[ovs-dev,Branch2.8] netdev-dpdk: Fix failure to configure flow control at netdev-init.

Message ID 20180809104617.3635-1-sugesh.chandran@intel.com
State New
Delegated to: Ian Stokes
Headers show
Series
  • [ovs-dev,Branch2.8] netdev-dpdk: Fix failure to configure flow control at netdev-init.
Related show

Commit Message

Chandran, Sugesh Aug. 9, 2018, 10:46 a.m.
This patch backports the commit from the latest OVS master to OVS-2.8.

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.
Also to avoid read error on unsupported ports, the flow control parameters
are now read only when user is trying to configure/update it.

Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com>
---
 lib/netdev-dpdk.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Comments

0-day Robot Aug. 9, 2018, 10:54 a.m. | #1
Bleep bloop.  Greetings Sugesh Chandran, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
Failed to merge in the changes.
Patch failed at 0001 netdev-dpdk: Fix failure to configure flow control at netdev-init.
The copy of the patch that failed is found in:
   /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Aaron Conole Aug. 9, 2018, 1:15 p.m. | #2
Hi Sugesh (and robot),

0-day Robot <robot@bytheb.org> writes:

> Bleep bloop.  Greetings Sugesh Chandran, I am a robot and I have tried out your patch.
> Thanks for your contribution.
>
> I encountered some error that I wasn't expecting.  See the details below.
>
>
> git-am:
> Failed to merge in the changes.
> Patch failed at 0001 netdev-dpdk: Fix failure to configure flow control at netdev-init.
> The copy of the patch that failed is found in:
>    /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --resolved".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".

This happened because Branch2.8 doesn't match any such branch in the
repository.  It isn't a problem with your patch, per-se.

In the future, if you use branch-x.y the bot will detect the branch and
switch to using that branch.  Then the checks will be more robust.

> Please check this out.  If you feel there has been an error, please
> email aconole@bytheb.org
>
> Thanks,
> 0-day Robot
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Chandran, Sugesh Aug. 9, 2018, 1:38 p.m. | #3
Hi Aron,
Thank you for pointing out.
I will send out the patch again with right branch convention to make the robot happy.


Regards
_Sugesh


> -----Original Message-----
> From: Aaron Conole [mailto:aconole@bytheb.org]
> Sent: Thursday, August 9, 2018 2:15 PM
> To: 0-day Robot <robot@bytheb.org>
> Cc: Chandran, Sugesh <sugesh.chandran@intel.com>; dev@openvswitch.org
> Subject: Re: [ovs-dev] [ovs-dev, Branch2.8] netdev-dpdk: Fix failure to configure
> flow control at netdev-init.
> 
> Hi Sugesh (and robot),
> 
> 0-day Robot <robot@bytheb.org> writes:
> 
> > Bleep bloop.  Greetings Sugesh Chandran, I am a robot and I have tried out
> your patch.
> > Thanks for your contribution.
> >
> > I encountered some error that I wasn't expecting.  See the details below.
> >
> >
> > git-am:
> > Failed to merge in the changes.
> > Patch failed at 0001 netdev-dpdk: Fix failure to configure flow control at
> netdev-init.
> > The copy of the patch that failed is found in:
> >
> > /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-app
> > ly/patch When you have resolved this problem, run "git am --resolved".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> 
> This happened because Branch2.8 doesn't match any such branch in the
> repository.  It isn't a problem with your patch, per-se.
> 
> In the future, if you use branch-x.y the bot will detect the branch and switch to
> using that branch.  Then the checks will be more robust.
> 
> > Please check this out.  If you feel there has been an error, please
> > email aconole@bytheb.org
> >
> > Thanks,
> > 0-day Robot
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Aaron Conole Aug. 9, 2018, 2:47 p.m. | #4
"Chandran, Sugesh" <sugesh.chandran@intel.com> writes:

> Hi Aron,
> Thank you for pointing out.
> I will send out the patch again with right branch convention to make the robot happy.

No need.  We can ignore the bot in this case.  A committer can do the
right thing.  Just an FYI for future commits.

Thanks!

> Regards
> _Sugesh
>
>
>> -----Original Message-----
>> From: Aaron Conole [mailto:aconole@bytheb.org]
>> Sent: Thursday, August 9, 2018 2:15 PM
>> To: 0-day Robot <robot@bytheb.org>
>> Cc: Chandran, Sugesh <sugesh.chandran@intel.com>; dev@openvswitch.org
>> Subject: Re: [ovs-dev] [ovs-dev, Branch2.8] netdev-dpdk: Fix failure to configure
>> flow control at netdev-init.
>> 
>> Hi Sugesh (and robot),
>> 
>> 0-day Robot <robot@bytheb.org> writes:
>> 
>> > Bleep bloop.  Greetings Sugesh Chandran, I am a robot and I have tried out
>> your patch.
>> > Thanks for your contribution.
>> >
>> > I encountered some error that I wasn't expecting.  See the details below.
>> >
>> >
>> > git-am:
>> > Failed to merge in the changes.
>> > Patch failed at 0001 netdev-dpdk: Fix failure to configure flow control at
>> netdev-init.
>> > The copy of the patch that failed is found in:
>> >
>> > /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-app
>> > ly/patch When you have resolved this problem, run "git am --resolved".
>> > If you prefer to skip this patch, run "git am --skip" instead.
>> > To restore the original branch and stop patching, run "git am --abort".
>> 
>> This happened because Branch2.8 doesn't match any such branch in the
>> repository.  It isn't a problem with your patch, per-se.
>> 
>> In the future, if you use branch-x.y the bot will detect the branch and switch to
>> using that branch.  Then the checks will be more robust.
>> 
>> > Please check this out.  If you feel there has been an error, please
>> > email aconole@bytheb.org
>> >
>> > Thanks,
>> > 0-day Robot
>> > _______________________________________________
>> > 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
Ian Stokes Aug. 10, 2018, 10:18 a.m. | #5
> This patch backports the commit from the latest OVS master to OVS-2.8.
> 
> 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.
> Also to avoid read error on unsupported ports, the flow control parameters
> are now read only when user is trying to configure/update it.
> 
> Signed-off-by: Sugesh Chandran <sugesh.chandran@intel.com>

Thanks Sugesh, I'll backport this and the 2.7 patch to the appropriate branches as part of today's pull request.

Ian

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 9bf76ee32..f7c80e0dc 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -866,13 +866,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=%"PRIu8", err=%d",
-                 dev->port_id, diag);
-    }
-
     return 0;
 }
 
@@ -1404,6 +1397,12 @@  netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
 
     fc_mode = fc_mode_set[tx_fc_en][rx_fc_en];
     if (dev->fc_conf.mode != fc_mode || autoneg != dev->fc_conf.autoneg) {
+        /* Get the Flow control configuration for DPDK-ETH */
+        err = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf);
+        if (err) {
+            VLOG_WARN("Cannot get flow control parameters on port "
+                "%"PRIu16", err=%d", dev->port_id, err);
+        }
         dev->fc_conf.mode = fc_mode;
         dev->fc_conf.autoneg = autoneg;
         dpdk_eth_flow_ctrl_setup(dev);