| Message ID | 20230321133609.49591-1-nbd@nbd.name (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,1/2] net: ethernet: mtk_eth_soc: add code for offloading flows from wlan devices | expand |
On Tue, Mar 21, 2023 at 02:36:08PM +0100, Felix Fietkau wrote: > WED version 2 (on MT7986 and later) can offload flows originating from wireless > devices. In order to make that work, ndo_setup_tc needs to be implemented on > the netdevs. This adds the required code to offload flows coming in from WED, > while keeping track of the incoming wed index used for selecting the correct > PPE device. > > Signed-off-by: Felix Fietkau <nbd@nbd.name> Hi Felix, A few nits from my side. First, please reformat the patch description to have a maximum of 75 characters per line, as suggested by checkpatch. ... > @@ -512,25 +514,15 @@ mtk_flow_offload_stats(struct mtk_eth *eth, struct flow_cls_offload *f) > > static DEFINE_MUTEX(mtk_flow_offload_mutex); > > -static int > -mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) > +int mtk_flow_offload_cmd(struct mtk_eth *eth, struct flow_cls_offload *cls, > + int ppe_index) > { > - struct flow_cls_offload *cls = type_data; > - struct net_device *dev = cb_priv; > - struct mtk_mac *mac = netdev_priv(dev); > - struct mtk_eth *eth = mac->hw; > int err; > > - if (!tc_can_offload(dev)) > - return -EOPNOTSUPP; > - > - if (type != TC_SETUP_CLSFLOWER) > - return -EOPNOTSUPP; > - > mutex_lock(&mtk_flow_offload_mutex); > switch (cls->command) { > case FLOW_CLS_REPLACE: > - err = mtk_flow_offload_replace(eth, cls); > + err = mtk_flow_offload_replace(eth, cls, ppe_index); > break; > case FLOW_CLS_DESTROY: > err = mtk_flow_offload_destroy(eth, cls); > @@ -547,6 +539,23 @@ mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_pri > return err; > } > > +static int > +mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) > +{ > + struct flow_cls_offload *cls = type_data; > + struct net_device *dev = cb_priv; > + struct mtk_mac *mac = netdev_priv(dev); > + struct mtk_eth *eth = mac->hw; Reverse xmas tree - longest line to shortest - for local variable declarations please. > + > + if (!tc_can_offload(dev)) > + return -EOPNOTSUPP; > + > + if (type != TC_SETUP_CLSFLOWER) > + return -EOPNOTSUPP; > + > + return mtk_flow_offload_cmd(eth, cls, 0); > +} > + > static int > mtk_eth_setup_tc_block(struct net_device *dev, struct flow_block_offload *f) > { > diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c > index 95d890870984..30fe1281d2d3 100644 > --- a/drivers/net/ethernet/mediatek/mtk_wed.c > +++ b/drivers/net/ethernet/mediatek/mtk_wed.c ... > @@ -1745,6 +1752,102 @@ void mtk_wed_flow_remove(int index) > mutex_unlock(&hw_lock); > } > > +static int > +mtk_wed_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) > +{ > + struct mtk_wed_flow_block_priv *priv = cb_priv; > + struct flow_cls_offload *cls = type_data; > + struct mtk_wed_hw *hw = priv->hw; > + > + if (!tc_can_offload(priv->dev)) > + return -EOPNOTSUPP; > + > + if (type != TC_SETUP_CLSFLOWER) > + return -EOPNOTSUPP; > + > + return mtk_flow_offload_cmd(hw->eth, cls, hw->index); This seems very similar to mtk_eth_setup_tc_block_cb(). Can further consolidation be considered? > +} > + > +static int > +mtk_wed_setup_tc_block(struct mtk_wed_hw *hw, struct net_device *dev, > + struct flow_block_offload *f) > +{ > + struct mtk_wed_flow_block_priv *priv; > + static LIST_HEAD(block_cb_list); > + struct flow_block_cb *block_cb; > + struct mtk_eth *eth = hw->eth; > + bool register_block = false; > + flow_setup_cb_t *cb; > + > + if (!eth->soc->offload_version) > + return -EOPNOTSUPP; > + > + if (f->binder_type != FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS) > + return -EOPNOTSUPP; > + > + cb = mtk_wed_setup_tc_block_cb; > + f->driver_block_list = &block_cb_list; > + > + switch (f->command) { > + case FLOW_BLOCK_BIND: > + block_cb = flow_block_cb_lookup(f->block, cb, dev); > + if (!block_cb) { I wonder if this could be written more idiomatically as: if (block_cb) { flow_block_cb_incref(block_cb); return 0; } priv = kzalloc(sizeof(*priv), GFP_KERNEL); ... > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->hw = hw; > + priv->dev = dev; > + block_cb = flow_block_cb_alloc(cb, dev, priv, NULL); > + if (IS_ERR(block_cb)) { > + kfree(priv); > + return PTR_ERR(block_cb); > + } > + > + register_block = true; > + } > + > + flow_block_cb_incref(block_cb); > + > + if (register_block) { > + flow_block_cb_add(block_cb, f); > + list_add_tail(&block_cb->driver_list, &block_cb_list); > + } > + return 0; > + case FLOW_BLOCK_UNBIND: > + block_cb = flow_block_cb_lookup(f->block, cb, dev); > + if (!block_cb) > + return -ENOENT; > + > + if (!flow_block_cb_decref(block_cb)) { > + flow_block_cb_remove(block_cb, f); > + list_del(&block_cb->driver_list); > + kfree(block_cb->cb_priv); > + } > + return 0; > + default: > + return -EOPNOTSUPP; > + } > +} ... > diff --git a/include/linux/soc/mediatek/mtk_wed.h b/include/linux/soc/mediatek/mtk_wed.h ... > @@ -237,6 +240,8 @@ mtk_wed_get_rx_capa(struct mtk_wed_device *dev) > (_dev)->ops->msg_update(_dev, _id, _msg, _len) > #define mtk_wed_device_stop(_dev) (_dev)->ops->stop(_dev) > #define mtk_wed_device_dma_reset(_dev) (_dev)->ops->reset_dma(_dev) > +#define mtk_wed_device_setup_tc(_dev, _netdev, _type, _type_data) \ > + (_dev)->ops->setup_tc(_dev, _netdev, _type, _type_data) nit: checkpatch says: include/linux/soc/mediatek/mtk_wed.h:243: ERROR: Macros with complex values should be enclosed in parentheses +#define mtk_wed_device_setup_tc(_dev, _netdev, _type, _type_data) \ + (_dev)->ops->setup_tc(_dev, _netdev, _type, _type_data) include/linux/soc/mediatek/mtk_wed.h:243: CHECK: Macro argument reuse '_dev' - possible side-effects? +#define mtk_wed_device_setup_tc(_dev, _netdev, _type, _type_data) \ + (_dev)->ops->setup_tc(_dev, _netdev, _type, _type_data) > #else > static inline bool mtk_wed_device_active(struct mtk_wed_device *dev) > {
On Wed, Mar 22, 2023 at 03:04:19PM +0100, Simon Horman wrote: > On Tue, Mar 21, 2023 at 02:36:08PM +0100, Felix Fietkau wrote: > > WED version 2 (on MT7986 and later) can offload flows originating from wireless > > devices. In order to make that work, ndo_setup_tc needs to be implemented on > > the netdevs. This adds the required code to offload flows coming in from WED, > > while keeping track of the incoming wed index used for selecting the correct > > PPE device. > > > > Signed-off-by: Felix Fietkau <nbd@nbd.name> > > Hi Felix, > > A few nits from my side. > > First, please reformat the patch description to have a maximum of 75 characters > per line, as suggested by checkpatch. > > ... One more. This seems to conflict with: [PATCH net 1/2] net: ethernet: mtk_eth_soc: fix flow block So I guess this series needs to be rebased on that one one at some point.
On 22.03.23 15:04, Simon Horman wrote: > On Tue, Mar 21, 2023 at 02:36:08PM +0100, Felix Fietkau wrote: >> WED version 2 (on MT7986 and later) can offload flows originating from wireless >> devices. In order to make that work, ndo_setup_tc needs to be implemented on >> the netdevs. This adds the required code to offload flows coming in from WED, >> while keeping track of the incoming wed index used for selecting the correct >> PPE device. >> >> Signed-off-by: Felix Fietkau <nbd@nbd.name> > > Hi Felix, > > A few nits from my side. > > First, please reformat the patch description to have a maximum of 75 characters > per line, as suggested by checkpatch. Will do >> @@ -512,25 +514,15 @@ mtk_flow_offload_stats(struct mtk_eth *eth, struct flow_cls_offload *f) >> >> static DEFINE_MUTEX(mtk_flow_offload_mutex); >> >> -static int >> -mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) >> +int mtk_flow_offload_cmd(struct mtk_eth *eth, struct flow_cls_offload *cls, >> + int ppe_index) >> { >> - struct flow_cls_offload *cls = type_data; >> - struct net_device *dev = cb_priv; >> - struct mtk_mac *mac = netdev_priv(dev); >> - struct mtk_eth *eth = mac->hw; >> int err; >> >> - if (!tc_can_offload(dev)) >> - return -EOPNOTSUPP; >> - >> - if (type != TC_SETUP_CLSFLOWER) >> - return -EOPNOTSUPP; >> - >> mutex_lock(&mtk_flow_offload_mutex); >> switch (cls->command) { >> case FLOW_CLS_REPLACE: >> - err = mtk_flow_offload_replace(eth, cls); >> + err = mtk_flow_offload_replace(eth, cls, ppe_index); >> break; >> case FLOW_CLS_DESTROY: >> err = mtk_flow_offload_destroy(eth, cls); >> @@ -547,6 +539,23 @@ mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_pri >> return err; >> } >> >> +static int >> +mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) >> +{ >> + struct flow_cls_offload *cls = type_data; >> + struct net_device *dev = cb_priv; >> + struct mtk_mac *mac = netdev_priv(dev); >> + struct mtk_eth *eth = mac->hw; > > Reverse xmas tree - longest line to shortest - > for local variable declarations please. Will do. >> diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c >> index 95d890870984..30fe1281d2d3 100644 >> --- a/drivers/net/ethernet/mediatek/mtk_wed.c >> +++ b/drivers/net/ethernet/mediatek/mtk_wed.c > > ... > >> @@ -1745,6 +1752,102 @@ void mtk_wed_flow_remove(int index) >> mutex_unlock(&hw_lock); >> } >> >> +static int >> +mtk_wed_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) >> +{ >> + struct mtk_wed_flow_block_priv *priv = cb_priv; >> + struct flow_cls_offload *cls = type_data; >> + struct mtk_wed_hw *hw = priv->hw; >> + >> + if (!tc_can_offload(priv->dev)) >> + return -EOPNOTSUPP; >> + >> + if (type != TC_SETUP_CLSFLOWER) >> + return -EOPNOTSUPP; >> + >> + return mtk_flow_offload_cmd(hw->eth, cls, hw->index); > > This seems very similar to mtk_eth_setup_tc_block_cb(). > Can further consolidation be considered? It's similar, but using different data structures and pointer chains. Consolidation does not make sense here. >> +} >> + >> +static int >> +mtk_wed_setup_tc_block(struct mtk_wed_hw *hw, struct net_device *dev, >> + struct flow_block_offload *f) >> +{ >> + struct mtk_wed_flow_block_priv *priv; >> + static LIST_HEAD(block_cb_list); >> + struct flow_block_cb *block_cb; >> + struct mtk_eth *eth = hw->eth; >> + bool register_block = false; >> + flow_setup_cb_t *cb; >> + >> + if (!eth->soc->offload_version) >> + return -EOPNOTSUPP; >> + >> + if (f->binder_type != FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS) >> + return -EOPNOTSUPP; >> + >> + cb = mtk_wed_setup_tc_block_cb; >> + f->driver_block_list = &block_cb_list; >> + >> + switch (f->command) { >> + case FLOW_BLOCK_BIND: >> + block_cb = flow_block_cb_lookup(f->block, cb, dev); >> + if (!block_cb) { > > I wonder if this could be written more idiomatically as: > > if (block_cb) { > flow_block_cb_incref(block_cb); > return 0; > } > > priv = kzalloc(sizeof(*priv), GFP_KERNEL); flow_block_cb_incref needs to be called for the newly allocated flow block cb as well. I was following the same pattern that several >> diff --git a/include/linux/soc/mediatek/mtk_wed.h b/include/linux/soc/mediatek/mtk_wed.h > > ... > >> @@ -237,6 +240,8 @@ mtk_wed_get_rx_capa(struct mtk_wed_device *dev) >> (_dev)->ops->msg_update(_dev, _id, _msg, _len) >> #define mtk_wed_device_stop(_dev) (_dev)->ops->stop(_dev) >> #define mtk_wed_device_dma_reset(_dev) (_dev)->ops->reset_dma(_dev) >> +#define mtk_wed_device_setup_tc(_dev, _netdev, _type, _type_data) \ >> + (_dev)->ops->setup_tc(_dev, _netdev, _type, _type_data) > > nit: checkpatch says: > > include/linux/soc/mediatek/mtk_wed.h:243: ERROR: Macros with complex values should be enclosed in parentheses > +#define mtk_wed_device_setup_tc(_dev, _netdev, _type, _type_data) \ > + (_dev)->ops->setup_tc(_dev, _netdev, _type, _type_data) > > include/linux/soc/mediatek/mtk_wed.h:243: CHECK: Macro argument reuse '_dev' - possible side-effects? > +#define mtk_wed_device_setup_tc(_dev, _netdev, _type, _type_data) \ > + (_dev)->ops->setup_tc(_dev, _netdev, _type, _type_data) In my opinion that's a false positive. The newly added macros follow the same pattern as the existing ones. Thanks, - Felix
On 22.03.23 15:29, Simon Horman wrote: > On Wed, Mar 22, 2023 at 03:04:19PM +0100, Simon Horman wrote: >> On Tue, Mar 21, 2023 at 02:36:08PM +0100, Felix Fietkau wrote: >> > WED version 2 (on MT7986 and later) can offload flows originating from wireless >> > devices. In order to make that work, ndo_setup_tc needs to be implemented on >> > the netdevs. This adds the required code to offload flows coming in from WED, >> > while keeping track of the incoming wed index used for selecting the correct >> > PPE device. >> > >> > Signed-off-by: Felix Fietkau <nbd@nbd.name> >> >> Hi Felix, >> >> A few nits from my side. >> >> First, please reformat the patch description to have a maximum of 75 characters >> per line, as suggested by checkpatch. >> >> ... > > One more. > > This seems to conflict with: > > [PATCH net 1/2] net: ethernet: mtk_eth_soc: fix flow block > > So I guess this series needs to be rebased on that one one at some point. I don't think this conflicts. My local rebase tests didn't show any issues either. - Felix
On Wed, Mar 22, 2023 at 04:18:29PM +0100, Felix Fietkau wrote: > On 22.03.23 15:04, Simon Horman wrote: > > On Tue, Mar 21, 2023 at 02:36:08PM +0100, Felix Fietkau wrote: > > > WED version 2 (on MT7986 and later) can offload flows originating from wireless > > > devices. In order to make that work, ndo_setup_tc needs to be implemented on > > > the netdevs. This adds the required code to offload flows coming in from WED, > > > while keeping track of the incoming wed index used for selecting the correct > > > PPE device. > > > > > > Signed-off-by: Felix Fietkau <nbd@nbd.name> > > > > Hi Felix, > > > > A few nits from my side. > > > > First, please reformat the patch description to have a maximum of 75 characters > > per line, as suggested by checkpatch. > Will do > > > > @@ -512,25 +514,15 @@ mtk_flow_offload_stats(struct mtk_eth *eth, struct flow_cls_offload *f) > > > static DEFINE_MUTEX(mtk_flow_offload_mutex); > > > -static int > > > -mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) > > > +int mtk_flow_offload_cmd(struct mtk_eth *eth, struct flow_cls_offload *cls, > > > + int ppe_index) > > > { > > > - struct flow_cls_offload *cls = type_data; > > > - struct net_device *dev = cb_priv; > > > - struct mtk_mac *mac = netdev_priv(dev); > > > - struct mtk_eth *eth = mac->hw; > > > int err; > > > - if (!tc_can_offload(dev)) > > > - return -EOPNOTSUPP; > > > - > > > - if (type != TC_SETUP_CLSFLOWER) > > > - return -EOPNOTSUPP; > > > - > > > mutex_lock(&mtk_flow_offload_mutex); > > > switch (cls->command) { > > > case FLOW_CLS_REPLACE: > > > - err = mtk_flow_offload_replace(eth, cls); > > > + err = mtk_flow_offload_replace(eth, cls, ppe_index); > > > break; > > > case FLOW_CLS_DESTROY: > > > err = mtk_flow_offload_destroy(eth, cls); > > > @@ -547,6 +539,23 @@ mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_pri > > > return err; > > > } > > > +static int > > > +mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) > > > +{ > > > + struct flow_cls_offload *cls = type_data; > > > + struct net_device *dev = cb_priv; > > > + struct mtk_mac *mac = netdev_priv(dev); > > > + struct mtk_eth *eth = mac->hw; > > > > Reverse xmas tree - longest line to shortest - > > for local variable declarations please. > Will do. > > > > diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c > > > index 95d890870984..30fe1281d2d3 100644 > > > --- a/drivers/net/ethernet/mediatek/mtk_wed.c > > > +++ b/drivers/net/ethernet/mediatek/mtk_wed.c > > > > ... > > > > > @@ -1745,6 +1752,102 @@ void mtk_wed_flow_remove(int index) > > > mutex_unlock(&hw_lock); > > > } > > > +static int > > > +mtk_wed_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) > > > +{ > > > + struct mtk_wed_flow_block_priv *priv = cb_priv; > > > + struct flow_cls_offload *cls = type_data; > > > + struct mtk_wed_hw *hw = priv->hw; > > > + > > > + if (!tc_can_offload(priv->dev)) > > > + return -EOPNOTSUPP; > > > + > > > + if (type != TC_SETUP_CLSFLOWER) > > > + return -EOPNOTSUPP; > > > + > > > + return mtk_flow_offload_cmd(hw->eth, cls, hw->index); > > > > This seems very similar to mtk_eth_setup_tc_block_cb(). > > Can further consolidation be considered? > It's similar, but using different data structures and pointer chains. > Consolidation does not make sense here. Thanks, I see that now. > > > +} > > > + > > > +static int > > > +mtk_wed_setup_tc_block(struct mtk_wed_hw *hw, struct net_device *dev, > > > + struct flow_block_offload *f) > > > +{ > > > + struct mtk_wed_flow_block_priv *priv; > > > + static LIST_HEAD(block_cb_list); > > > + struct flow_block_cb *block_cb; > > > + struct mtk_eth *eth = hw->eth; > > > + bool register_block = false; > > > + flow_setup_cb_t *cb; > > > + > > > + if (!eth->soc->offload_version) > > > + return -EOPNOTSUPP; > > > + > > > + if (f->binder_type != FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS) > > > + return -EOPNOTSUPP; > > > + > > > + cb = mtk_wed_setup_tc_block_cb; > > > + f->driver_block_list = &block_cb_list; > > > + > > > + switch (f->command) { > > > + case FLOW_BLOCK_BIND: > > > + block_cb = flow_block_cb_lookup(f->block, cb, dev); > > > + if (!block_cb) { > > > > I wonder if this could be written more idiomatically as: > > > > if (block_cb) { > > flow_block_cb_incref(block_cb); > > return 0; > > } > > > > priv = kzalloc(sizeof(*priv), GFP_KERNEL); > flow_block_cb_incref needs to be called for the newly allocated flow block > cb as well. I was following the same pattern that several I guess that to some extent it is a question of style. It seems to me that having separate calls to flow_block_cb_incref() for the different cases leads to nicer, and less indented, code. But its just a suggestion from my side. > > > diff --git a/include/linux/soc/mediatek/mtk_wed.h b/include/linux/soc/mediatek/mtk_wed.h > > > > ... > > > > > @@ -237,6 +240,8 @@ mtk_wed_get_rx_capa(struct mtk_wed_device *dev) > > > (_dev)->ops->msg_update(_dev, _id, _msg, _len) > > > #define mtk_wed_device_stop(_dev) (_dev)->ops->stop(_dev) > > > #define mtk_wed_device_dma_reset(_dev) (_dev)->ops->reset_dma(_dev) > > > +#define mtk_wed_device_setup_tc(_dev, _netdev, _type, _type_data) \ > > > + (_dev)->ops->setup_tc(_dev, _netdev, _type, _type_data) > > > > nit: checkpatch says: > > > > include/linux/soc/mediatek/mtk_wed.h:243: ERROR: Macros with complex values should be enclosed in parentheses > > +#define mtk_wed_device_setup_tc(_dev, _netdev, _type, _type_data) \ > > + (_dev)->ops->setup_tc(_dev, _netdev, _type, _type_data) > > > > include/linux/soc/mediatek/mtk_wed.h:243: CHECK: Macro argument reuse '_dev' - possible side-effects? > > +#define mtk_wed_device_setup_tc(_dev, _netdev, _type, _type_data) \ > > + (_dev)->ops->setup_tc(_dev, _netdev, _type, _type_data) > In my opinion that's a false positive. The newly added macros follow the > same pattern as the existing ones. Ack.
On Wed, Mar 22, 2023 at 04:19:41PM +0100, Felix Fietkau wrote: > On 22.03.23 15:29, Simon Horman wrote: > > On Wed, Mar 22, 2023 at 03:04:19PM +0100, Simon Horman wrote: > > > On Tue, Mar 21, 2023 at 02:36:08PM +0100, Felix Fietkau wrote: > > > > WED version 2 (on MT7986 and later) can offload flows originating from wireless > > > > devices. In order to make that work, ndo_setup_tc needs to be implemented on > > > > the netdevs. This adds the required code to offload flows coming in from WED, > > > > while keeping track of the incoming wed index used for selecting the correct > > > > PPE device. > > > > > > > > Signed-off-by: Felix Fietkau <nbd@nbd.name> > > > > > > Hi Felix, > > > > > > A few nits from my side. > > > > > > First, please reformat the patch description to have a maximum of 75 characters > > > per line, as suggested by checkpatch. > > > > > > ... > > > > One more. > > > > This seems to conflict with: > > > > [PATCH net 1/2] net: ethernet: mtk_eth_soc: fix flow block > > > > So I guess this series needs to be rebased on that one one at some point. > I don't think this conflicts. My local rebase tests didn't show any issues > either. Ok, sorry for the noise there. I must have messed something up when I checked earlier.
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h index 084a6badef6d..f7092b532643 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h @@ -1322,6 +1322,9 @@ int mtk_gmac_rgmii_path_setup(struct mtk_eth *eth, int mac_id); int mtk_eth_offload_init(struct mtk_eth *eth); int mtk_eth_setup_tc(struct net_device *dev, enum tc_setup_type type, void *type_data); +int mtk_flow_offload_cmd(struct mtk_eth *eth, struct flow_cls_offload *cls, + int ppe_index); +void mtk_flow_offload_cleanup(struct mtk_eth *eth, struct list_head *list); void mtk_eth_set_dma_device(struct mtk_eth *eth, struct device *dma_dev); diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c index 81afd5ee3fbf..0c5e049b8141 100644 --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c @@ -235,7 +235,8 @@ mtk_flow_set_output_device(struct mtk_eth *eth, struct mtk_foe_entry *foe, } static int -mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f) +mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f, + int ppe_index) { struct flow_rule *rule = flow_cls_offload_flow_rule(f); struct flow_action_entry *act; @@ -452,6 +453,7 @@ mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f) entry->cookie = f->cookie; memcpy(&entry->data, &foe, sizeof(entry->data)); entry->wed_index = wed_index; + entry->ppe_index = ppe_index; err = mtk_foe_entry_commit(eth->ppe[entry->ppe_index], entry); if (err < 0) @@ -512,25 +514,15 @@ mtk_flow_offload_stats(struct mtk_eth *eth, struct flow_cls_offload *f) static DEFINE_MUTEX(mtk_flow_offload_mutex); -static int -mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) +int mtk_flow_offload_cmd(struct mtk_eth *eth, struct flow_cls_offload *cls, + int ppe_index) { - struct flow_cls_offload *cls = type_data; - struct net_device *dev = cb_priv; - struct mtk_mac *mac = netdev_priv(dev); - struct mtk_eth *eth = mac->hw; int err; - if (!tc_can_offload(dev)) - return -EOPNOTSUPP; - - if (type != TC_SETUP_CLSFLOWER) - return -EOPNOTSUPP; - mutex_lock(&mtk_flow_offload_mutex); switch (cls->command) { case FLOW_CLS_REPLACE: - err = mtk_flow_offload_replace(eth, cls); + err = mtk_flow_offload_replace(eth, cls, ppe_index); break; case FLOW_CLS_DESTROY: err = mtk_flow_offload_destroy(eth, cls); @@ -547,6 +539,23 @@ mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_pri return err; } +static int +mtk_eth_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) +{ + struct flow_cls_offload *cls = type_data; + struct net_device *dev = cb_priv; + struct mtk_mac *mac = netdev_priv(dev); + struct mtk_eth *eth = mac->hw; + + if (!tc_can_offload(dev)) + return -EOPNOTSUPP; + + if (type != TC_SETUP_CLSFLOWER) + return -EOPNOTSUPP; + + return mtk_flow_offload_cmd(eth, cls, 0); +} + static int mtk_eth_setup_tc_block(struct net_device *dev, struct flow_block_offload *f) { diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c index 95d890870984..30fe1281d2d3 100644 --- a/drivers/net/ethernet/mediatek/mtk_wed.c +++ b/drivers/net/ethernet/mediatek/mtk_wed.c @@ -13,6 +13,8 @@ #include <linux/mfd/syscon.h> #include <linux/debugfs.h> #include <linux/soc/mediatek/mtk_wed.h> +#include <net/flow_offload.h> +#include <net/pkt_cls.h> #include "mtk_eth_soc.h" #include "mtk_wed_regs.h" #include "mtk_wed.h" @@ -41,6 +43,11 @@ static struct mtk_wed_hw *hw_list[2]; static DEFINE_MUTEX(hw_lock); +struct mtk_wed_flow_block_priv { + struct mtk_wed_hw *hw; + struct net_device *dev; +}; + static void wed_m32(struct mtk_wed_device *dev, u32 reg, u32 mask, u32 val) { @@ -1745,6 +1752,102 @@ void mtk_wed_flow_remove(int index) mutex_unlock(&hw_lock); } +static int +mtk_wed_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) +{ + struct mtk_wed_flow_block_priv *priv = cb_priv; + struct flow_cls_offload *cls = type_data; + struct mtk_wed_hw *hw = priv->hw; + + if (!tc_can_offload(priv->dev)) + return -EOPNOTSUPP; + + if (type != TC_SETUP_CLSFLOWER) + return -EOPNOTSUPP; + + return mtk_flow_offload_cmd(hw->eth, cls, hw->index); +} + +static int +mtk_wed_setup_tc_block(struct mtk_wed_hw *hw, struct net_device *dev, + struct flow_block_offload *f) +{ + struct mtk_wed_flow_block_priv *priv; + static LIST_HEAD(block_cb_list); + struct flow_block_cb *block_cb; + struct mtk_eth *eth = hw->eth; + bool register_block = false; + flow_setup_cb_t *cb; + + if (!eth->soc->offload_version) + return -EOPNOTSUPP; + + if (f->binder_type != FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS) + return -EOPNOTSUPP; + + cb = mtk_wed_setup_tc_block_cb; + f->driver_block_list = &block_cb_list; + + switch (f->command) { + case FLOW_BLOCK_BIND: + block_cb = flow_block_cb_lookup(f->block, cb, dev); + if (!block_cb) { + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->hw = hw; + priv->dev = dev; + block_cb = flow_block_cb_alloc(cb, dev, priv, NULL); + if (IS_ERR(block_cb)) { + kfree(priv); + return PTR_ERR(block_cb); + } + + register_block = true; + } + + flow_block_cb_incref(block_cb); + + if (register_block) { + flow_block_cb_add(block_cb, f); + list_add_tail(&block_cb->driver_list, &block_cb_list); + } + return 0; + case FLOW_BLOCK_UNBIND: + block_cb = flow_block_cb_lookup(f->block, cb, dev); + if (!block_cb) + return -ENOENT; + + if (!flow_block_cb_decref(block_cb)) { + flow_block_cb_remove(block_cb, f); + list_del(&block_cb->driver_list); + kfree(block_cb->cb_priv); + } + return 0; + default: + return -EOPNOTSUPP; + } +} + +static int +mtk_wed_setup_tc(struct mtk_wed_device *wed, struct net_device *dev, + enum tc_setup_type type, void *type_data) +{ + struct mtk_wed_hw *hw = wed->hw; + + if (hw->version < 2) + return -EOPNOTSUPP; + + switch (type) { + case TC_SETUP_BLOCK: + case TC_SETUP_FT: + return mtk_wed_setup_tc_block(hw, dev, type_data); + default: + return -EOPNOTSUPP; + } +} + void mtk_wed_add_hw(struct device_node *np, struct mtk_eth *eth, void __iomem *wdma, phys_addr_t wdma_phy, int index) @@ -1764,6 +1867,7 @@ void mtk_wed_add_hw(struct device_node *np, struct mtk_eth *eth, .irq_set_mask = mtk_wed_irq_set_mask, .detach = mtk_wed_detach, .ppe_check = mtk_wed_ppe_check, + .setup_tc = mtk_wed_setup_tc, }; struct device_node *eth_np = eth->dev->of_node; struct platform_device *pdev; diff --git a/include/linux/soc/mediatek/mtk_wed.h b/include/linux/soc/mediatek/mtk_wed.h index fd0b0605cf90..b2b28180dff7 100644 --- a/include/linux/soc/mediatek/mtk_wed.h +++ b/include/linux/soc/mediatek/mtk_wed.h @@ -6,6 +6,7 @@ #include <linux/regmap.h> #include <linux/pci.h> #include <linux/skbuff.h> +#include <linux/netdevice.h> #define MTK_WED_TX_QUEUES 2 #define MTK_WED_RX_QUEUES 2 @@ -179,6 +180,8 @@ struct mtk_wed_ops { u32 (*irq_get)(struct mtk_wed_device *dev, u32 mask); void (*irq_set_mask)(struct mtk_wed_device *dev, u32 mask); + int (*setup_tc)(struct mtk_wed_device *wed, struct net_device *dev, + enum tc_setup_type type, void *type_data); }; extern const struct mtk_wed_ops __rcu *mtk_soc_wed_ops; @@ -237,6 +240,8 @@ mtk_wed_get_rx_capa(struct mtk_wed_device *dev) (_dev)->ops->msg_update(_dev, _id, _msg, _len) #define mtk_wed_device_stop(_dev) (_dev)->ops->stop(_dev) #define mtk_wed_device_dma_reset(_dev) (_dev)->ops->reset_dma(_dev) +#define mtk_wed_device_setup_tc(_dev, _netdev, _type, _type_data) \ + (_dev)->ops->setup_tc(_dev, _netdev, _type, _type_data) #else static inline bool mtk_wed_device_active(struct mtk_wed_device *dev) { @@ -255,6 +260,7 @@ static inline bool mtk_wed_device_active(struct mtk_wed_device *dev) #define mtk_wed_device_update_msg(_dev, _id, _msg, _len) -ENODEV #define mtk_wed_device_stop(_dev) do {} while (0) #define mtk_wed_device_dma_reset(_dev) do {} while (0) +#define mtk_wed_device_setup_tc(_dev, _netdev, _type, _type_data) -EOPNOTSUPP #endif #endif
WED version 2 (on MT7986 and later) can offload flows originating from wireless devices. In order to make that work, ndo_setup_tc needs to be implemented on the netdevs. This adds the required code to offload flows coming in from WED, while keeping track of the incoming wed index used for selecting the correct PPE device. Signed-off-by: Felix Fietkau <nbd@nbd.name> --- drivers/net/ethernet/mediatek/mtk_eth_soc.h | 3 + .../net/ethernet/mediatek/mtk_ppe_offload.c | 37 ++++--- drivers/net/ethernet/mediatek/mtk_wed.c | 104 ++++++++++++++++++ include/linux/soc/mediatek/mtk_wed.h | 6 + 4 files changed, 136 insertions(+), 14 deletions(-)