netdev - Re: [PATCH v3 net-next 2/4] net: dsa: Link aggregation support

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]

Message-ID: <87h7ov5pcu.fsf@waldekranz.com>
Date:   Wed, 09 Dec 2020 09:57:05 +0100
From:   Tobias Waldekranz <tobias@...dekranz.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     davem@...emloft.net, kuba@...nel.org, vivien.didelot@...il.com,
        f.fainelli@...il.com, olteanv@...il.com, j.vosburgh@...il.com,
        vfalico@...il.com, andy@...yhouse.net, netdev@...r.kernel.org
Subject: Re: [PATCH v3 net-next 2/4] net: dsa: Link aggregation support

On Tue, Dec 08, 2020 at 00:26, Andrew Lunn <andrew@...n.ch> wrote:
> On Mon, Dec 07, 2020 at 10:19:47PM +0100, Tobias Waldekranz wrote:
>> On Fri, Dec 04, 2020 at 03:20, Andrew Lunn <andrew@...n.ch> wrote:
>> >> +static inline bool dsa_port_can_offload(struct dsa_port *dp,
>> >> +					struct net_device *dev)
>> >
>> > That name is a bit generic. We have a number of different offloads.
>> > The mv88E6060 cannot offload anything!
>> 
>> The name is intentionally generic as it answers the question "can this
>> dp offload requests for this netdev?"
>
> I think it is a bit more specific. Mirroring is an offload, but is not
> taken into account here, and does mirroring setup call this to see if
> mirroring can be offloaded? The hardware has rate control traffic
> shaping which we might sometime add support for via TC. That again is
> an offload.

I see what you are saying.

>> >> +{
>> >> +	/* Switchdev offloading can be configured on: */
>> >> +
>> >> +	if (dev == dp->slave)
>> >> +		/* DSA ports directly connected to a bridge. */
>> >> +		return true;
>> 
>> This condition is the normal case of a bridged port, i.e. no LAG
>> involved.
>> 
>> >> +	if (dp->lag && dev == rtnl_dereference(dp->lag->dev))
>> >> +		/* DSA ports connected to a bridge via a LAG */
>> >> +		return true;
>> 
>> And then the indirect case of a bridged port under a LAG.
>> 
>> I am happy to take requests for a better name though.
>
> There probably needs to be lag in the name.

I disagree. A LAG is one type of netdev that a DSA port can offload. The
other one is the DSA port's own netdev, i.e. what we have had since time
immemorial.

dsa_port_offloads_netdev(dp, dev)?