Message ID | 20210713160745.59707-1-nbd@nbd.name |
---|---|
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: > 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?
On 2021-07-13 20:31, Andrew Lunn wrote: >> diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c >> + >> +static inline void >> +wed_m32(struct mtk_wed_device *dev, u32 reg, u32 mask, u32 val) >> +{ >> + regmap_update_bits(dev->hw->regs, reg, mask | val, val); >> +} > > Please don't use inline functions in .c files. Let the compiler > decide. Will drop inline >> +static void >> +mtk_wed_reset(struct mtk_wed_device *dev, u32 mask) >> +{ >> + int i; >> + >> + wed_w32(dev, MTK_WED_RESET, mask); >> + for (i = 0; i < 100; i++) { >> + if (wed_r32(dev, MTK_WED_RESET) & mask) >> + continue; >> + >> + return; >> + } > > It may be better to use something from iopoll.h Will do >> +static inline int >> +mtk_wed_device_attach(struct mtk_wed_device *dev) >> +{ >> + int ret = -ENODEV; >> + >> +#ifdef CONFIG_NET_MEDIATEK_SOC_WED > > if (IS_ENABLED(CONFIG_NET_MEDIATEK_SOC_WED) is better, since it > compiles the code, and then the optimizer throws away. This one is intentional, since struct mtk_wed_device will be empty if CONFIG_NET_MEDIATEK_SOC_WED is not set. The code would not compile without the #ifdef. Thanks, - Felix
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 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
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