[ovs-dev] [PATCH v3 3/3] netdev-dpdk: Add vhost-user 'get_features' & 'get_status' functions

[Traynor, Kevin]

> -----Original Message-----
> From: Loftus, Ciara
> Sent: Wednesday, May 18, 2016 3:56 PM
> To: Traynor, Kevin <kevin.traynor at intel.com>; dev at openvswitch.org
> Subject: RE: [ovs-dev] [PATCH v3 3/3] netdev-dpdk: Add vhost-user
> 'get_features' & 'get_status' functions
> 
> > >
> > > Implementations for the netdev functions 'get_features' and
> > > 'get_status' are now available for vhost-user thanks to the
> addition of
> > > the vHost PMD.
> > >
> > > Signed-off-by: Ciara Loftus <ciara.loftus at intel.com>
> > > ---
> > >  lib/netdev-dpdk.c | 23 +++++++++++++----------
> > >  1 file changed, 13 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > > index 814ef83..fce1655 100644
> > > --- a/lib/netdev-dpdk.c
> > > +++ b/lib/netdev-dpdk.c
> > > @@ -2301,15 +2301,18 @@ netdev_dpdk_get_status(const struct netdev
> > > *netdev, struct smap *args)
> > >      smap_add_format(args, "max_rx_queues", "%u",
> > > dev_info.max_rx_queues);
> > >      smap_add_format(args, "max_tx_queues", "%u",
> > > dev_info.max_tx_queues);
> > >      smap_add_format(args, "max_mac_addrs", "%u",
> > > dev_info.max_mac_addrs);
> > > -    smap_add_format(args, "max_hash_mac_addrs", "%u",
> > > dev_info.max_hash_mac_addrs);
> > > -    smap_add_format(args, "max_vfs", "%u", dev_info.max_vfs);
> > > -    smap_add_format(args, "max_vmdq_pools", "%u",
> > > dev_info.max_vmdq_pools);
> > >
> > > -    if (dev_info.pci_dev) {
> > > -        smap_add_format(args, "pci-vendor_id", "0x%u",
> > > -                        dev_info.pci_dev->id.vendor_id);
> > > -        smap_add_format(args, "pci-device_id", "0x%x",
> > > -                        dev_info.pci_dev->id.device_id);
> > > +    if (dev->type == DPDK_DEV_ETH) {
> > > +        smap_add_format(args, "max_hash_mac_addrs", "%u",
> > > +                        dev_info.max_hash_mac_addrs);
> > > +        smap_add_format(args, "max_vfs", "%u", dev_info.max_vfs);
> > > +        smap_add_format(args, "max_vmdq_pools", "%u",
> > > dev_info.max_vmdq_pools);
> > > +        if (dev_info.pci_dev) {
> > > +            smap_add_format(args, "pci-vendor_id", "0x%u",
> > > +                            dev_info.pci_dev->id.vendor_id);
> > > +            smap_add_format(args, "pci-device_id", "0x%x",
> > > +                            dev_info.pci_dev->id.device_id);
> > > +        }
> > >      }
> > >
> > >      return 0;
> > > @@ -3431,8 +3434,8 @@ static const struct netdev_class OVS_UNUSED
> > > dpdk_vhost_user_class =
> > >          netdev_dpdk_vhost_user_send,
> > >          netdev_dpdk_get_carrier,
> > >          netdev_dpdk_get_stats,
> > > -        NULL,
> > > -        NULL,
> > > +        netdev_dpdk_get_features,
> > > +        netdev_dpdk_get_status,
> >
> > Maybe a comment for 1/3 but just thought of it while reviewing this:
> > do you need to call check_link_status() in netdev_dpdk_get_status()
> now
> > that it's not on a timer anymore? or is it guaranteed to be called
> for
> > all interfaces prior to netdev_dpdk_get_status().
> 
> Do you mean in netdev_dpdk_get_carrier() ?
> Perhaps the call isn't needed anymore... but there's maybe one corner
> case.
> Not sure if it's possible but if both the link status changed
> interrupt and netdev_dpdk_get_carrier are both called, it is whoever
> takes the dpdk_mutex first that will continue first. If
> netdev_dpdk_get_carrier() gets the mutex, it will only get up-to-date
> link info if the check_link_status() is called. If the call is not
> there, get_carrier() will return the old link status, free the mutex,
> and the interrupt will continue on and update the status to the new
> one immediately after.
> So I think it should be kept, but I don't have strong feelings about
> it either way.

That's not what I was getting at. I was just wondering if the dev->link
status structure will always be populated before we report it out.

Now I see there is a call to rte_eth_link_get_nowait() in
dpdk_eth_dev_init() so it will be fine - thanks.

> 
> Thanks,
> Ciara
> 
> >
> >
> > >          netdev_dpdk_vhost_user_rxq_recv);
> > >
> > >  void
> > > --
> > > 2.4.3
> > >
> > > _______________________________________________
> > > dev mailing list
> > > dev at openvswitch.org
> > > http://openvswitch.org/mailman/listinfo/dev