| Message ID | 20210713160745.59707-4-nbd@nbd.name |
|---|---|
| State | New |
| Headers | show |
| Series | Ethernet->WLAN hardware flow offloading support on MT7622 | expand |
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 >
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
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.
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 --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(ð->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(ð->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(ð->ppe, entry->hash); rhashtable_remove_fast(ð->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;
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(-)