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 <ktray...@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 <zealot0...@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>
>>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to