ACK: [PATCH] net/sched: store the last executed chain also for clsact egress

Zachary Tahenakos zachary.tahenakos at canonical.com
Mon Aug 1 12:45:35 UTC 2022


Acked-by Zachary Tahenakos <zachary.tahenakos at canonical.com>

On Wed, Jul 27, 2022 at 5:53 PM Bodong Wang <bodong at nvidia.com> wrote:

> From: Davide Caratti <dcaratti at redhat.com>
>
> BugLink: https://bugs.launchpad.net/bugs/1982980
>
> currently, only 'ingress' and 'clsact ingress' qdiscs store the tc 'chain
> id' in the skb extension. However, userspace programs (like ovs) are able
> to setup egress rules, and datapath gets confused in case it doesn't find
> the 'chain id' for a packet that's "recirculated" by tc.
> Change tcf_classify() to have the same semantic as tcf_classify_ingress()
> so that a single function can be called in ingress / egress, using the tc
> ingress / egress block respectively.
>
> Suggested-by: Alaa Hleilel <alaa at nvidia.com>
> Signed-off-by: Davide Caratti <dcaratti at redhat.com>
> Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner at gmail.com>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> (backported from commit 3aa2605594556c676fb88744bd9845acae60683d)
> [paulb: Removed tcf_qevent_handle change in cls_api]
> Signed-off-by: Paul Blakey <paulb at nvidia.com>
> Signed-off-by: Bodong Wang <bodong at nvidia.com>
> ---
>  include/net/pkt_cls.h    | 22 +++++++---------------
>  net/core/dev.c           |  5 ++---
>  net/sched/cls_api.c      | 40 ++++++++++++++++------------------------
>  net/sched/sch_atm.c      |  2 +-
>  net/sched/sch_cake.c     |  2 +-
>  net/sched/sch_cbq.c      |  2 +-
>  net/sched/sch_drr.c      |  2 +-
>  net/sched/sch_dsmark.c   |  2 +-
>  net/sched/sch_fq_codel.c |  2 +-
>  net/sched/sch_hfsc.c     |  2 +-
>  net/sched/sch_htb.c      |  2 +-
>  net/sched/sch_multiq.c   |  2 +-
>  net/sched/sch_prio.c     |  2 +-
>  net/sched/sch_qfq.c      |  2 +-
>  net/sched/sch_sfb.c      |  2 +-
>  net/sched/sch_sfq.c      |  2 +-
>  16 files changed, 38 insertions(+), 55 deletions(-)
>
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index d23d7fe..18e9373 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -70,12 +70,10 @@ static inline struct Qdisc *tcf_block_q(struct
> tcf_block *block)
>         return block->q;
>  }
>
> -int tcf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
> -                struct tcf_result *res, bool compat_mode);
> -int tcf_classify_ingress(struct sk_buff *skb,
> -                        const struct tcf_block *ingress_block,
> -                        const struct tcf_proto *tp, struct tcf_result
> *res,
> -                        bool compat_mode);
> +int tcf_classify(struct sk_buff *skb,
> +                const struct tcf_block *block,
> +                const struct tcf_proto *tp, struct tcf_result *res,
> +                bool compat_mode);
>
>  #else
>  static inline bool tcf_block_shared(struct tcf_block *block)
> @@ -132,20 +130,14 @@ void tc_setup_cb_block_unregister(struct tcf_block
> *block, flow_setup_cb_t *cb,
>  {
>  }
>
> -static inline int tcf_classify(struct sk_buff *skb, const struct
> tcf_proto *tp,
> +static inline int tcf_classify(struct sk_buff *skb,
> +                              const struct tcf_block *block,
> +                              const struct tcf_proto *tp,
>                                struct tcf_result *res, bool compat_mode)
>  {
>         return TC_ACT_UNSPEC;
>  }
>
> -static inline int tcf_classify_ingress(struct sk_buff *skb,
> -                                      const struct tcf_block
> *ingress_block,
> -                                      const struct tcf_proto *tp,
> -                                      struct tcf_result *res, bool
> compat_mode)
> -{
> -       return TC_ACT_UNSPEC;
> -}
> -
>  #endif
>
>  static inline unsigned long
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 4a32897..a50c5dd 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3513,7 +3513,7 @@ int dev_loopback_xmit(struct net *net, struct sock
> *sk, struct sk_buff *skb)
>         tc_skb_cb(skb)->post_ct = false;
>         mini_qdisc_bstats_cpu_update(miniq, skb);
>
> -       switch (tcf_classify(skb, miniq->filter_list, &cl_res, false)) {
> +       switch (tcf_classify(skb, miniq->block, miniq->filter_list,
> &cl_res, false)) {
>         case TC_ACT_OK:
>         case TC_ACT_RECLASSIFY:
>                 skb->tc_index = TC_H_MIN(cl_res.classid);
> @@ -4608,8 +4608,7 @@ static __latent_entropy void net_tx_action(struct
> softirq_action *h)
>         skb->tc_at_ingress = 1;
>         mini_qdisc_bstats_cpu_update(miniq, skb);
>
> -       switch (tcf_classify_ingress(skb, miniq->block, miniq->filter_list,
> -                                    &cl_res, false)) {
> +       switch (tcf_classify(skb, miniq->block, miniq->filter_list,
> &cl_res, false)) {
>         case TC_ACT_OK:
>         case TC_ACT_RECLASSIFY:
>                 skb->tc_index = TC_H_MIN(cl_res.classid);
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 698648a..ddc2d94 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -1628,21 +1628,11 @@ static inline int __tcf_classify(struct sk_buff
> *skb,
>  #endif
>  }
>
> -int tcf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
> +int tcf_classify(struct sk_buff *skb,
> +                const struct tcf_block *block,
> +                const struct tcf_proto *tp,
>                  struct tcf_result *res, bool compat_mode)
>  {
> -       u32 last_executed_chain = 0;
> -
> -       return __tcf_classify(skb, tp, tp, res, compat_mode,
> -                             &last_executed_chain);
> -}
> -EXPORT_SYMBOL(tcf_classify);
> -
> -int tcf_classify_ingress(struct sk_buff *skb,
> -                        const struct tcf_block *ingress_block,
> -                        const struct tcf_proto *tp,
> -                        struct tcf_result *res, bool compat_mode)
> -{
>  #if !IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
>         u32 last_executed_chain = 0;
>
> @@ -1654,20 +1644,22 @@ int tcf_classify_ingress(struct sk_buff *skb,
>         struct tc_skb_ext *ext;
>         int ret;
>
> -       ext = skb_ext_find(skb, TC_SKB_EXT);
> +       if (block) {
> +               ext = skb_ext_find(skb, TC_SKB_EXT);
>
> -       if (ext && ext->chain) {
> -               struct tcf_chain *fchain;
> +               if (ext && ext->chain) {
> +                       struct tcf_chain *fchain;
>
> -               fchain = tcf_chain_lookup_rcu(ingress_block, ext->chain);
> -               if (!fchain)
> -                       return TC_ACT_SHOT;
> +                       fchain = tcf_chain_lookup_rcu(block, ext->chain);
> +                       if (!fchain)
> +                               return TC_ACT_SHOT;
>
> -               /* Consume, so cloned/redirect skbs won't inherit ext */
> -               skb_ext_del(skb, TC_SKB_EXT);
> +                       /* Consume, so cloned/redirect skbs won't inherit
> ext */
> +                       skb_ext_del(skb, TC_SKB_EXT);
>
> -               tp = rcu_dereference_bh(fchain->filter_chain);
> -               last_executed_chain = fchain->index;
> +                       tp = rcu_dereference_bh(fchain->filter_chain);
> +                       last_executed_chain = fchain->index;
> +               }
>         }
>
>         ret = __tcf_classify(skb, tp, orig_tp, res, compat_mode,
> @@ -1691,7 +1683,7 @@ int tcf_classify_ingress(struct sk_buff *skb,
>         return ret;
>  #endif
>  }
> -EXPORT_SYMBOL(tcf_classify_ingress);
> +EXPORT_SYMBOL(tcf_classify);
>
>  struct tcf_chain_info {
>         struct tcf_proto __rcu **pprev;
> diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c
> index 6385995..98a52c3 100644
> --- a/net/sched/sch_atm.c
> +++ b/net/sched/sch_atm.c
> @@ -393,7 +393,7 @@ static int atm_tc_enqueue(struct sk_buff *skb, struct
> Qdisc *sch,
>                 list_for_each_entry(flow, &p->flows, list) {
>                         fl = rcu_dereference_bh(flow->filter_list);
>                         if (fl) {
> -                               result = tcf_classify(skb, fl, &res, true);
> +                               result = tcf_classify(skb, NULL, fl, &res,
> true);
>                                 if (result < 0)
>                                         continue;
>                                 flow = (struct atm_flow_data *)res.class;
> diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
> index 0eb4d4a..e36c34a 100644
> --- a/net/sched/sch_cake.c
> +++ b/net/sched/sch_cake.c
> @@ -1629,7 +1629,7 @@ static u32 cake_classify(struct Qdisc *sch, struct
> cake_tin_data **t,
>                 goto hash;
>
>         *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
> -       result = tcf_classify(skb, filter, &res, false);
> +       result = tcf_classify(skb, NULL, filter, &res, false);
>
>         if (result >= 0) {
>  #ifdef CONFIG_NET_CLS_ACT
> diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
> index e597288..5b7ea18 100644
> --- a/net/sched/sch_cbq.c
> +++ b/net/sched/sch_cbq.c
> @@ -228,7 +228,7 @@ struct cbq_sched_data {
>                 /*
>                  * Step 2+n. Apply classifier.
>                  */
> -               result = tcf_classify(skb, fl, &res, true);
> +               result = tcf_classify(skb, NULL, fl, &res, true);
>                 if (!fl || result < 0)
>                         goto fallback;
>
> diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
> index 07a2b0b..60a5a56 100644
> --- a/net/sched/sch_drr.c
> +++ b/net/sched/sch_drr.c
> @@ -316,7 +316,7 @@ static struct drr_class *drr_classify(struct sk_buff
> *skb, struct Qdisc *sch,
>
>         *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
>         fl = rcu_dereference_bh(q->filter_list);
> -       result = tcf_classify(skb, fl, &res, false);
> +       result = tcf_classify(skb, NULL, fl, &res, false);
>         if (result >= 0) {
>  #ifdef CONFIG_NET_CLS_ACT
>                 switch (result) {
> diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
> index 76ed1a0..5be11c4 100644
> --- a/net/sched/sch_dsmark.c
> +++ b/net/sched/sch_dsmark.c
> @@ -241,7 +241,7 @@ static int dsmark_enqueue(struct sk_buff *skb, struct
> Qdisc *sch,
>         else {
>                 struct tcf_result res;
>                 struct tcf_proto *fl = rcu_dereference_bh(p->filter_list);
> -               int result = tcf_classify(skb, fl, &res, false);
> +               int result = tcf_classify(skb, NULL, fl, &res, false);
>
>                 pr_debug("result %d class 0x%04x\n", result, res.classid);
>
> diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
> index 86fb2f9..1d2f3f4 100644
> --- a/net/sched/sch_fq_codel.c
> +++ b/net/sched/sch_fq_codel.c
> @@ -92,7 +92,7 @@ static unsigned int fq_codel_classify(struct sk_buff
> *skb, struct Qdisc *sch,
>                 return fq_codel_hash(q, skb) + 1;
>
>         *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
> -       result = tcf_classify(skb, filter, &res, false);
> +       result = tcf_classify(skb, NULL, filter, &res, false);
>         if (result >= 0) {
>  #ifdef CONFIG_NET_CLS_ACT
>                 switch (result) {
> diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
> index 433f219..091eea4 100644
> --- a/net/sched/sch_hfsc.c
> +++ b/net/sched/sch_hfsc.c
> @@ -1129,7 +1129,7 @@ struct hfsc_sched {
>         *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
>         head = &q->root;
>         tcf = rcu_dereference_bh(q->root.filter_list);
> -       while (tcf && (result = tcf_classify(skb, tcf, &res, false)) >= 0)
> {
> +       while (tcf && (result = tcf_classify(skb, NULL, tcf, &res, false))
> >= 0) {
>  #ifdef CONFIG_NET_CLS_ACT
>                 switch (result) {
>                 case TC_ACT_QUEUED:
> diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
> index 8184c87..176359f 100644
> --- a/net/sched/sch_htb.c
> +++ b/net/sched/sch_htb.c
> @@ -232,7 +232,7 @@ static struct htb_class *htb_classify(struct sk_buff
> *skb, struct Qdisc *sch,
>         }
>
>         *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
> -       while (tcf && (result = tcf_classify(skb, tcf, &res, false)) >= 0)
> {
> +       while (tcf && (result = tcf_classify(skb, NULL, tcf, &res, false))
> >= 0) {
>  #ifdef CONFIG_NET_CLS_ACT
>                 switch (result) {
>                 case TC_ACT_QUEUED:
> diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
> index 1330ad2..bc5f288 100644
> --- a/net/sched/sch_multiq.c
> +++ b/net/sched/sch_multiq.c
> @@ -36,7 +36,7 @@ struct multiq_sched_data {
>         int err;
>
>         *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
> -       err = tcf_classify(skb, fl, &res, false);
> +       err = tcf_classify(skb, NULL, fl, &res, false);
>  #ifdef CONFIG_NET_CLS_ACT
>         switch (err) {
>         case TC_ACT_STOLEN:
> diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
> index 6479417..41573b8 100644
> --- a/net/sched/sch_prio.c
> +++ b/net/sched/sch_prio.c
> @@ -39,7 +39,7 @@ struct prio_sched_data {
>         *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
>         if (TC_H_MAJ(skb->priority) != sch->handle) {
>                 fl = rcu_dereference_bh(q->filter_list);
> -               err = tcf_classify(skb, fl, &res, false);
> +               err = tcf_classify(skb, NULL, fl, &res, false);
>  #ifdef CONFIG_NET_CLS_ACT
>                 switch (err) {
>                 case TC_ACT_STOLEN:
> diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> index 1eb339d..d6827b54 100644
> --- a/net/sched/sch_qfq.c
> +++ b/net/sched/sch_qfq.c
> @@ -689,7 +689,7 @@ static struct qfq_class *qfq_classify(struct sk_buff
> *skb, struct Qdisc *sch,
>
>         *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
>         fl = rcu_dereference_bh(q->filter_list);
> -       result = tcf_classify(skb, fl, &res, false);
> +       result = tcf_classify(skb, NULL, fl, &res, false);
>         if (result >= 0) {
>  #ifdef CONFIG_NET_CLS_ACT
>                 switch (result) {
> diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
> index 4074c50..28a7ed6 100644
> --- a/net/sched/sch_sfb.c
> +++ b/net/sched/sch_sfb.c
> @@ -257,7 +257,7 @@ static bool sfb_classify(struct sk_buff *skb, struct
> tcf_proto *fl,
>         struct tcf_result res;
>         int result;
>
> -       result = tcf_classify(skb, fl, &res, false);
> +       result = tcf_classify(skb, NULL, fl, &res, false);
>         if (result >= 0) {
>  #ifdef CONFIG_NET_CLS_ACT
>                 switch (result) {
> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
> index b92bafa..a090c53 100644
> --- a/net/sched/sch_sfq.c
> +++ b/net/sched/sch_sfq.c
> @@ -178,7 +178,7 @@ static unsigned int sfq_classify(struct sk_buff *skb,
> struct Qdisc *sch,
>                 return sfq_hash(q, skb) + 1;
>
>         *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
> -       result = tcf_classify(skb, fl, &res, false);
> +       result = tcf_classify(skb, NULL, fl, &res, false);
>         if (result >= 0) {
>  #ifdef CONFIG_NET_CLS_ACT
>                 switch (result) {
> --
> 1.8.3.1
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20220801/3deaf83a/attachment-0001.html>


More information about the kernel-team mailing list