diff mbox

[ovs-dev] netdev: Initialize netdev's features before getting them

Message ID f7ty41tk8ax.fsf@redhat.com
State Not Applicable
Headers show

Commit Message

Aaron Conole Oct. 12, 2016, 1:26 p.m. UTC
Hi Binbin,

Binbin Xu <xu.binbin1@zte.com.cn> writes:

> When OVS&DPDK is used, DPDK doesn't support features 'advertised',
> 'supported' and 'peer'. If a physical port added to bridge, features
> descirbed above can't be assigned, and the values are random.
>
> Signed-off-by: Binbin Xu <xu.binbin1@zte.com.cn>
> ---

Thanks for reporting this.  I consider this a bug in dpdk, not
something that requires changing the netdev framework.  A look at other
netdev classes confirms this.

Please re-submit something that fixes DPDK, like the following.  If I
read it correctly, you might also submit an additional patch to cleanup
the interface in lib/netdev-bsd.c (which seems to be 'mismatched').

NOTE patch is *completely* untested, not even compile tested.

---

Comments

Xu Binbin Oct. 13, 2016, 12:54 a.m. UTC | #1
Hi Aaron,

Thanks for your suggestion, I'll resubmit a patch for DPDK, and take a 
look at
lib/netdev-bsd.c to confirm that the additional patch is needed or not.

Thanks,

Aaron Conole <aconole@redhat.com> 写于 2016/10/12 21:26:14:

> 发件人:  Aaron Conole <aconole@redhat.com>

> 收件人:  Binbin Xu <xu.binbin1@zte.com.cn>, 

> 抄送: dev@openvswitch.org

> 日期:  2016/10/12 21:26

> 主题: Re: [ovs-dev] [PATCH] netdev: Initialize netdev's features 

> before getting them

> 

> Hi Binbin,

> 

> Binbin Xu <xu.binbin1@zte.com.cn> writes:

> 

> > When OVS&DPDK is used, DPDK doesn't support features 'advertised',

> > 'supported' and 'peer'. If a physical port added to bridge, features

> > descirbed above can't be assigned, and the values are random.

> >

> > Signed-off-by: Binbin Xu <xu.binbin1@zte.com.cn>

> > ---

> 

> Thanks for reporting this.  I consider this a bug in dpdk, not

> something that requires changing the netdev framework.  A look at other

> netdev classes confirms this.

> 

> Please re-submit something that fixes DPDK, like the following.  If I

> read it correctly, you might also submit an additional patch to cleanup

> the interface in lib/netdev-bsd.c (which seems to be 'mismatched').

> 

> NOTE patch is *completely* untested, not even compile tested.


I have tested the patch in my environment, you means that i didn't make 
the
unit test?

> 

> ---

> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c

> index 39bf930..6f5ec43 100644

> --- a/lib/netdev-dpdk.c

> +++ b/lib/netdev-dpdk.c

> @@ -1978,13 +1978,15 @@ out:

>  static int

>  netdev_dpdk_get_features(const struct netdev *netdev,

>                           enum netdev_features *current,

> -                         enum netdev_features *advertised OVS_UNUSED,

> -                         enum netdev_features *supported OVS_UNUSED,

> -                         enum netdev_features *peer OVS_UNUSED)

> +                         enum netdev_features *advertised,

> +                         enum netdev_features *supported,

> +                         enum netdev_features *peer)

>  {

>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);

>      struct rte_eth_link link;

> 

> +    *advertised = *peer = *supported = 0;

> +

>      ovs_mutex_lock(&dev->mutex);

>      link = dev->link;

>      ovs_mutex_unlock(&dev->mutex);
Aaron Conole Oct. 13, 2016, 1:16 p.m. UTC | #2
xu.binbin1@zte.com.cn writes:

> Hi Aaron, 
>
> Thanks for your suggestion, I'll resubmit a patch for DPDK, and take a look at 
> lib/netdev-bsd.c to confirm that the additional patch is needed or not. 
>
> Thanks, 
>
> Aaron Conole <aconole@redhat.com> 写于 2016/10/12 21:26:14:
>
>> 发件人:  Aaron Conole <aconole@redhat.com> 
>> 收件人:  Binbin Xu <xu.binbin1@zte.com.cn>, 
>> 抄送: dev@openvswitch.org 
>> 日期:  2016/10/12 21:26 
>> 主题: Re: [ovs-dev] [PATCH] netdev: Initialize netdev's features 
>> before getting them 
>> 
>> Hi Binbin,
>> 
>> Binbin Xu <xu.binbin1@zte.com.cn> writes:
>> 
>> > When OVS&DPDK is used, DPDK doesn't support features 'advertised',
>> > 'supported' and 'peer'. If a physical port added to bridge, features
>> > descirbed above can't be assigned, and the values are random.
>> >
>> > Signed-off-by: Binbin Xu <xu.binbin1@zte.com.cn>
>> > ---
>> 
>> Thanks for reporting this.  I consider this a bug in dpdk, not
>> something that requires changing the netdev framework.  A look at other
>> netdev classes confirms this.
>> 
>> Please re-submit something that fixes DPDK, like the following.  If I
>> read it correctly, you might also submit an additional patch to cleanup
>> the interface in lib/netdev-bsd.c (which seems to be 'mismatched').
>> 
>> NOTE patch is *completely* untested, not even compile tested. 
>
> I have tested the patch in my environment, you means that i didn't make the 
> unit test? 

I meant I have not tested the code I posted.

>> 
>> ---
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 39bf930..6f5ec43 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1978,13 +1978,15 @@ out:
>>  static int
>>  netdev_dpdk_get_features(const struct netdev *netdev,
>>                           enum netdev_features *current,
>> -                         enum netdev_features *advertised OVS_UNUSED,
>> -                         enum netdev_features *supported OVS_UNUSED,
>> -                         enum netdev_features *peer OVS_UNUSED)
>> +                         enum netdev_features *advertised,
>> +                         enum netdev_features *supported,
>> +                         enum netdev_features *peer)
>>  {
>>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>>      struct rte_eth_link link;
>>  
>> +    *advertised = *peer = *supported = 0;
>> +
>>      ovs_mutex_lock(&dev->mutex);
>>      link = dev->link;
>>      ovs_mutex_unlock(&dev->mutex);
diff mbox

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 39bf930..6f5ec43 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1978,13 +1978,15 @@  out:
 static int
 netdev_dpdk_get_features(const struct netdev *netdev,
                          enum netdev_features *current,
-                         enum netdev_features *advertised OVS_UNUSED,
-                         enum netdev_features *supported OVS_UNUSED,
-                         enum netdev_features *peer OVS_UNUSED)
+                         enum netdev_features *advertised,
+                         enum netdev_features *supported,
+                         enum netdev_features *peer)
 {
     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
     struct rte_eth_link link;
 
+    *advertised = *peer = *supported = 0;
+
     ovs_mutex_lock(&dev->mutex);
     link = dev->link;
     ovs_mutex_unlock(&dev->mutex);