[dpdk-dev,1/2] net/i40e: enable VF untag drop
Checks
Commit Message
Add a new private API to support the untag drop enable/disable
for specific VF.
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 49 +++++++++++++++++++++++++++++++++++++++++
drivers/net/i40e/rte_pmd_i40e.h | 18 +++++++++++++++
2 files changed, 67 insertions(+)
Comments
On 3/3/2017 1:59 AM, Qi Zhang wrote:
> Add a new private API to support the untag drop enable/disable
> for specific VF.
>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 49 +++++++++++++++++++++++++++++++++++++++++
> drivers/net/i40e/rte_pmd_i40e.h | 18 +++++++++++++++
Shared library is giving build error because of API is missing in
*version.map file
> 2 files changed, 67 insertions(+)
>
<...>
> diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
> index a0ad88c..895e2cc 100644
> --- a/drivers/net/i40e/rte_pmd_i40e.h
> +++ b/drivers/net/i40e/rte_pmd_i40e.h
> @@ -332,4 +332,22 @@ int rte_pmd_i40e_get_vf_stats(uint8_t port,
> int rte_pmd_i40e_reset_vf_stats(uint8_t port,
> uint16_t vf_id);
>
> +/**
> + * Enable/Disable VF untag drop
> + *
> + * @param port
> + * The port identifier of the Ethernet device.
> + * @param vf_id
> + * VF on witch to enable/disable
> + * @param on
> + * Enable or Disable
> + * @retura
@return
> + * - (0) if successful.
> + * -(-ENODEVE) if *port* invalid
> + * -(-EINVAL) if bad parameter.
> + */
> +int rte_pmd_i40e_set_vf_vlan_untag_drop(uint8_t port,
> + uint16_t vf_id,
> + uint8_t on);
As discussed previously, I believe it is good to keep following syntax
in API:
<name_space>_<object>_<action>, for this API it becomes:
rte_pmd_i40e_vf_vlan_untag_drop_set(), and perhaps "set" can be removed?
> +
> #endif /* _PMD_I40E_H_ */
>
Hi Ferruh:
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, March 7, 2017 6:51 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: enable VF untag drop
>
> On 3/3/2017 1:59 AM, Qi Zhang wrote:
> > Add a new private API to support the untag drop enable/disable for
> > specific VF.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 49
> > +++++++++++++++++++++++++++++++++++++++++
> > drivers/net/i40e/rte_pmd_i40e.h | 18 +++++++++++++++
>
> Shared library is giving build error because of API is missing in *version.map file
>
> > 2 files changed, 67 insertions(+)
> >
>
> <...>
>
> > diff --git a/drivers/net/i40e/rte_pmd_i40e.h
> > b/drivers/net/i40e/rte_pmd_i40e.h index a0ad88c..895e2cc 100644
> > --- a/drivers/net/i40e/rte_pmd_i40e.h
> > +++ b/drivers/net/i40e/rte_pmd_i40e.h
> > @@ -332,4 +332,22 @@ int rte_pmd_i40e_get_vf_stats(uint8_t port, int
> > rte_pmd_i40e_reset_vf_stats(uint8_t port,
> > uint16_t vf_id);
> >
> > +/**
> > + * Enable/Disable VF untag drop
> > + *
> > + * @param port
> > + * The port identifier of the Ethernet device.
> > + * @param vf_id
> > + * VF on witch to enable/disable
> > + * @param on
> > + * Enable or Disable
> > + * @retura
>
> @return
>
> > + * - (0) if successful.
> > + * -(-ENODEVE) if *port* invalid
> > + * -(-EINVAL) if bad parameter.
> > + */
> > +int rte_pmd_i40e_set_vf_vlan_untag_drop(uint8_t port,
> > + uint16_t vf_id,
> > + uint8_t on);
>
> As discussed previously, I believe it is good to keep following syntax in API:
> <name_space>_<object>_<action>, for this API it becomes:
I think, current naming rule is <name_space>_<action>_<object> right? See below
rte_pmd_i40e_set_vf_vlan_anti_spoof;
rte_pmd_i40e_set_vf_vlan_filter;
rte_pmd_i40e_set_vf_vlan_insert;
rte_pmd_i40e_set_vf_vlan_stripq;
rte_pmd_i40e_set_vf_vlan_tag;
so what's wrong with this?
>
> rte_pmd_i40e_vf_vlan_untag_drop_set(), and perhaps "set" can be removed?
>
> > +
> > #endif /* _PMD_I40E_H_ */
> >
On 3/9/2017 3:24 AM, Zhang, Qi Z wrote:
> Hi Ferruh:
>
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Tuesday, March 7, 2017 6:51 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
>> Zhang, Helin <helin.zhang@intel.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: enable VF untag drop
>>
>> On 3/3/2017 1:59 AM, Qi Zhang wrote:
>>> Add a new private API to support the untag drop enable/disable for
>>> specific VF.
>>>
>>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>>> ---
>>> drivers/net/i40e/i40e_ethdev.c | 49
>>> +++++++++++++++++++++++++++++++++++++++++
>>> drivers/net/i40e/rte_pmd_i40e.h | 18 +++++++++++++++
>>
>> Shared library is giving build error because of API is missing in *version.map file
>>
>>> 2 files changed, 67 insertions(+)
>>>
>>
>> <...>
>>
>>> diff --git a/drivers/net/i40e/rte_pmd_i40e.h
>>> b/drivers/net/i40e/rte_pmd_i40e.h index a0ad88c..895e2cc 100644
>>> --- a/drivers/net/i40e/rte_pmd_i40e.h
>>> +++ b/drivers/net/i40e/rte_pmd_i40e.h
>>> @@ -332,4 +332,22 @@ int rte_pmd_i40e_get_vf_stats(uint8_t port, int
>>> rte_pmd_i40e_reset_vf_stats(uint8_t port,
>>> uint16_t vf_id);
>>>
>>> +/**
>>> + * Enable/Disable VF untag drop
>>> + *
>>> + * @param port
>>> + * The port identifier of the Ethernet device.
>>> + * @param vf_id
>>> + * VF on witch to enable/disable
>>> + * @param on
>>> + * Enable or Disable
>>> + * @retura
>>
>> @return
>>
>>> + * - (0) if successful.
>>> + * -(-ENODEVE) if *port* invalid
>>> + * -(-EINVAL) if bad parameter.
>>> + */
>>> +int rte_pmd_i40e_set_vf_vlan_untag_drop(uint8_t port,
>>> + uint16_t vf_id,
>>> + uint8_t on);
>>
>> As discussed previously, I believe it is good to keep following syntax in API:
>> <name_space>_<object>_<action>, for this API it becomes:
> I think, current naming rule is <name_space>_<action>_<object> right?
Overall, I am not aware of any defined naming rule, I am for defining one.
> See below
> rte_pmd_i40e_set_vf_vlan_anti_spoof;
> rte_pmd_i40e_set_vf_vlan_filter;
> rte_pmd_i40e_set_vf_vlan_insert;
> rte_pmd_i40e_set_vf_vlan_stripq;
> rte_pmd_i40e_set_vf_vlan_tag;
> so what's wrong with this?
This breaks hierarchical approach, if you think API name as tree. Easier
to see this when you sort the APIs, ns_set_x, ns_reset_x, ns_del_x will
spread to different locations.
This looks OK when you work on one type of object already, but with all
APIs in concern, I believe object based grouping is better than action
based grouping.
And why do you think above one is better? Again, as long as one is
agreed on, I am OK.
>>
>> rte_pmd_i40e_vf_vlan_untag_drop_set(), and perhaps "set" can be removed?
>>
>>> +
>>> #endif /* _PMD_I40E_H_ */
>>>
>
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, March 14, 2017 9:30 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: enable VF untag drop
>
> On 3/9/2017 3:24 AM, Zhang, Qi Z wrote:
> > Hi Ferruh:
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh
> >> Sent: Tuesday, March 7, 2017 6:51 PM
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing
> >> <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> >> Cc: dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: enable VF untag drop
> >>
> >> On 3/3/2017 1:59 AM, Qi Zhang wrote:
> >>> Add a new private API to support the untag drop enable/disable for
> >>> specific VF.
> >>>
> >>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> >>> ---
> >>> drivers/net/i40e/i40e_ethdev.c | 49
> >>> +++++++++++++++++++++++++++++++++++++++++
> >>> drivers/net/i40e/rte_pmd_i40e.h | 18 +++++++++++++++
> >>
> >> Shared library is giving build error because of API is missing in
> >> *version.map file
> >>
> >>> 2 files changed, 67 insertions(+)
> >>>
> >>
> >> <...>
> >>
> >>> diff --git a/drivers/net/i40e/rte_pmd_i40e.h
> >>> b/drivers/net/i40e/rte_pmd_i40e.h index a0ad88c..895e2cc 100644
> >>> --- a/drivers/net/i40e/rte_pmd_i40e.h
> >>> +++ b/drivers/net/i40e/rte_pmd_i40e.h
> >>> @@ -332,4 +332,22 @@ int rte_pmd_i40e_get_vf_stats(uint8_t port,
> >>> int rte_pmd_i40e_reset_vf_stats(uint8_t port,
> >>> uint16_t vf_id);
> >>>
> >>> +/**
> >>> + * Enable/Disable VF untag drop
> >>> + *
> >>> + * @param port
> >>> + * The port identifier of the Ethernet device.
> >>> + * @param vf_id
> >>> + * VF on witch to enable/disable
> >>> + * @param on
> >>> + * Enable or Disable
> >>> + * @retura
> >>
> >> @return
> >>
> >>> + * - (0) if successful.
> >>> + * -(-ENODEVE) if *port* invalid
> >>> + * -(-EINVAL) if bad parameter.
> >>> + */
> >>> +int rte_pmd_i40e_set_vf_vlan_untag_drop(uint8_t port,
> >>> + uint16_t vf_id,
> >>> + uint8_t on);
> >>
> >> As discussed previously, I believe it is good to keep following syntax in
> API:
> >> <name_space>_<object>_<action>, for this API it becomes:
> > I think, current naming rule is <name_space>_<action>_<object> right?
>
> Overall, I am not aware of any defined naming rule, I am for defining one.
>
> > See below
> > rte_pmd_i40e_set_vf_vlan_anti_spoof;
> > rte_pmd_i40e_set_vf_vlan_filter;
> > rte_pmd_i40e_set_vf_vlan_insert;
> > rte_pmd_i40e_set_vf_vlan_stripq;
> > rte_pmd_i40e_set_vf_vlan_tag;
> > so what's wrong with this
>
> This breaks hierarchical approach, if you think API name as tree. Easier to
> see this when you sort the APIs, ns_set_x, ns_reset_x, ns_del_x will spread
> to different locations.
I agree with your point, I had thought your concern is only about this patch, but actually it's not.
>
> This looks OK when you work on one type of object already, but with all APIs
> in concern, I believe object based grouping is better than action based
> grouping.
>
> And why do you think above one is better? Again, as long as one is agreed on,
I don't, sorry for make you misunderstand
>
> >>
> >> rte_pmd_i40e_vf_vlan_untag_drop_set(), and perhaps "set" can be
> removed?
> >>
> >>> +
> >>> #endif /* _PMD_I40E_H_ */
> >>>
> >
Hi Ferruh, Qi,
<snip>
> > >> Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: enable VF untag drop
> > >>
> > >> On 3/3/2017 1:59 AM, Qi Zhang wrote:
> > >>> Add a new private API to support the untag drop enable/disable for
> > >>> specific VF.
> > >>>
> > >>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > >>> ---
> > >>> drivers/net/i40e/i40e_ethdev.c | 49
> > >>> +++++++++++++++++++++++++++++++++++++++++
> > >>> drivers/net/i40e/rte_pmd_i40e.h | 18 +++++++++++++++
> > >>
> > >> Shared library is giving build error because of API is missing in
> > >> *version.map file
> > >>
> > >>> 2 files changed, 67 insertions(+)
> > >>>
> > >>
> > >> <...>
> > >>
> > >>> diff --git a/drivers/net/i40e/rte_pmd_i40e.h
> > >>> b/drivers/net/i40e/rte_pmd_i40e.h index a0ad88c..895e2cc 100644
> > >>> --- a/drivers/net/i40e/rte_pmd_i40e.h
> > >>> +++ b/drivers/net/i40e/rte_pmd_i40e.h
> > >>> @@ -332,4 +332,22 @@ int rte_pmd_i40e_get_vf_stats(uint8_t port,
> > >>> int rte_pmd_i40e_reset_vf_stats(uint8_t port,
> > >>> uint16_t vf_id);
> > >>>
> > >>> +/**
> > >>> + * Enable/Disable VF untag drop
> > >>> + *
> > >>> + * @param port
> > >>> + * The port identifier of the Ethernet device.
> > >>> + * @param vf_id
> > >>> + * VF on witch to enable/disable
> > >>> + * @param on
> > >>> + * Enable or Disable
> > >>> + * @retura
> > >>
> > >> @return
> > >>
> > >>> + * - (0) if successful.
> > >>> + * -(-ENODEVE) if *port* invalid
> > >>> + * -(-EINVAL) if bad parameter.
> > >>> + */
> > >>> +int rte_pmd_i40e_set_vf_vlan_untag_drop(uint8_t port,
> > >>> + uint16_t vf_id,
> > >>> + uint8_t on);
> > >>
> > >> As discussed previously, I believe it is good to keep following
> > >> syntax in
> > API:
> > >> <name_space>_<object>_<action>, for this API it becomes:
> > > I think, current naming rule is <name_space>_<action>_<object> right?
This seems to be the existing naming convention.
> >
> > Overall, I am not aware of any defined naming rule, I am for defining one.
> >
> > > See below
> > > rte_pmd_i40e_set_vf_vlan_anti_spoof;
> > > rte_pmd_i40e_set_vf_vlan_filter;
> > > rte_pmd_i40e_set_vf_vlan_insert;
> > > rte_pmd_i40e_set_vf_vlan_stripq;
> > > rte_pmd_i40e_set_vf_vlan_tag; so what's wrong with this
> >
> > This breaks hierarchical approach, if you think API name as tree.
> > Easier to see this when you sort the APIs, ns_set_x, ns_reset_x,
> > ns_del_x will spread to different locations.
> I agree with your point, I had thought your concern is only about this patch,
> but actually it's not.
> >
> > This looks OK when you work on one type of object already, but with
> > all APIs in concern, I believe object based grouping is better than
> > action based grouping.
>
> >
> > And why do you think above one is better? Again, as long as one is
> > agreed on,
> I don't, sorry for make you misunderstand
I don't think changing the name convention at this point is a good idea.
It would be better to remain consistent with the existing naming convention.
Otherwise both naming conventions will exist for the rte_pmd_i40e_* API's.
> > >> rte_pmd_i40e_vf_vlan_untag_drop_set(), and perhaps "set" can be
> > removed?
> > >>
> > >>> +
> > >>> #endif /* _PMD_I40E_H_ */
> > >>>
> > >
Regards,
Bernard.
On 3/14/2017 6:16 PM, Iremonger, Bernard wrote:
> Hi Ferruh, Qi,
>
> <snip>
>
>>>>> Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: enable VF untag drop
>>>>>
>>>>> On 3/3/2017 1:59 AM, Qi Zhang wrote:
>>>>>> Add a new private API to support the untag drop enable/disable for
>>>>>> specific VF.
>>>>>>
>>>>>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>>>>>> ---
>>>>>> drivers/net/i40e/i40e_ethdev.c | 49
>>>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>>> drivers/net/i40e/rte_pmd_i40e.h | 18 +++++++++++++++
>>>>>
>>>>> Shared library is giving build error because of API is missing in
>>>>> *version.map file
>>>>>
>>>>>> 2 files changed, 67 insertions(+)
>>>>>>
>>>>>
>>>>> <...>
>>>>>
>>>>>> diff --git a/drivers/net/i40e/rte_pmd_i40e.h
>>>>>> b/drivers/net/i40e/rte_pmd_i40e.h index a0ad88c..895e2cc 100644
>>>>>> --- a/drivers/net/i40e/rte_pmd_i40e.h
>>>>>> +++ b/drivers/net/i40e/rte_pmd_i40e.h
>>>>>> @@ -332,4 +332,22 @@ int rte_pmd_i40e_get_vf_stats(uint8_t port,
>>>>>> int rte_pmd_i40e_reset_vf_stats(uint8_t port,
>>>>>> uint16_t vf_id);
>>>>>>
>>>>>> +/**
>>>>>> + * Enable/Disable VF untag drop
>>>>>> + *
>>>>>> + * @param port
>>>>>> + * The port identifier of the Ethernet device.
>>>>>> + * @param vf_id
>>>>>> + * VF on witch to enable/disable
>>>>>> + * @param on
>>>>>> + * Enable or Disable
>>>>>> + * @retura
>>>>>
>>>>> @return
>>>>>
>>>>>> + * - (0) if successful.
>>>>>> + * -(-ENODEVE) if *port* invalid
>>>>>> + * -(-EINVAL) if bad parameter.
>>>>>> + */
>>>>>> +int rte_pmd_i40e_set_vf_vlan_untag_drop(uint8_t port,
>>>>>> + uint16_t vf_id,
>>>>>> + uint8_t on);
>>>>>
>>>>> As discussed previously, I believe it is good to keep following
>>>>> syntax in
>>> API:
>>>>> <name_space>_<object>_<action>, for this API it becomes:
>>>> I think, current naming rule is <name_space>_<action>_<object> right?
>
> This seems to be the existing naming convention.
>
>>>
>>> Overall, I am not aware of any defined naming rule, I am for defining one.
>>>
>>>> See below
>>>> rte_pmd_i40e_set_vf_vlan_anti_spoof;
>>>> rte_pmd_i40e_set_vf_vlan_filter;
>>>> rte_pmd_i40e_set_vf_vlan_insert;
>>>> rte_pmd_i40e_set_vf_vlan_stripq;
>>>> rte_pmd_i40e_set_vf_vlan_tag; so what's wrong with this
>>>
>>> This breaks hierarchical approach, if you think API name as tree.
>>> Easier to see this when you sort the APIs, ns_set_x, ns_reset_x,
>>> ns_del_x will spread to different locations.
>> I agree with your point, I had thought your concern is only about this patch,
>> but actually it's not.
>>>
>>> This looks OK when you work on one type of object already, but with
>>> all APIs in concern, I believe object based grouping is better than
>>> action based grouping.
>>
>>>
>>> And why do you think above one is better? Again, as long as one is
>>> agreed on,
>> I don't, sorry for make you misunderstand
>
> I don't think changing the name convention at this point is a good idea.
I am not suggesting changing existing ones, for the sake compatibility.
But that can also be an option, since these are PMD specific API, I
expect usage will be limited and these does not carry as high standard
as library APIs.
> It would be better to remain consistent with the existing naming convention.
Existing i40e ones added this way to be compatible with existing ixgbe
ones. But I don't think we need to follow old usage with new APIs.
> Otherwise both naming conventions will exist for the rte_pmd_i40e_* API's.
It can be for a while, later all can be fixed. If you think proposed
convention is not better, that is something else.
>
>
>>>>> rte_pmd_i40e_vf_vlan_untag_drop_set(), and perhaps "set" can be
>>> removed?
>>>>>
>>>>>> +
>>>>>> #endif /* _PMD_I40E_H_ */
>>>>>>
>>>>
>
> Regards,
>
> Bernard.
>
Hi Ferruh,
<snip>
> >>>>> Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: enable VF untag drop
> >>>>>
> >>>>> On 3/3/2017 1:59 AM, Qi Zhang wrote:
> >>>>>> Add a new private API to support the untag drop enable/disable
> >>>>>> for specific VF.
> >>>>>>
> >>>>>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> >>>>>> ---
> >>>>>> drivers/net/i40e/i40e_ethdev.c | 49
> >>>>>> +++++++++++++++++++++++++++++++++++++++++
> >>>>>> drivers/net/i40e/rte_pmd_i40e.h | 18 +++++++++++++++
> >>>>>
> >>>>> Shared library is giving build error because of API is missing in
> >>>>> *version.map file
> >>>>>
> >>>>>> 2 files changed, 67 insertions(+)
> >>>>>>
> >>>>>
> >>>>> <...>
> >>>>>
> >>>>>> diff --git a/drivers/net/i40e/rte_pmd_i40e.h
> >>>>>> b/drivers/net/i40e/rte_pmd_i40e.h index a0ad88c..895e2cc 100644
> >>>>>> --- a/drivers/net/i40e/rte_pmd_i40e.h
> >>>>>> +++ b/drivers/net/i40e/rte_pmd_i40e.h
> >>>>>> @@ -332,4 +332,22 @@ int rte_pmd_i40e_get_vf_stats(uint8_t
> port,
> >>>>>> int rte_pmd_i40e_reset_vf_stats(uint8_t port,
> >>>>>> uint16_t vf_id);
> >>>>>>
> >>>>>> +/**
> >>>>>> + * Enable/Disable VF untag drop
> >>>>>> + *
> >>>>>> + * @param port
> >>>>>> + * The port identifier of the Ethernet device.
> >>>>>> + * @param vf_id
> >>>>>> + * VF on witch to enable/disable
> >>>>>> + * @param on
> >>>>>> + * Enable or Disable
> >>>>>> + * @retura
> >>>>>
> >>>>> @return
> >>>>>
> >>>>>> + * - (0) if successful.
> >>>>>> + * -(-ENODEVE) if *port* invalid
> >>>>>> + * -(-EINVAL) if bad parameter.
> >>>>>> + */
> >>>>>> +int rte_pmd_i40e_set_vf_vlan_untag_drop(uint8_t port,
> >>>>>> + uint16_t vf_id,
> >>>>>> + uint8_t on);
> >>>>>
> >>>>> As discussed previously, I believe it is good to keep following
> >>>>> syntax in
> >>> API:
> >>>>> <name_space>_<object>_<action>, for this API it becomes:
> >>>> I think, current naming rule is <name_space>_<action>_<object>
> right?
> >
> > This seems to be the existing naming convention.
> >
> >>>
> >>> Overall, I am not aware of any defined naming rule, I am for defining
> one.
> >>>
> >>>> See below
> >>>> rte_pmd_i40e_set_vf_vlan_anti_spoof;
> >>>> rte_pmd_i40e_set_vf_vlan_filter;
> >>>> rte_pmd_i40e_set_vf_vlan_insert;
> >>>> rte_pmd_i40e_set_vf_vlan_stripq;
> >>>> rte_pmd_i40e_set_vf_vlan_tag; so what's wrong with this
> >>>
> >>> This breaks hierarchical approach, if you think API name as tree.
> >>> Easier to see this when you sort the APIs, ns_set_x, ns_reset_x,
> >>> ns_del_x will spread to different locations.
> >> I agree with your point, I had thought your concern is only about
> >> this patch, but actually it's not.
> >>>
> >>> This looks OK when you work on one type of object already, but with
> >>> all APIs in concern, I believe object based grouping is better than
> >>> action based grouping.
> >>
> >>>
> >>> And why do you think above one is better? Again, as long as one is
> >>> agreed on,
> >> I don't, sorry for make you misunderstand
> >
> > I don't think changing the name convention at this point is a good idea.
>
> I am not suggesting changing existing ones, for the sake compatibility.
> But that can also be an option, since these are PMD specific API, I expect
> usage will be limited and these does not carry as high standard as library APIs.
These API's are already in use with users.
> > It would be better to remain consistent with the existing naming
> convention.
>
> Existing i40e ones added this way to be compatible with existing ixgbe ones.
> But I don't think we need to follow old usage with new APIs.
>
> > Otherwise both naming conventions will exist for the rte_pmd_i40e_*
> API's.
>
> It can be for a while, later all can be fixed. If you think proposed convention is
> not better, that is something else.
I don't see anything to be gained by using two naming conventions side by side.
I don't have a strong view on which name convention is better, however the existing naming convention is what is in use with users at present. It is also the naming convention used librte_ether.
Changing API naming conventions is something that should probably be discussed in a separate thread, as it can have a wider impact than the i40e and ixgbe PMD's.
> >
> >
> >>>>> rte_pmd_i40e_vf_vlan_untag_drop_set(), and perhaps "set" can be
> >>> removed?
> >>>>>
> >>>>>> +
> >>>>>> #endif /* _PMD_I40E_H_ */
> >>>>>>
> >>>>
Regards,
Bernard.
@@ -11212,3 +11212,52 @@ rte_pmd_i40e_reset_vf_stats(uint8_t port,
return 0;
}
+
+int rte_pmd_i40e_set_vf_vlan_untag_drop(uint8_t port,
+ uint16_t vf_id,
+ uint8_t on)
+{
+ struct rte_eth_dev *dev;
+ struct i40e_pf *pf;
+ struct i40e_vsi *vsi;
+ struct i40e_aqc_add_remove_vlan_element_data vlan_data = {0};
+ struct i40e_hw *hw;
+ int ret;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+ dev = &rte_eth_devices[port];
+
+ if (!is_device_supported(dev, &rte_i40e_pmd))
+ return -ENOTSUP;
+
+ pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+ if (vf_id >= pf->vf_num || !pf->vfs) {
+ PMD_DRV_LOG(ERR, "Invalid VF ID.");
+ return -EINVAL;
+ }
+
+ vsi = pf->vfs[vf_id].vsi;
+ if (!vsi) {
+ PMD_DRV_LOG(ERR, "Invalid VSI.");
+ return -EINVAL;
+ }
+
+ hw = I40E_VSI_TO_HW(vsi);
+ vlan_data.vlan_tag = 0;
+ vlan_data.vlan_flags = 0x1;
+
+ if (on) {
+ ret = i40e_aq_add_vlan(hw, vsi->seid, &vlan_data, 1, NULL);
+ if (ret != I40E_SUCCESS)
+ PMD_DRV_LOG(ERR, "Failed to add vlan filter");
+ } else {
+ ret = i40e_aq_remove_vlan(hw, vsi->seid,
+ &vlan_data, 1, NULL);
+ if (ret != I40E_SUCCESS)
+ PMD_DRV_LOG(ERR, "Failed to remove vlan filter");
+ }
+
+ return ret;
+}
@@ -332,4 +332,22 @@ int rte_pmd_i40e_get_vf_stats(uint8_t port,
int rte_pmd_i40e_reset_vf_stats(uint8_t port,
uint16_t vf_id);
+/**
+ * Enable/Disable VF untag drop
+ *
+ * @param port
+ * The port identifier of the Ethernet device.
+ * @param vf_id
+ * VF on witch to enable/disable
+ * @param on
+ * Enable or Disable
+ * @retura
+ * - (0) if successful.
+ * -(-ENODEVE) if *port* invalid
+ * -(-EINVAL) if bad parameter.
+ */
+int rte_pmd_i40e_set_vf_vlan_untag_drop(uint8_t port,
+ uint16_t vf_id,
+ uint8_t on);
+
#endif /* _PMD_I40E_H_ */