netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2] net: openvswitch: Be liberal in tcp conntrack.
@ 2020-11-16 13:01 nusiddiq
  2020-11-20  4:57 ` Jakub Kicinski
  0 siblings, 1 reply; 4+ messages in thread
From: nusiddiq @ 2020-11-16 13:01 UTC (permalink / raw)
  To: dev, netdev
  Cc: Pravin B Shelar, Florian Westphal, netfilter-devel,
	Pablo Neira Ayuso, Numan Siddique

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>
---
 include/net/netfilter/nf_conntrack_l4proto.h | 14 ++++++++++++++
 net/netfilter/nf_conntrack_proto_tcp.c       |  6 ------
 net/openvswitch/conntrack.c                  |  8 ++++++++
 3 files changed, 22 insertions(+), 6 deletions(-)

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;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next v2] net: openvswitch: Be liberal in tcp conntrack.
  2020-11-16 13:01 [PATCH net-next v2] net: openvswitch: Be liberal in tcp conntrack nusiddiq
@ 2020-11-20  4:57 ` Jakub Kicinski
  2020-11-20  6:32   ` Florian Westphal
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Kicinski @ 2020-11-20  4:57 UTC (permalink / raw)
  To: nusiddiq
  Cc: dev, netdev, Pravin B Shelar, Florian Westphal, netfilter-devel,
	Pablo Neira Ayuso

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?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next v2] net: openvswitch: Be liberal in tcp conntrack.
  2020-11-20  4:57 ` Jakub Kicinski
@ 2020-11-20  6:32   ` Florian Westphal
  2020-11-20 17:55     ` Jakub Kicinski
  0 siblings, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2020-11-20  6:32 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: nusiddiq, dev, netdev, Pravin B Shelar, Florian Westphal,
	netfilter-devel, Pablo Neira Ayuso

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>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next v2] net: openvswitch: Be liberal in tcp conntrack.
  2020-11-20  6:32   ` Florian Westphal
@ 2020-11-20 17:55     ` Jakub Kicinski
  0 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2020-11-20 17:55 UTC (permalink / raw)
  To: Florian Westphal
  Cc: nusiddiq, dev, netdev, Pravin B Shelar, netfilter-devel,
	Pablo Neira Ayuso

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-11-20 17:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-16 13:01 [PATCH net-next v2] net: openvswitch: Be liberal in tcp conntrack nusiddiq
2020-11-20  4:57 ` Jakub Kicinski
2020-11-20  6:32   ` Florian Westphal
2020-11-20 17:55     ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).