Re: [PATCH nf-next] netfilter: flowtable: carry a priority into the offload

> A flow rule handed to a driver describes NAT, encapsulation and the
> output device, but not how the flow should be treated on the way out,
> so hardware with priority queues can only fall back on the DSCP the
> packet carries.
> 
> Carry skb->priority of the packet that created the flow and emit it as
> FLOW_ACTION_PRIORITY, the action act_skbedit already emits on the tc
> path. "meta priority set" before "flow add" then reaches the hardware.
> A flow without a priority emits no action; mtk and airoha, the two
> in-tree consumers of these rules, ignore the new one as they do
> FLOW_ACTION_CSUM.
> 
> The flowtable holds one flow for both directions and the expression
> runs once, so the priority applies to both; per-direction
> classification is not carried.
> 
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Julius Bairaktaris <[email protected]>

Hi Julius,

we have already discussed about this kind of approach to offload skb priority
into drivers that support flowtable hw offload. The main blocking point is we
need the same feature in the flowtable sw path in order to not introduce any
missing capability in the kernel tx path.

Regards,
Lorenzo

> ---
> The consumer of the emitted action is a DSA driver for the IPQ8074 PPE,
> maintained in OpenWrt; measured there, "meta priority set" ahead of
> "flow add" places offloaded flows in the port's hardware priority
> queues. The mtk and airoha hunks are compile-tested only. The new field
> grows struct flow_offload by eight bytes on 64-bit; the entry allocates
> from its own kmem_cache, so no allocation-class change.
> 
>  drivers/net/ethernet/airoha/airoha_ppe.c        |  1 +
>  drivers/net/ethernet/mediatek/mtk_ppe_offload.c |  1 +
>  include/net/netfilter/nf_flow_table.h           |  1 +
>  net/netfilter/nf_flow_table_offload.c           | 11 +++++++++++
>  net/netfilter/nft_flow_offload.c                |  7 +++++++
>  5 files changed, 21 insertions(+)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
> index 92611802801e..2afce76ad131 100644
> --- a/drivers/net/ethernet/airoha/airoha_ppe.c
> +++ b/drivers/net/ethernet/airoha/airoha_ppe.c
> @@ -1161,6 +1161,7 @@ static int airoha_ppe_flow_offload_replace(struct airoha_eth *eth,
>  		case FLOW_ACTION_REDIRECT:
>  			odev = act->dev;
>  			break;
> +		case FLOW_ACTION_PRIORITY:
>  		case FLOW_ACTION_CSUM:
>  			break;
>  		case FLOW_ACTION_VLAN_PUSH:
> diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> index 99b28aaa7cc4..4ee99e8e4a34 100644
> --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c
> @@ -378,6 +378,7 @@ mtk_flow_offload_replace(struct mtk_eth *eth, struct flow_cls_offload *f,
>  		case FLOW_ACTION_REDIRECT:
>  			odev = act->dev;
>  			break;
> +		case FLOW_ACTION_PRIORITY:
>  		case FLOW_ACTION_CSUM:
>  			break;
>  		case FLOW_ACTION_VLAN_PUSH:
> diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
> index f2e2771f188f..23218c8cbc3d 100644
> --- a/include/net/netfilter/nf_flow_table.h
> +++ b/include/net/netfilter/nf_flow_table.h
> @@ -202,6 +202,7 @@ struct flow_offload {
>  	unsigned long				flags;
>  	u16					type;
>  	u32					timeout;
> +	u32					priority;
>  	struct rcu_head				rcu_head;
>  };
>  
> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> index 801a3dd9ceea..caaadffc2563 100644
> --- a/net/netfilter/nf_flow_table_offload.c
> +++ b/net/netfilter/nf_flow_table_offload.c
> @@ -696,6 +696,17 @@ nf_flow_rule_route_common(struct net *net, const struct flow_offload *flow,
>  	    flow_offload_eth_dst(net, flow, dir, flow_rule) < 0)
>  		return -1;
>  
> +	if (flow->priority) {
> +		struct flow_action_entry *entry;
> +
> +		entry = flow_action_entry_next(flow_rule);
> +		if (!entry)
> +			return -1;
> +
> +		entry->id = FLOW_ACTION_PRIORITY;
> +		entry->priority = flow->priority;
> +	}
> +
>  	tuple = &flow->tuplehash[dir].tuple;
>  
>  	for (i = 0; i < tuple->encap_num; i++) {
> diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
> index 32b4281038dd..dd3ac2b9c963 100644
> --- a/net/netfilter/nft_flow_offload.c
> +++ b/net/netfilter/nft_flow_offload.c
> @@ -117,6 +117,13 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
>  	if (tcph)
>  		flow_offload_ct_tcp(ct);
>  
> +	/* Whatever classified this packet before it reached the flowtable also
> +	 * describes every packet the hardware will forward in its place, so
> +	 * carry it into the offload rather than losing it with the software
> +	 * path.
> +	 */
> +	flow->priority = pkt->skb->priority;
> +
>  	__set_bit(NF_FLOW_HW_BIDIRECTIONAL, &flow->flags);
>  	ret = flow_offload_add(flowtable, flow);
>  	if (ret < 0)
> 
> base-commit: 91ec2035134982b98fab0609a9fd8480e8217dc1
> -- 
> 2.53.0
>

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCapaixwAKCRA6cBh0uS2t
rO6BAQDATMMj9psNMDyU5heE/YH01LAOWYjr6ZhqoY+od26axgEAmapqfjkDrUmq
l70nCQ9+MTNflnFu+hLQGuQ4HGwL3AI=
=wTb6
-----END PGP SIGNATURE-----