| Message ID | 1541111461-29556-1-git-send-email-ian.stokes@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2,1/2] netdev-dpdk: Fix netdev_dpdk_get_features(). | expand |
On 02.11.2018 1:31, 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.") > Co-authored-by: Flavio Leitner <fbl@sysclose.org> > Signed-off-by: Flavio Leitner <fbl@sysclose.org> > Signed-off-by: Ian Stokes <ian.stokes@intel.com> > --- > > v1 -> v2 > * Moved link speed comment to apply to all of link feature statement. > * Remove reference to enum ofp_port_features in link feature comment. > * Moved else if to same line as bracket from previous if statement. > * Added Co-author and Sign-Off for Flavio. > --- > lib/netdev-dpdk.c | 64 +++++++++++++++++++++++++++++++++---------------------- > 1 file changed, 39 insertions(+), 25 deletions(-) > Minor thing is that you could add a period to the end of a comment to match with the coding-style. Acked-by: Ilya Maximets <i.maximets@samsung.com>
> On 02.11.2018 1:31, 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.") > > Co-authored-by: Flavio Leitner <fbl@sysclose.org> > > Signed-off-by: Flavio Leitner <fbl@sysclose.org> > > Signed-off-by: Ian Stokes <ian.stokes@intel.com> > > --- > > > > v1 -> v2 > > * Moved link speed comment to apply to all of link feature statement. > > * Remove reference to enum ofp_port_features in link feature comment. > > * Moved else if to same line as bracket from previous if statement. > > * Added Co-author and Sign-Off for Flavio. > > --- > > lib/netdev-dpdk.c | 64 > > +++++++++++++++++++++++++++++++++---------------------- > > 1 file changed, 39 insertions(+), 25 deletions(-) > > > > Minor thing is that you could add a period to the end of a comment to > match with the coding-style. Good catch, will do. Thanks Ian > > Acked-by: Ilya Maximets <i.maximets@samsung.com>
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index f91aa27cd..59f4ccbfe 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -2707,43 +2707,57 @@ 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; + /* Match against OpenFlow defined link speed values */ + if (link.link_duplex == ETH_LINK_FULL_DUPLEX) { + switch (link.link_speed) { + 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;
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.") Co-authored-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ian Stokes <ian.stokes@intel.com> --- v1 -> v2 * Moved link speed comment to apply to all of link feature statement. * Remove reference to enum ofp_port_features in link feature comment. * Moved else if to same line as bracket from previous if statement. * Added Co-author and Sign-Off for Flavio. --- lib/netdev-dpdk.c | 64 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 39 insertions(+), 25 deletions(-)