ACK/Cmnt: [PATCH] net/sched: store the last executed chain also for clsact egress
Tim Gardner
tim.gardner at canonical.com
Thu Jul 28 12:31:41 UTC 2022
On 7/27/22 15:52, Bodong Wang 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) {
Acked-by: Tim Gardner <tim.gardner at canonical.com>
One can only assume this patch is destined for focal/bluefield
--
-----------
Tim Gardner
Canonical, Inc
More information about the kernel-team
mailing list