diff mbox series

[RFC,net-next,2/3] Expose tc block ports to the datapath

Message ID 20230815162530.150994-3-jhs@mojatatu.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series Introduce tc block ports tracking and use | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 2741 this patch: 2742
netdev/cc_maintainers warning 4 maintainers not CCed: kuba@kernel.org edumazet@google.com davem@davemloft.net pabeni@redhat.com
netdev/build_clang fail Errors and warnings before: 1502 this patch: 1504
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 2788 this patch: 2789
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 42 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jamal Hadi Salim Aug. 15, 2023, 4:25 p.m. UTC 
The datapath can now find the block of the port in which the packet arrived at.
It can then use it for various activities.

In the next patch we show a simple action that multicast to all ports except for
the port in which the packet arrived on.

Co-developed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
Co-developed-by: Pedro Tammela <pctammela@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 include/net/sch_generic.h | 4 ++++
 net/sched/cls_api.c       | 6 +++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

Comments

Stephen Hemminger Aug. 15, 2023, 5:52 p.m. UTC | #1 
On Tue, 15 Aug 2023 12:25:29 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:

> +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)

Use EXPORT_SYMBOL_GPL?
Simon Horman Aug. 16, 2023, 9:04 a.m. UTC | #2 
On Tue, Aug 15, 2023 at 12:25:29PM -0400, Jamal Hadi Salim 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.
> 
> In the next patch we show a simple action that multicast to all ports except for
> the port in which the packet arrived on.
> 
> Co-developed-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Victor Nogueira <victor@mojatatu.com>
> Co-developed-by: Pedro Tammela <pctammela@mojatatu.com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

...

> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index a976792ef02f..be4555714519 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c

...

> @@ -1737,9 +1738,12 @@ 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);

Hi Jamal,

Does the line above belong inside the condition immediately below?
It seems potentially unused otherwise.

>  #if !IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
>  	u32 last_executed_chain = 0;
>  
> +	qdisc_cb->block_index = block->index;
> +
>  	return __tcf_classify(skb, tp, tp, res, compat_mode, NULL, 0,
>  			      &last_executed_chain);
>  #else
> -- 
> 2.34.1
> 
>
Jamal Hadi Salim Aug. 16, 2023, 11:06 a.m. UTC | #3 
On Tue, Aug 15, 2023 at 1:52 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 15 Aug 2023 12:25:29 -0400
> Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> > +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)
>
> Use EXPORT_SYMBOL_GPL?

Sure.

On Wed, Aug 16, 2023 at 5:04 AM Simon Horman <horms@kernel.org> wrote:
>
On Wed, Aug 16, 2023 at 5:04 AM Simon Horman <horms@kernel.org> wrote:
>
> On Tue, Aug 15, 2023 at 12:25:29PM -0400, Jamal Hadi Salim 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.
> >
> > In the next patch we show a simple action that multicast to all ports except for
> > the port in which the packet arrived on.
> >
> > Co-developed-by: Victor Nogueira <victor@mojatatu.com>
> > Signed-off-by: Victor Nogueira <victor@mojatatu.com>
> > Co-developed-by: Pedro Tammela <pctammela@mojatatu.com>
> > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>
> ...
>
> > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> > index a976792ef02f..be4555714519 100644
> > --- a/net/sched/cls_api.c
> > +++ b/net/sched/cls_api.c
>
> ...
>
> > @@ -1737,9 +1738,12 @@ 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);
>
> Hi Jamal,
>
> Does the line above belong inside the condition immediately below?
> It seems potentially unused otherwise.

Indeed.
The Intel bot also complained about this. I guess we'll need an
additional patch and move up "last_executed_chain" variable which is
repeated twice. Then i can add the assignment on top of this.
Something like:
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 4af48f76f..5d9959381 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1736,13 +1736,12 @@ int tcf_classify(struct sk_buff *skb,
                 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;
-
+#if !IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
        return __tcf_classify(skb, tp, tp, res, compat_mode, NULL, 0,
                              &last_executed_chain);
 #else
-       u32 last_executed_chain = tp ? tp->chain->index : 0;
+       last_executed_chain = tp ? tp->chain->index : 0;
        struct tcf_exts_miss_cookie_node *n = NULL;
        const struct tcf_proto *orig_tp = tp;
        struct tc_skb_ext *ext;

cheers,
jamal
Jamal Hadi Salim Aug. 16, 2023, 12:58 p.m. UTC | #4 
On Tue, Aug 15, 2023 at 1:52 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 15 Aug 2023 12:25:29 -0400
> Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> > +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)
>
> Use EXPORT_SYMBOL_GPL?

Actually... all the other symbols exported in that area use
EXPORT_SYMBOL() - i just cutnpasted. For consistency shouldnt i keep
it?

cheers,
jamal
diff mbox series

Patch

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index f002b0423efc..a99ac60426b3 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..be4555714519 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,12 @@  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->index;
+
 	return __tcf_classify(skb, tp, tp, res, compat_mode, NULL, 0,
 			      &last_executed_chain);
 #else