| Message ID | 20180710132337.18211-1-sugesh.chandran@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] netdev-dpdk: Fix failure to configure flow control at netdev-init. | expand |
On 7/10/2018 2:23 PM, Sugesh Chandran wrote: > Configuring flow control at ixgbe netdev-init is throwing error in port > start. > > For eg: without this fix, user cannot configure flow control on ixgbe dpdk > port as below, > > " > ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ > options:dpdk-devargs=0000:05:00.1 options:rx-flow-ctrl=true > " > > Instead, it must be configured as two different commands, > > " > ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ > options:dpdk-devargs=0000:05:00.1 > ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true > " > > The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' fields before > trying to configuring the dpdk ethdev. Hence OVS can no longer set the > 'dont care' fields to just '0' as before. This commit make sure all the > 'rte_eth_fc_conf' fields are populated with default values before the dev > init. > Thanks for the patch Sugesh, I've applied to dpdk_merge and it will be part of this weeks pull request. I assume it should be backported to OVS 2.9 at least, do you know if it should go to 2.8/2.7 also? Ian > Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> > --- > lib/netdev-dpdk.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index bb4d60f26..023b80d3e 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -1065,14 +1065,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) > > mbp_priv = rte_mempool_get_priv(dev->dpdk_mp->mp); > dev->buf_size = mbp_priv->mbuf_data_room_size - RTE_PKTMBUF_HEADROOM; > - > - /* Get the Flow control configuration for DPDK-ETH */ > - diag = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); > - if (diag) { > - VLOG_DBG("cannot get flow control parameters on port "DPDK_PORT_ID_FMT > - ", err=%d", dev->port_id, diag); > - } > - > return 0; > } > > @@ -1773,6 +1765,13 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, > autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false); > > fc_mode = fc_mode_set[tx_fc_en][rx_fc_en]; > + /* Get the Flow control configuration for DPDK-ETH */ > + err = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); > + if (err) { > + VLOG_INFO("cannot get flow control parameters on port "DPDK_PORT_ID_FMT > + ", err=%d", dev->port_id, err); > + } > + > if (dev->fc_conf.mode != fc_mode || autoneg != dev->fc_conf.autoneg) { > dev->fc_conf.mode = fc_mode; > dev->fc_conf.autoneg = autoneg; >
Hi Ian, Regards _Sugesh > -----Original Message----- ... > > make sure all the 'rte_eth_fc_conf' fields are populated with default > > values before the dev init. > > > > Thanks for the patch Sugesh, I've applied to dpdk_merge and it will be part of > this weeks pull request. > > I assume it should be backported to OVS 2.9 at least, do you know if it should go > to 2.8/2.7 also? > [Sugesh]Yes, that’s right. It need to backported to 2.8 and 2.7 too. Should I send separate patch for those branches as well? > Ian > > Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> ...
> Hi Ian, > > Regards > _Sugesh > > > -----Original Message----- > ... > > > make sure all the 'rte_eth_fc_conf' fields are populated with > > > default values before the dev init. > > > > > > > Thanks for the patch Sugesh, I've applied to dpdk_merge and it will be > > part of this weeks pull request. > > > > I assume it should be backported to OVS 2.9 at least, do you know if > > it should go to 2.8/2.7 also? > > > [Sugesh]Yes, that’s right. It need to backported to 2.8 and 2.7 too. > Should I send separate patch for those branches as well? Yes, it doesn't apply cleanly for 2.8/2.7 so a patch for those branches would be needed. Thanks Ian > > > Ian > > > Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> > ...
Regards _Sugesh > -----Original Message----- > From: Stokes, Ian > Sent: Monday, July 16, 2018 1:52 PM > To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs- > dev@openvswitch.org > Subject: RE: [ovs-dev] [PATCH] netdev-dpdk: Fix failure to configure flow control > at netdev-init. > > > Hi Ian, > > > > Regards > > _Sugesh > > > > > -----Original Message----- > > ... > > > > make sure all the 'rte_eth_fc_conf' fields are populated with > > > > default values before the dev init. > > > > > > > > > > Thanks for the patch Sugesh, I've applied to dpdk_merge and it will > > > be part of this weeks pull request. > > > > > > I assume it should be backported to OVS 2.9 at least, do you know if > > > it should go to 2.8/2.7 also? > > > > > [Sugesh]Yes, that’s right. It need to backported to 2.8 and 2.7 too. > > Should I send separate patch for those branches as well? > > Yes, it doesn't apply cleanly for 2.8/2.7 so a patch for those branches would be > needed. [Sugesh] Sure, I will send patches for those versions. > > Thanks > Ian > > > > > Ian > > > > Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> > > ...
On 7/13/2018 6:13 PM, Ian Stokes wrote: > On 7/10/2018 2:23 PM, Sugesh Chandran wrote: >> Configuring flow control at ixgbe netdev-init is throwing error in port >> start. >> >> For eg: without this fix, user cannot configure flow control on ixgbe >> dpdk >> port as below, >> >> " >> ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ >> options:dpdk-devargs=0000:05:00.1 options:rx-flow-ctrl=true >> " >> >> Instead, it must be configured as two different commands, >> >> " >> ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ >> options:dpdk-devargs=0000:05:00.1 >> ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true >> " >> >> The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' >> fields before >> trying to configuring the dpdk ethdev. Hence OVS can no longer set the >> 'dont care' fields to just '0' as before. This commit make sure all the >> 'rte_eth_fc_conf' fields are populated with default values before the dev >> init. >> > > Thanks for the patch Sugesh, I've applied to dpdk_merge and it will be > part of this weeks pull request. > > I assume it should be backported to OVS 2.9 at least, do you know if it > should go to 2.8/2.7 also? Hi Sugesh, during further testing I found this breaks functionality for Virtual functions with OVS (specifically tested with igb_vf, i40e_vf and ixgbe_vf). The port itself will fail to initialize with the following error: netdev_dpdk|INFO|cannot get flow control parameters on port 2, err=-95 And is not added. as such I think flow control should only be optional as there is no guarantee it will be available for a given dpdk device and if unavailable it should not stop the port from being initialized. I've pulled this patch from the merge request for master, I think the branch patches will have to be reworked also. Ian > > Ian >> Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> >> --- >> lib/netdev-dpdk.c | 15 +++++++-------- >> 1 file changed, 7 insertions(+), 8 deletions(-) >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> index bb4d60f26..023b80d3e 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -1065,14 +1065,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) >> mbp_priv = rte_mempool_get_priv(dev->dpdk_mp->mp); >> dev->buf_size = mbp_priv->mbuf_data_room_size - >> RTE_PKTMBUF_HEADROOM; >> - >> - /* Get the Flow control configuration for DPDK-ETH */ >> - diag = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); >> - if (diag) { >> - VLOG_DBG("cannot get flow control parameters on port >> "DPDK_PORT_ID_FMT >> - ", err=%d", dev->port_id, diag); >> - } >> - >> return 0; >> } >> @@ -1773,6 +1765,13 @@ netdev_dpdk_set_config(struct netdev *netdev, >> const struct smap *args, >> autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false); >> fc_mode = fc_mode_set[tx_fc_en][rx_fc_en]; >> + /* Get the Flow control configuration for DPDK-ETH */ >> + err = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); >> + if (err) { >> + VLOG_INFO("cannot get flow control parameters on port >> "DPDK_PORT_ID_FMT >> + ", err=%d", dev->port_id, err); >> + } >> + >> if (dev->fc_conf.mode != fc_mode || autoneg != >> dev->fc_conf.autoneg) { >> dev->fc_conf.mode = fc_mode; >> dev->fc_conf.autoneg = autoneg; >> > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Ian, Thank you for testing it on VF. Please find my comments below, Regards _Sugesh > -----Original Message----- > From: Stokes, Ian > Sent: Friday, July 20, 2018 6:30 PM > To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs- > dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: Fix failure to configure flow control > at netdev-init. > > On 7/13/2018 6:13 PM, Ian Stokes wrote: > > On 7/10/2018 2:23 PM, Sugesh Chandran wrote: > >> Configuring flow control at ixgbe netdev-init is throwing error in > >> port start. > >> > >> For eg: without this fix, user cannot configure flow control on ixgbe > >> dpdk port as below, > >> > >> " > >> ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ > >> options:dpdk-devargs=0000:05:00.1 options:rx-flow-ctrl=true > >> " > >> > >> Instead, it must be configured as two different commands, > >> > >> " > >> ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ > >> options:dpdk-devargs=0000:05:00.1 > >> ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true " > >> > >> The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' > >> fields before > >> trying to configuring the dpdk ethdev. Hence OVS can no longer set > >> the 'dont care' fields to just '0' as before. This commit make sure > >> all the 'rte_eth_fc_conf' fields are populated with default values > >> before the dev init. > >> > > > > Thanks for the patch Sugesh, I've applied to dpdk_merge and it will be > > part of this weeks pull request. > > > > I assume it should be backported to OVS 2.9 at least, do you know if > > it should go to 2.8/2.7 also? > > Hi Sugesh, > > during further testing I found this breaks functionality for Virtual functions with > OVS (specifically tested with igb_vf, i40e_vf and ixgbe_vf). > > The port itself will fail to initialize with the following error: > > netdev_dpdk|INFO|cannot get flow control parameters on port 2, err=-95 > > And is not added. as such I think flow control should only be optional as there is > no guarantee it will be available for a given dpdk device and if unavailable it > should not stop the port from being initialized. [Sugesh] I am not sure if we need to add this condition in OVS. As a virtual switch, OVS should able to use these APIs irrespective of NIC/port types. Actually we are masking this error , without the patch. It is possible to hit this issue, when we configure the flow control on a VF port. Adding a condition in OVS to check a VF port for similar parameters doesn’t looks like a clean approach? What do you think? This leads to another question , whats the impact of modifying other netdev parameters on a VF port. Does it error out/silently fail/work as normal netdev ports? > > I've pulled this patch from the merge request for master, I think the branch > patches will have to be reworked also. > > Ian > > > > Ian > >> Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> > >> --- > >> lib/netdev-dpdk.c | 15 +++++++-------- > >> 1 file changed, 7 insertions(+), 8 deletions(-) > >> > >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > >> bb4d60f26..023b80d3e 100644 > >> --- a/lib/netdev-dpdk.c > >> +++ b/lib/netdev-dpdk.c > >> @@ -1065,14 +1065,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) > >> mbp_priv = rte_mempool_get_priv(dev->dpdk_mp->mp); > >> dev->buf_size = mbp_priv->mbuf_data_room_size - > >> RTE_PKTMBUF_HEADROOM; > >> - > >> - /* Get the Flow control configuration for DPDK-ETH */ > >> - diag = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); > >> - if (diag) { > >> - VLOG_DBG("cannot get flow control parameters on port > >> "DPDK_PORT_ID_FMT > >> - ", err=%d", dev->port_id, diag); > >> - } > >> - > >> return 0; > >> } > >> @@ -1773,6 +1765,13 @@ netdev_dpdk_set_config(struct netdev *netdev, > >> const struct smap *args, > >> autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false); > >> fc_mode = fc_mode_set[tx_fc_en][rx_fc_en]; > >> + /* Get the Flow control configuration for DPDK-ETH */ > >> + err = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); > >> + if (err) { > >> + VLOG_INFO("cannot get flow control parameters on port > >> "DPDK_PORT_ID_FMT > >> + ", err=%d", dev->port_id, err); > >> + } > >> + > >> if (dev->fc_conf.mode != fc_mode || autoneg != > >> dev->fc_conf.autoneg) { > >> dev->fc_conf.mode = fc_mode; > >> dev->fc_conf.autoneg = autoneg; > >> > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 7/20/2018 7:24 PM, Chandran, Sugesh wrote: > Hi Ian, > > Thank you for testing it on VF. Please find my comments below, > > Regards > _Sugesh > >> -----Original Message----- >> From: Stokes, Ian >> Sent: Friday, July 20, 2018 6:30 PM >> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs- >> dev@openvswitch.org >> Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: Fix failure to configure flow control >> at netdev-init. >> >> On 7/13/2018 6:13 PM, Ian Stokes wrote: >>> On 7/10/2018 2:23 PM, Sugesh Chandran wrote: >>>> Configuring flow control at ixgbe netdev-init is throwing error in >>>> port start. >>>> >>>> For eg: without this fix, user cannot configure flow control on ixgbe >>>> dpdk port as below, >>>> >>>> " >>>> ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ >>>> options:dpdk-devargs=0000:05:00.1 options:rx-flow-ctrl=true >>>> " >>>> >>>> Instead, it must be configured as two different commands, >>>> >>>> " >>>> ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ >>>> options:dpdk-devargs=0000:05:00.1 >>>> ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true " >>>> >>>> The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' >>>> fields before >>>> trying to configuring the dpdk ethdev. Hence OVS can no longer set >>>> the 'dont care' fields to just '0' as before. This commit make sure >>>> all the 'rte_eth_fc_conf' fields are populated with default values >>>> before the dev init. >>>> >>> >>> Thanks for the patch Sugesh, I've applied to dpdk_merge and it will be >>> part of this weeks pull request. >>> >>> I assume it should be backported to OVS 2.9 at least, do you know if >>> it should go to 2.8/2.7 also? >> >> Hi Sugesh, >> >> during further testing I found this breaks functionality for Virtual functions with >> OVS (specifically tested with igb_vf, i40e_vf and ixgbe_vf). >> >> The port itself will fail to initialize with the following error: >> >> netdev_dpdk|INFO|cannot get flow control parameters on port 2, err=-95 >> >> And is not added. as such I think flow control should only be optional as there is >> no guarantee it will be available for a given dpdk device and if unavailable it >> should not stop the port from being initialized. > [Sugesh] I am not sure if we need to add this condition in OVS. > As a virtual switch, OVS should able to use these APIs irrespective of NIC/port types. > Actually we are masking this error , without the patch. It is possible to hit this issue, > when we configure the flow control on a VF port. Ok, but now we have a situation that when adding a VF port, even without specifying the flow control option, it fails as now flow control parameters are being passed on init irregardless of whether the user intends to use it or not. Now that I think of it I think this would probably break other port types as well that don't support flow control (such as vdevs). > Adding a condition in OVS to check a VF port for similar parameters doesn’t looks like > a clean approach? What do you think? We've already had to do this for a few items (HW CRC enablement, LSC, MTU support). It's not clean but until DPDK provides a way of checking capabilities before init it's the best we can do. > > This leads to another question , whats the impact of modifying other netdev parameters > on a VF port. Does it error out/silently fail/work as normal netdev ports? I think we can't fail completely based on an optional capability. And this is not just for VF ports, conceivably you can have HW ports that don't support a particular feature either supported by a netdev. For the current patch you could warn a user that the feature is not supported and continue the initialization process (similar to the set mtu capability). Ian > > >> >> I've pulled this patch from the merge request for master, I think the branch >> patches will have to be reworked also. >> >> Ian >>> >>> Ian >>>> Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> >>>> --- >>>> lib/netdev-dpdk.c | 15 +++++++-------- >>>> 1 file changed, 7 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >>>> bb4d60f26..023b80d3e 100644 >>>> --- a/lib/netdev-dpdk.c >>>> +++ b/lib/netdev-dpdk.c >>>> @@ -1065,14 +1065,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) >>>> mbp_priv = rte_mempool_get_priv(dev->dpdk_mp->mp); >>>> dev->buf_size = mbp_priv->mbuf_data_room_size - >>>> RTE_PKTMBUF_HEADROOM; >>>> - >>>> - /* Get the Flow control configuration for DPDK-ETH */ >>>> - diag = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); >>>> - if (diag) { >>>> - VLOG_DBG("cannot get flow control parameters on port >>>> "DPDK_PORT_ID_FMT >>>> - ", err=%d", dev->port_id, diag); >>>> - } >>>> - >>>> return 0; >>>> } >>>> @@ -1773,6 +1765,13 @@ netdev_dpdk_set_config(struct netdev *netdev, >>>> const struct smap *args, >>>> autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false); >>>> fc_mode = fc_mode_set[tx_fc_en][rx_fc_en]; >>>> + /* Get the Flow control configuration for DPDK-ETH */ >>>> + err = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); >>>> + if (err) { >>>> + VLOG_INFO("cannot get flow control parameters on port >>>> "DPDK_PORT_ID_FMT >>>> + ", err=%d", dev->port_id, err); >>>> + } >>>> + >>>> if (dev->fc_conf.mode != fc_mode || autoneg != >>>> dev->fc_conf.autoneg) { >>>> dev->fc_conf.mode = fc_mode; >>>> dev->fc_conf.autoneg = autoneg; >>>> >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Regards _Sugesh > -----Original Message----- > From: Stokes, Ian > Sent: Friday, July 20, 2018 8:41 PM > To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs- > dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: Fix failure to configure flow control > at netdev-init. > > On 7/20/2018 7:24 PM, Chandran, Sugesh wrote: > > Hi Ian, > > > > Thank you for testing it on VF. Please find my comments below, > > > > Regards > > _Sugesh > > > >> -----Original Message----- > >> From: Stokes, Ian > >> Sent: Friday, July 20, 2018 6:30 PM > >> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs- > >> dev@openvswitch.org > >> Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: Fix failure to configure > >> flow control at netdev-init. > >> > >> On 7/13/2018 6:13 PM, Ian Stokes wrote: > >>> On 7/10/2018 2:23 PM, Sugesh Chandran wrote: > >>>> Configuring flow control at ixgbe netdev-init is throwing error in > >>>> port start. > >>>> > >>>> For eg: without this fix, user cannot configure flow control on > >>>> ixgbe dpdk port as below, > >>>> > >>>> " > >>>> ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk > >>>> \ > >>>> options:dpdk-devargs=0000:05:00.1 > >>>> options:rx-flow-ctrl=true " > >>>> > >>>> Instead, it must be configured as two different commands, > >>>> > >>>> " > >>>> ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk > >>>> \ > >>>> options:dpdk-devargs=0000:05:00.1 > >>>> ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true " > >>>> > >>>> The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' > >>>> fields before > >>>> trying to configuring the dpdk ethdev. Hence OVS can no longer set > >>>> the 'dont care' fields to just '0' as before. This commit make sure > >>>> all the 'rte_eth_fc_conf' fields are populated with default values > >>>> before the dev init. > >>>> > >>> > >>> Thanks for the patch Sugesh, I've applied to dpdk_merge and it will > >>> be part of this weeks pull request. > >>> > >>> I assume it should be backported to OVS 2.9 at least, do you know if > >>> it should go to 2.8/2.7 also? > >> > >> Hi Sugesh, > >> > >> during further testing I found this breaks functionality for Virtual > >> functions with OVS (specifically tested with igb_vf, i40e_vf and ixgbe_vf). > >> > >> The port itself will fail to initialize with the following error: > >> > >> netdev_dpdk|INFO|cannot get flow control parameters on port 2, > >> err=-95 > >> > >> And is not added. as such I think flow control should only be > >> optional as there is no guarantee it will be available for a given > >> dpdk device and if unavailable it should not stop the port from being > initialized. > [Sugesh] I am not sure if we need to add this condition in OVS. > > As a virtual switch, OVS should able to use these APIs irrespective of NIC/port > types. > > Actually we are masking this error , without the patch. It is possible > > to hit this issue, when we configure the flow control on a VF port. > > Ok, but now we have a situation that when adding a VF port, even without > specifying the flow control option, it fails as now flow control parameters are > being passed on init irregardless of whether the user intends to use it or not. > > Now that I think of it I think this would probably break other port types as well > that don't support flow control (such as vdevs). > > > Adding a condition in OVS to check a VF port for similar parameters > > doesn’t looks like a clean approach? What do you think? > > We've already had to do this for a few items (HW CRC enablement, LSC, MTU > support). It's not clean but until DPDK provides a way of checking capabilities > before init it's the best we can do. [Sugesh] We may need to feed this back to DPDK to see if we can get a standard way to know the capabilities. The flow control is slightly different than other parameters that mentioned above. It involved setting multiple parameters to enable . OVS doesn’t really configure all of those parameters .Instead It just only enable/disable the feature when user is requested. Remaining parameters are left to its defaults(which is not zero). So OVS must first query the flow control before trying to configure it. Now the query will fail for unsupported hardware, and the error message will be reported as of now. AFAIK DPDK doesn’t have any way to check if a port can support flow control/not, so we cannot avoid the error. But what we can do here is, Instead of calling the 'get' blindly, limit it to call only when user trying to configure flow control in the system. That will avoid the error in normal scenario. Does it make sense? I will send out the patch with those modifications. > > > > > This leads to another question , whats the impact of modifying other netdev > parameters > > on a VF port. Does it error out/silently fail/work as normal netdev ports? > > I think we can't fail completely based on an optional capability. And this is not > just for VF ports, conceivably you can have HW ports that don't support a > particular feature either supported by a netdev. > > For the current patch you could warn a user that the feature is not supported > and continue the initialization process (similar to the set mtu capability). > > Ian > > > > > >> > >> I've pulled this patch from the merge request for master, I think the > >> branch patches will have to be reworked also. > >> > >> Ian > >>> > >>> Ian > >>>> Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> > >>>> --- > >>>> lib/netdev-dpdk.c | 15 +++++++-------- > >>>> 1 file changed, 7 insertions(+), 8 deletions(-) > >>>> > >>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > >>>> bb4d60f26..023b80d3e 100644 > >>>> --- a/lib/netdev-dpdk.c > >>>> +++ b/lib/netdev-dpdk.c > >>>> @@ -1065,14 +1065,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) > >>>> mbp_priv = rte_mempool_get_priv(dev->dpdk_mp->mp); > >>>> dev->buf_size = mbp_priv->mbuf_data_room_size - > >>>> RTE_PKTMBUF_HEADROOM; > >>>> - > >>>> - /* Get the Flow control configuration for DPDK-ETH */ > >>>> - diag = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); > >>>> - if (diag) { > >>>> - VLOG_DBG("cannot get flow control parameters on port > >>>> "DPDK_PORT_ID_FMT > >>>> - ", err=%d", dev->port_id, diag); > >>>> - } > >>>> - > >>>> return 0; > >>>> } > >>>> @@ -1773,6 +1765,13 @@ netdev_dpdk_set_config(struct netdev > >>>> *netdev, const struct smap *args, > >>>> autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false); > >>>> fc_mode = fc_mode_set[tx_fc_en][rx_fc_en]; > >>>> + /* Get the Flow control configuration for DPDK-ETH */ > >>>> + err = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); > >>>> + if (err) { > >>>> + VLOG_INFO("cannot get flow control parameters on port > >>>> "DPDK_PORT_ID_FMT > >>>> + ", err=%d", dev->port_id, err); > >>>> + } > >>>> + > >>>> if (dev->fc_conf.mode != fc_mode || autoneg != > >>>> dev->fc_conf.autoneg) { > >>>> dev->fc_conf.mode = fc_mode; > >>>> dev->fc_conf.autoneg = autoneg; > >>>> > >>> > >>> _______________________________________________ > >>> dev mailing list > >>> dev@openvswitch.org > >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index bb4d60f26..023b80d3e 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -1065,14 +1065,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) mbp_priv = rte_mempool_get_priv(dev->dpdk_mp->mp); dev->buf_size = mbp_priv->mbuf_data_room_size - RTE_PKTMBUF_HEADROOM; - - /* Get the Flow control configuration for DPDK-ETH */ - diag = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); - if (diag) { - VLOG_DBG("cannot get flow control parameters on port "DPDK_PORT_ID_FMT - ", err=%d", dev->port_id, diag); - } - return 0; } @@ -1773,6 +1765,13 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false); fc_mode = fc_mode_set[tx_fc_en][rx_fc_en]; + /* Get the Flow control configuration for DPDK-ETH */ + err = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); + if (err) { + VLOG_INFO("cannot get flow control parameters on port "DPDK_PORT_ID_FMT + ", err=%d", dev->port_id, err); + } + if (dev->fc_conf.mode != fc_mode || autoneg != dev->fc_conf.autoneg) { dev->fc_conf.mode = fc_mode; dev->fc_conf.autoneg = autoneg;
Configuring flow control at ixgbe netdev-init is throwing error in port start. For eg: without this fix, user cannot configure flow control on ixgbe dpdk port as below, " ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ options:dpdk-devargs=0000:05:00.1 options:rx-flow-ctrl=true " Instead, it must be configured as two different commands, " ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \ options:dpdk-devargs=0000:05:00.1 ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true " The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' fields before trying to configuring the dpdk ethdev. Hence OVS can no longer set the 'dont care' fields to just '0' as before. This commit make sure all the 'rte_eth_fc_conf' fields are populated with default values before the dev init. Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com> --- lib/netdev-dpdk.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)