| Message ID | 20230819163515.2266246-3-victor@mojatatu.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/sched: Introduce tc block ports tracking and use | expand |
On Sat, Aug 19, 2023 at 01:35:13PM -0300, Victor Nogueira wrote: > The datapath can now find the block of the port in which the packet arrived > at. It can then use it for various activities. I think $subject needs a s/ports//. Because, well, the patch is exposing the block, which contains the ports.. The first sentence here goes along with this rationale. more below > > In the next patch we show a simple action that multicasts to all ports > excep for the port in which the packet arrived on. "except" > > Co-developed-by: Jamal Hadi Salim <jhs@mojatatu.com> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> > Co-developed-by: Pedro Tammela <pctammela@mojatatu.com> > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> > Signed-off-by: Victor Nogueira <victor@mojatatu.com> > --- > include/net/sch_generic.h | 4 ++++ > net/sched/cls_api.c | 10 +++++++++- > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h > index 824a0ecb5afc..c5defb166ef6 100644 > --- a/include/net/sch_generic.h > +++ b/include/net/sch_generic.h > @@ -440,6 +440,8 @@ struct qdisc_skb_cb { > }; > #define QDISC_CB_PRIV_LEN 20 > unsigned char data[QDISC_CB_PRIV_LEN]; > + /* This should allow eBPF to continue to align */ Not sure if this comment really belongs in here. Up to you but it seems better suited in the patch description. Hopefully the next one won't do something like: /* This should allow eBPF to continue to align */ u32 block_index; + /* This one too */ + u32 my_var; :-) > + u32 block_index; > }; > > typedef void tcf_chain_head_change_t(struct tcf_proto *tp_head, void *priv);
On Wed, Aug 23, 2023 at 1:33 PM Marcelo Ricardo Leitner <mleitner@redhat.com> wrote: > > On Sat, Aug 19, 2023 at 01:35:13PM -0300, Victor Nogueira wrote: > > The datapath can now find the block of the port in which the packet arrived > > at. It can then use it for various activities. > > I think $subject needs a s/ports//. Because, well, the patch is > exposing the block, which contains the ports.. The first sentence here > goes along with this rationale. > > more below > > > > > In the next patch we show a simple action that multicasts to all ports > > excep for the port in which the packet arrived on. > > "except" > Thanks Marcelo. We'll fix both in the next version. cheers, jamal > > Co-developed-by: Jamal Hadi Salim <jhs@mojatatu.com> > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> > > Co-developed-by: Pedro Tammela <pctammela@mojatatu.com> > > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> > > Signed-off-by: Victor Nogueira <victor@mojatatu.com> > > --- > > include/net/sch_generic.h | 4 ++++ > > net/sched/cls_api.c | 10 +++++++++- > > 2 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h > > index 824a0ecb5afc..c5defb166ef6 100644 > > --- a/include/net/sch_generic.h > > +++ b/include/net/sch_generic.h > > @@ -440,6 +440,8 @@ struct qdisc_skb_cb { > > }; > > #define QDISC_CB_PRIV_LEN 20 > > unsigned char data[QDISC_CB_PRIV_LEN]; > > + /* This should allow eBPF to continue to align */ > > Not sure if this comment really belongs in here. Up to you but it > seems better suited in the patch description. Hopefully the next one > won't do something like: > > /* This should allow eBPF to continue to align */ > u32 block_index; > + /* This one too */ > + u32 my_var; > > :-) > > > + u32 block_index; > > }; > > > > typedef void tcf_chain_head_change_t(struct tcf_proto *tp_head, void *priv); >
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 824a0ecb5afc..c5defb166ef6 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -440,6 +440,8 @@ struct qdisc_skb_cb { }; #define QDISC_CB_PRIV_LEN 20 unsigned char data[QDISC_CB_PRIV_LEN]; + /* This should allow eBPF to continue to align */ + u32 block_index; }; typedef void tcf_chain_head_change_t(struct tcf_proto *tp_head, void *priv); @@ -488,6 +490,8 @@ struct tcf_block { struct mutex proto_destroy_lock; /* Lock for proto_destroy hashtable. */ }; +struct tcf_block *tcf_block_lookup(struct net *net, u32 block_index); + static inline bool lockdep_tcf_chain_is_locked(struct tcf_chain *chain) { return lockdep_is_held(&chain->filter_chain_lock); diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index a976792ef02f..00e776cdd3fc 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -1011,12 +1011,13 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q, return block; } -static struct tcf_block *tcf_block_lookup(struct net *net, u32 block_index) +struct tcf_block *tcf_block_lookup(struct net *net, u32 block_index) { struct tcf_net *tn = net_generic(net, tcf_net_id); return idr_find(&tn->idr, block_index); } +EXPORT_SYMBOL(tcf_block_lookup); static struct tcf_block *tcf_block_refcnt_get(struct net *net, u32 block_index) { @@ -1737,9 +1738,13 @@ int tcf_classify(struct sk_buff *skb, const struct tcf_proto *tp, struct tcf_result *res, bool compat_mode) { + struct qdisc_skb_cb *qdisc_cb = qdisc_skb_cb(skb); + #if !IS_ENABLED(CONFIG_NET_TC_SKB_EXT) u32 last_executed_chain = 0; + qdisc_cb->block_index = block ? block->index : 0; + return __tcf_classify(skb, tp, tp, res, compat_mode, NULL, 0, &last_executed_chain); #else @@ -1751,6 +1756,7 @@ int tcf_classify(struct sk_buff *skb, int ret; if (block) { + qdisc_cb->block_index = block->index; ext = skb_ext_find(skb, TC_SKB_EXT); if (ext && (ext->chain || ext->act_miss)) { @@ -1778,6 +1784,8 @@ int tcf_classify(struct sk_buff *skb, tp = rcu_dereference_bh(fchain->filter_chain); last_executed_chain = fchain->index; } + } else { + qdisc_cb->block_index = 0; } ret = __tcf_classify(skb, tp, orig_tp, res, compat_mode, n, act_index,