diff mbox

[ovs-dev] datapath: compat: keep skb mark across tunnel devices.

Message ID 1470969813-18568-1-git-send-email-pshelar@ovn.org
State Accepted
Headers show

Commit Message

Pravin Shelar Aug. 12, 2016, 2:43 a.m. UTC
Older kernel skb_scrub_packet() has bug which resets skb mark for
all packet. It is fixed during 3.18 release where it is reset
only for packets crossing namespace. So OVS is forced to use
compat skb_scrub_packet() on older kernel.
This is related to upstream bug fix commit ca7c7b9059e3
("skbuff: Do not scrub skb mark within the same name space").

VMware-BZ: #1710701
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
---
 acinclude.m4                                 | 2 --
 datapath/linux/compat/include/linux/skbuff.h | 2 +-
 datapath/linux/compat/skbuff-openvswitch.c   | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

Comments

Joe Stringer Aug. 12, 2016, 5:33 a.m. UTC | #1
On 11 August 2016 at 19:43, Pravin B Shelar <pshelar@ovn.org> wrote:
> Older kernel skb_scrub_packet() has bug which resets skb mark for
> all packet. It is fixed during 3.18 release where it is reset
> only for packets crossing namespace. So OVS is forced to use
> compat skb_scrub_packet() on older kernel.
> This is related to upstream bug fix commit ca7c7b9059e3
> ("skbuff: Do not scrub skb mark within the same name space").
>
> VMware-BZ: #1710701
> Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
> ---

This looks good to me, although the original upstream commit seems to
be in Linus' tree only from v4.1:
https://github.com/torvalds/linux/commit/213dd74aee765d4e5f3f4b9607fef0cf97faa2af

I guess it was backported to some kernel versions, but it would be
more consistent to use this number if in doubt.

Acked-by: Joe Stringer <joe@ovn.org>
Pravin Shelar Aug. 12, 2016, 4:37 p.m. UTC | #2
On Thu, Aug 11, 2016 at 10:33 PM, Joe Stringer <joe@ovn.org> wrote:
> On 11 August 2016 at 19:43, Pravin B Shelar <pshelar@ovn.org> wrote:
>> Older kernel skb_scrub_packet() has bug which resets skb mark for
>> all packet. It is fixed during 3.18 release where it is reset
>> only for packets crossing namespace. So OVS is forced to use
>> compat skb_scrub_packet() on older kernel.
>> This is related to upstream bug fix commit ca7c7b9059e3
>> ("skbuff: Do not scrub skb mark within the same name space").
>>
>> VMware-BZ: #1710701
>> Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
>> ---
>
> This looks good to me, although the original upstream commit seems to
> be in Linus' tree only from v4.1:
> https://github.com/torvalds/linux/commit/213dd74aee765d4e5f3f4b9607fef0cf97faa2af
>
> I guess it was backported to some kernel versions, but it would be
> more consistent to use this number if in doubt.
>

I only worry about latest stable kernel release. If ovs starts to
support all point releases our compatibility footprint would balloon
up.
In this case the code is already there, so it does not matter much win
this case. So I changed the version check.

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

Thanks for review, pushed to master and branch 2.5.
Joe Stringer Aug. 12, 2016, 5:22 p.m. UTC | #3
On 12 August 2016 at 09:37, pravin shelar <pshelar@ovn.org> wrote:
> On Thu, Aug 11, 2016 at 10:33 PM, Joe Stringer <joe@ovn.org> wrote:
>> On 11 August 2016 at 19:43, Pravin B Shelar <pshelar@ovn.org> wrote:
>>> Older kernel skb_scrub_packet() has bug which resets skb mark for
>>> all packet. It is fixed during 3.18 release where it is reset
>>> only for packets crossing namespace. So OVS is forced to use
>>> compat skb_scrub_packet() on older kernel.
>>> This is related to upstream bug fix commit ca7c7b9059e3
>>> ("skbuff: Do not scrub skb mark within the same name space").
>>>
>>> VMware-BZ: #1710701
>>> Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
>>> ---
>>
>> This looks good to me, although the original upstream commit seems to
>> be in Linus' tree only from v4.1:
>> https://github.com/torvalds/linux/commit/213dd74aee765d4e5f3f4b9607fef0cf97faa2af
>>
>> I guess it was backported to some kernel versions, but it would be
>> more consistent to use this number if in doubt.
>>
>
> I only worry about latest stable kernel release. If ovs starts to
> support all point releases our compatibility footprint would balloon
> up.
> In this case the code is already there, so it does not matter much win
> this case. So I changed the version check.

Fair enough, as a general rule it's going to be easier to just support
the latest point releases.
diff mbox

Patch

diff --git a/acinclude.m4 b/acinclude.m4
index 25295c5..3ffdb9e 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -551,8 +551,6 @@  AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [inner_protocol_type])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_inner_transport_offset])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [kfree_skb_list])
-  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_scrub_packet.*xnet],
-		  [OVS_DEFINE([HAVE_SKB_SCRUB_PACKET_XNET])])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [rxhash])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [u16.*rxhash],
                   [OVS_DEFINE([HAVE_U16_RXHASH])])
diff --git a/datapath/linux/compat/include/linux/skbuff.h b/datapath/linux/compat/include/linux/skbuff.h
index ad5a558..c0ad7b9 100644
--- a/datapath/linux/compat/include/linux/skbuff.h
+++ b/datapath/linux/compat/include/linux/skbuff.h
@@ -310,7 +310,7 @@  static inline unsigned char *skb_pull_rcsum(struct sk_buff *skb, unsigned int le
 
 #endif
 
-#ifndef HAVE_SKB_SCRUB_PACKET_XNET
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0)
 #define skb_scrub_packet rpl_skb_scrub_packet
 void rpl_skb_scrub_packet(struct sk_buff *skb, bool xnet);
 #endif
diff --git a/datapath/linux/compat/skbuff-openvswitch.c b/datapath/linux/compat/skbuff-openvswitch.c
index 3a76823..34cf1a7 100644
--- a/datapath/linux/compat/skbuff-openvswitch.c
+++ b/datapath/linux/compat/skbuff-openvswitch.c
@@ -278,7 +278,7 @@  void rpl_kfree_skb_list(struct sk_buff *segs)
 EXPORT_SYMBOL(rpl_kfree_skb_list);
 #endif
 
-#ifndef HAVE_SKB_SCRUB_PACKET_XNET
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0)
 
 #define nf_reset_trace rpl_nf_reset_trace
 static void nf_reset_trace(struct sk_buff *skb)