Message ID | 1642186652-10799-6-git-send-email-bodong@nvidia.com |
---|---|
State | New |
Headers | show |
Series | Fix ct_state nat matching and nat action not being executed | expand |
This patch is almost upstream: commit 6f022c2ddbcefaee79502ce5386dfe351d457070 ("net: openvswitch: Fix ct_state nat flags for conns arriving from tc") from linux-next. How about a v3 ? On 1/14/22 11:57 AM, Bodong Wang wrote: > From: Paul Blakey <paulb@nvidia.com> > > BugLink: https://bugs.launchpad.net/bugs/1957807 > > 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: b57dc7c13ea9 ("net/sched: Introduce action ct") > Signed-off-by: Paul Blakey <paulb@nvidia.com> > Signed-off-by: Bodong Wang <bodong@nvidia.com> > --- > include/linux/skbuff.h | 4 +++- > include/net/pkt_sched.h | 4 +++- > net/openvswitch/flow.c | 16 +++++++++++++--- > net/sched/act_ct.c | 6 ++++++ > net/sched/cls_api.c | 2 ++ > 5 files changed, 27 insertions(+), 5 deletions(-) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index b53877d..9701643 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -285,7 +285,9 @@ struct tc_skb_ext { > __u32 chain; > __u16 mru; > __u16 zone; > - bool post_ct; > + u8 post_ct:1; > + u8 post_ct_snat:1; > + u8 post_ct_dnat:1; > }; > #endif > > diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h > index b52882e..6250acb 100644 > --- a/include/net/pkt_sched.h > +++ b/include/net/pkt_sched.h > @@ -178,7 +178,9 @@ struct tc_skb_cb { > struct qdisc_skb_cb qdisc_cb; > > u16 mru; > - bool post_ct; > + u8 post_ct:1; > + u8 post_ct_snat:1; > + u8 post_ct_dnat:1; > u16 zone; /* Only valid if post_ct = true */ > }; > > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c > index 89e00eb..090dc2f 100644 > --- a/net/openvswitch/flow.c > +++ b/net/openvswitch/flow.c > @@ -846,7 +846,7 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info, > #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT) > struct tc_skb_ext *tc_ext; > #endif > - bool post_ct = false; > + bool post_ct = false, post_ct_snat = false, post_ct_dnat = false; > int res, err; > u16 zone = 0; > > @@ -887,6 +887,8 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info, > key->recirc_id = tc_ext ? tc_ext->chain : 0; > OVS_CB(skb)->mru = tc_ext ? tc_ext->mru : 0; > post_ct = tc_ext ? tc_ext->post_ct : false; > + post_ct_snat = post_ct ? tc_ext->post_ct_snat : false; > + post_ct_dnat = post_ct ? tc_ext->post_ct_dnat : false; > zone = post_ct ? tc_ext->zone : 0; > } else { > key->recirc_id = 0; > @@ -898,8 +900,16 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info, > err = key_extract(skb, key); > if (!err) { > ovs_ct_fill_key(skb, key, post_ct); /* Must be after key_extract(). */ > - if (post_ct && !skb_get_nfct(skb)) > - key->ct_zone = zone; > + if (post_ct) { > + if (!skb_get_nfct(skb)) { > + key->ct_zone = zone; > + } else { > + if (!post_ct_dnat) > + key->ct_state &= ~OVS_CS_F_DST_NAT; > + if (!post_ct_snat) > + key->ct_state &= ~OVS_CS_F_SRC_NAT; > + } > + } > } > return err; > } > diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c > index 796089c..8a78cbd 100644 > --- a/net/sched/act_ct.c > +++ b/net/sched/act_ct.c > @@ -832,6 +832,12 @@ static int ct_nat_execute(struct sk_buff *skb, struct nf_conn *ct, > } > > err = nf_nat_packet(ct, ctinfo, hooknum, skb); > + if (err == NF_ACCEPT) { > + if (maniptype == NF_NAT_MANIP_SRC) > + tc_skb_cb(skb)->post_ct_snat = 1; > + if (maniptype == NF_NAT_MANIP_DST) > + tc_skb_cb(skb)->post_ct_dnat = 1; > + } > out: > return err; > } > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c > index 5df4155..aecc4f0 100644 > --- a/net/sched/cls_api.c > +++ b/net/sched/cls_api.c > @@ -1683,6 +1683,8 @@ int tcf_classify_ingress(struct sk_buff *skb, > ext->chain = last_executed_chain; > ext->mru = cb->mru; > ext->post_ct = cb->post_ct; > + ext->post_ct_snat = cb->post_ct_snat; > + ext->post_ct_dnat = cb->post_ct_dnat; > ext->zone = cb->zone; > } > >
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index b53877d..9701643 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -285,7 +285,9 @@ struct tc_skb_ext { __u32 chain; __u16 mru; __u16 zone; - bool post_ct; + u8 post_ct:1; + u8 post_ct_snat:1; + u8 post_ct_dnat:1; }; #endif diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h index b52882e..6250acb 100644 --- a/include/net/pkt_sched.h +++ b/include/net/pkt_sched.h @@ -178,7 +178,9 @@ struct tc_skb_cb { struct qdisc_skb_cb qdisc_cb; u16 mru; - bool post_ct; + u8 post_ct:1; + u8 post_ct_snat:1; + u8 post_ct_dnat:1; u16 zone; /* Only valid if post_ct = true */ }; diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c index 89e00eb..090dc2f 100644 --- a/net/openvswitch/flow.c +++ b/net/openvswitch/flow.c @@ -846,7 +846,7 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info, #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT) struct tc_skb_ext *tc_ext; #endif - bool post_ct = false; + bool post_ct = false, post_ct_snat = false, post_ct_dnat = false; int res, err; u16 zone = 0; @@ -887,6 +887,8 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info, key->recirc_id = tc_ext ? tc_ext->chain : 0; OVS_CB(skb)->mru = tc_ext ? tc_ext->mru : 0; post_ct = tc_ext ? tc_ext->post_ct : false; + post_ct_snat = post_ct ? tc_ext->post_ct_snat : false; + post_ct_dnat = post_ct ? tc_ext->post_ct_dnat : false; zone = post_ct ? tc_ext->zone : 0; } else { key->recirc_id = 0; @@ -898,8 +900,16 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info, err = key_extract(skb, key); if (!err) { ovs_ct_fill_key(skb, key, post_ct); /* Must be after key_extract(). */ - if (post_ct && !skb_get_nfct(skb)) - key->ct_zone = zone; + if (post_ct) { + if (!skb_get_nfct(skb)) { + key->ct_zone = zone; + } else { + if (!post_ct_dnat) + key->ct_state &= ~OVS_CS_F_DST_NAT; + if (!post_ct_snat) + key->ct_state &= ~OVS_CS_F_SRC_NAT; + } + } } return err; } diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c index 796089c..8a78cbd 100644 --- a/net/sched/act_ct.c +++ b/net/sched/act_ct.c @@ -832,6 +832,12 @@ static int ct_nat_execute(struct sk_buff *skb, struct nf_conn *ct, } err = nf_nat_packet(ct, ctinfo, hooknum, skb); + if (err == NF_ACCEPT) { + if (maniptype == NF_NAT_MANIP_SRC) + tc_skb_cb(skb)->post_ct_snat = 1; + if (maniptype == NF_NAT_MANIP_DST) + tc_skb_cb(skb)->post_ct_dnat = 1; + } out: return err; } diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 5df4155..aecc4f0 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -1683,6 +1683,8 @@ int tcf_classify_ingress(struct sk_buff *skb, ext->chain = last_executed_chain; ext->mru = cb->mru; ext->post_ct = cb->post_ct; + ext->post_ct_snat = cb->post_ct_snat; + ext->post_ct_dnat = cb->post_ct_dnat; ext->zone = cb->zone; }