Re: [PATCH net-next] netfilter: nf_table_offload: Fix zero prio of flow_cls_common_offload

[Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>]

On Thu, Jul 11, 2019 at 04:03:30PM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
> 
> The flow_cls_common_offload prio should be not zero
> 
> It leads the invalid table prio in hw.
> 
> # nft add table netdev firewall
> # nft add chain netdev firewall acl { type filter hook ingress device mlx_pf0vf0 priority - 300 \; }
> # nft add rule netdev firewall acl ip daddr 1.1.1.7 drop
> Error: Could not process rule: Invalid argument
> 
> kernel log
> mlx5_core 0000:81:00.0: E-Switch: Failed to create FDB Table err -22 (table prio: 65535, level: 0, size: 4194304)
> 
> Fixes: c9626a2cbdb2 ("netfilter: nf_tables: add hardware offload support")
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
>  net/netfilter/nf_tables_offload.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
> index 2c33028..01d8133 100644
> --- a/net/netfilter/nf_tables_offload.c
> +++ b/net/netfilter/nf_tables_offload.c
> @@ -7,6 +7,8 @@
>  #include <net/netfilter/nf_tables_offload.h>
>  #include <net/pkt_cls.h>
>  
> +#define FLOW_OFFLOAD_DEFAUT_PRIO 1U
> +
>  static struct nft_flow_rule *nft_flow_rule_alloc(int num_actions)
>  {
>  	struct nft_flow_rule *flow;
> @@ -107,6 +109,7 @@ static void nft_flow_offload_common_init(struct flow_cls_common_offload *common,
>  					struct netlink_ext_ack *extack)
>  {
>  	common->protocol = proto;
> +	common->prio = TC_H_MAKE(FLOW_OFFLOAD_DEFAUT_PRIO << 16, 0);

Note that tc semantics for this is to auto-generate a priority in such
cases, instead of using a default.

@tc_new_tfilter():
        if (prio == 0) {
                /* If no priority is provided by the user,
                 * we allocate one.
                 */
                if (n->nlmsg_flags & NLM_F_CREATE) {
                        prio = TC_H_MAKE(0x80000000U, 0U);
                        prio_allocate = true;
...
                if (prio_allocate)
                        prio = tcf_auto_prio(tcf_chain_tp_prev(chain,
                                                               &chain_info));

>  	common->extack = extack;
>  }
>  
> -- 
> 1.8.3.1
>