[ovs-dev] Revert "Revert "utilities/ovs-ctl: Force removal of ip_gre/gre""

Message ID 1536673955-2687-2-git-send-email-pkusunyifeng@gmail.com
State Accepted
Headers show
Series
  • [ovs-dev] Revert "Revert "utilities/ovs-ctl: Force removal of ip_gre/gre""
Related show

Commit Message

Yifeng Sun Sept. 11, 2018, 1:52 p.m.
Please backport this patch to upstream OVS down to 2.10.

Author: Greg Rose <roseg@vmware.com>
Date:   Wed Jun 6 15:34:44 2018 -0700

This reverts commit a94f9524dbc11c78c83d1a49959497f5e73bf949.

Signed-off-by: Greg Rose <roseg@vmware.com>
---
 utilities/ovs-lib.in | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Gregory Rose Sept. 11, 2018, 10:24 p.m. | #1
On 9/11/2018 6:52 AM, Yifeng Sun wrote:
> Please backport this patch to upstream OVS down to 2.10.
>
> Author: Greg Rose <roseg@vmware.com>
> Date:   Wed Jun 6 15:34:44 2018 -0700
>
> This reverts commit a94f9524dbc11c78c83d1a49959497f5e73bf949.
>
> Signed-off-by: Greg Rose <roseg@vmware.com>

I guess someone else will have to review this one.  Pretty darn basic tho.

- Greg


> ---
>   utilities/ovs-lib.in | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index 090a144..f6b5393 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -627,6 +627,14 @@ force_reload_kmod () {
>           action "Removing datapath: $dp" ovs-dpctl del-dp "$dp"
>       done
>   
> +    if test -e /sys/module/ip_gre; then
> +        action "Forcing removal of ip_gre module" rmmod ip_gre
> +    fi
> +
> +    if test -e /sys/module/gre; then
> +        action "Forcing removal of gre module" rmmod gre
> +    fi
> +
>       ovs_kmod_ctl remove
>   
>       # Start vswitchd by asking it to wait till flow restore is finished.
Ben Pfaff Sept. 12, 2018, 10:09 p.m. | #2
On Tue, Sep 11, 2018 at 06:52:35AM -0700, Yifeng Sun wrote:
> Please backport this patch to upstream OVS down to 2.10.
> 
> Author: Greg Rose <roseg@vmware.com>
> Date:   Wed Jun 6 15:34:44 2018 -0700
> 
> This reverts commit a94f9524dbc11c78c83d1a49959497f5e73bf949.
> 
> Signed-off-by: Greg Rose <roseg@vmware.com>

The log message should explain why the commit is being reverted.
Gregory Rose Sept. 13, 2018, 4:42 p.m. | #3
On 9/12/2018 3:09 PM, Ben Pfaff wrote:
> On Tue, Sep 11, 2018 at 06:52:35AM -0700, Yifeng Sun wrote:
>> Please backport this patch to upstream OVS down to 2.10.
>>
>> Author: Greg Rose <roseg@vmware.com>
>> Date:   Wed Jun 6 15:34:44 2018 -0700
>>
>> This reverts commit a94f9524dbc11c78c83d1a49959497f5e73bf949.
>>
>> Signed-off-by: Greg Rose <roseg@vmware.com>
> The log message should explain why the commit is being reverted.
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Ben,

Here's a patch with a better commit messsage:

From: Yifeng Sun <pkusunyifeng@gmail.com>
Date: Tue, 11 Sep 2018 06:52:35 -0700
Subject: [PATCH] Revert "Revert "utilities/ovs-ctl: Force removal of
  ip_gre/gre""

Please backport this patch to upstream OVS down to 2.10.

Author: Greg Rose <roseg@vmware.com>
Date:   Wed Jun 6 15:34:44 2018 -0700

This reverts commit a94f9524dbc11c78c83d1a49959497f5e73bf949.

This is a revert of a previously reverted commit
2bdd1f3d96a86bea6bdb8788f23ec7dd99b289e3.

When we originally added commit 2bdd1f3d96 it was part of an
effort to work around gre module conflicts found while enabling
the ERSPAN feature. Testing at the time did not show any benefit
so in commit a94f9524db we reverted it.  However, further
developments showed that in some corner cases it did have a
benefit and it did not do any harm so we reverted the original
revert to restore the code.

Signed-off-by: Greg Rose <roseg@vmware.com>
---
  utilities/ovs-lib.in | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index 090a144..f6b5393 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -627,6 +627,14 @@ force_reload_kmod () {
          action "Removing datapath: $dp" ovs-dpctl del-dp "$dp"
      done

+    if test -e /sys/module/ip_gre; then
+        action "Forcing removal of ip_gre module" rmmod ip_gre
+    fi
+
+    if test -e /sys/module/gre; then
+        action "Forcing removal of gre module" rmmod gre
+    fi
+
      ovs_kmod_ctl remove

      # Start vswitchd by asking it to wait till flow restore is finished.
--
1.8.3.1
Yifeng Sun Sept. 13, 2018, 5:04 p.m. | #4
Looks good to me and testing shows no problem. Thanks Greg.

Tested-by: Yifeng Sun <pkusunyifeng@gmail.com>
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>

On Thu, Sep 13, 2018 at 9:42 AM Gregory Rose <gvrose8192@gmail.com> wrote:

> On 9/12/2018 3:09 PM, Ben Pfaff wrote:
> > On Tue, Sep 11, 2018 at 06:52:35AM -0700, Yifeng Sun wrote:
> >> Please backport this patch to upstream OVS down to 2.10.
> >>
> >> Author: Greg Rose <roseg@vmware.com>
> >> Date:   Wed Jun 6 15:34:44 2018 -0700
> >>
> >> This reverts commit a94f9524dbc11c78c83d1a49959497f5e73bf949.
> >>
> >> Signed-off-by: Greg Rose <roseg@vmware.com>
> > The log message should explain why the commit is being reverted.
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> Ben,
>
> Here's a patch with a better commit messsage:
>
> From: Yifeng Sun <pkusunyifeng@gmail.com>
> Date: Tue, 11 Sep 2018 06:52:35 -0700
> Subject: [PATCH] Revert "Revert "utilities/ovs-ctl: Force removal of
>   ip_gre/gre""
>
> Please backport this patch to upstream OVS down to 2.10.
>
> Author: Greg Rose <roseg@vmware.com>
> Date:   Wed Jun 6 15:34:44 2018 -0700
>
> This reverts commit a94f9524dbc11c78c83d1a49959497f5e73bf949.
>
> This is a revert of a previously reverted commit
> 2bdd1f3d96a86bea6bdb8788f23ec7dd99b289e3.
>
> When we originally added commit 2bdd1f3d96 it was part of an
> effort to work around gre module conflicts found while enabling
> the ERSPAN feature. Testing at the time did not show any benefit
> so in commit a94f9524db we reverted it.  However, further
> developments showed that in some corner cases it did have a
> benefit and it did not do any harm so we reverted the original
> revert to restore the code.
>
> Signed-off-by: Greg Rose <roseg@vmware.com>
> ---
>   utilities/ovs-lib.in | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index 090a144..f6b5393 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -627,6 +627,14 @@ force_reload_kmod () {
>           action "Removing datapath: $dp" ovs-dpctl del-dp "$dp"
>       done
>
> +    if test -e /sys/module/ip_gre; then
> +        action "Forcing removal of ip_gre module" rmmod ip_gre
> +    fi
> +
> +    if test -e /sys/module/gre; then
> +        action "Forcing removal of gre module" rmmod gre
> +    fi
> +
>       ovs_kmod_ctl remove
>
>       # Start vswitchd by asking it to wait till flow restore is finished.
> --
> 1.8.3.1
>
>
Justin Pettit Sept. 14, 2018, 7:43 p.m. | #5
Thanks, guys.  I pushed this to master and branch-2.10.

--Justin


> On Sep 13, 2018, at 10:04 AM, Yifeng Sun <pkusunyifeng@gmail.com> wrote:
> 
> Looks good to me and testing shows no problem. Thanks Greg.
> 
> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com>
> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
> 
> On Thu, Sep 13, 2018 at 9:42 AM Gregory Rose <gvrose8192@gmail.com> wrote:
> 
>> On 9/12/2018 3:09 PM, Ben Pfaff wrote:
>>> On Tue, Sep 11, 2018 at 06:52:35AM -0700, Yifeng Sun wrote:
>>>> Please backport this patch to upstream OVS down to 2.10.
>>>> 
>>>> Author: Greg Rose <roseg@vmware.com>
>>>> Date:   Wed Jun 6 15:34:44 2018 -0700
>>>> 
>>>> This reverts commit a94f9524dbc11c78c83d1a49959497f5e73bf949.
>>>> 
>>>> Signed-off-by: Greg Rose <roseg@vmware.com>
>>> The log message should explain why the commit is being reverted.
>>> _______________________________________________
>>> dev mailing list
>>> dev@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> 
>> Ben,
>> 
>> Here's a patch with a better commit messsage:
>> 
>> From: Yifeng Sun <pkusunyifeng@gmail.com>
>> Date: Tue, 11 Sep 2018 06:52:35 -0700
>> Subject: [PATCH] Revert "Revert "utilities/ovs-ctl: Force removal of
>>  ip_gre/gre""
>> 
>> Please backport this patch to upstream OVS down to 2.10.
>> 
>> Author: Greg Rose <roseg@vmware.com>
>> Date:   Wed Jun 6 15:34:44 2018 -0700
>> 
>> This reverts commit a94f9524dbc11c78c83d1a49959497f5e73bf949.
>> 
>> This is a revert of a previously reverted commit
>> 2bdd1f3d96a86bea6bdb8788f23ec7dd99b289e3.
>> 
>> When we originally added commit 2bdd1f3d96 it was part of an
>> effort to work around gre module conflicts found while enabling
>> the ERSPAN feature. Testing at the time did not show any benefit
>> so in commit a94f9524db we reverted it.  However, further
>> developments showed that in some corner cases it did have a
>> benefit and it did not do any harm so we reverted the original
>> revert to restore the code.
>> 
>> Signed-off-by: Greg Rose <roseg@vmware.com>
>> ---
>>  utilities/ovs-lib.in | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
>> index 090a144..f6b5393 100644
>> --- a/utilities/ovs-lib.in
>> +++ b/utilities/ovs-lib.in
>> @@ -627,6 +627,14 @@ force_reload_kmod () {
>>          action "Removing datapath: $dp" ovs-dpctl del-dp "$dp"
>>      done
>> 
>> +    if test -e /sys/module/ip_gre; then
>> +        action "Forcing removal of ip_gre module" rmmod ip_gre
>> +    fi
>> +
>> +    if test -e /sys/module/gre; then
>> +        action "Forcing removal of gre module" rmmod gre
>> +    fi
>> +
>>      ovs_kmod_ctl remove
>> 
>>      # Start vswitchd by asking it to wait till flow restore is finished.
>> --
>> 1.8.3.1
>> 
>> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Gregory Rose Sept. 14, 2018, 8:09 p.m. | #6
Thanks Justin!

-----Original Message-----
From: Justin Pettit <jpettit@ovn.org> 
Sent: Friday, September 14, 2018 12:44 PM
To: Yifeng Sun <pkusunyifeng@gmail.com>
Cc: Gregory Rose <gvrose8192@gmail.com>; ovs dev <dev@openvswitch.org>; Gregory Rose <roseg@vmware.com>
Subject: Re: [ovs-dev] Revert "Revert "utilities/ovs-ctl: Force removal of ip_gre/gre""

Thanks, guys.  I pushed this to master and branch-2.10.

--Justin


> On Sep 13, 2018, at 10:04 AM, Yifeng Sun <pkusunyifeng@gmail.com> wrote:
> 
> Looks good to me and testing shows no problem. Thanks Greg.
> 
> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com>
> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
> 
> On Thu, Sep 13, 2018 at 9:42 AM Gregory Rose <gvrose8192@gmail.com> wrote:
> 
>> On 9/12/2018 3:09 PM, Ben Pfaff wrote:
>>> On Tue, Sep 11, 2018 at 06:52:35AM -0700, Yifeng Sun wrote:
>>>> Please backport this patch to upstream OVS down to 2.10.
>>>> 
>>>> Author: Greg Rose <roseg@vmware.com>
>>>> Date:   Wed Jun 6 15:34:44 2018 -0700
>>>> 
>>>> This reverts commit a94f9524dbc11c78c83d1a49959497f5e73bf949.
>>>> 
>>>> Signed-off-by: Greg Rose <roseg@vmware.com>
>>> The log message should explain why the commit is being reverted.
>>> _______________________________________________
>>> dev mailing list
>>> dev@openvswitch.org
>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai
>>> l.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&amp;data=02%7C01%7C
>>> roseg%40vmware.com%7C3bb83bd2848f4d0f2ce108d61a7a603f%7Cb39138ca3cee
>>> 4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636725510249773317&amp;sdata=OzpW7RX8
>>> uYB8KIzRiwWrRUVwsS496PBPt%2FYbe8x2uXQ%3D&amp;reserved=0
>> 
>> Ben,
>> 
>> Here's a patch with a better commit messsage:
>> 
>> From: Yifeng Sun <pkusunyifeng@gmail.com>
>> Date: Tue, 11 Sep 2018 06:52:35 -0700
>> Subject: [PATCH] Revert "Revert "utilities/ovs-ctl: Force removal of  
>> ip_gre/gre""
>> 
>> Please backport this patch to upstream OVS down to 2.10.
>> 
>> Author: Greg Rose <roseg@vmware.com>
>> Date:   Wed Jun 6 15:34:44 2018 -0700
>> 
>> This reverts commit a94f9524dbc11c78c83d1a49959497f5e73bf949.
>> 
>> This is a revert of a previously reverted commit 
>> 2bdd1f3d96a86bea6bdb8788f23ec7dd99b289e3.
>> 
>> When we originally added commit 2bdd1f3d96 it was part of an effort 
>> to work around gre module conflicts found while enabling the ERSPAN 
>> feature. Testing at the time did not show any benefit so in commit 
>> a94f9524db we reverted it.  However, further developments showed that 
>> in some corner cases it did have a benefit and it did not do any harm 
>> so we reverted the original revert to restore the code.
>> 
>> Signed-off-by: Greg Rose <roseg@vmware.com>
>> ---
>>  utilities/ovs-lib.in | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in index 
>> 090a144..f6b5393 100644
>> --- a/utilities/ovs-lib.in
>> +++ b/utilities/ovs-lib.in
>> @@ -627,6 +627,14 @@ force_reload_kmod () {
>>          action "Removing datapath: $dp" ovs-dpctl del-dp "$dp"
>>      done
>> 
>> +    if test -e /sys/module/ip_gre; then
>> +        action "Forcing removal of ip_gre module" rmmod ip_gre
>> +    fi
>> +
>> +    if test -e /sys/module/gre; then
>> +        action "Forcing removal of gre module" rmmod gre
>> +    fi
>> +
>>      ovs_kmod_ctl remove
>> 
>>      # Start vswitchd by asking it to wait till flow restore is finished.
>> --
>> 1.8.3.1
>> 
>> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.
> openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&amp;data=02%7C01%7Crose
> g%40vmware.com%7C3bb83bd2848f4d0f2ce108d61a7a603f%7Cb39138ca3cee4b4aa4
> d6cd83d9dd62f0%7C1%7C0%7C636725510249773317&amp;sdata=OzpW7RX8uYB8KIzR
> iwWrRUVwsS496PBPt%2FYbe8x2uXQ%3D&amp;reserved=0

Patch

diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index 090a144..f6b5393 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -627,6 +627,14 @@  force_reload_kmod () {
         action "Removing datapath: $dp" ovs-dpctl del-dp "$dp"
     done
 
+    if test -e /sys/module/ip_gre; then
+        action "Forcing removal of ip_gre module" rmmod ip_gre
+    fi
+
+    if test -e /sys/module/gre; then
+        action "Forcing removal of gre module" rmmod gre
+    fi
+
     ovs_kmod_ctl remove
 
     # Start vswitchd by asking it to wait till flow restore is finished.