| Message ID | 20201116130126.3065077-1-nusiddiq@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [net-next,v2] net: openvswitch: Be liberal in tcp conntrack. | expand |
On Mon, 16 Nov 2020 18:31:26 +0530 nusiddiq@redhat.com wrote: > From: Numan Siddique <nusiddiq@redhat.com> > > There is no easy way to distinguish if a conntracked tcp packet is > marked invalid because of tcp_in_window() check error or because > it doesn't belong to an existing connection. With this patch, > openvswitch sets liberal tcp flag for the established sessions so > that out of window packets are not marked invalid. > > A helper function - nf_ct_set_tcp_be_liberal(nf_conn) is added which > sets this flag for both the directions of the nf_conn. > > Suggested-by: Florian Westphal <fw@strlen.de> > Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Florian, LGTY?
Jakub Kicinski <kuba@kernel.org> wrote: > On Mon, 16 Nov 2020 18:31:26 +0530 nusiddiq@redhat.com wrote: > > From: Numan Siddique <nusiddiq@redhat.com> > > > > There is no easy way to distinguish if a conntracked tcp packet is > > marked invalid because of tcp_in_window() check error or because > > it doesn't belong to an existing connection. With this patch, > > openvswitch sets liberal tcp flag for the established sessions so > > that out of window packets are not marked invalid. > > > > A helper function - nf_ct_set_tcp_be_liberal(nf_conn) is added which > > sets this flag for both the directions of the nf_conn. > > > > Suggested-by: Florian Westphal <fw@strlen.de> > > Signed-off-by: Numan Siddique <nusiddiq@redhat.com> > > Florian, LGTY? Sorry, this one sailed past me. Acked-by: Florian Westphal <fw@strlen.de>
On Fri, 20 Nov 2020 07:32:11 +0100 Florian Westphal wrote: > Jakub Kicinski <kuba@kernel.org> wrote: > > On Mon, 16 Nov 2020 18:31:26 +0530 nusiddiq@redhat.com wrote: > > > From: Numan Siddique <nusiddiq@redhat.com> > > > > > > There is no easy way to distinguish if a conntracked tcp packet is > > > marked invalid because of tcp_in_window() check error or because > > > it doesn't belong to an existing connection. With this patch, > > > openvswitch sets liberal tcp flag for the established sessions so > > > that out of window packets are not marked invalid. > > > > > > A helper function - nf_ct_set_tcp_be_liberal(nf_conn) is added which > > > sets this flag for both the directions of the nf_conn. > > > > > > Suggested-by: Florian Westphal <fw@strlen.de> > > > Signed-off-by: Numan Siddique <nusiddiq@redhat.com> > > Acked-by: Florian Westphal <fw@strlen.de> Thanks! Applied.
diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h index 88186b95b3c2..9be7320b994f 100644 --- a/include/net/netfilter/nf_conntrack_l4proto.h +++ b/include/net/netfilter/nf_conntrack_l4proto.h @@ -203,6 +203,20 @@ static inline struct nf_icmp_net *nf_icmpv6_pernet(struct net *net) { return &net->ct.nf_ct_proto.icmpv6; } + +/* Caller must check nf_ct_protonum(ct) is IPPROTO_TCP before calling. */ +static inline void nf_ct_set_tcp_be_liberal(struct nf_conn *ct) +{ + ct->proto.tcp.seen[0].flags |= IP_CT_TCP_FLAG_BE_LIBERAL; + ct->proto.tcp.seen[1].flags |= IP_CT_TCP_FLAG_BE_LIBERAL; +} + +/* Caller must check nf_ct_protonum(ct) is IPPROTO_TCP before calling. */ +static inline bool nf_conntrack_tcp_established(const struct nf_conn *ct) +{ + return ct->proto.tcp.state == TCP_CONNTRACK_ESTABLISHED && + test_bit(IPS_ASSURED_BIT, &ct->status); +} #endif #ifdef CONFIG_NF_CT_PROTO_DCCP diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c index c8fb2187ad4b..811c6c9b59e1 100644 --- a/net/netfilter/nf_conntrack_proto_tcp.c +++ b/net/netfilter/nf_conntrack_proto_tcp.c @@ -834,12 +834,6 @@ static noinline bool tcp_new(struct nf_conn *ct, const struct sk_buff *skb, return true; } -static bool nf_conntrack_tcp_established(const struct nf_conn *ct) -{ - return ct->proto.tcp.state == TCP_CONNTRACK_ESTABLISHED && - test_bit(IPS_ASSURED_BIT, &ct->status); -} - /* Returns verdict for packet, or -1 for invalid. */ int nf_conntrack_tcp_packet(struct nf_conn *ct, struct sk_buff *skb, diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c index 4beb96139d77..6a88daab0190 100644 --- a/net/openvswitch/conntrack.c +++ b/net/openvswitch/conntrack.c @@ -1037,6 +1037,14 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key, ovs_ct_helper(skb, info->family) != NF_ACCEPT) { return -EINVAL; } + + if (nf_ct_protonum(ct) == IPPROTO_TCP && + nf_ct_is_confirmed(ct) && nf_conntrack_tcp_established(ct)) { + /* Be liberal for tcp packets so that out-of-window + * packets are not marked invalid. + */ + nf_ct_set_tcp_be_liberal(ct); + } } return 0;