[ovs-dev] Fix: vhost user port status
diff mbox

Message ID 1503547983-13850-1-git-send-email-wangzhike@jd.com
State Superseded
Delegated to: Darrell Ball
Headers show

Commit Message

Zhike Wang Aug. 24, 2017, 4:13 a.m. UTC
After ovs-vswitchd reboots, vhost user status is displayed as
LINK DOWN though the traffic is OK.

The problem is that the port may be udpated while the vhost_reconfigured
is false. Then the vhost_reconfigured is updated to true. As a result,
the vhost user status is kept as LINK-DOWN.

Signed-off-by: wangzhike <wangzhike@jd.com>
---
 lib/netdev-dpdk.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Darrell Ball Aug. 25, 2017, 1:35 a.m. UTC | #1
Hi Lawrence 

I am not very particular about the title prefix, but could we use something more
technically specific than ‘Fix’; maybe netdev-dpdk  ?
Title should also end with a period.

On 8/23/17, 9:13 PM, "ovs-dev-bounces@openvswitch.org on behalf of wangzhike" <ovs-dev-bounces@openvswitch.org on behalf of wangzhike@jd.com> wrote:

    After ovs-vswitchd reboots, vhost user status is displayed as
    LINK DOWN though the traffic is OK.
    
    The problem is that the port may be udpated while the vhost_reconfigured
    is false. Then the vhost_reconfigured is updated to true. As a result,
    the vhost user status is kept as LINK-DOWN.

[Darrell]
Just so I understand, are you actually using vhost-user or really vhost-user-client ports ?
JTBS, can you describe what you did after vswitchd restart ?

    
    Signed-off-by: wangzhike <wangzhike@jd.com>

    ---
     lib/netdev-dpdk.c | 6 +++++-
     1 file changed, 5 insertions(+), 1 deletion(-)
    
    diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
    index 1aaf6f7..e90fd0e 100644
    --- a/lib/netdev-dpdk.c
    +++ b/lib/netdev-dpdk.c
    @@ -3227,7 +3227,11 @@ dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev)
         }
     
         if (netdev_dpdk_get_vid(dev) >= 0) {
    -        dev->vhost_reconfigured = true;
    +        if (dev->vhost_reconfigured == false) {
    +            dev->vhost_reconfigured = true;
    +            /* change vhost_reconfigured may affect carrier status */

[Darrell]
Maybe we can say ?
/* Carrier status may need updating. */
Would this be ok ?
BTW, comments should end with a period.

    +            netdev_change_seq_changed(&dev->up);
    +        }
         }
     
         return 0;
    -- 
    1.8.3.1
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=TOaqootDYDg45mYkwMOQnfh-LiXCCGZAYC37gEf9aWE&s=5_PkYdvwnY5DuxfvSrVJ2WbjCGRVTpwE6kjLs6FLbzI&e=
Zhike Wang Aug. 25, 2017, 3:10 a.m. UTC | #2
Hi Darrell,

Thanks for your comment. I just create a new patch accordingly.

Yes, I am testing vhost user client port.
After ovs-vswitchd restarts, I did nothing. I just used command "ovs-ofctl show br" to get the port status, and found the status is "LINK DOWN".
With the patch, the status is 0 as expected.

Br,
Wang Zhike

-----Original Message-----
From: Darrell Ball [mailto:dball@vmware.com] 

Sent: Friday, August 25, 2017 9:35 AM
To: 王志克; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Fix: vhost user port status

Hi Lawrence 

I am not very particular about the title prefix, but could we use something more
technically specific than ‘Fix’; maybe netdev-dpdk  ?
Title should also end with a period.

On 8/23/17, 9:13 PM, "ovs-dev-bounces@openvswitch.org on behalf of wangzhike" <ovs-dev-bounces@openvswitch.org on behalf of wangzhike@jd.com> wrote:

    After ovs-vswitchd reboots, vhost user status is displayed as
    LINK DOWN though the traffic is OK.
    
    The problem is that the port may be udpated while the vhost_reconfigured
    is false. Then the vhost_reconfigured is updated to true. As a result,
    the vhost user status is kept as LINK-DOWN.

[Darrell]
Just so I understand, are you actually using vhost-user or really vhost-user-client ports ?
JTBS, can you describe what you did after vswitchd restart ?

    
    Signed-off-by: wangzhike <wangzhike@jd.com>

    ---
     lib/netdev-dpdk.c | 6 +++++-
     1 file changed, 5 insertions(+), 1 deletion(-)
    
    diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
    index 1aaf6f7..e90fd0e 100644
    --- a/lib/netdev-dpdk.c
    +++ b/lib/netdev-dpdk.c
    @@ -3227,7 +3227,11 @@ dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev)
         }
     
         if (netdev_dpdk_get_vid(dev) >= 0) {
    -        dev->vhost_reconfigured = true;
    +        if (dev->vhost_reconfigured == false) {
    +            dev->vhost_reconfigured = true;
    +            /* change vhost_reconfigured may affect carrier status */

[Darrell]
Maybe we can say ?
/* Carrier status may need updating. */
Would this be ok ?
BTW, comments should end with a period.

    +            netdev_change_seq_changed(&dev->up);
    +        }
         }
     
         return 0;
    -- 
    1.8.3.1
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=TOaqootDYDg45mYkwMOQnfh-LiXCCGZAYC37gEf9aWE&s=5_PkYdvwnY5DuxfvSrVJ2WbjCGRVTpwE6kjLs6FLbzI&e=

Patch
diff mbox

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 1aaf6f7..e90fd0e 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -3227,7 +3227,11 @@  dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev)
     }
 
     if (netdev_dpdk_get_vid(dev) >= 0) {
-        dev->vhost_reconfigured = true;
+        if (dev->vhost_reconfigured == false) {
+            dev->vhost_reconfigured = true;
+            /* change vhost_reconfigured may affect carrier status */
+            netdev_change_seq_changed(&dev->up);
+        }
     }
 
     return 0;