by Felix Fietkau

[permalink] [raw]

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

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