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

Submitted by Zang MingJie on March 15, 2017, 8:46 a.m.

Details

Message ID 20170315084605.14762-1-zealot0630@gmail.com
State New
Headers show

Commit Message

Zang MingJie March 15, 2017, 8:46 a.m.
dpdk-strip-vlan option specifies whether strip vlan for the dpdk interface.

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

Comments

Kevin Traynor April 4, 2017, 2:03 p.m.
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 <zealot0630@gmail.com>
> ---
>  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(&dev->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(&dev->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 @@
>          </p>
>        </column>
>  
> +      <column name="options" key="dpdk-strip-vlan" type='{"type": "boolean"}'>
> +        <p>
> +          Specifies whether strip vlan for the dpdk interface. It is useful
> +          when using ixgbevf interface with a vlan filter.
> +        </p>
> +      </column>
> +
>        <column name="options" key="vhost-server-path"
>                type='{"type": "string"}'>
>          <p>
>
Zang MingJie April 18, 2017, 10:48 a.m.
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 <ktraynor@redhat.com> 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 <zealot0630@gmail.com>
>> ---
>>  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(&dev->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(&dev->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 @@
>>          </p>
>>        </column>
>>
>> +      <column name="options" key="dpdk-strip-vlan" type='{"type": "boolean"}'>
>> +        <p>
>> +          Specifies whether strip vlan for the dpdk interface. It is useful
>> +          when using ixgbevf interface with a vlan filter.
>> +        </p>
>> +      </column>
>> +
>>        <column name="options" key="vhost-server-path"
>>                type='{"type": "string"}'>
>>          <p>
>>
>

Patch hide | download patch | download mbox

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(&dev->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(&dev->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 @@ 
         </p>
       </column>
 
+      <column name="options" key="dpdk-strip-vlan" type='{"type": "boolean"}'>
+        <p>
+          Specifies whether strip vlan for the dpdk interface. It is useful
+          when using ixgbevf interface with a vlan filter.
+        </p>
+      </column>
+
       <column name="options" key="vhost-server-path"
               type='{"type": "string"}'>
         <p>