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

[Tobias Waldekranz <tobias@waldekranz.com>]

On Tue, Dec 08, 2020 at 00:26, Andrew Lunn <andrew@lunn.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@lunn.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)?