diff mbox series

[RFC,3/7] net: ethernet: mtk_eth_soc: implement flow offloading to WED devices

Message ID 20210713160745.59707-4-nbd@nbd.name (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series Ethernet->WLAN hardware flow offloading support on MT7622 | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 15 maintainers not CCed: john@phrozen.org Mark-MC.Lee@mediatek.com linux-arm-kernel@lists.infradead.org sean.wang@mediatek.com andriin@fb.com matthias.bgg@gmail.com ast@kernel.org ap420073@gmail.com edumazet@google.com linux-mediatek@lists.infradead.org kuba@kernel.org alobakin@pm.me atenart@kernel.org davem@davemloft.net weiwan@google.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 4828 this patch: 4838
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch fail ERROR: do not use assignment in if condition WARNING: suspect code indent for conditional statements (8, 12)
netdev/build_allmodconfig_warn fail Errors and warnings before: 4897 this patch: 4897
netdev/header_inline success Link

Commit Message

Felix Fietkau July 13, 2021, 4:07 p.m. UTC
This allows hardware flow offloading from Ethernet to WLAN on MT7622 SoC

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 drivers/net/ethernet/mediatek/mtk_ppe.c       | 18 +++++
 drivers/net/ethernet/mediatek/mtk_ppe.h       | 14 ++--
 .../net/ethernet/mediatek/mtk_ppe_offload.c   | 67 ++++++++++++-------
 include/linux/netdevice.h                     |  7 ++
 net/core/dev.c                                |  4 ++
 5 files changed, 78 insertions(+), 32 deletions(-)

Comments

Pablo Neira Ayuso July 13, 2021, 6:40 p.m. UTC | #1
On Tue, Jul 13, 2021 at 06:07:41PM +0200, Felix Fietkau wrote:
[...]
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c253c2aafe97..7ea6a1db0338 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -885,6 +885,10 @@ int dev_fill_forward_path(const struct net_device *dev, const u8 *daddr,
>  		if (WARN_ON_ONCE(last_dev == ctx.dev))
>  			return -1;
>  	}
> +
> +	if (!ctx.dev)
> +		return ret;

This is not a safety check, right? After this update ctx.dev might be NULL?

> +
>  	path = dev_fwd_path(stack);
>  	if (!path)
>  		return -1;
> -- 
> 2.30.1
>
Pablo Neira Ayuso July 13, 2021, 6:56 p.m. UTC | #2
On Tue, Jul 13, 2021 at 06:07:41PM +0200, Felix Fietkau wrote:
> This allows hardware flow offloading from Ethernet to WLAN on MT7622 SoC
> 
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>  drivers/net/ethernet/mediatek/mtk_ppe.c       | 18 +++++
>  drivers/net/ethernet/mediatek/mtk_ppe.h       | 14 ++--
>  .../net/ethernet/mediatek/mtk_ppe_offload.c   | 67 ++++++++++++-------
>  include/linux/netdevice.h                     |  7 ++
>  net/core/dev.c                                |  4 ++
>  5 files changed, 78 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c b/drivers/net/ethernet/mediatek/mtk_ppe.c
> index 3ad10c793308..472bcd3269a7 100644
> --- a/drivers/net/ethernet/mediatek/mtk_ppe.c
> +++ b/drivers/net/ethernet/mediatek/mtk_ppe.c
> @@ -329,6 +329,24 @@ int mtk_foe_entry_set_pppoe(struct mtk_foe_entry *entry, int sid)
>  	return 0;
>  }
>  
> +int mtk_foe_entry_set_wdma(struct mtk_foe_entry *entry, int wdma_idx, int txq,
> +			   int bss, int wcid)
> +{
> +	struct mtk_foe_mac_info *l2 = mtk_foe_entry_l2(entry);
> +	u32 *ib2 = mtk_foe_entry_ib2(entry);
> +
> +	*ib2 &= ~MTK_FOE_IB2_PORT_MG;
> +	*ib2 |= MTK_FOE_IB2_WDMA_WINFO;
> +	if (wdma_idx)
> +		*ib2 |= MTK_FOE_IB2_WDMA_DEVIDX;
> +
> +	l2->vlan2 = FIELD_PREP(MTK_FOE_VLAN2_WINFO_BSS, bss) |
> +		    FIELD_PREP(MTK_FOE_VLAN2_WINFO_WCID, wcid) |
> +		    FIELD_PREP(MTK_FOE_VLAN2_WINFO_RING, txq);
> +
> +	return 0;
> +}
> +
>  static inline bool mtk_foe_entry_usable(struct mtk_foe_entry *entry)
>  {
>  	return !(entry->ib1 & MTK_FOE_IB1_STATIC) &&
> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.h b/drivers/net/ethernet/mediatek/mtk_ppe.h
> index 242fb8f2ae65..df8ccaf48171 100644
> --- a/drivers/net/ethernet/mediatek/mtk_ppe.h
> +++ b/drivers/net/ethernet/mediatek/mtk_ppe.h
> @@ -48,9 +48,9 @@ enum {
>  #define MTK_FOE_IB2_DEST_PORT		GENMASK(7, 5)
>  #define MTK_FOE_IB2_MULTICAST		BIT(8)
>  
> -#define MTK_FOE_IB2_WHNAT_QID2		GENMASK(13, 12)
> -#define MTK_FOE_IB2_WHNAT_DEVIDX	BIT(16)
> -#define MTK_FOE_IB2_WHNAT_NAT		BIT(17)
> +#define MTK_FOE_IB2_WDMA_QID2		GENMASK(13, 12)
> +#define MTK_FOE_IB2_WDMA_DEVIDX		BIT(16)
> +#define MTK_FOE_IB2_WDMA_WINFO		BIT(17)
>  
>  #define MTK_FOE_IB2_PORT_MG		GENMASK(17, 12)
>  
> @@ -58,9 +58,9 @@ enum {
>  
>  #define MTK_FOE_IB2_DSCP		GENMASK(31, 24)
>  
> -#define MTK_FOE_VLAN2_WHNAT_BSS		GEMMASK(5, 0)
> -#define MTK_FOE_VLAN2_WHNAT_WCID	GENMASK(13, 6)
> -#define MTK_FOE_VLAN2_WHNAT_RING	GENMASK(15, 14)
> +#define MTK_FOE_VLAN2_WINFO_BSS		GENMASK(5, 0)
> +#define MTK_FOE_VLAN2_WINFO_WCID	GENMASK(13, 6)
> +#define MTK_FOE_VLAN2_WINFO_RING	GENMASK(15, 14)
>  
>  enum {
>  	MTK_FOE_STATE_INVALID,
> @@ -281,6 +281,8 @@ int mtk_foe_entry_set_ipv6_tuple(struct mtk_foe_entry *entry,
>  int mtk_foe_entry_set_dsa(struct mtk_foe_entry *entry, int port);
>  int mtk_foe_entry_set_vlan(struct mtk_foe_entry *entry, int vid);
>  int mtk_foe_entry_set_pppoe(struct mtk_foe_entry *entry, int sid);
> +int mtk_foe_entry_set_wdma(struct mtk_foe_entry *entry, int wdma_idx, int txq,
> +			   int bss, int wcid);
>  int mtk_foe_entry_commit(struct mtk_ppe *ppe, struct mtk_foe_entry *entry,
>  			 u16 timestamp);
>  int mtk_ppe_debugfs_init(struct mtk_ppe *ppe);
> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> index b5f68f66d42a..00b1d06f60d1 100644
> --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> @@ -10,6 +10,7 @@
>  #include <net/pkt_cls.h>
>  #include <net/dsa.h>
>  #include "mtk_eth_soc.h"
> +#include "mtk_wed.h"
>  
>  struct mtk_flow_data {
>  	struct ethhdr eth;
> @@ -39,6 +40,7 @@ struct mtk_flow_entry {
>  	struct rhash_head node;
>  	unsigned long cookie;
>  	u16 hash;
> +	s8 wed_index;
>  };
>  
>  static const struct rhashtable_params mtk_flow_ht_params = {
> @@ -127,35 +129,38 @@ mtk_flow_mangle_ipv4(const struct flow_action_entry *act,
>  }
>  
>  static int
> -mtk_flow_get_dsa_port(struct net_device **dev)
> +mtk_flow_set_output_device(struct mtk_eth *eth, struct mtk_foe_entry *foe,
> +			   struct net_device *dev, const u8 *dest_mac,
> +			   int *wed_index)
>  {
> -#if IS_ENABLED(CONFIG_NET_DSA)
> -	struct dsa_port *dp;
> -
> -	dp = dsa_port_from_netdev(*dev);
> -	if (IS_ERR(dp))
> -		return -ENODEV;
> -
> -	if (dp->cpu_dp->tag_ops->proto != DSA_TAG_PROTO_MTK)
> -		return -ENODEV;
> +	struct net_device_path_ctx ctx = {
> +		.dev    = dev,
> +		.daddr  = dest_mac,
> +	};
> +	struct net_device_path path = {};
> +	int pse_port;
>  
> -	*dev = dp->cpu_dp->master;
> +	if (!dev->netdev_ops->ndo_fill_forward_path ||
> +	    dev->netdev_ops->ndo_fill_forward_path(&ctx, &path) < 0)
> +		path.type = DEV_PATH_ETHERNET;

Maybe expose this through flow offload API so there is no need to call
ndo_fill_forward_path again from the driver?

> -	return dp->index;
> -#else
> -	return -ENODEV;
> -#endif
> -}
> -
> -static int
> -mtk_flow_set_output_device(struct mtk_eth *eth, struct mtk_foe_entry *foe,
> -			   struct net_device *dev)
> -{
> -	int pse_port, dsa_port;
> +	switch (path.type) {
> +	case DEV_PATH_DSA:

This DSA update is not related, right?
Felix Fietkau July 14, 2021, 8:21 a.m. UTC | #3
On 2021-07-13 20:40, Pablo Neira Ayuso wrote:
> On Tue, Jul 13, 2021 at 06:07:41PM +0200, Felix Fietkau wrote:
> [...]
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index c253c2aafe97..7ea6a1db0338 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -885,6 +885,10 @@ int dev_fill_forward_path(const struct net_device *dev, const u8 *daddr,
>>  		if (WARN_ON_ONCE(last_dev == ctx.dev))
>>  			return -1;
>>  	}
>> +
>> +	if (!ctx.dev)
>> +		return ret;
> 
> This is not a safety check, right? After this update ctx.dev might be NULL?
Right. I added this check to be able to prevent dev_fill_forward_path
from adding an extra DEV_PATH_ETHERNET entry, which is not applicable
for wlan devices.

- Felix
Felix Fietkau July 14, 2021, 8:26 a.m. UTC | #4
On 2021-07-13 20:56, Pablo Neira Ayuso wrote:
> On Tue, Jul 13, 2021 at 06:07:41PM +0200, Felix Fietkau wrote:
>> This allows hardware flow offloading from Ethernet to WLAN on MT7622 SoC
>> 
>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>> ---
>>  drivers/net/ethernet/mediatek/mtk_ppe.c       | 18 +++++
>>  drivers/net/ethernet/mediatek/mtk_ppe.h       | 14 ++--
>>  .../net/ethernet/mediatek/mtk_ppe_offload.c   | 67 ++++++++++++-------
>>  include/linux/netdevice.h                     |  7 ++
>>  net/core/dev.c                                |  4 ++
>>  5 files changed, 78 insertions(+), 32 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c b/drivers/net/ethernet/mediatek/mtk_ppe.c
>> index 3ad10c793308..472bcd3269a7 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_ppe.c
>> +++ b/drivers/net/ethernet/mediatek/mtk_ppe.c
>> @@ -329,6 +329,24 @@ int mtk_foe_entry_set_pppoe(struct mtk_foe_entry *entry, int sid)
>>  	return 0;
>>  }
>>  
>> +int mtk_foe_entry_set_wdma(struct mtk_foe_entry *entry, int wdma_idx, int txq,
>> +			   int bss, int wcid)
>> +{
>> +	struct mtk_foe_mac_info *l2 = mtk_foe_entry_l2(entry);
>> +	u32 *ib2 = mtk_foe_entry_ib2(entry);
>> +
>> +	*ib2 &= ~MTK_FOE_IB2_PORT_MG;
>> +	*ib2 |= MTK_FOE_IB2_WDMA_WINFO;
>> +	if (wdma_idx)
>> +		*ib2 |= MTK_FOE_IB2_WDMA_DEVIDX;
>> +
>> +	l2->vlan2 = FIELD_PREP(MTK_FOE_VLAN2_WINFO_BSS, bss) |
>> +		    FIELD_PREP(MTK_FOE_VLAN2_WINFO_WCID, wcid) |
>> +		    FIELD_PREP(MTK_FOE_VLAN2_WINFO_RING, txq);
>> +
>> +	return 0;
>> +}
>> +
>>  static inline bool mtk_foe_entry_usable(struct mtk_foe_entry *entry)
>>  {
>>  	return !(entry->ib1 & MTK_FOE_IB1_STATIC) &&
>> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.h b/drivers/net/ethernet/mediatek/mtk_ppe.h
>> index 242fb8f2ae65..df8ccaf48171 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_ppe.h
>> +++ b/drivers/net/ethernet/mediatek/mtk_ppe.h
>> @@ -48,9 +48,9 @@ enum {
>>  #define MTK_FOE_IB2_DEST_PORT		GENMASK(7, 5)
>>  #define MTK_FOE_IB2_MULTICAST		BIT(8)
>>  
>> -#define MTK_FOE_IB2_WHNAT_QID2		GENMASK(13, 12)
>> -#define MTK_FOE_IB2_WHNAT_DEVIDX	BIT(16)
>> -#define MTK_FOE_IB2_WHNAT_NAT		BIT(17)
>> +#define MTK_FOE_IB2_WDMA_QID2		GENMASK(13, 12)
>> +#define MTK_FOE_IB2_WDMA_DEVIDX		BIT(16)
>> +#define MTK_FOE_IB2_WDMA_WINFO		BIT(17)
>>  
>>  #define MTK_FOE_IB2_PORT_MG		GENMASK(17, 12)
>>  
>> @@ -58,9 +58,9 @@ enum {
>>  
>>  #define MTK_FOE_IB2_DSCP		GENMASK(31, 24)
>>  
>> -#define MTK_FOE_VLAN2_WHNAT_BSS		GEMMASK(5, 0)
>> -#define MTK_FOE_VLAN2_WHNAT_WCID	GENMASK(13, 6)
>> -#define MTK_FOE_VLAN2_WHNAT_RING	GENMASK(15, 14)
>> +#define MTK_FOE_VLAN2_WINFO_BSS		GENMASK(5, 0)
>> +#define MTK_FOE_VLAN2_WINFO_WCID	GENMASK(13, 6)
>> +#define MTK_FOE_VLAN2_WINFO_RING	GENMASK(15, 14)
>>  
>>  enum {
>>  	MTK_FOE_STATE_INVALID,
>> @@ -281,6 +281,8 @@ int mtk_foe_entry_set_ipv6_tuple(struct mtk_foe_entry *entry,
>>  int mtk_foe_entry_set_dsa(struct mtk_foe_entry *entry, int port);
>>  int mtk_foe_entry_set_vlan(struct mtk_foe_entry *entry, int vid);
>>  int mtk_foe_entry_set_pppoe(struct mtk_foe_entry *entry, int sid);
>> +int mtk_foe_entry_set_wdma(struct mtk_foe_entry *entry, int wdma_idx, int txq,
>> +			   int bss, int wcid);
>>  int mtk_foe_entry_commit(struct mtk_ppe *ppe, struct mtk_foe_entry *entry,
>>  			 u16 timestamp);
>>  int mtk_ppe_debugfs_init(struct mtk_ppe *ppe);
>> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
>> index b5f68f66d42a..00b1d06f60d1 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
>> +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
>> @@ -10,6 +10,7 @@
>>  #include <net/pkt_cls.h>
>>  #include <net/dsa.h>
>>  #include "mtk_eth_soc.h"
>> +#include "mtk_wed.h"
>>  
>>  struct mtk_flow_data {
>>  	struct ethhdr eth;
>> @@ -39,6 +40,7 @@ struct mtk_flow_entry {
>>  	struct rhash_head node;
>>  	unsigned long cookie;
>>  	u16 hash;
>> +	s8 wed_index;
>>  };
>>  
>>  static const struct rhashtable_params mtk_flow_ht_params = {
>> @@ -127,35 +129,38 @@ mtk_flow_mangle_ipv4(const struct flow_action_entry *act,
>>  }
>>  
>>  static int
>> -mtk_flow_get_dsa_port(struct net_device **dev)
>> +mtk_flow_set_output_device(struct mtk_eth *eth, struct mtk_foe_entry *foe,
>> +			   struct net_device *dev, const u8 *dest_mac,
>> +			   int *wed_index)
>>  {
>> -#if IS_ENABLED(CONFIG_NET_DSA)
>> -	struct dsa_port *dp;
>> -
>> -	dp = dsa_port_from_netdev(*dev);
>> -	if (IS_ERR(dp))
>> -		return -ENODEV;
>> -
>> -	if (dp->cpu_dp->tag_ops->proto != DSA_TAG_PROTO_MTK)
>> -		return -ENODEV;
>> +	struct net_device_path_ctx ctx = {
>> +		.dev    = dev,
>> +		.daddr  = dest_mac,
>> +	};
>> +	struct net_device_path path = {};
>> +	int pse_port;
>>  
>> -	*dev = dp->cpu_dp->master;
>> +	if (!dev->netdev_ops->ndo_fill_forward_path ||
>> +	    dev->netdev_ops->ndo_fill_forward_path(&ctx, &path) < 0)
>> +		path.type = DEV_PATH_ETHERNET;
> 
> Maybe expose this through flow offload API so there is no need to call
> ndo_fill_forward_path again from the driver?
Can you give me a pseudo-code example? I'm not sure how you want it to
be exposed through the flow offload API.
To me it seems easier and cleaner to just have a single
ndo_fill_forward_path call for the final output device to check the
device types that don't have any corresponding sw offload.

>> -	return dp->index;
>> -#else
>> -	return -ENODEV;
>> -#endif
>> -}
>> -
>> -static int
>> -mtk_flow_set_output_device(struct mtk_eth *eth, struct mtk_foe_entry *foe,
>> -			   struct net_device *dev)
>> -{
>> -	int pse_port, dsa_port;
>> +	switch (path.type) {
>> +	case DEV_PATH_DSA:
> 
> This DSA update is not related, right?
I consider it related. Since I'm calling ndo_fill_forward_path now, it's
better to use it for both DSA and WLAN instead of having independent checks.

- Felix
Pablo Neira Ayuso July 15, 2021, 9:36 p.m. UTC | #5
On Wed, Jul 14, 2021 at 10:26:08AM +0200, Felix Fietkau wrote:
> On 2021-07-13 20:56, Pablo Neira Ayuso wrote:
[...]
> >> --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> >> +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> >> @@ -10,6 +10,7 @@
> >>  #include <net/pkt_cls.h>
> >>  #include <net/dsa.h>
> >>  #include "mtk_eth_soc.h"
> >> +#include "mtk_wed.h"
> >>  
> >>  struct mtk_flow_data {
> >>  	struct ethhdr eth;
> >> @@ -39,6 +40,7 @@ struct mtk_flow_entry {
> >>  	struct rhash_head node;
> >>  	unsigned long cookie;
> >>  	u16 hash;
> >> +	s8 wed_index;
> >>  };
> >>  
> >>  static const struct rhashtable_params mtk_flow_ht_params = {
> >> @@ -127,35 +129,38 @@ mtk_flow_mangle_ipv4(const struct flow_action_entry *act,
> >>  }
> >>  
> >>  static int
> >> -mtk_flow_get_dsa_port(struct net_device **dev)
> >> +mtk_flow_set_output_device(struct mtk_eth *eth, struct mtk_foe_entry *foe,
> >> +			   struct net_device *dev, const u8 *dest_mac,
> >> +			   int *wed_index)
> >>  {
> >> -#if IS_ENABLED(CONFIG_NET_DSA)
> >> -	struct dsa_port *dp;
> >> -
> >> -	dp = dsa_port_from_netdev(*dev);
> >> -	if (IS_ERR(dp))
> >> -		return -ENODEV;
> >> -
> >> -	if (dp->cpu_dp->tag_ops->proto != DSA_TAG_PROTO_MTK)
> >> -		return -ENODEV;
> >> +	struct net_device_path_ctx ctx = {
> >> +		.dev    = dev,
> >> +		.daddr  = dest_mac,
> >> +	};
> >> +	struct net_device_path path = {};
> >> +	int pse_port;
> >>  
> >> -	*dev = dp->cpu_dp->master;
> >> +	if (!dev->netdev_ops->ndo_fill_forward_path ||
> >> +	    dev->netdev_ops->ndo_fill_forward_path(&ctx, &path) < 0)
> >> +		path.type = DEV_PATH_ETHERNET;
> > 
> > Maybe expose this through flow offload API so there is no need to call
> > ndo_fill_forward_path again from the driver?
>
> Can you give me a pseudo-code example? I'm not sure how you want it to
> be exposed through the flow offload API.

in a few steps:

1) Extend nft_dev_path_info() to deal with DEV_PATH_WDMA, it will
   just actually fetch a pointer to structure that is allocated
   by the driver.

- Update the net_device_path structure with this layout:

        struct flow_action_wdma {
                enum wdma_type type;    // MTK_WDMA goes here
                union {
                        struct {
                                ...;
                        } mtk;
                };
        } wdma;

Add:
        struct flow_action_wdma         *wdma;

to net_device_path.

2) Pass on this pointer to structure to the nf_flow_route
   wheelbarrow.

3) Store this information in the struct flow_offload_tuple,
   in a new struct flow_offload_hw *field to store all hardware
   offload specific information (not needed by software path). There
   is already hw_outdev that can be placed there.

4) Add a FLOW_ACTION_WDMA action to the flow offload API to
   pass on the flow_action_wdma structure.

It's a bit of work the first time to accomodate the requirements of
new API, but then all drivers will benefit from this.

It's also a bit of layering, but with more drivers in the tree, this
API can be simplified incrementally.

I can take a stab at it and send you a patch.

> To me it seems easier and cleaner to just have a single
> ndo_fill_forward_path call for the final output device to check the
> device types that don't have any corresponding sw offload.

It's simpler yes, but this results in two calls for
ndo_fill_forward_path, one from the core and another from the driver.
I think it's better there's a single point to call to
ndo_fill_forward_path for consolidation.

My proposal requires a bit more plumbing, but all drivers will
get the information that represents the offload in the same way.
Felix Fietkau July 16, 2021, 6:09 a.m. UTC | #6
On 2021-07-15 23:36, Pablo Neira Ayuso wrote:
> On Wed, Jul 14, 2021 at 10:26:08AM +0200, Felix Fietkau wrote:
>> On 2021-07-13 20:56, Pablo Neira Ayuso wrote:
> [...]
>> >> --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
>> >> +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
>> >> @@ -10,6 +10,7 @@
>> >>  #include <net/pkt_cls.h>
>> >>  #include <net/dsa.h>
>> >>  #include "mtk_eth_soc.h"
>> >> +#include "mtk_wed.h"
>> >>  
>> >>  struct mtk_flow_data {
>> >>  	struct ethhdr eth;
>> >> @@ -39,6 +40,7 @@ struct mtk_flow_entry {
>> >>  	struct rhash_head node;
>> >>  	unsigned long cookie;
>> >>  	u16 hash;
>> >> +	s8 wed_index;
>> >>  };
>> >>  
>> >>  static const struct rhashtable_params mtk_flow_ht_params = {
>> >> @@ -127,35 +129,38 @@ mtk_flow_mangle_ipv4(const struct flow_action_entry *act,
>> >>  }
>> >>  
>> >>  static int
>> >> -mtk_flow_get_dsa_port(struct net_device **dev)
>> >> +mtk_flow_set_output_device(struct mtk_eth *eth, struct mtk_foe_entry *foe,
>> >> +			   struct net_device *dev, const u8 *dest_mac,
>> >> +			   int *wed_index)
>> >>  {
>> >> -#if IS_ENABLED(CONFIG_NET_DSA)
>> >> -	struct dsa_port *dp;
>> >> -
>> >> -	dp = dsa_port_from_netdev(*dev);
>> >> -	if (IS_ERR(dp))
>> >> -		return -ENODEV;
>> >> -
>> >> -	if (dp->cpu_dp->tag_ops->proto != DSA_TAG_PROTO_MTK)
>> >> -		return -ENODEV;
>> >> +	struct net_device_path_ctx ctx = {
>> >> +		.dev    = dev,
>> >> +		.daddr  = dest_mac,
>> >> +	};
>> >> +	struct net_device_path path = {};
>> >> +	int pse_port;
>> >>  
>> >> -	*dev = dp->cpu_dp->master;
>> >> +	if (!dev->netdev_ops->ndo_fill_forward_path ||
>> >> +	    dev->netdev_ops->ndo_fill_forward_path(&ctx, &path) < 0)
>> >> +		path.type = DEV_PATH_ETHERNET;
>> > 
>> > Maybe expose this through flow offload API so there is no need to call
>> > ndo_fill_forward_path again from the driver?
>>
>> Can you give me a pseudo-code example? I'm not sure how you want it to
>> be exposed through the flow offload API.
> 
> in a few steps:
> 
> 1) Extend nft_dev_path_info() to deal with DEV_PATH_WDMA, it will
>    just actually fetch a pointer to structure that is allocated
>    by the driver.
> 
> - Update the net_device_path structure with this layout:
> 
>         struct flow_action_wdma {
>                 enum wdma_type type;    // MTK_WDMA goes here
>                 union {
>                         struct {
>                                 ...;
>                         } mtk;
>                 };
>         } wdma;
> 
> Add:
>         struct flow_action_wdma         *wdma;
> 
> to net_device_path.
> 
> 2) Pass on this pointer to structure to the nf_flow_route
>    wheelbarrow.
> 
> 3) Store this information in the struct flow_offload_tuple,
>    in a new struct flow_offload_hw *field to store all hardware
>    offload specific information (not needed by software path). There
>    is already hw_outdev that can be placed there.
> 
> 4) Add a FLOW_ACTION_WDMA action to the flow offload API to
>    pass on the flow_action_wdma structure.
I think it should probably be called FLOW_ACTION_MTK_WDMA (and be
mediatek specific), or we should pick a more generic name for it.

> It's a bit of work the first time to accomodate the requirements of
> new API, but then all drivers will benefit from this.
> 
> It's also a bit of layering, but with more drivers in the tree, this
> API can be simplified incrementally.
> 
> I can take a stab at it and send you a patch.
Thanks. If we do this for WDMA, shouldn't we also do it for DSA to keep
things consistent?

- Felix
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c b/drivers/net/ethernet/mediatek/mtk_ppe.c
index 3ad10c793308..472bcd3269a7 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe.c
+++ b/drivers/net/ethernet/mediatek/mtk_ppe.c
@@ -329,6 +329,24 @@  int mtk_foe_entry_set_pppoe(struct mtk_foe_entry *entry, int sid)
 	return 0;
 }
 
+int mtk_foe_entry_set_wdma(struct mtk_foe_entry *entry, int wdma_idx, int txq,
+			   int bss, int wcid)
+{
+	struct mtk_foe_mac_info *l2 = mtk_foe_entry_l2(entry);
+	u32 *ib2 = mtk_foe_entry_ib2(entry);
+
+	*ib2 &= ~MTK_FOE_IB2_PORT_MG;
+	*ib2 |= MTK_FOE_IB2_WDMA_WINFO;
+	if (wdma_idx)
+		*ib2 |= MTK_FOE_IB2_WDMA_DEVIDX;
+
+	l2->vlan2 = FIELD_PREP(MTK_FOE_VLAN2_WINFO_BSS, bss) |
+		    FIELD_PREP(MTK_FOE_VLAN2_WINFO_WCID, wcid) |
+		    FIELD_PREP(MTK_FOE_VLAN2_WINFO_RING, txq);
+
+	return 0;
+}
+
 static inline bool mtk_foe_entry_usable(struct mtk_foe_entry *entry)
 {
 	return !(entry->ib1 & MTK_FOE_IB1_STATIC) &&
diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.h b/drivers/net/ethernet/mediatek/mtk_ppe.h
index 242fb8f2ae65..df8ccaf48171 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe.h
+++ b/drivers/net/ethernet/mediatek/mtk_ppe.h
@@ -48,9 +48,9 @@  enum {
 #define MTK_FOE_IB2_DEST_PORT		GENMASK(7, 5)
 #define MTK_FOE_IB2_MULTICAST		BIT(8)
 
-#define MTK_FOE_IB2_WHNAT_QID2		GENMASK(13, 12)
-#define MTK_FOE_IB2_WHNAT_DEVIDX	BIT(16)
-#define MTK_FOE_IB2_WHNAT_NAT		BIT(17)
+#define MTK_FOE_IB2_WDMA_QID2		GENMASK(13, 12)
+#define MTK_FOE_IB2_WDMA_DEVIDX		BIT(16)
+#define MTK_FOE_IB2_WDMA_WINFO		BIT(17)
 
 #define MTK_FOE_IB2_PORT_MG		GENMASK(17, 12)
 
@@ -58,9 +58,9 @@  enum {
 
 #define MTK_FOE_IB2_DSCP		GENMASK(31, 24)
 
-#define MTK_FOE_VLAN2_WHNAT_BSS		GEMMASK(5, 0)
-#define MTK_FOE_VLAN2_WHNAT_WCID	GENMASK(13, 6)
-#define MTK_FOE_VLAN2_WHNAT_RING	GENMASK(15, 14)
+#define MTK_FOE_VLAN2_WINFO_BSS		GENMASK(5, 0)
+#define MTK_FOE_VLAN2_WINFO_WCID	GENMASK(13, 6)
+#define MTK_FOE_VLAN2_WINFO_RING	GENMASK(15, 14)
 
 enum {
 	MTK_FOE_STATE_INVALID,
@@ -281,6 +281,8 @@  int mtk_foe_entry_set_ipv6_tuple(struct mtk_foe_entry *entry,
 int mtk_foe_entry_set_dsa(struct mtk_foe_entry *entry, int port);
 int mtk_foe_entry_set_vlan(struct mtk_foe_entry *entry, int vid);
 int mtk_foe_entry_set_pppoe(struct mtk_foe_entry *entry, int sid);
+int mtk_foe_entry_set_wdma(struct mtk_foe_entry *entry, int wdma_idx, int txq,
+			   int bss, int wcid);
 int mtk_foe_entry_commit(struct mtk_ppe *ppe, struct mtk_foe_entry *entry,
 			 u16 timestamp);
 int mtk_ppe_debugfs_init(struct mtk_ppe *ppe);
diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
index b5f68f66d42a..00b1d06f60d1 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
+++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
@@ -10,6 +10,7 @@ 
 #include <net/pkt_cls.h>
 #include <net/dsa.h>
 #include "mtk_eth_soc.h"
+#include "mtk_wed.h"
 
 struct mtk_flow_data {
 	struct ethhdr eth;
@@ -39,6 +40,7 @@  struct mtk_flow_entry {
 	struct rhash_head node;
 	unsigned long cookie;
 	u16 hash;
+	s8 wed_index;
 };
 
 static const struct rhashtable_params mtk_flow_ht_params = {
@@ -127,35 +129,38 @@  mtk_flow_mangle_ipv4(const struct flow_action_entry *act,
 }
 
 static int
-mtk_flow_get_dsa_port(struct net_device **dev)
+mtk_flow_set_output_device(struct mtk_eth *eth, struct mtk_foe_entry *foe,
+			   struct net_device *dev, const u8 *dest_mac,
+			   int *wed_index)
 {
-#if IS_ENABLED(CONFIG_NET_DSA)
-	struct dsa_port *dp;
-
-	dp = dsa_port_from_netdev(*dev);
-	if (IS_ERR(dp))
-		return -ENODEV;
-
-	if (dp->cpu_dp->tag_ops->proto != DSA_TAG_PROTO_MTK)
-		return -ENODEV;
+	struct net_device_path_ctx ctx = {
+		.dev    = dev,
+		.daddr  = dest_mac,
+	};
+	struct net_device_path path = {};
+	int pse_port;
 
-	*dev = dp->cpu_dp->master;
+	if (!dev->netdev_ops->ndo_fill_forward_path ||
+	    dev->netdev_ops->ndo_fill_forward_path(&ctx, &path) < 0)
+		path.type = DEV_PATH_ETHERNET;
 
-	return dp->index;
-#else
-	return -ENODEV;
-#endif
-}
-
-static int
-mtk_flow_set_output_device(struct mtk_eth *eth, struct mtk_foe_entry *foe,
-			   struct net_device *dev)
-{
-	int pse_port, dsa_port;
+	switch (path.type) {
+	case DEV_PATH_DSA:
+		if (path.dsa.proto != DSA_TAG_PROTO_MTK)
+			break;
 
-	dsa_port = mtk_flow_get_dsa_port(&dev);
-	if (dsa_port >= 0)
-		mtk_foe_entry_set_dsa(foe, dsa_port);
+		mtk_foe_entry_set_dsa(foe, path.dsa.port);
+		break;
+	case DEV_PATH_MTK_WDMA:
+		mtk_foe_entry_set_wdma(foe, path.mtk_wdma.wdma_idx,
+				       path.mtk_wdma.queue, path.mtk_wdma.bss,
+				       path.mtk_wdma.wcid);
+		mtk_foe_entry_set_pse_port(foe, 3);
+		*wed_index = path.mtk_wdma.wdma_idx;
+		return 0;
+	default:
+		break;
+	}
 
 	if (dev == eth->netdev[0])
 		pse_port = 1;
@@ -179,6 +184,7 @@  mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f)
 	struct net_device *odev = NULL;
 	struct mtk_flow_entry *entry;
 	int offload_type = 0;
+	int wed_index = -1;
 	u16 addr_type = 0;
 	u32 timestamp;
 	u8 l4proto = 0;
@@ -323,10 +329,14 @@  mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f)
 	if (data.pppoe.num == 1)
 		mtk_foe_entry_set_pppoe(&foe, data.pppoe.sid);
 
-	err = mtk_flow_set_output_device(eth, &foe, odev);
+	err = mtk_flow_set_output_device(eth, &foe, odev, data.eth.h_dest,
+					 &wed_index);
 	if (err)
 		return err;
 
+	if (wed_index >= 0 && (err = mtk_wed_flow_add(wed_index)) < 0)
+		return err;
+
 	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
 	if (!entry)
 		return -ENOMEM;
@@ -340,6 +350,7 @@  mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f)
 	}
 
 	entry->hash = hash;
+	entry->wed_index = wed_index;
 	err = rhashtable_insert_fast(&eth->flow_table, &entry->node,
 				     mtk_flow_ht_params);
 	if (err < 0)
@@ -350,6 +361,8 @@  mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f)
 	mtk_foe_entry_clear(&eth->ppe, hash);
 free:
 	kfree(entry);
+	if (wed_index >= 0)
+	    mtk_wed_flow_remove(wed_index);
 	return err;
 }
 
@@ -366,6 +379,8 @@  mtk_flow_offload_destroy(struct mtk_eth *eth, struct flow_cls_offload *f)
 	mtk_foe_entry_clear(&eth->ppe, entry->hash);
 	rhashtable_remove_fast(&eth->flow_table, &entry->node,
 			       mtk_flow_ht_params);
+	if (entry->wed_index >= 0)
+		mtk_wed_flow_remove(entry->wed_index);
 	kfree(entry);
 
 	return 0;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eaf5bb008aa9..67bba2826bc4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -856,6 +856,7 @@  enum net_device_path_type {
 	DEV_PATH_BRIDGE,
 	DEV_PATH_PPPOE,
 	DEV_PATH_DSA,
+	DEV_PATH_MTK_WDMA,
 };
 
 struct net_device_path {
@@ -881,6 +882,12 @@  struct net_device_path {
 			int port;
 			u16 proto;
 		} dsa;
+		struct {
+			u8 wdma_idx;
+			u8 queue;
+			u8 bss;
+			u8 wcid;
+		} mtk_wdma;
 	};
 };
 
diff --git a/net/core/dev.c b/net/core/dev.c
index c253c2aafe97..7ea6a1db0338 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -885,6 +885,10 @@  int dev_fill_forward_path(const struct net_device *dev, const u8 *daddr,
 		if (WARN_ON_ONCE(last_dev == ctx.dev))
 			return -1;
 	}
+
+	if (!ctx.dev)
+		return ret;
+
 	path = dev_fwd_path(stack);
 	if (!path)
 		return -1;