netdev - Re: [PATCH net-next v2 6/6] sfc: add offloading of 'foreign' TC (decap) rules

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]

Date:   Sun, 26 Mar 2023 11:15:26 +0200
From:   Simon Horman <simon.horman@...igine.com>
To:     edward.cree@....com
Cc:     linux-net-drivers@....com, davem@...emloft.net, kuba@...nel.org,
        pabeni@...hat.com, edumazet@...gle.com,
        Edward Cree <ecree.xilinx@...il.com>, netdev@...r.kernel.org,
        habetsm.xilinx@...il.com, michal.swiatkowski@...ux.intel.com
Subject: Re: [PATCH net-next v2 6/6] sfc: add offloading of 'foreign' TC
 (decap) rules

On Thu, Mar 23, 2023 at 08:45:14PM +0000, edward.cree@....com wrote:
> From: Edward Cree <ecree.xilinx@...il.com>
> 
> A 'foreign' rule is one for which the net_dev is not the sfc netdevice
>  or any of its representors.  The driver registers indirect flow blocks
>  for tunnel netdevs so that it can offload decap rules.  For example:
> 
>     tc filter add dev vxlan0 parent ffff: protocol ipv4 flower \
>         enc_src_ip 10.1.0.2 enc_dst_ip 10.1.0.1 \
>         enc_key_id 1000 enc_dst_port 4789 \
>         action tunnel_key unset \
>         action mirred egress redirect dev $REPRESENTOR
> 
> When notified of a rule like this, register an encap match on the IP
>  and dport tuple (creating an Outer Rule table entry) and insert an MAE
>  action rule to perform the decapsulation and deliver to the representee.
> 
> Moved efx_tc_delete_rule() below efx_tc_flower_release_encap_match() to
>  avoid the need for a forward declaration.
> 
> Signed-off-by: Edward Cree <ecree.xilinx@...il.com>

Looks good to me.


Reviewed-by: Simon Horman <simon.horman@...igine.com>

...

> +			list_add_tail(&act->list, &rule->acts.list);
> +			act = NULL;
> +			if (fa->id == FLOW_ACTION_REDIRECT)
> +				break; /* end of the line */

nit: 'act = NULL;' could go inside this if clause.

> +			/* Mirror, so continue on with saved act */
> +			act = kzalloc(sizeof(*act), GFP_USER);
> +			if (!act) {
> +				rc = -ENOMEM;
> +				goto release;
> +			}
> +			*act = save;
> +			break;

...