netdev - Re: [PATCH v3 net-next 1/2] net: ethernet: mtk_eth_soc: add code for offloading flows from wlan devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZCdEvEJTJewWnuU9@corigine.com>
Date:   Fri, 31 Mar 2023 22:38:20 +0200
From:   Simon Horman <simon.horman@...igine.com>
To:     Felix Fietkau <nbd@....name>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH v3 net-next 1/2] net: ethernet: mtk_eth_soc: add code for
 offloading flows from wlan devices

On Fri, Mar 31, 2023 at 02:47:06PM +0200, 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@....name>

...

> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c

...

> +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 mtk_mac *mac = netdev_priv(dev);

This does not compile because dev is undefined at this point.

> +	struct net_device *dev = cb_priv;
> +	struct mtk_eth *eth = mac->hw;

I would suggest something like this:

        struct flow_cls_offload *cls = type_data;
        struct net_device *dev = cb_priv;
        struct mtk_mac *mac;
        struct mtk_eth *eth;

        mac = netdev_priv(cb_priv);
        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

...

> +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;

gcc-12 with W=1 tellsme that register_block is unused.
It should be removed.

> +	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) {
> +			flow_block_cb_incref(block_cb);
> +			return 0;
> +		}
> +
> +		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);
> +		}
> +
> +		flow_block_cb_incref(block_cb);
> +		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;
> +	}
> +}

...