| Message ID | 1534127262-12980-1-git-send-email-xu.binbin1@zte.com.cn |
|---|---|
| State | Accepted |
| Delegated to: | Ian Stokes |
| Headers | show |
| Series | [ovs-dev] netdev-dpdk: Support the link speed of XL710 | expand |
> In the scenario of XL710, the link speed which stored in the table of > Interface is not 40G. Because the implementation of query of link speed > only support to 10G, the parameter 'current' will be a random value in the > scenario of higher link speed. In this case, incorrect link speed of XL710 > nic will be stored in the database. > Good catch, I've tested and it works as expected. I'll add this to the dpdk_merge pull request for this week and backport it to the previous release branches also. Thanks Ian > Signed-off-by: Xu Binbin <xu.binbin1@zte.com.cn> > --- > lib/netdev-dpdk.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ac02a09..e4b6ced > 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -2735,6 +2735,9 @@ netdev_dpdk_get_features(const struct netdev > *netdev, > if (link.link_speed == ETH_SPEED_NUM_10G) { > *current = NETDEV_F_10GB_FD; > } > + if (link.link_speed == ETH_SPEED_NUM_40G) { > + *current = NETDEV_F_40GB_FD; > + } > } > > if (link.link_autoneg) { > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
DPDK exposes API all the way from 10Mbps to 100Gbps. http://doc.dpdk.org/api/rte__ethdev_8h_source.html Can other cards be added? 25G is now getting really popular. Thanks On Wed, 22 Aug 2018 at 16:28, Stokes, Ian <ian.stokes@intel.com> wrote: > > In the scenario of XL710, the link speed which stored in the table of > > Interface is not 40G. Because the implementation of query of link speed > > only support to 10G, the parameter 'current' will be a random value in > the > > scenario of higher link speed. In this case, incorrect link speed of > XL710 > > nic will be stored in the database. > > > > Good catch, I've tested and it works as expected. I'll add this to the > dpdk_merge pull request for this week and backport it to the previous > release branches also. > > Thanks > Ian > > > Signed-off-by: Xu Binbin <xu.binbin1@zte.com.cn> > > --- > > lib/netdev-dpdk.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ac02a09..e4b6ced > > 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -2735,6 +2735,9 @@ netdev_dpdk_get_features(const struct netdev > > *netdev, > > if (link.link_speed == ETH_SPEED_NUM_10G) { > > *current = NETDEV_F_10GB_FD; > > } > > + if (link.link_speed == ETH_SPEED_NUM_40G) { > > + *current = NETDEV_F_40GB_FD; > > + } > > } > > > > if (link.link_autoneg) { > > -- > > 1.8.3.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 8/22/2018 6:14 PM, Federico Iezzi wrote: > DPDK exposes API all the way from 10Mbps to 100Gbps. > http://doc.dpdk.org/api/rte__ethdev_8h_source.html > > Can other cards be added? 25G is now getting really popular. > > Thanks It’s a good point, technically there’s nothing stopping users from using 25/50/56/100 Gbp HW. 25/50/56 Gb are not defined specifically as a port feature rate in the openflow specifications at this time so they would have to be defined as NETDEV_F_OTHER to correlate to the feature rate not being in the ofp_port feature list in openflow. The following incremental on the patch below should suffice: @@ -2735,9 +2735,21 @@ netdev_dpdk_get_features(const struct netdev *netdev, if (link.link_speed == ETH_SPEED_NUM_10G) { *current = NETDEV_F_10GB_FD; } + if (link.link_speed == ETH_SPEED_NUM_25G) { + *current = NETDEV_F_OTHER; + } if (link.link_speed == ETH_SPEED_NUM_40G) { *current = NETDEV_F_40GB_FD; } + if (link.link_speed == ETH_SPEED_NUM_50G) { + *current = NETDEV_F_OTHER; + } + if (link.link_speed == ETH_SPEED_NUM_56G) { + *current = NETDEV_F_OTHER; + } + if (link.link_speed == ETH_SPEED_NUM_100G) { + *current = NETDEV_F_100GB_FD; + } What are peoples thoughts? I can submit this as a separate patch if preferred. Thanks Ian Ian > > On Wed, 22 Aug 2018 at 16:28, Stokes, Ian <ian.stokes@intel.com > <mailto:ian.stokes@intel.com>> wrote: > > > In the scenario of XL710, the link speed which stored in the table of > > Interface is not 40G. Because the implementation of query of link > speed > > only support to 10G, the parameter 'current' will be a random > value in the > > scenario of higher link speed. In this case, incorrect link speed > of XL710 > > nic will be stored in the database. > > > > Good catch, I've tested and it works as expected. I'll add this to > the dpdk_merge pull request for this week and backport it to the > previous release branches also. > > Thanks > Ian > > > Signed-off-by: Xu Binbin <xu.binbin1@zte.com.cn > <mailto:xu.binbin1@zte.com.cn>> > > --- > > lib/netdev-dpdk.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > ac02a09..e4b6ced > > 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -2735,6 +2735,9 @@ netdev_dpdk_get_features(const struct netdev > > *netdev, > > if (link.link_speed == ETH_SPEED_NUM_10G) { > > *current = NETDEV_F_10GB_FD; > > } > > + if (link.link_speed == ETH_SPEED_NUM_40G) { > > + *current = NETDEV_F_40GB_FD; > > + } > > } > > > > if (link.link_autoneg) { > > -- > > 1.8.3.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org <mailto:dev@openvswitch.org> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > dev@openvswitch.org <mailto:dev@openvswitch.org> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Any comment here? This seems like a very easy commit :-) On Thu, 23 Aug 2018 at 13:34, Ian Stokes <ian.stokes@intel.com> wrote: > On 8/22/2018 6:14 PM, Federico Iezzi wrote: > > DPDK exposes API all the way from 10Mbps to 100Gbps. > > http://doc.dpdk.org/api/rte__ethdev_8h_source.html > > > > Can other cards be added? 25G is now getting really popular. > > > > Thanks > > It’s a good point, technically there’s nothing stopping users from using > 25/50/56/100 Gbp HW. > > 25/50/56 Gb are not defined specifically as a port feature rate in the > openflow specifications at this time so they would have to be defined as > NETDEV_F_OTHER to correlate to the feature rate not being in the > ofp_port feature list in openflow. > > The following incremental on the patch below should suffice: > > @@ -2735,9 +2735,21 @@ netdev_dpdk_get_features(const struct netdev > *netdev, > if (link.link_speed == ETH_SPEED_NUM_10G) { > *current = NETDEV_F_10GB_FD; > } > + if (link.link_speed == ETH_SPEED_NUM_25G) { > + *current = NETDEV_F_OTHER; > + } > if (link.link_speed == ETH_SPEED_NUM_40G) { > *current = NETDEV_F_40GB_FD; > } > + if (link.link_speed == ETH_SPEED_NUM_50G) { > + *current = NETDEV_F_OTHER; > + } > + if (link.link_speed == ETH_SPEED_NUM_56G) { > + *current = NETDEV_F_OTHER; > + } > + if (link.link_speed == ETH_SPEED_NUM_100G) { > + *current = NETDEV_F_100GB_FD; > + } > > What are peoples thoughts? I can submit this as a separate patch if > preferred. > > Thanks > Ian > > Ian > > > > > On Wed, 22 Aug 2018 at 16:28, Stokes, Ian <ian.stokes@intel.com > > <mailto:ian.stokes@intel.com>> wrote: > > > > > In the scenario of XL710, the link speed which stored in the > table of > > > Interface is not 40G. Because the implementation of query of link > > speed > > > only support to 10G, the parameter 'current' will be a random > > value in the > > > scenario of higher link speed. In this case, incorrect link speed > > of XL710 > > > nic will be stored in the database. > > > > > > > Good catch, I've tested and it works as expected. I'll add this to > > the dpdk_merge pull request for this week and backport it to the > > previous release branches also. > > > > Thanks > > Ian > > > > > Signed-off-by: Xu Binbin <xu.binbin1@zte.com.cn > > <mailto:xu.binbin1@zte.com.cn>> > > > --- > > > lib/netdev-dpdk.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > > ac02a09..e4b6ced > > > 100644 > > > --- a/lib/netdev-dpdk.c > > > +++ b/lib/netdev-dpdk.c > > > @@ -2735,6 +2735,9 @@ netdev_dpdk_get_features(const struct netdev > > > *netdev, > > > if (link.link_speed == ETH_SPEED_NUM_10G) { > > > *current = NETDEV_F_10GB_FD; > > > } > > > + if (link.link_speed == ETH_SPEED_NUM_40G) { > > > + *current = NETDEV_F_40GB_FD; > > > + } > > > } > > > > > > if (link.link_autoneg) { > > > -- > > > 1.8.3.1 > > > > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org <mailto:dev@openvswitch.org> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org <mailto:dev@openvswitch.org> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > >
On Fri, Aug 31, 2018 at 05:54:25AM +0200, Federico Iezzi wrote: > Any comment here? > This seems like a very easy commit :-) > > > > On Thu, 23 Aug 2018 at 13:34, Ian Stokes <ian.stokes@intel.com> wrote: > > > On 8/22/2018 6:14 PM, Federico Iezzi wrote: > > > DPDK exposes API all the way from 10Mbps to 100Gbps. > > > http://doc.dpdk.org/api/rte__ethdev_8h_source.html > > > > > > Can other cards be added? 25G is now getting really popular. > > > > > > Thanks > > > > It’s a good point, technically there’s nothing stopping users from using > > 25/50/56/100 Gbp HW. > > > > 25/50/56 Gb are not defined specifically as a port feature rate in the > > openflow specifications at this time so they would have to be defined as > > NETDEV_F_OTHER to correlate to the feature rate not being in the > > ofp_port feature list in openflow. > > > > The following incremental on the patch below should suffice: > > > > @@ -2735,9 +2735,21 @@ netdev_dpdk_get_features(const struct netdev > > *netdev, > > if (link.link_speed == ETH_SPEED_NUM_10G) { > > *current = NETDEV_F_10GB_FD; > > } > > + if (link.link_speed == ETH_SPEED_NUM_25G) { > > + *current = NETDEV_F_OTHER; > > + } > > if (link.link_speed == ETH_SPEED_NUM_40G) { > > *current = NETDEV_F_40GB_FD; > > } > > + if (link.link_speed == ETH_SPEED_NUM_50G) { > > + *current = NETDEV_F_OTHER; > > + } > > + if (link.link_speed == ETH_SPEED_NUM_56G) { > > + *current = NETDEV_F_OTHER; > > + } > > + if (link.link_speed == ETH_SPEED_NUM_100G) { > > + *current = NETDEV_F_100GB_FD; > > + } > > > > What are peoples thoughts? I can submit this as a separate patch if > > preferred. That's all the supported speeds OF defines as you mentioned, but there are free bits there if it's important to report a more accurate speed. Alternatively we could expose that information through get_status() as a translated string most probably. What worries me is that 'current' variable is allocated in the stack and dpdk doesn't zero it like bsd or linux does, so if speed falls out of those values, it will use whatever was in the stack before as reported originally. Perhaps we could do: uint32_t supported = 0; if (link.link_duplex == ETH_LINK_FULL_DUPLEX) { switch (link.link_speed) { /* OpenFlow defined values: see enum ofp_port_features */ [...] case ETH_SPEED_NUM_10G: supported |= NETDEV_F_10GB_FD; break; case ETH_SPEED_NUM_40G: supported |= NETDEV_F_40GB_FD; break; case ETH_SPEED_NUM_100G: supported |= NETDEV_F_100GB_FD break; default: supported |= NETDEV_F_OTHER; } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) { [...] } if (link.link_autoneg) { supported |= NETDEV_F_AUTONEG; } *current = supported; *advertised = *supported = *peer = 0; return 0; fbl > > > > Thanks > > Ian > > > > Ian > > > > > > > > On Wed, 22 Aug 2018 at 16:28, Stokes, Ian <ian.stokes@intel.com > > > <mailto:ian.stokes@intel.com>> wrote: > > > > > > > In the scenario of XL710, the link speed which stored in the > > table of > > > > Interface is not 40G. Because the implementation of query of link > > > speed > > > > only support to 10G, the parameter 'current' will be a random > > > value in the > > > > scenario of higher link speed. In this case, incorrect link speed > > > of XL710 > > > > nic will be stored in the database. > > > > > > > > > > Good catch, I've tested and it works as expected. I'll add this to > > > the dpdk_merge pull request for this week and backport it to the > > > previous release branches also. > > > > > > Thanks > > > Ian > > > > > > > Signed-off-by: Xu Binbin <xu.binbin1@zte.com.cn > > > <mailto:xu.binbin1@zte.com.cn>> > > > > --- > > > > lib/netdev-dpdk.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > > > ac02a09..e4b6ced > > > > 100644 > > > > --- a/lib/netdev-dpdk.c > > > > +++ b/lib/netdev-dpdk.c > > > > @@ -2735,6 +2735,9 @@ netdev_dpdk_get_features(const struct netdev > > > > *netdev, > > > > if (link.link_speed == ETH_SPEED_NUM_10G) { > > > > *current = NETDEV_F_10GB_FD; > > > > } > > > > + if (link.link_speed == ETH_SPEED_NUM_40G) { > > > > + *current = NETDEV_F_40GB_FD; > > > > + } > > > > } > > > > > > > > if (link.link_autoneg) { > > > > -- > > > > 1.8.3.1 > > > > > > > > _______________________________________________ > > > > dev mailing list > > > > dev@openvswitch.org <mailto:dev@openvswitch.org> > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org <mailto:dev@openvswitch.org> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
So, any news on the other link speeds like 25, 50, and 100Gbps? Thanks On Mon, 3 Sep 2018 at 19:54, Flavio Leitner <fbl@sysclose.org> wrote: > On Fri, Aug 31, 2018 at 05:54:25AM +0200, Federico Iezzi wrote: > > Any comment here? > > This seems like a very easy commit :-) > > > > > > > > On Thu, 23 Aug 2018 at 13:34, Ian Stokes <ian.stokes@intel.com> wrote: > > > > > On 8/22/2018 6:14 PM, Federico Iezzi wrote: > > > > DPDK exposes API all the way from 10Mbps to 100Gbps. > > > > http://doc.dpdk.org/api/rte__ethdev_8h_source.html > > > > > > > > Can other cards be added? 25G is now getting really popular. > > > > > > > > Thanks > > > > > > It’s a good point, technically there’s nothing stopping users from > using > > > 25/50/56/100 Gbp HW. > > > > > > 25/50/56 Gb are not defined specifically as a port feature rate in the > > > openflow specifications at this time so they would have to be defined > as > > > NETDEV_F_OTHER to correlate to the feature rate not being in the > > > ofp_port feature list in openflow. > > > > > > The following incremental on the patch below should suffice: > > > > > > @@ -2735,9 +2735,21 @@ netdev_dpdk_get_features(const struct netdev > > > *netdev, > > > if (link.link_speed == ETH_SPEED_NUM_10G) { > > > *current = NETDEV_F_10GB_FD; > > > } > > > + if (link.link_speed == ETH_SPEED_NUM_25G) { > > > + *current = NETDEV_F_OTHER; > > > + } > > > if (link.link_speed == ETH_SPEED_NUM_40G) { > > > *current = NETDEV_F_40GB_FD; > > > } > > > + if (link.link_speed == ETH_SPEED_NUM_50G) { > > > + *current = NETDEV_F_OTHER; > > > + } > > > + if (link.link_speed == ETH_SPEED_NUM_56G) { > > > + *current = NETDEV_F_OTHER; > > > + } > > > + if (link.link_speed == ETH_SPEED_NUM_100G) { > > > + *current = NETDEV_F_100GB_FD; > > > + } > > > > > > What are peoples thoughts? I can submit this as a separate patch if > > > preferred. > > That's all the supported speeds OF defines as you mentioned, but there > are free bits there if it's important to report a more accurate speed. > > Alternatively we could expose that information through get_status() as > a translated string most probably. > > What worries me is that 'current' variable is allocated in the stack and > dpdk doesn't zero it like bsd or linux does, so if speed falls out of those > values, it will use whatever was in the stack before as reported > originally. > > Perhaps we could do: > > uint32_t supported = 0; > > if (link.link_duplex == ETH_LINK_FULL_DUPLEX) { > switch (link.link_speed) { > /* OpenFlow defined values: see enum ofp_port_features */ > [...] > case ETH_SPEED_NUM_10G: > supported |= NETDEV_F_10GB_FD; > break; > case ETH_SPEED_NUM_40G: > supported |= NETDEV_F_40GB_FD; > break; > case ETH_SPEED_NUM_100G: > supported |= NETDEV_F_100GB_FD > break; > default: > supported |= NETDEV_F_OTHER; > } > else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) { > [...] > } > > if (link.link_autoneg) { > supported |= NETDEV_F_AUTONEG; > } > > *current = supported; > *advertised = *supported = *peer = 0; > > return 0; > > > fbl > > > > > > > Thanks > > > Ian > > > > > > Ian > > > > > > > > > > > On Wed, 22 Aug 2018 at 16:28, Stokes, Ian <ian.stokes@intel.com > > > > <mailto:ian.stokes@intel.com>> wrote: > > > > > > > > > In the scenario of XL710, the link speed which stored in the > > > table of > > > > > Interface is not 40G. Because the implementation of query of > link > > > > speed > > > > > only support to 10G, the parameter 'current' will be a random > > > > value in the > > > > > scenario of higher link speed. In this case, incorrect link > speed > > > > of XL710 > > > > > nic will be stored in the database. > > > > > > > > > > > > > Good catch, I've tested and it works as expected. I'll add this > to > > > > the dpdk_merge pull request for this week and backport it to the > > > > previous release branches also. > > > > > > > > Thanks > > > > Ian > > > > > > > > > Signed-off-by: Xu Binbin <xu.binbin1@zte.com.cn > > > > <mailto:xu.binbin1@zte.com.cn>> > > > > > --- > > > > > lib/netdev-dpdk.c | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > > > > ac02a09..e4b6ced > > > > > 100644 > > > > > --- a/lib/netdev-dpdk.c > > > > > +++ b/lib/netdev-dpdk.c > > > > > @@ -2735,6 +2735,9 @@ netdev_dpdk_get_features(const struct > netdev > > > > > *netdev, > > > > > if (link.link_speed == ETH_SPEED_NUM_10G) { > > > > > *current = NETDEV_F_10GB_FD; > > > > > } > > > > > + if (link.link_speed == ETH_SPEED_NUM_40G) { > > > > > + *current = NETDEV_F_40GB_FD; > > > > > + } > > > > > } > > > > > > > > > > if (link.link_autoneg) { > > > > > -- > > > > > 1.8.3.1 > > > > > > > > > > _______________________________________________ > > > > > dev mailing list > > > > > dev@openvswitch.org <mailto:dev@openvswitch.org> > > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > _______________________________________________ > > > > dev mailing list > > > > dev@openvswitch.org <mailto:dev@openvswitch.org> > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > -- > Flavio > > >
On 10/11/2018 11:39 AM, Federico Iezzi wrote: > So, any news on the other link speeds like 25, 50, and 100Gbps? Hi Federico, I've sent out a patch series to help address this based on the previous discussion. https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353292.html https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353293.html Feedback welcome. Thanks Ian > > Thanks > > On Mon, 3 Sep 2018 at 19:54, Flavio Leitner <fbl@sysclose.org > <mailto:fbl@sysclose.org>> wrote: > > On Fri, Aug 31, 2018 at 05:54:25AM +0200, Federico Iezzi wrote: > > Any comment here? > > This seems like a very easy commit :-) > > > > > > > > On Thu, 23 Aug 2018 at 13:34, Ian Stokes <ian.stokes@intel.com > <mailto:ian.stokes@intel.com>> wrote: > > > > > On 8/22/2018 6:14 PM, Federico Iezzi wrote: > > > > DPDK exposes API all the way from 10Mbps to 100Gbps. > > > > http://doc.dpdk.org/api/rte__ethdev_8h_source.html > > > > > > > > Can other cards be added? 25G is now getting really popular. > > > > > > > > Thanks > > > > > > It’s a good point, technically there’s nothing stopping users > from using > > > 25/50/56/100 Gbp HW. > > > > > > 25/50/56 Gb are not defined specifically as a port feature rate > in the > > > openflow specifications at this time so they would have to be > defined as > > > NETDEV_F_OTHER to correlate to the feature rate not being in the > > > ofp_port feature list in openflow. > > > > > > The following incremental on the patch below should suffice: > > > > > > @@ -2735,9 +2735,21 @@ netdev_dpdk_get_features(const struct netdev > > > *netdev, > > > if (link.link_speed == ETH_SPEED_NUM_10G) { > > > *current = NETDEV_F_10GB_FD; > > > } > > > + if (link.link_speed == ETH_SPEED_NUM_25G) { > > > + *current = NETDEV_F_OTHER; > > > + } > > > if (link.link_speed == ETH_SPEED_NUM_40G) { > > > *current = NETDEV_F_40GB_FD; > > > } > > > + if (link.link_speed == ETH_SPEED_NUM_50G) { > > > + *current = NETDEV_F_OTHER; > > > + } > > > + if (link.link_speed == ETH_SPEED_NUM_56G) { > > > + *current = NETDEV_F_OTHER; > > > + } > > > + if (link.link_speed == ETH_SPEED_NUM_100G) { > > > + *current = NETDEV_F_100GB_FD; > > > + } > > > > > > What are peoples thoughts? I can submit this as a separate patch if > > > preferred. > > That's all the supported speeds OF defines as you mentioned, but there > are free bits there if it's important to report a more accurate speed. > > Alternatively we could expose that information through get_status() as > a translated string most probably. > > What worries me is that 'current' variable is allocated in the stack and > dpdk doesn't zero it like bsd or linux does, so if speed falls out > of those > values, it will use whatever was in the stack before as reported > originally. > > Perhaps we could do: > > uint32_t supported = 0; > > if (link.link_duplex == ETH_LINK_FULL_DUPLEX) { > switch (link.link_speed) { > /* OpenFlow defined values: see enum ofp_port_features */ > [...] > case ETH_SPEED_NUM_10G: > supported |= NETDEV_F_10GB_FD; > break; > case ETH_SPEED_NUM_40G: > supported |= NETDEV_F_40GB_FD; > break; > case ETH_SPEED_NUM_100G: > supported |= NETDEV_F_100GB_FD > break; > default: > supported |= NETDEV_F_OTHER; > } > else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) { > [...] > } > > if (link.link_autoneg) { > supported |= NETDEV_F_AUTONEG; > } > > *current = supported; > *advertised = *supported = *peer = 0; > > return 0; > > > fbl > > > > > > > Thanks > > > Ian > > > > > > Ian > > > > > > > > > > > On Wed, 22 Aug 2018 at 16:28, Stokes, Ian > <ian.stokes@intel.com <mailto:ian.stokes@intel.com> > > > > <mailto:ian.stokes@intel.com <mailto:ian.stokes@intel.com>>> > wrote: > > > > > > > > > In the scenario of XL710, the link speed which stored > in the > > > table of > > > > > Interface is not 40G. Because the implementation of > query of link > > > > speed > > > > > only support to 10G, the parameter 'current' will be a > random > > > > value in the > > > > > scenario of higher link speed. In this case, incorrect > link speed > > > > of XL710 > > > > > nic will be stored in the database. > > > > > > > > > > > > > Good catch, I've tested and it works as expected. I'll > add this to > > > > the dpdk_merge pull request for this week and backport it > to the > > > > previous release branches also. > > > > > > > > Thanks > > > > Ian > > > > > > > > > Signed-off-by: Xu Binbin <xu.binbin1@zte.com.cn > <mailto:xu.binbin1@zte.com.cn> > > > > <mailto:xu.binbin1@zte.com.cn > <mailto:xu.binbin1@zte.com.cn>>> > > > > > --- > > > > > lib/netdev-dpdk.c | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > > > > ac02a09..e4b6ced > > > > > 100644 > > > > > --- a/lib/netdev-dpdk.c > > > > > +++ b/lib/netdev-dpdk.c > > > > > @@ -2735,6 +2735,9 @@ netdev_dpdk_get_features(const > struct netdev > > > > > *netdev, > > > > > if (link.link_speed == ETH_SPEED_NUM_10G) { > > > > > *current = NETDEV_F_10GB_FD; > > > > > } > > > > > + if (link.link_speed == ETH_SPEED_NUM_40G) { > > > > > + *current = NETDEV_F_40GB_FD; > > > > > + } > > > > > } > > > > > > > > > > if (link.link_autoneg) { > > > > > -- > > > > > 1.8.3.1 > > > > > > > > > > _______________________________________________ > > > > > dev mailing list > > > > > dev@openvswitch.org <mailto:dev@openvswitch.org> > <mailto:dev@openvswitch.org <mailto:dev@openvswitch.org>> > > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > _______________________________________________ > > > > dev mailing list > > > > dev@openvswitch.org <mailto:dev@openvswitch.org> > <mailto:dev@openvswitch.org <mailto:dev@openvswitch.org>> > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org <mailto:dev@openvswitch.org> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > -- > Flavio > >
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ac02a09..e4b6ced 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -2735,6 +2735,9 @@ netdev_dpdk_get_features(const struct netdev *netdev, if (link.link_speed == ETH_SPEED_NUM_10G) { *current = NETDEV_F_10GB_FD; } + if (link.link_speed == ETH_SPEED_NUM_40G) { + *current = NETDEV_F_40GB_FD; + } } if (link.link_autoneg) {
In the scenario of XL710, the link speed which stored in the table of Interface is not 40G. Because the implementation of query of link speed only support to 10G, the parameter 'current' will be a random value in the scenario of higher link speed. In this case, incorrect link speed of XL710 nic will be stored in the database. Signed-off-by: Xu Binbin <xu.binbin1@zte.com.cn> --- lib/netdev-dpdk.c | 3 +++ 1 file changed, 3 insertions(+)