Re: [ovs-dev] [PATCH] netdev-dpdk: add dpdk interface strip_vlan option

[Zang MingJie]

Yes, It is better to use the bit-field. I'll update the patch after
the refereed patch has been applied

On Tue, Apr 4, 2017 at 10:03 PM, Kevin Traynor  wrote:
> On 03/15/2017 08:46 AM, Zang MingJie wrote:
>> dpdk-strip-vlan option specifies whether strip vlan for the dpdk interface.
>>
>> Signed-off-by: Zang MingJie 
>> ---
>>  lib/netdev-dpdk.c| 23 ++-
>>  vswitchd/vswitch.xml |  7 +++
>>  2 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index ddc651bf9..ea49adf3e 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -373,6 +373,7 @@ struct netdev_dpdk {
>>  int requested_n_rxq;
>>  int requested_rxq_size;
>>  int requested_txq_size;
>> +bool requested_strip_vlan;
>>
>>  /* Number of rx/tx descriptors for physical devices */
>>  int rxq_size;
>> @@ -395,6 +396,8 @@ struct netdev_dpdk {
>>  /* DPDK-ETH hardware offload features,
>>   * from the enum set 'dpdk_hw_ol_features' */
>>  uint32_t hw_ol_features;
>> +
>> +bool strip_vlan;
>
> I don't think this should have it's own fields in the data struct as
> there is a hw_ol_feature bitmask intended for that. At the moment it is
> only used for rx checksum offload and could easily be extended for strip
> vlan if people think it's a good feature to expose. Maybe the reason you
> didn't use it in this patch is because it's currently broken and does
> nothing on reconfiguration :|
>
> I've sent a patch to fix that here
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330481.html
>
>>  };
>>
>>  struct netdev_rxq_dpdk {
>> @@ -646,6 +649,7 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int 
>> n_rxq, int n_txq)
>>  conf.rxmode.jumbo_frame = 0;
>>  conf.rxmode.max_rx_pkt_len = 0;
>>  }
>> +conf.rxmode.hw_vlan_strip = dev->strip_vlan;
>>  conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>>NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>>  /* A device may report more queues than it makes available (this has
>> @@ -1133,6 +1137,19 @@ netdev_dpdk_process_devargs(const char *devargs, char 
>> **errp)
>>  }
>>
>>  static void
>> +dpdk_set_strip_vlan_config(struct netdev_dpdk *dev, const struct smap *args)
>> +OVS_REQUIRES(dev->mutex)
>> +{
>> +bool strip_vlan;
>> +
>> +strip_vlan = smap_get_bool(args, "dpdk-strip-vlan", false);
>> +if (strip_vlan != dev->requested_strip_vlan) {
>> +dev->requested_strip_vlan = strip_vlan;
>> +netdev_request_reconfigure(>up);
>> +}
>> +}
>> +
>> +static void
>>  dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args)
>>  OVS_REQUIRES(dev->mutex)
>>  {
>> @@ -1182,6 +1199,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
>> struct smap *args,
>>  ovs_mutex_lock(>mutex);
>>
>>  dpdk_set_rxq_config(dev, args);
>> +dpdk_set_strip_vlan_config(dev, args);
>>
>>  dpdk_process_queue_size(netdev, args, "n_rxq_desc",
>>  NIC_PORT_DEFAULT_RXQ_SIZE,
>> @@ -3123,7 +3141,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
>>  && dev->mtu == dev->requested_mtu
>>  && dev->rxq_size == dev->requested_rxq_size
>>  && dev->txq_size == dev->requested_txq_size
>> -&& dev->socket_id == dev->requested_socket_id) {
>> +&& dev->socket_id == dev->requested_socket_id
>> +&& dev->strip_vlan == dev->requested_strip_vlan) {
>>  /* Reconfiguration is unnecessary */
>>
>>  goto out;
>> @@ -3145,6 +3164,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
>>  dev->rxq_size = dev->requested_rxq_size;
>>  dev->txq_size = dev->requested_txq_size;
>>
>> +dev->strip_vlan = dev->requested_strip_vlan;
>> +
>>  rte_free(dev->tx_q);
>>  err = dpdk_eth_dev_init(dev);
>>  dev->tx_q = netdev_dpdk_alloc_txq(netdev->n_txq);
>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> index a91be59b0..5c0083188 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -2360,6 +2360,13 @@
>>  
>>
>>
>> +  
>> +
>> +  Specifies whether strip vlan for the dpdk interface. It is useful
>> +  when using ixgbevf interface with a vlan filter.
>> +
>> +  
>> +
>>>type='{"type": "string"}'>
>>  
>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-dpdk: add dpdk interface strip_vlan option

[Kevin Traynor]

On 03/15/2017 08:46 AM, Zang MingJie wrote:
> dpdk-strip-vlan option specifies whether strip vlan for the dpdk interface.
> 
> Signed-off-by: Zang MingJie 
> ---
>  lib/netdev-dpdk.c| 23 ++-
>  vswitchd/vswitch.xml |  7 +++
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index ddc651bf9..ea49adf3e 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -373,6 +373,7 @@ struct netdev_dpdk {
>  int requested_n_rxq;
>  int requested_rxq_size;
>  int requested_txq_size;
> +bool requested_strip_vlan;
>  
>  /* Number of rx/tx descriptors for physical devices */
>  int rxq_size;
> @@ -395,6 +396,8 @@ struct netdev_dpdk {
>  /* DPDK-ETH hardware offload features,
>   * from the enum set 'dpdk_hw_ol_features' */
>  uint32_t hw_ol_features;
> +
> +bool strip_vlan;

I don't think this should have it's own fields in the data struct as
there is a hw_ol_feature bitmask intended for that. At the moment it is
only used for rx checksum offload and could easily be extended for strip
vlan if people think it's a good feature to expose. Maybe the reason you
didn't use it in this patch is because it's currently broken and does
nothing on reconfiguration :|

I've sent a patch to fix that here
https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330481.html

>  };
>  
>  struct netdev_rxq_dpdk {
> @@ -646,6 +649,7 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int 
> n_rxq, int n_txq)
>  conf.rxmode.jumbo_frame = 0;
>  conf.rxmode.max_rx_pkt_len = 0;
>  }
> +conf.rxmode.hw_vlan_strip = dev->strip_vlan;
>  conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>  /* A device may report more queues than it makes available (this has
> @@ -1133,6 +1137,19 @@ netdev_dpdk_process_devargs(const char *devargs, char 
> **errp)
>  }
>  
>  static void
> +dpdk_set_strip_vlan_config(struct netdev_dpdk *dev, const struct smap *args)
> +OVS_REQUIRES(dev->mutex)
> +{
> +bool strip_vlan;
> +
> +strip_vlan = smap_get_bool(args, "dpdk-strip-vlan", false);
> +if (strip_vlan != dev->requested_strip_vlan) {
> +dev->requested_strip_vlan = strip_vlan;
> +netdev_request_reconfigure(>up);
> +}
> +}
> +
> +static void
>  dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args)
>  OVS_REQUIRES(dev->mutex)
>  {
> @@ -1182,6 +1199,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
> struct smap *args,
>  ovs_mutex_lock(>mutex);
>  
>  dpdk_set_rxq_config(dev, args);
> +dpdk_set_strip_vlan_config(dev, args);
>  
>  dpdk_process_queue_size(netdev, args, "n_rxq_desc",
>  NIC_PORT_DEFAULT_RXQ_SIZE,
> @@ -3123,7 +3141,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
>  && dev->mtu == dev->requested_mtu
>  && dev->rxq_size == dev->requested_rxq_size
>  && dev->txq_size == dev->requested_txq_size
> -&& dev->socket_id == dev->requested_socket_id) {
> +&& dev->socket_id == dev->requested_socket_id
> +&& dev->strip_vlan == dev->requested_strip_vlan) {
>  /* Reconfiguration is unnecessary */
>  
>  goto out;
> @@ -3145,6 +3164,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
>  dev->rxq_size = dev->requested_rxq_size;
>  dev->txq_size = dev->requested_txq_size;
>  
> +dev->strip_vlan = dev->requested_strip_vlan;
> +
>  rte_free(dev->tx_q);
>  err = dpdk_eth_dev_init(dev);
>  dev->tx_q = netdev_dpdk_alloc_txq(netdev->n_txq);
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index a91be59b0..5c0083188 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -2360,6 +2360,13 @@
>  
>
>  
> +  
> +
> +  Specifies whether strip vlan for the dpdk interface. It is useful
> +  when using ixgbevf interface with a vlan filter.
> +
> +  
> +
>type='{"type": "string"}'>
>  
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] netdev-dpdk: add dpdk interface strip_vlan option

[Zang MingJie]

dpdk-strip-vlan option specifies whether strip vlan for the dpdk interface.

Signed-off-by: Zang MingJie 
---
 lib/netdev-dpdk.c| 23 ++-
 vswitchd/vswitch.xml |  7 +++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ddc651bf9..ea49adf3e 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -373,6 +373,7 @@ struct netdev_dpdk {
 int requested_n_rxq;
 int requested_rxq_size;
 int requested_txq_size;
+bool requested_strip_vlan;
 
 /* Number of rx/tx descriptors for physical devices */
 int rxq_size;
@@ -395,6 +396,8 @@ struct netdev_dpdk {
 /* DPDK-ETH hardware offload features,
  * from the enum set 'dpdk_hw_ol_features' */
 uint32_t hw_ol_features;
+
+bool strip_vlan;
 };
 
 struct netdev_rxq_dpdk {
@@ -646,6 +649,7 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int 
n_rxq, int n_txq)
 conf.rxmode.jumbo_frame = 0;
 conf.rxmode.max_rx_pkt_len = 0;
 }
+conf.rxmode.hw_vlan_strip = dev->strip_vlan;
 conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
   NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
 /* A device may report more queues than it makes available (this has
@@ -1133,6 +1137,19 @@ netdev_dpdk_process_devargs(const char *devargs, char 
**errp)
 }
 
 static void
+dpdk_set_strip_vlan_config(struct netdev_dpdk *dev, const struct smap *args)
+OVS_REQUIRES(dev->mutex)
+{
+bool strip_vlan;
+
+strip_vlan = smap_get_bool(args, "dpdk-strip-vlan", false);
+if (strip_vlan != dev->requested_strip_vlan) {
+dev->requested_strip_vlan = strip_vlan;
+netdev_request_reconfigure(>up);
+}
+}
+
+static void
 dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args)
 OVS_REQUIRES(dev->mutex)
 {
@@ -1182,6 +1199,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
struct smap *args,
 ovs_mutex_lock(>mutex);
 
 dpdk_set_rxq_config(dev, args);
+dpdk_set_strip_vlan_config(dev, args);
 
 dpdk_process_queue_size(netdev, args, "n_rxq_desc",
 NIC_PORT_DEFAULT_RXQ_SIZE,
@@ -3123,7 +3141,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
 && dev->mtu == dev->requested_mtu
 && dev->rxq_size == dev->requested_rxq_size
 && dev->txq_size == dev->requested_txq_size
-&& dev->socket_id == dev->requested_socket_id) {
+&& dev->socket_id == dev->requested_socket_id
+&& dev->strip_vlan == dev->requested_strip_vlan) {
 /* Reconfiguration is unnecessary */
 
 goto out;
@@ -3145,6 +3164,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
 dev->rxq_size = dev->requested_rxq_size;
 dev->txq_size = dev->requested_txq_size;
 
+dev->strip_vlan = dev->requested_strip_vlan;
+
 rte_free(dev->tx_q);
 err = dpdk_eth_dev_init(dev);
 dev->tx_q = netdev_dpdk_alloc_txq(netdev->n_txq);
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index a91be59b0..5c0083188 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -2360,6 +2360,13 @@
 
   
 
+  
+
+  Specifies whether strip vlan for the dpdk interface. It is useful
+  when using ixgbevf interface with a vlan filter.
+
+  
+
   
 
-- 
2.11.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev