netdev - Re: [PATCH iwl-next v2] ice: Add support for packet mirroring using hardware in switchdev mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 12 Sep 2023 18:43:22 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: Andrii Staikov <andrii.staikov@...el.com>, 
	intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org
Subject: Re: [PATCH iwl-next v2] ice: Add support for packet mirroring using
 hardware in switchdev mode

On Tue, 2023-09-12 at 16:41 +0200, Alexander Lobakin wrote:
> From: Paolo Abeni <pabeni@...hat.com>
> Date: Tue, 12 Sep 2023 11:56:15 +0200
> 
> > Hi all,
> > 
> > On Tue, 2023-09-12 at 11:29 +0200, Andrii Staikov wrote:
> > > Switchdev mode allows to add mirroring rules to mirror
> > > incoming and outgoing packets to the interface's port
> > > representor. Previously, this was available only using
> > > software functionality. Add possibility to offload this
> > > functionality to the NIC hardware.
> > > 
> > > Introduce ICE_MIRROR_PACKET filter action to the
> > > ice_sw_fwd_act_type enum to identify the desired action
> > > and pass it to the hardware as well as the VSI to mirror.
> > > 
> > > Example of tc mirror command using hardware:
> > > tc filter add dev ens1f0np0 ingress protocol ip prio 1 flower
> > > src_mac b4:96:91:a5:c7:a7 skip_sw action mirred egress mirror dev eth1
> > > 
> > > ens1f0np0 - PF
> > > b4:96:91:a5:c7:a7 - source MAC address
> > > eth1 - PR of a VF to mirror to
> > > 
> > > Signed-off-by: Andrii Staikov <andrii.staikov@...el.com>
> > 
> > The amount of patches that IMHO should land only into intel-specific
> > MLs and instead reaches also netdev, recently increased.
> 
> Let's clarify what you mean by "intel-specific MLs".
> Do you mean our internal MLs and review or the open one, IWL?
> 
> IWL is mostly rudimentary. It's open, but almost nobody outside of Intel
> sits there, which means 2/3 of patches doesn't get enough attention and
> reviews. It's used by our validation as well, but that's it.
> Our internal ML for Ethernet patches works as usually. I realize roughly
> half of all patches pass it without a Reviewed-by tag and it's something
> we're actively working on. If all goes well, no patches without a proper
> review will go outside Intel's internal Ethernet MLs.
> Now, the second part,
> 
> > 
> > Please try harder to apply proper constraints to your traffic, netdev
> > is already busy enough!
> 
> Do you want us to stop CCing netdev when we send patches to the outer
> review to IWL?
> This would mean they will once again start missing enough attention from
> the outside. I hope you don't want our patches to be reviewed *only* by
> Intel folks, right? I don't feel this a good idea.
> That's why we started CCing netdev this year. And we do that when we
> send patches to IWL, i.e. outside. It's not like "ok, let's Cc netdev
> instead of going through our internal review process".
> 
> Our clients, partners (e.g. Czech RedHat), our developers, want our
> patches to have proper complex review. Dropping netdev would mean that a
> patch of some non-corpo guy will be reviewed more carefully and at the
> end will have better quality than an Intel patch, which "shouldn't
> overburden netdev".
> Saying "we'll see them when Tony sends a PR" also doesn't work well for
> me. A patch gets taken into a PR once it passes internal review, then
> validation, this always do take a while. Imagine waiting for a month for
> your patch to be sent in a PR to get a negative review, so that you have
> to repeat this process again and wait for another month to get some more
> change requests and again :D
> 
> In a couple months, no our patches will hit netdev without a proper
> Reviewed-by obtained during the internal review, let's not take corner
> cases and effectively hide our code from the world?
> I don't think you'd like to put a huge banner on netdev's lore saying
> "please also take a look at intel-wired-lan" :z I also don't want ppl to
> behave like Greg KH some time ago when he said "where's your damn
> internal RB, stop abusing LKML" in reply to my early RFC PoC sent only
> for an open discussion xD

I was under the impression that some patches landed on IWL cc-ing
netdev possibly unintentionally, e.g.:

https://lore.kernel.org/netdev/20230904021455.3944605-1-junfeng.guo@intel.com/

My intention was to raise attention on such events.

Cheers,

Paolo