netdev - Re: [RFC Draft PATCHv2 net-next] Doc: update bridge doc

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: Thu, 2 Nov 2023 12:09:09 +0800
From: Hangbin Liu <liuhangbin@...il.com>
To: Nikolay Aleksandrov <razor@...ckwall.org>
Cc: netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
	David Ahern <dsahern@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Ido Schimmel <idosch@...sch.org>, Roopa Prabhu <roopa@...dia.com>,
	Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [RFC Draft PATCHv2 net-next] Doc: update bridge doc

Hi Nikolay,

On Wed, Nov 01, 2023 at 01:29:34PM +0200, Nikolay Aleksandrov wrote:
> Hi,
> I have written some initial comments, there will definitely be more.
> One general thing - please split this in 2 patches at least. 1 for the
> documentation, and 1 for the netlink uAPI changes. You can even split it

Sure, I will.
> further into logical parts if you'd like, it will make it easier to
> review and people can focus on different parts better. Please CC DSA
> folks as well.
> 
> > diff --git a/Documentation/networking/bridge.rst b/Documentation/networking/bridge.rst
> > index c859f3c1636e..b36bd737c05e 100644
> > --- a/Documentation/networking/bridge.rst
> > +++ b/Documentation/networking/bridge.rst
> > -The bridge-utilities are maintained at:
> > -   git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/bridge-utils.git
> > +Bridge internals
> > +================
> > -Additionally, the iproute2 utilities can be used to configure
> > -bridge devices.
> > +Here are the core structs of bridge code.
> 
> the core structs? These are outdated structures used in ioctl.

Ah, I just notice this. In next patch I will use `struct net_bridge_vlan`
as example.

> > +Bridge sysfs
> > +------------
> > +
> > +All the sysfs parameters are also exported via the bridge netlink API.
> > +Here you can find the explanation based on the correspond netlink attributes.
> 
> I don't get this one?

Some users/admins may still config bridge via sysfs. So I added this part
to let users know what's the meaning/usage of each sysfs field.

Do you want to remove this part for the doc?

> Also please mention the sysfs interface is deprecated and should not be
> extended if new options are added.

OK, I will add this note if we will keep this part.
> > +
> > +Switchdev
> > +=========
> > +
> > +Linux Bridge Switchdev is a feature in the Linux kernel that extends the
> > +capabilities of the traditional Linux bridge to work more efficiently with
> > +hardware switches that support switchdev. This technology is particularly
> > +useful in data center and networking environments where high-performance
> > +and low-latency packet forwarding is essential.
> 
> The last sentence is misleading, switchdev is used for many different types
> of devices.
> > +
> > +With Linux Bridge Switchdev, certain networking functions like forwarding,
> > +filtering, and learning of Ethernet frames can be offloaded to the hardware
> 
> "to a hardware switch"
> 
> > +switch. This offloading reduces the burden on the Linux kernel and CPU,
> > +leading to improved network performance and lower latency.
> > +
> > +To use Linux Bridge Switchdev, you need hardware switches that support the
> > +switchdev interface. This means that the switch hardware needs to have the
> > +necessary drivers and functionality to work in conjunction with the Linux
> > +kernel.
> 
> I'd add DSA maintainers to the CC list, and also ask switchdev driver
> maintainers to add more here. Switchdev can be explained much better.

Yes, you are right. I will add them in next version.
> > +
> > +Is it protocol independent?
> 
> Unclear, what layer?

How about "Is it L3/L4 protocol independent?"

> > +---------------------------
> > +
> > +Yes. The bridge knows nothing about protocols, it only sees Ethernet frames.
> 
> It sees all frames, it *uses* only L2 headers/information.
> 
> > diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> > index f95326fce6bb..63e39de1055b 100644
> > --- a/include/uapi/linux/if_bridge.h
> > +++ b/include/uapi/linux/if_bridge.h
> > + *
> > + * @IFLA_BR_MCAST_STARTUP_QUERY_CNT
> > + *   Set the number of IGMP queries to send during startup phase.
> 
> What is a startup phase?

https://datatracker.ietf.org/doc/html/rfc2236#section-8.7

I think it means when the bridge/switch up.
> 
> > + *
> > + *   The default value is 1.
> 
> What is 1?

1 second.

> 
> > + *
> > + * @IFLA_BR_MCAST_MEMBERSHIP_INTVL
> > + *   The interval after which the bridge will leave a group, if no membership
> > + *   reports for this group are received.
> > + *
> > + *   The default value is 260.
> 
> What is 260? Please be more specific.

OK, I will.

Thanks
Hangbin