Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b2420602-8d64-a75b-2296-059ac0ba26ef@nbd.name>
Date:   Sat, 12 Nov 2022 01:05:10 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.4.2
Content-Language: en-US
To:     Vladimir Oltean <olteanv@gmail.com>
Cc:     netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        linux-kernel@vger.kernel.org
References: <20221110212212.96825-1-nbd@nbd.name>
 <20221110212212.96825-2-nbd@nbd.name> <20221111233714.pmbc5qvq3g3hemhr@skbuf>
From:   Felix Fietkau <nbd@nbd.name>
Subject: Re: [PATCH net-next v3 1/4] net: dsa: add support for DSA rx
 offloading via metadata dst
In-Reply-To: <20221111233714.pmbc5qvq3g3hemhr@skbuf>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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