Re: [PATCH net-next v3 1/4] net: dsa: add support for DSA rx offloading via metadata dst

[Felix Fietkau <nbd@nbd.name>]

On 12.11.22 00:37, Vladimir Oltean wrote:
> On Thu, Nov 10, 2022 at 10:22:08PM +0100, Felix Fietkau wrote:
>> If a metadata dst is present with the type METADATA_HW_PORT_MUX on a dsa cpu
>> port netdev, assume that it carries the port number and that there is no DSA
>> tag present in the skb data.
>> 
>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>>  				else
>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>> index 64b14f655b23..0b67622cf905 100644
>> --- a/net/dsa/dsa.c
>> +++ b/net/dsa/dsa.c
>> @@ -11,6 +11,7 @@
>>  #include <linux/netdevice.h>
>>  #include <linux/sysfs.h>
>>  #include <linux/ptp_classify.h>
>> +#include <net/dst_metadata.h>
>>  
>>  #include "dsa_priv.h"
>>  
>> @@ -216,6 +217,7 @@ static bool dsa_skb_defer_rx_timestamp(struct dsa_slave_priv *p,
>>  static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
>>  			  struct packet_type *pt, struct net_device *unused)
>>  {
>> +	struct metadata_dst *md_dst = skb_metadata_dst(skb);
>>  	struct dsa_port *cpu_dp = dev->dsa_ptr;
>>  	struct sk_buff *nskb = NULL;
>>  	struct dsa_slave_priv *p;
>> @@ -229,7 +231,22 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
>>  	if (!skb)
>>  		return 0;
>>  
>> -	nskb = cpu_dp->rcv(skb, dev);
>> +	if (md_dst && md_dst->type == METADATA_HW_PORT_MUX) {
>> +		unsigned int port = md_dst->u.port_info.port_id;
>> +
>> +		skb_dst_set(skb, NULL);
> 
> If you insist on not using the refcounting feature and free your
> metadata_dst in the master's remove() function, that's going to
> invalidate absolutely any point I'm trying to make. Normally I'd leave
> you alone, however I really don't like that this is also forcing DSA to
> not use the refcount, and therefore, that it's forcing any other driver
> to do the same as mtk_eth_soc. Not sure how that's gonna scale in the
> hypothetical future when there will be a DSA master which can offload
> RX DSA tags, *and* the switch can change tagging protocols dynamically
> (which will force the master to allocate/free its metadata dst's at
> runtime too). I guess that will be for me to figure out, which I don't
> like.
> 
> Jakub, what do you think? Refcounting or no refcounting?
If think that refcounting for the metadata dst is useful in this case, I 
can replace the skb_dst_set call with skb_dst_drop() in v4, so it would 
work for both cases.

- Felix