On Thu, Oct 25, 2018 at 03:17:40PM +0100, Ian Stokes wrote: > This commit fixes netdev_dpdk_get_features() by initializing a bitmap > that represents current features to zero and accounting for non defined > link speed values in the OpenFlow spec. > > The current approach for retrieving netdev dpdk features uses a > pointer allocated in the stack without being initialized. As such there > is no guarantee that the bitmap will be accurate. Fix this by declaring > and initializing local variable 'feature' to be used when building the > bitmap, with its value then assigned to the pointer. Also account for > link speeds not defined in the OpenFlow spec by defaulting to > NETDEV_F_OTHER for undefined link speeds. > > Fixes: 8a9562d21a40 ("dpif-netdev: Add DPDK netdev.") > Signed-off-by: Ian Stokes <ian.sto...@intel.com> > --- > Flavio, this patch is based on suggestions from you > > https://mail.openvswitch.org/pipermail/ovs-dev/2018-September/351809.html > > I'd like to add > > Co-authored-by: Flavio Leitner <f...@sysclose.org> > Signed-off-by: Flavio Leitner <f...@sysclose.org> > > if you agree to sign off on this.
Yup, sounds good to me. Thanks Ian! fbl > --- > lib/netdev-dpdk.c | 65 > ++++++++++++++++++++++++++++++++++--------------------- > 1 file changed, 40 insertions(+), 25 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index f91aa27cd..35eb30f8d 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -2707,43 +2707,58 @@ netdev_dpdk_get_features(const struct netdev *netdev, > { > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > struct rte_eth_link link; > + uint32_t feature = 0; > > ovs_mutex_lock(&dev->mutex); > link = dev->link; > ovs_mutex_unlock(&dev->mutex); > > - if (link.link_duplex == ETH_LINK_HALF_DUPLEX) { > - if (link.link_speed == ETH_SPEED_NUM_10M) { > - *current = NETDEV_F_10MB_HD; > - } > - if (link.link_speed == ETH_SPEED_NUM_100M) { > - *current = NETDEV_F_100MB_HD; > - } > - if (link.link_speed == ETH_SPEED_NUM_1G) { > - *current = NETDEV_F_1GB_HD; > - } > - } else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) { > - if (link.link_speed == ETH_SPEED_NUM_10M) { > - *current = NETDEV_F_10MB_FD; > - } > - if (link.link_speed == ETH_SPEED_NUM_100M) { > - *current = NETDEV_F_100MB_FD; > - } > - if (link.link_speed == ETH_SPEED_NUM_1G) { > - *current = NETDEV_F_1GB_FD; > - } > - if (link.link_speed == ETH_SPEED_NUM_10G) { > - *current = NETDEV_F_10GB_FD; > + if (link.link_duplex == ETH_LINK_FULL_DUPLEX) { > + switch (link.link_speed) { > + /* OpenFlow defined values: see enum ofp_port_features */ > + case ETH_SPEED_NUM_10M: > + feature |= NETDEV_F_10MB_FD; > + break; > + case ETH_SPEED_NUM_100M: > + feature |= NETDEV_F_100MB_FD; > + break; > + case ETH_SPEED_NUM_1G: > + feature |= NETDEV_F_1GB_FD; > + break; > + case ETH_SPEED_NUM_10G: > + feature |= NETDEV_F_10GB_FD; > + break; > + case ETH_SPEED_NUM_40G: > + feature |= NETDEV_F_40GB_FD; > + break; > + case ETH_SPEED_NUM_100G: > + feature |= NETDEV_F_100GB_FD; > + break; > + default: > + feature |= NETDEV_F_OTHER; > } > - if (link.link_speed == ETH_SPEED_NUM_40G) { > - *current = NETDEV_F_40GB_FD; > + } > + else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) { > + switch (link.link_speed) { > + case ETH_SPEED_NUM_10M: > + feature |= NETDEV_F_10MB_HD; > + break; > + case ETH_SPEED_NUM_100M: > + feature |= NETDEV_F_100MB_HD; > + break; > + case ETH_SPEED_NUM_1G: > + feature |= NETDEV_F_1GB_HD; > + break; > + default: > + feature |= NETDEV_F_OTHER; > } > } > > if (link.link_autoneg) { > - *current |= NETDEV_F_AUTONEG; > + feature |= NETDEV_F_AUTONEG; > } > > + *current = feature; > *advertised = *supported = *peer = 0; > > return 0; > -- > 2.13.6 > -- Flavio _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev