netdev - Re: [PATCH net 1/1] net: openvswitch: Fix ct_state nat flags for conns arriving from tc

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 4 Jan 2022 10:08:35 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Paul Blakey <paulb@...dia.com>
Cc:     <dev@...nvswitch.org>, <netdev@...r.kernel.org>,
        Cong Wang <xiyou.wangcong@...il.com>,
        "Jamal Hadi Salim" <jhs@...atatu.com>,
        Pravin B Shelar <pshelar@....org>, <davem@...emloft.net>,
        Jiri Pirko <jiri@...dia.com>,
        Saeed Mahameed <saeedm@...dia.com>,
        Oz Shlomo <ozsh@...dia.com>, "Vlad Buslov" <vladbu@...dia.com>,
        Roi Dayan <roid@...dia.com>
Subject: Re: [PATCH net 1/1] net: openvswitch: Fix ct_state nat flags for
 conns arriving from tc

On Tue, 4 Jan 2022 10:28:21 +0200 Paul Blakey wrote:
> Netfilter conntrack maintains NAT flags per connection indicating
> whether NAT was configured for the connection. Openvswitch maintains
> NAT flags on the per packet flow key ct_state field, indicating
> whether NAT was actually executed on the packet.
> 
> When a packet misses from tc to ovs the conntrack NAT flags are set.
> However, NAT was not necessarily executed on the packet because the
> connection's state might still be in NEW state. As such, openvswitch wrongly
> assumes that NAT was executed and sets an incorrect flow key NAT flags.
> 
> Fix this, by flagging to openvswitch which NAT was actually done in
> act_ct via tc_skb_ext and tc_skb_cb to the openvswitch module, so
> the packet flow key NAT flags will be correctly set.

Fixes ?

> Signed-off-by: Paul Blakey <paulb@...dia.com>

> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 4507d77d6941..bab45a009310 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -287,7 +287,9 @@ struct tc_skb_ext {
>  	__u32 chain;
>  	__u16 mru;
>  	__u16 zone;
> -	bool post_ct;
> +	bool post_ct:1;
> +	bool post_ct_snat:1;
> +	bool post_ct_dnat:1;

single bit bool variables seem weird, use a unsigned int type, like u8.

>  };
>  #endif
>  
> diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
> index 9e71691c491b..a171dfa91910 100644
> --- a/include/net/pkt_sched.h
> +++ b/include/net/pkt_sched.h
> @@ -197,7 +197,9 @@ struct tc_skb_cb {
>  	struct qdisc_skb_cb qdisc_cb;
>  
>  	u16 mru;
> -	bool post_ct;
> +	bool post_ct: 1;

extra space

> +	bool post_ct_snat:1;
> +	bool post_ct_dnat:1;
>  	u16 zone; /* Only valid if post_ct = true */
>  };