All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Ido Schimmel <idosch@nvidia.com>
Cc: taras.chornyi@plvision.eu, petrm@nvidia.com,
	alexandre.belloni@bootlin.com, jiri@resnulli.us,
	taspelund@nvidia.com, leon@kernel.org, netdev@vger.kernel.org,
	razor@blackwall.org, bridge@lists.linux-foundation.org,
	claudiu.manoil@nxp.com, UNGLinuxDriver@microchip.com,
	vladimir.oltean@nxp.com, edumazet@google.com, jhs@mojatatu.com,
	roopa@nvidia.com, xiyou.wangcong@gmail.com, pabeni@redhat.com,
	saeedm@nvidia.com, davem@davemloft.net
Subject: Re: [Bridge] [PATCH net-next 1/5] skbuff: bridge: Add layer 2 miss indication
Date: Tue, 23 May 2023 13:29:51 -0700	[thread overview]
Message-ID: <20230523132951.623288cb@kernel.org> (raw)
In-Reply-To: <ZGx0/hwPmFFN2ivS@shredder>

On Tue, 23 May 2023 11:10:38 +0300 Ido Schimmel wrote:
> > Can we possibly put the new field at the end of the CB and then have TC
> > look at it in the CB? We already do a bit of such CB juggling in strp
> > (first member of struct sk_skb_cb).  
> 
> Using the CB between different layers is very fragile and I would like
> to avoid it. Note that the skb can pass various layers until hitting the
> classifier, each of which can decide to memset() the CB.
> 
> Anyway, I think I have a better alternative. I added the 'l2_miss' bit
> to the tc skb extension and adjusted the bridge to mark packets via this
> extension. The entire thing is protected by the existing 'tc_skb_ext_tc'
> static key, so overhead is kept to a minimum when feature is disabled.
> Extended flower to enable / disable this key when filters that match on
> 'l2_miss' are added / removed.
> 
> bridge change to mark the packet:
> https://github.com/idosch/linux/commit/3fab206492fcad9177f2340680f02ced1b9a0dec.patch
> 
> flow_dissector change to dissect the info from the extension:
> https://github.com/idosch/linux/commit/1533c078b02586547817a4e63989a0db62aa5315.patch
> 
> flower change to enable / disable the key:
> https://github.com/idosch/linux/commit/cf84b277511ec80fe565c41271abc6b2e2f629af.patch
> 
> Advantages compared to the previous approach are that we do not need a
> new bit in the skb and that overhead is kept to a minimum when feature
> is disabled. Disadvantage is that overhead is higher when feature is
> enabled.
> 
> WDYT?

Sounds good, yup. Thanks!

WARNING: multiple messages have this Message-ID (diff)
From: Jakub Kicinski <kuba@kernel.org>
To: Ido Schimmel <idosch@nvidia.com>
Cc: razor@blackwall.org, netdev@vger.kernel.org,
	bridge@lists.linux-foundation.org, davem@davemloft.net,
	pabeni@redhat.com, edumazet@google.com, roopa@nvidia.com,
	taras.chornyi@plvision.eu, saeedm@nvidia.com, leon@kernel.org,
	petrm@nvidia.com, vladimir.oltean@nxp.com,
	claudiu.manoil@nxp.com, alexandre.belloni@bootlin.com,
	UNGLinuxDriver@microchip.com, jhs@mojatatu.com,
	xiyou.wangcong@gmail.com, jiri@resnulli.us, taspelund@nvidia.com
Subject: Re: [PATCH net-next 1/5] skbuff: bridge: Add layer 2 miss indication
Date: Tue, 23 May 2023 13:29:51 -0700	[thread overview]
Message-ID: <20230523132951.623288cb@kernel.org> (raw)
In-Reply-To: <ZGx0/hwPmFFN2ivS@shredder>

On Tue, 23 May 2023 11:10:38 +0300 Ido Schimmel wrote:
> > Can we possibly put the new field at the end of the CB and then have TC
> > look at it in the CB? We already do a bit of such CB juggling in strp
> > (first member of struct sk_skb_cb).  
> 
> Using the CB between different layers is very fragile and I would like
> to avoid it. Note that the skb can pass various layers until hitting the
> classifier, each of which can decide to memset() the CB.
> 
> Anyway, I think I have a better alternative. I added the 'l2_miss' bit
> to the tc skb extension and adjusted the bridge to mark packets via this
> extension. The entire thing is protected by the existing 'tc_skb_ext_tc'
> static key, so overhead is kept to a minimum when feature is disabled.
> Extended flower to enable / disable this key when filters that match on
> 'l2_miss' are added / removed.
> 
> bridge change to mark the packet:
> https://github.com/idosch/linux/commit/3fab206492fcad9177f2340680f02ced1b9a0dec.patch
> 
> flow_dissector change to dissect the info from the extension:
> https://github.com/idosch/linux/commit/1533c078b02586547817a4e63989a0db62aa5315.patch
> 
> flower change to enable / disable the key:
> https://github.com/idosch/linux/commit/cf84b277511ec80fe565c41271abc6b2e2f629af.patch
> 
> Advantages compared to the previous approach are that we do not need a
> new bit in the skb and that overhead is kept to a minimum when feature
> is disabled. Disadvantage is that overhead is higher when feature is
> enabled.
> 
> WDYT?

Sounds good, yup. Thanks!

  parent reply	other threads:[~2023-05-23 20:29 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-18 11:33 [Bridge] [PATCH net-next 0/5] Add layer 2 miss indication and filtering Ido Schimmel
2023-05-18 11:33 ` Ido Schimmel
2023-05-18 11:33 ` [Bridge] [PATCH net-next 1/5] skbuff: bridge: Add layer 2 miss indication Ido Schimmel
2023-05-18 11:33   ` Ido Schimmel
2023-05-18 16:08   ` [Bridge] " Nikolay Aleksandrov
2023-05-18 16:08     ` Nikolay Aleksandrov
2023-05-19 13:51     ` [Bridge] " Ido Schimmel
2023-05-19 13:51       ` Ido Schimmel
2023-05-19 21:52       ` [Bridge] " Jakub Kicinski
2023-05-19 21:52         ` Jakub Kicinski
2023-05-23  8:10         ` [Bridge] " Ido Schimmel
2023-05-23  8:10           ` Ido Schimmel
2023-05-23  9:04           ` [Bridge] " Paolo Abeni
2023-05-23  9:04             ` Paolo Abeni
2023-05-23 11:34             ` [Bridge] " Ido Schimmel
2023-05-23 11:34               ` Ido Schimmel
2023-05-23 13:03           ` [Bridge] " Nikolay Aleksandrov
2023-05-23 13:03             ` Nikolay Aleksandrov
2023-05-23 20:29           ` Jakub Kicinski [this message]
2023-05-23 20:29             ` Jakub Kicinski
2023-05-21  7:34       ` [Bridge] " Nikolay Aleksandrov
2023-05-21  7:34         ` Nikolay Aleksandrov
2023-05-18 11:33 ` [Bridge] [PATCH net-next 2/5] net/sched: flower: Allow matching on layer 2 miss Ido Schimmel
2023-05-18 11:33   ` Ido Schimmel
2023-05-19 11:27   ` [Bridge] " Simon Horman
2023-05-19 11:27     ` Simon Horman
2023-05-18 11:33 ` [Bridge] [PATCH net-next 3/5] flow_offload: Reject " Ido Schimmel
2023-05-18 11:33   ` Ido Schimmel
2023-05-19 11:33   ` [Bridge] " Simon Horman
2023-05-19 11:33     ` Simon Horman
2023-05-19 14:10     ` [Bridge] " Ido Schimmel
2023-05-19 14:10       ` Ido Schimmel
2023-05-19 14:15       ` [Bridge] " Simon Horman
2023-05-19 14:15         ` Simon Horman
2023-05-18 11:33 ` [Bridge] [PATCH net-next 4/5] mlxsw: spectrum_flower: Add ability to match " Ido Schimmel
2023-05-18 11:33   ` Ido Schimmel
2023-05-19 11:35   ` [Bridge] " Simon Horman
2023-05-19 11:35     ` Simon Horman
2023-05-18 11:33 ` [Bridge] [PATCH net-next 5/5] selftests: forwarding: Add layer 2 miss test cases Ido Schimmel
2023-05-18 11:33   ` Ido Schimmel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230523132951.623288cb@kernel.org \
    --to=kuba@kernel.org \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=idosch@nvidia.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=leon@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=petrm@nvidia.com \
    --cc=razor@blackwall.org \
    --cc=roopa@nvidia.com \
    --cc=saeedm@nvidia.com \
    --cc=taras.chornyi@plvision.eu \
    --cc=taspelund@nvidia.com \
    --cc=vladimir.oltean@nxp.com \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.