[ovs-dev] [PATCH V2] utilities: On RHEL 7 systems clean up after upgrade

[Aaron Conole]

Gregory Rose <gvrose8192 at gmail.com> writes:

> On 6/29/2018 10:48 AM, Aaron Conole wrote:
>> Justin Pettit <jpettit at ovn.org> writes:
>>
>>>> On Jun 26, 2018, at 10:19 AM, Greg Rose <gvrose8192 at gmail.com> wrote:
>>>>
>>>> When upgrading from older versions of OVS that used the built-in geneve
>>>> kernel module on RHEL 7 systems to newer versions that use the 'compat'
>>>> vport_geneve and vport_vxlan drivers we need to clean up some cruft
>>>> that might have been left over after the upgrade.
>>>>
>>>> Remove any genev_sys_6081 and vxlan_sys_4789 interfaces and then if
>>>> the RHEL 7 geneve or vxlan built-in drivers are loaded remove them
>>>> before loading the new drivers.
>>>>
>>>> Removing the geneve and vxlan built-in drivers will prevent occurrences
>>>> of the "unassociated datapath" errors that can sometimes occur in some
>>>> environments.
>>>>
>>>> Signed-off-by: Greg Rose <gvrose8192 at gmail.com>
>>>>
>>>> ---
>>>>
>>>> V2 - Move the changes to the ovs-kmod-ctl command as per Aaron
>>>>      Conole's suggestion.
>>>> ---
>>>> utilities/ovs-kmod-ctl.in | 19 +++++++++++++++++++
>>>> utilities/ovs-lib.in      |  2 +-
>>>> 2 files changed, 20 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/utilities/ovs-kmod-ctl.in b/utilities/ovs-kmod-ctl.in
>>>> index 7c50d64..9b09e22 100644
>>>> --- a/utilities/ovs-kmod-ctl.in
>>>> +++ b/utilities/ovs-kmod-ctl.in
>>>> @@ -72,6 +72,25 @@ remove_kmods() {
>>>>      if test -e /sys/module/openvswitch; then
>>>>          action "Removing openvswitch module" rmmod openvswitch
>>>>      fi
>>>> +
>>>> +    # Older releases may be using the rtnetlink interface while a
>>>> +    # newer release will want to use the internal compat interface
>>>> +    # for geneve and vxlan.
>>>> +    if test -e /sys/class/net/genev_sys_6081; then
>>>> +        action "ip link del link genev_sys_6081 dev genev_sys_6081" \
>>>> +                ip link del link genev_sys_6081 dev genev_sys_6081
>>>> +    fi
>>>> +    if test -e /sys/class/net/vxlan_sys_4789; then
>>>> +        action "ip link del link vxlan_sys_4789 dev vxlan_sys_4789" \
>>>> +                ip link del link vxlan_sys_4789 dev vxlan_sys_4789
>>>> +    fi
>>>> +
>>>> +    if test -e /sys/module/geneve; then
>>>> +        action "rmmod geneve" rmmod geneve
>>>> +    fi
>>>> +    if test -e /sys/module/vxlan; then
>>>> +        action "rmmod vxlan" rmmod vxlan
>>>> +    fi
>>>> }
>>>>
>>>> usage () {
>>>> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
>>>> index 6a958cb..765cd06 100644
>>>> --- a/utilities/ovs-lib.in
>>>> +++ b/utilities/ovs-lib.in
>>>> @@ -605,6 +605,7 @@ force_reload_kmod () {
>>>>      stop_ovsdb
>>>>      start_ovsdb || return 1
>>>>
>>>> +    action "ovs-appctl dpctl/flush-conntrack" ovs-appctl dpctl/flush-conntrack
>>>>      stop_forwarding
>>>>
>>>>      if action "Saving interface configuration" save_interfaces; then
>>>> @@ -620,7 +621,6 @@ force_reload_kmod () {
>>>>      for dp in `ovs-dpctl dump-dps`; do
>>>>          action "Removing datapath: $dp" ovs-dpctl del-dp "$dp"
>>>>      done
>>>> -    action "ovs-appctl dpctl/flush-conntrack"
>>> This looks like a separate bug fix that seems like it would be worth its own patch.
>> +1 - I completely missed it the first time around.
>>
>>> I think the point of string is to be human-readable.  What about
>>> strings like "Flush conntrack entries" and "Delete geneve device", for
>>> example?
>>>
>>> I assume these patches should be backported, correct?
>>>
>>> Aaron, since you commented on the previous version and have more
>>> expertise in this area, can you take a look at this (or the respin if
>>> Greg agrees with my comments)?
>>>
>>> Thanks,
>> Will do.  I have to admit, I don't understand why to prefer the compat
>> interface over rtnetlink.
>
> Purely to support ERSPAN which is not yet in RHEL 7.  If Red Hat ports
> ERSPAN back to RHEL 7 we will
> be ecstatic and more than happy to go back  to the rtnetlink interface.

Got it.  Thanks.

> Thanks,
>
> - Greg
>
>>
>>> --Justin