> -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Monday, March 7, 2016 5:29 PM > To: Zhang, Helin > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 0/3] i40e setting ether type of VLANs > > 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. Won't it be reviewed by Bruce, and applied on his maintainer branch first?
> > > 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. Let me think it a little bit more. I don't think I have understood that versioning quite well. > > About the API change, you want to be able to insert a QinQ inner-vlan, right? No, I just want to configure the ether type of vlan the hardware can recognize, but not to insert a vlan. > 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. Yes, I agree with you the comments should be reworked. > > 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. Accepted. > > 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? No, they are different. I am trying to configure the hardware recognized ether type of both inner and outer vlan, but not to configure the vlan itself.
