diff mbox

[ovs-dev,v2,2/2] datapath: nf_connlabels_replace() backport.

Message ID 1493086183-114264-2-git-send-email-jarno@ovn.org
State Accepted
Delegated to: Joe Stringer
Headers show

Commit Message

Jarno Rajahalme April 25, 2017, 2:09 a.m. UTC
Linux 4.7 changed nf_connlabels_replace() to trigger conntrack event
for a label change only when the labels actually changed.  Without
this change an update event is triggered even if the labels already
have the values they are being set to.

There is no way we can detect this functional change from Linux
headers, so provide replacements that work the same for older Linux
releases regardless if a distribution provides backports or not.

VMware-BZ: #1837218
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
v2: New for v2.

 .../include/net/netfilter/nf_conntrack_labels.h    | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)

Comments

Joe Stringer April 27, 2017, 5:31 p.m. UTC | #1
On 24 April 2017 at 19:09, Jarno Rajahalme <jarno@ovn.org> wrote:
> Linux 4.7 changed nf_connlabels_replace() to trigger conntrack event
> for a label change only when the labels actually changed.  Without
> this change an update event is triggered even if the labels already
> have the values they are being set to.
>
> There is no way we can detect this functional change from Linux
> headers, so provide replacements that work the same for older Linux
> releases regardless if a distribution provides backports or not.
>
> VMware-BZ: #1837218
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> ---

I assume that by "Linux 4.7", you're actually referring to the following commit?
5a8145f7b222 ("netfilter: labels: don't emit ct event if labels were
not changed")

It might be helpful for future reference (including review) to refer
to this commit in the code comment below.

Otherwise LGTM.

Acked-by: Joe Stringer <joe@ovn.org>
Jarno Rajahalme April 27, 2017, 5:46 p.m. UTC | #2
> On Apr 27, 2017, at 10:31 AM, Joe Stringer <joe@ovn.org> wrote:
> 
> On 24 April 2017 at 19:09, Jarno Rajahalme <jarno@ovn.org> wrote:
>> Linux 4.7 changed nf_connlabels_replace() to trigger conntrack event
>> for a label change only when the labels actually changed.  Without
>> this change an update event is triggered even if the labels already
>> have the values they are being set to.
>> 
>> There is no way we can detect this functional change from Linux
>> headers, so provide replacements that work the same for older Linux
>> releases regardless if a distribution provides backports or not.
>> 
>> VMware-BZ: #1837218
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>> ---
> 
> I assume that by "Linux 4.7", you're actually referring to the following commit?
> 5a8145f7b222 ("netfilter: labels: don't emit ct event if labels were
> not changed")
> 
> It might be helpful for future reference (including review) to refer
> to this commit in the code comment below.
> 

Right, thanks for suggesting this. I added this to both the commit message and the code comment.

> Otherwise LGTM.
> 
> Acked-by: Joe Stringer <joe@ovn.org>

Pushed to master,

  Jarno
diff mbox

Patch

diff --git a/datapath/linux/compat/include/net/netfilter/nf_conntrack_labels.h b/datapath/linux/compat/include/net/netfilter/nf_conntrack_labels.h
index 5af5a9a..61cf19e 100644
--- a/datapath/linux/compat/include/net/netfilter/nf_conntrack_labels.h
+++ b/datapath/linux/compat/include/net/netfilter/nf_conntrack_labels.h
@@ -57,4 +57,49 @@  static inline int nf_connlabels_get(struct net *net, unsigned int bits)
 static inline void nf_connlabels_put(struct net *net) { }
 #endif /* CONFIG_NF_CONNTRACK_LABELS */
 #endif /* HAVE_NF_CONNLABELS_GET_TAKES_BIT */
+
+/* Linux 4.7 introduced a functional change to trigger conntrack event for a
+ * label change only when the labels actually changed.  There is no way we can
+ * detect this from the headers, so provide replacements that work the same for
+ * OVS (where labels size is 128 bits == 16 bytes == 4 4-byte words). */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4,7,0)
+static int replace_u32(u32 *address, u32 mask, u32 new)
+{
+	u32 old, tmp;
+
+	do {
+		old = *address;
+		tmp = (old & mask) ^ new;
+		if (old == tmp)
+			return 0;
+	} while (cmpxchg(address, old, tmp) != old);
+
+	return 1;
+}
+
+static int rpl_nf_connlabels_replace(struct nf_conn *ct,
+				     const u32 *data,
+				     const u32 *mask, unsigned int words32)
+{
+	struct nf_conn_labels *labels;
+	unsigned int i;
+	int changed = 0;
+	u32 *dst;
+
+	labels = nf_ct_labels_find(ct);
+	if (!labels)
+		return -ENOSPC;
+
+	dst = (u32 *) labels->bits;
+	for (i = 0; i < words32; i++)
+		changed |= replace_u32(&dst[i], mask ? ~mask[i] : 0, data[i]);
+
+	if (changed)
+		nf_conntrack_event_cache(IPCT_LABEL, ct);
+
+	return 0;
+}
+#define nf_connlabels_replace rpl_nf_connlabels_replace
+#endif
+
 #endif /* _NF_CONNTRACK_LABELS_WRAPPER_H */