[ovs-dev] [PATCH 1/2] netdev-dpdk: Restore txq/rxq number if initialization fails.

[Ethan Jackson]

Ben, Justin, should this be backported?  I'm not up on the policy at the moment.

Etha

On Thu, Jul 23, 2015 at 3:42 AM, Traynor, Kevin <kevin.traynor at intel.com> wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Daniele Di
>> Proietto
>> Sent: Thursday, July 16, 2015 7:48 PM
>> To: dev at openvswitch.org
>> Subject: [ovs-dev] [PATCH 1/2] netdev-dpdk: Restore txq/rxq number if
>> initialization fails.
>>
>> netdev_dpdk_set_multiq() should not set the number of configured rxq
>> and txq if the driver initialization fails (meaning that the driver
>> failed to setup the queues).  Otherwise, on a subsequent call to
>> netdev_dpdk_set_multiq(), the code may believe that the queues have
>> already been setup and there's no work to be done.
>>
>> This commit fixes the problem by restoring the old values if
>> dpdk_eth_dev_init() fails.
>>
>> Reported-by: Ian Stokes <ian.stokes at intel.com>
>> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
>> ---
>>  lib/netdev-dpdk.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 8b843db..5ae805e 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -743,6 +743,7 @@ netdev_dpdk_set_multiq(struct netdev *netdev_, unsigned
>> int n_txq,
>>  {
>>      struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
>>      int err = 0;
>> +    int old_rxq, old_txq;
>>
>>      if (netdev->up.n_txq == n_txq && netdev->up.n_rxq == n_rxq) {
>>          return err;
>> @@ -753,12 +754,20 @@ netdev_dpdk_set_multiq(struct netdev *netdev_, unsigned
>> int n_txq,
>>
>>      rte_eth_dev_stop(netdev->port_id);
>>
>> +    old_txq = netdev->up.n_txq;
>> +    old_rxq = netdev->up.n_rxq;
>>      netdev->up.n_txq = n_txq;
>>      netdev->up.n_rxq = n_rxq;
>>
>>      rte_free(netdev->tx_q);
>>      err = dpdk_eth_dev_init(netdev);
>>      netdev_dpdk_alloc_txq(netdev, netdev->real_n_txq);
>> +    if (err) {
>> +        /* If there has been an error, it means that the requested queues
>> +         * have not been created.  Restore the old numbers. */
>> +        netdev->up.n_txq = old_txq;
>> +        netdev->up.n_rxq = old_rxq;
>
> I had thought that we should restore the previous netdev->tx_q but at present
> txq's are fixed, so I think it is fine. If txq's become configurable we can change.
>
> It would be good to get these patches into OVS2.4 branch if there is still time?
>
> Acked-by: Kevin Traynor <kevin.traynor at intel.com>
>
>> +    }
>>
>>      netdev->txq_needs_locking = netdev->real_n_txq != netdev->up.n_txq;
>>
>> --
>> 2.1.4
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev