All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Helin Zhang <helin.zhang@intel.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v2 0/3] i40e setting ether type of VLANs
Date: Mon, 07 Mar 2016 10:28:47 +0100	[thread overview]
Message-ID: <1571443.EPqn3bY1d6@xps13> (raw)
In-Reply-To: <1457338370-32744-1-git-send-email-helin.zhang@intel.com>

2016-03-07 16:12, Helin Zhang:
> The patch set was branched off rel_16_04 of repo dpdk-next-net,
> on below commit.
>  - commit 4ac366ba647909c3b71818f9be9db86ba5e871da
>      nfp: fix non-x86 build

Currently, changes on ethdev are directly applied on dpdk.git.

> v2:
>  - Used RTE_NEXT_ABI to avoid ABI change issue.

RTE_NEXT_ABI must be used only when it is really too difficult to keep
the compatibility with librte_compat.
Here you are just adding a parameter to some functions, so you should
try versionning the functions with the help of macros in librte_compat.

About the API change, you want to be able to insert a QinQ inner-vlan, right?
The current comment of rte_eth_dev_set_vlan_ether_type is:
 * Set the Outer VLAN Ether Type by an Ethernet device, it can be inserted to
 * the VLAN Header. This is a register setup available on some Intel NIC, not
 * but all, please check the data sheet for availability.

2 comments:
- you haven't changed "Outer VLAN" so the API description is wrong
- it is announced as something Intel-specific

About the new enum:
+ * VLAN types to indicate if it is for single VLAN, inner VLAN or outer VLAN.
+ * Note that most of time single VLAN is treated the same as inner VLAN.

You cannot say "most of time" in an API.

More generally, I am not convinced by the current VLAN API that you are extending.
Why this function is not merged with rte_eth_dev_set_vlan_pvid?

  parent reply	other threads:[~2016-03-07  9:30 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-22  1:37 [PATCH 0/2] i40e setting ether type of VLANs Helin Zhang
2016-01-22  1:37 ` [PATCH 1/2] ethdev: add vlan type for setting ether type Helin Zhang
2016-01-22  1:37 ` [PATCH 2/2] i40e: add VLAN ether type config Helin Zhang
2016-03-07  8:12 ` [PATCH v2 0/3] i40e setting ether type of VLANs Helin Zhang
2016-03-07  8:12   ` [PATCH v2 1/3] ethdev: add vlan type for setting ether type Helin Zhang
2016-03-07  8:12   ` [PATCH v2 2/3] i40e: add VLAN ether type config Helin Zhang
2016-03-07  8:12   ` [PATCH v2 3/3] i40e: fix the overflow issue Helin Zhang
2016-03-07  9:28   ` Thomas Monjalon [this message]
2016-03-09 15:20     ` [PATCH v2 0/3] i40e setting ether type of VLANs Zhang, Helin
2016-03-10 16:36   ` [PATCH v3 0/2] " Helin Zhang
2016-03-10 16:36     ` [PATCH v3 1/2] ethdev: add vlan type for setting ether type Helin Zhang
2016-03-10 16:36     ` [PATCH v3 2/2] i40e: fix the overflow issue Helin Zhang
2016-03-11  2:36     ` [PATCH v3 0/2] i40e setting ether type of VLANs Lu, Wenzhuo
2016-03-11  8:49     ` [PATCH v4 " Helin Zhang
2016-03-11  8:49       ` [PATCH v4 1/2] ethdev: add vlan type for setting ether type Helin Zhang
2016-03-11 11:19         ` Panu Matilainen
2016-03-11 11:20           ` Thomas Monjalon
2016-03-11 14:17             ` Zhang, Helin
2016-03-11 14:20               ` Thomas Monjalon
2016-03-11  8:49       ` [PATCH v4 2/2] i40e: fix the overflow issue Helin Zhang
2016-03-11 16:50       ` [PATCH v5 0/2] i40e setting ether type of VLANs Helin Zhang
2016-03-11 16:50         ` [PATCH v5 1/2] ethdev: add vlan type for setting ether type Helin Zhang
2016-03-11 16:50         ` [PATCH v5 2/2] i40e: fix the overflow issue Helin Zhang
2016-03-11 20:20         ` [PATCH v5 0/2] i40e setting ether type of VLANs Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1571443.EPqn3bY1d6@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=dev@dpdk.org \
    --cc=helin.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.