Re: [dpdk-dev] [PATCH v6 00/26] linux/eal: Remove most causes of panic on init

[Bruce Richardson]

On Wed, Mar 08, 2017 at 10:58:27PM +0100, Thomas Monjalon wrote:
> Hi,
> 
> Thanks for the work.
> I think it needs to be completed to have the same behaviour on bsdapp.

Ideally, yes, but I also don't think the lack of BSD changes should
block the inclusion of this set. In terms of application writers, the
apps don't need to be written differently for BSD compared to Linux
because of this change. All that is different is that the BSD version
will panic rather than return the error code. 

Regards,
/Bruce

> 

Re: [dpdk-dev] [PATCH v2 00/13] introduce fail-safe PMD

[Bruce Richardson]

On Wed, Mar 08, 2017 at 11:54:02AM -0500, Neil Horman wrote:
> On Wed, Mar 08, 2017 at 04:15:33PM +0100, Gaetan Rivet wrote:
> > This PMD intercepts and manages Ethernet device removal events issued by
> > slave PMDs and re-initializes them transparently when brought back so that
> > existing applications do not need to be modified to benefit from true
> > hot-plugging support.
> > 
> > The stacked PMD approach shares many similarities with the bonding PMD but
> > with a different purpose. While bonding provides the ability to group
> > several links into a single logical device for enhanced throughput and
> > supports fail-over at link level, this one manages the sudden disappearance
> > of the underlying device; it guarantees applications face a valid device in
> > working order at all times.
> > 
> Why not just add this feature to the bonding pmd then?  A bond is perfectly
> capable of handling the trivial case of a single underlying device, and adding
> an option to make the underly slave 'persistent' seem both much simpler in 
> terms
> of implementation and code size, than adding an entire new pmd, along with its
> supporting code.
> 
> Neil
>
+1 
I don't like the idea of having multiple PMDs in DPDK to handle
combining multiple other devices into one.

/Bruce

Re: [dpdk-dev] [PATCH v6 00/26] linux/eal: Remove most causes of panic on init

[Thomas Monjalon]

2017-03-09 09:11, Bruce Richardson:
> On Wed, Mar 08, 2017 at 10:58:27PM +0100, Thomas Monjalon wrote:
> > Hi,
> > 
> > Thanks for the work.
> > I think it needs to be completed to have the same behaviour on bsdapp.
> 
> Ideally, yes, but I also don't think the lack of BSD changes should
> block the inclusion of this set. In terms of application writers, the
> apps don't need to be written differently for BSD compared to Linux
> because of this change. All that is different is that the BSD version
> will panic rather than return the error code. 

So you do not have any issue about having a different behaviour
on Linux and BSD?
You are the bsdapp maintainer, so it is your call.

Re: [dpdk-dev] [PATCH v6 00/26] linux/eal: Remove most causes of panic on init

[Richardson, Bruce]

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monja...@6wind.com]
> Sent: Thursday, March 9, 2017 9:26 AM
> To: Richardson, Bruce 
> Cc: Aaron Conole ; dev@dpdk.org; Stephen Hemminger
> 
> Subject: Re: [dpdk-dev] [PATCH v6 00/26] linux/eal: Remove most causes of
> panic on init
> 
> 2017-03-09 09:11, Bruce Richardson:
> > On Wed, Mar 08, 2017 at 10:58:27PM +0100, Thomas Monjalon wrote:
> > > Hi,
> > >
> > > Thanks for the work.
> > > I think it needs to be completed to have the same behaviour on bsdapp.
> >
> > Ideally, yes, but I also don't think the lack of BSD changes should
> > block the inclusion of this set. In terms of application writers, the
> > apps don't need to be written differently for BSD compared to Linux
> > because of this change. All that is different is that the BSD version
> > will panic rather than return the error code.
> 
> So you do not have any issue about having a different behaviour on Linux
> and BSD?
> You are the bsdapp maintainer, so it is your call.

I would infinitely prefer to have the same behavior. However, so long as this 
does not require a user to change their app to be different on BSD, I don't 
think lack of BSD support should block improving Linux.

Aaron - will you be able to do equivalent changes to BSD within the 17.05 
timeframe?

/Bruce

Re: [dpdk-dev] [PATCH 1/4] net/i40e: support replace filter type

[Ferruh Yigit]

On 3/9/2017 5:59 AM, Xing, Beilei wrote:
> 
> 
>> -Original Message-
>> From: Yigit, Ferruh
>> Sent: Wednesday, March 8, 2017 11:50 PM
>> To: Xing, Beilei ; Wu, Jingjing 
>> 
>> Cc: Zhang, Helin ; dev@dpdk.org; Iremonger,
>> Bernard ; Stroe, Laura
>> 
>> Subject: Re: [dpdk-dev] [PATCH 1/4] net/i40e: support replace filter type
>>
>> On 3/3/2017 9:31 AM, Beilei Xing wrote:
>>> Add new admin queue function and extended fields in DCR 288:
>>>  - Add admin queue function for Replace filter
>>>command (Opcode: 0x025F)
>>>  - Add General fields for Add/Remove Cloud filters
>>>command
>>>
>>> This patch will be removed to base driver in future.
>>>
>>> Signed-off-by: Bernard Iremonger 
>>> Signed-off-by: Stroe Laura 
>>> Signed-off-by: Jingjing Wu 
>>> Signed-off-by: Beilei Xing 
>>> ---
>>>  drivers/net/i40e/i40e_ethdev.h | 106 
>>>  drivers/net/i40e/i40e_flow.c   | 152
>> +
>>>  2 files changed, 258 insertions(+)
>>>
>>> diff --git a/drivers/net/i40e/i40e_ethdev.h
>>> b/drivers/net/i40e/i40e_ethdev.h index f545850..3a49865 100644
>>> --- a/drivers/net/i40e/i40e_ethdev.h
>>> +++ b/drivers/net/i40e/i40e_ethdev.h
>>> @@ -729,6 +729,100 @@ struct i40e_valid_pattern {
>>> parse_filter_t parse_filter;
>>>  };
>>>
>>> +/* Support replace filter */
>>> +
>>> +/* i40e_aqc_add_remove_cloud_filters_element_big_data is used when
>>> + * I40E_AQC_ADD_REM_CLOUD_CMD_BIG_BUFFER flag is set. refer to
>>> + * DCR288
>>
>> Please do not refer to DCR, unless you can provide a public link for it.
> OK, got it.
> 
>>
>>> + */
>>> +struct i40e_aqc_add_remove_cloud_filters_element_big_data {
>>> +   struct i40e_aqc_add_remove_cloud_filters_element_data element;
>>
>> What is the difference between
>> "i40e_aqc_add_remove_cloud_filters_element_big_data" and
>> "i40e_aqc_add_remove_cloud_filters_element_data", why need big_data
>> one?
> 
> As ' Add/Remove Cloud filters -command buffer ' is changed in the DCR288, 
> 'general fields' exists only when big_buffer is set.

What does it mean having "big_buffer" set? What changes functionally
being big_buffer set or not?

> But we don't want to change the  " 
> i40e_aqc_add_remove_cloud_filters_element_data " as it will cause ABI/API 
> change in kernel driver.
> 
<...>

Re: [dpdk-dev] ip_pipeline firewall customization

[Dumitrescu, Cristian]

Hi Shyam,

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shyam Shrivastav
> Sent: Wednesday, March 8, 2017 3:55 PM
> To: dev@dpdk.org
> Cc: Shyam Shrivastav 
> Subject: [dpdk-dev] ip_pipeline firewall customization
> 
> Hi All
> 
> I am using ip_pipeline firewall as base for our project, need
> comments/suggestions/corrections regarding following
> 
> 1) We can not configure firewall  rule to drop packets, as portid is
> mandatory in command. I am planning to allow this for our requirement with
> following code changes
>   a) Allow "port" as optional parameter (pipeline_firewall.c), pass -1
> as port id if "port" is not specified to app_pipeline_firewall_add_rule and
> change that parameter to int32_t.
>   b) Make required changes in pipeline_firewall_msg_req_add_handler if
> portid is -1, that is table entry action to be .action =
> RTE_PIPELINE_ACTION_DROP.
> 

You don't really need to do this for implementing a drop action. You can simply 
create a SINK output port (which basically drops all the packets directed to 
it) and set this as the output port for all rules that drop packet.

See ip_pipeline/config/firewall.cfg as example.

> 2) I am registering a f_action_hit function for firewall table to perform
> certain translations if action is pass (RTE_PIPELINE_ACTION_PORT).

What type of actions are you performing? If generic enough, it would be 
interesting to add them to this pipeline, so I encourage you to contribute with 
ideas and code patches.

> Configured a rule like following
> 
> pipeline>p 1 firewall add priority 1 ipv4 0.0.0.0 0 0.0.0.0 0 0 65535 0
> 65535 0 0 port 0
> 
> which should be hit only by ipv4 packets. However even ARP packets are hit
> by this ACL and my routine is called. If I configure a specific src or dst
> ip then everything works fine and arp packets are not hit , for example
> following rule hits only ipv4 icmp packets
> 
> pipeline>  p 1 firewall add priority 1 ipv4 0.0.0.0 0 45.35.70.12 32 0
> 65535 0 65535 1 0xf port 0
> 
> Is this a bug or am I missing something ?
> 

You can make sure no ARP packets are received by the firewall pipeline by 
simply filtering all the ARP packets to a separate RXQ of the NIC port, which 
can be further handled by a separate function.

See ip_pipeline/config/network_layers.cfg as example:
[LINK0]
arp_q = 4

> 
> Thanks
> Shyam

Regards,
Cristian

Re: [dpdk-dev] [v4 0/3] Merge l3fwd-acl and l3fwd

[Mcnamara, John]

> From: Ravi Kerur [mailto:rke...@gmail.com] 
> Sent: Monday, March 6, 2017 11:21 PM
> To: Mcnamara, John 
> Cc: dev@dpdk.org; Ananyev, Konstantin ; 
> Richardson, Bruce 
> Subject: Re: [dpdk-dev] [v4 0/3] Merge l3fwd-acl and l3fwd
> 
> 
> Should I work with documentation team to update the document? If yes, 
> please let me know the contact information.

Hi Ravi,

There isn't any documentation team. Unfortunately. :-)

The raw documentation is in text format and is generally updated by
the developer submitting a patch.

There is already docs for l3fwd and l3fwd_acl so all that is required
is to merge them. However, you will need to take some care to remove
the duplicate sections and to make sure that the merged doc makes
sense as a whole.

See the documentation guidelines:

http://dpdk.org/doc/guides/contributing/documentation.html
 
John

Re: [dpdk-dev] [v4 0/3] Merge l3fwd-acl and l3fwd

[Mcnamara, John]

> From: Ravi Kerur [mailto:rke...@gmail.com] 
> Sent: Wednesday, March 8, 2017 9:51 PM
> To: Mcnamara, John 
> Cc: dev@dpdk.org; Ananyev, Konstantin ; 
> Richardson, Bruce 
> Subject: Re: [dpdk-dev] [v4 0/3] Merge l3fwd-acl and l3fwd


> Kindly let me know new 'v5' patch follows dpdk guidelines?


Hi Ravi,

In general the commit messages don't meet the guidelines, so it is 
probably worth reading through the guidelines again:

   http://dpdk.org/doc/guides/contributing/patches.html

In particular have a look at the sections on the "subject line" and the
"message body".

I'll make some other comments to your patches to help you along.

Thanks,

John

Re: [dpdk-dev] [PATCH v5 1/3] examples/l3fwd: merge l3fwd-acl code into l3fwd

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ravi Kerur
> Sent: Wednesday, March 8, 2017 9:32 PM
> To: dev@dpdk.org; Ananyev, Konstantin ;
> Richardson, Bruce 
> Cc: Ravi Kerur 
> Subject: [dpdk-dev] [PATCH v5 1/3] examples/l3fwd: merge l3fwd-acl code
> into l3fwd
> 
> Merge l3fwd-acl code into l3fwd with '-A' cmdline option to run ACL.

Hi Ravi,

The subject line and commit message are okay. the only issue is that
the version information should be after a triple dash line so that
is doesn't appear in the commit message.

The version information is intended for reviewers so that know what
has changed since the last version. Just put the following before it:

---


> 
> v5:
>   > None.
> 
> v4:
> > Initialize rss_hf to IP for LPM, EM and ACL.
> > Update rss_hf with l4 in parse_args for ACL.
> > Fix pending checkpatch code indentation warning.


This applies to all the commit messages.

John

Re: [dpdk-dev] [PATCH 1/4] net/i40e: support replace filter type

[Xing, Beilei]

> -Original Message-
> From: Yigit, Ferruh
> Sent: Thursday, March 9, 2017 6:02 PM
> To: Xing, Beilei ; Wu, Jingjing 
> Cc: Zhang, Helin ; dev@dpdk.org; Iremonger,
> Bernard ; Stroe, Laura
> 
> Subject: Re: [dpdk-dev] [PATCH 1/4] net/i40e: support replace filter type
> 
> On 3/9/2017 5:59 AM, Xing, Beilei wrote:
> >
> >
> >> -Original Message-
> >> From: Yigit, Ferruh
> >> Sent: Wednesday, March 8, 2017 11:50 PM
> >> To: Xing, Beilei ; Wu, Jingjing
> >> 
> >> Cc: Zhang, Helin ; dev@dpdk.org; Iremonger,
> >> Bernard ; Stroe, Laura
> >> 
> >> Subject: Re: [dpdk-dev] [PATCH 1/4] net/i40e: support replace filter
> >> type
> >>
> >> On 3/3/2017 9:31 AM, Beilei Xing wrote:
> >>> Add new admin queue function and extended fields in DCR 288:
> >>>  - Add admin queue function for Replace filter
> >>>command (Opcode: 0x025F)
> >>>  - Add General fields for Add/Remove Cloud filters
> >>>command
> >>>
> >>> This patch will be removed to base driver in future.
> >>>
> >>> Signed-off-by: Bernard Iremonger 
> >>> Signed-off-by: Stroe Laura 
> >>> Signed-off-by: Jingjing Wu 
> >>> Signed-off-by: Beilei Xing 
> >>> ---
> >>>  drivers/net/i40e/i40e_ethdev.h | 106
> 
> >>>  drivers/net/i40e/i40e_flow.c   | 152
> >> +
> >>>  2 files changed, 258 insertions(+)
> >>>
> >>> diff --git a/drivers/net/i40e/i40e_ethdev.h
> >>> b/drivers/net/i40e/i40e_ethdev.h index f545850..3a49865 100644
> >>> --- a/drivers/net/i40e/i40e_ethdev.h
> >>> +++ b/drivers/net/i40e/i40e_ethdev.h
> >>> @@ -729,6 +729,100 @@ struct i40e_valid_pattern {
> >>>   parse_filter_t parse_filter;
> >>>  };
> >>>
> >>> +/* Support replace filter */
> >>> +
> >>> +/* i40e_aqc_add_remove_cloud_filters_element_big_data is used
> when
> >>> + * I40E_AQC_ADD_REM_CLOUD_CMD_BIG_BUFFER flag is set. refer to
> >>> + * DCR288
> >>
> >> Please do not refer to DCR, unless you can provide a public link for it.
> > OK, got it.
> >
> >>
> >>> + */
> >>> +struct i40e_aqc_add_remove_cloud_filters_element_big_data {
> >>> + struct i40e_aqc_add_remove_cloud_filters_element_data element;
> >>
> >> What is the difference between
> >> "i40e_aqc_add_remove_cloud_filters_element_big_data" and
> >> "i40e_aqc_add_remove_cloud_filters_element_data", why need
> big_data
> >> one?
> >
> > As ' Add/Remove Cloud filters -command buffer ' is changed in the DCR288,
> 'general fields' exists only when big_buffer is set.
> 
> What does it mean having "big_buffer" set? What changes functionally being
> big_buffer set or not?

According to DCR288, "Add/Remove Cloud Filter Command" should add 'Big Buffer' 
in byte20, but we can't change ' struct i40e_aqc_add_remove_cloud_filters ' in 
base code,
struct i40e_aqc_add_remove_cloud_filters {
u8  num_filters;
u8  reserved;
__le16  seid;
#define I40E_AQC_ADD_CLOUD_CMD_SEID_NUM_SHIFT   0
#define I40E_AQC_ADD_CLOUD_CMD_SEID_NUM_MASK(0x3FF << \
I40E_AQC_ADD_CLOUD_CMD_SEID_NUM_SHIFT)
u8  reserved2[4];
__le32  addr_high;
__le32  addr_low;
};

So we use reserverd[0] for 'Big Buffer' here, in the patch for ND, we changed 
above structure with following:

struct i40e_aqc_add_remove_cloud_filters {
u8  num_filters;
u8  reserved;
__le16  seid;
#define I40E_AQC_ADD_CLOUD_CMD_SEID_NUM_SHIFT   0
#define I40E_AQC_ADD_CLOUD_CMD_SEID_NUM_MASK(0x3FF << \
I40E_AQC_ADD_CLOUD_CMD_SEID_NUM_SHIFT)
u8  big_buffer;
u8  reserved2[3];
__le32  addr_high;
__le32  addr_low;
};


> 
> > But we don't want to change the  "
> i40e_aqc_add_remove_cloud_filters_element_data " as it will cause ABI/API
> change in kernel driver.
> >
> <...>

Re: [dpdk-dev] [PATCH v5 2/3] examples/l3fwd: add config file support for LPM

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ravi Kerur
> Sent: Wednesday, March 8, 2017 9:32 PM
> To: dev@dpdk.org; Ananyev, Konstantin ;
> Richardson, Bruce 
> Cc: Ravi Kerur 
> Subject: [dpdk-dev] [PATCH v5 2/3] examples/l3fwd: add config file support
> for LPM
> 
> Add config file support for v4 and v6 to build forwarding tables.


Hi Ravi,

The subject line is fine. The body might be better saying IPv4 and IPv6 since
I initially thought you were referring to the versions below.

> 
> v5:
>   > Changes is_bypass_line from inline to non-line.
> 
> v4:
>   > No changes.


Same comment as before about ---.

John.

Re: [dpdk-dev] [PATCH v5 3/3] examples/l3fwd: add config file support for EM

[Mcnamara, John]

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ravi Kerur
> Sent: Wednesday, March 8, 2017 9:32 PM
> To: dev@dpdk.org; Ananyev, Konstantin ;
> Richardson, Bruce 
> Cc: Ravi Kerur 
> Subject: [dpdk-dev] [PATCH v5 3/3] examples/l3fwd: add config file support
> for EM
> 
> Add config file support for v4 and v6 to build exact match forwarding
> tables.

Hi Ravi,

Same comment about Ipv4 and IPv6 and ---.

Also it is probably better to use "exact match" instead of "EM" in the
subject. It isn't as well known an acronym as LPM.

John

Re: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter for MPLS

[Iremonger, Bernard]

Hi Beilei,

> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Beilei Xing
> Sent: Friday, March 3, 2017 9:44 AM
> To: Wu, Jingjing 
> Cc: Zhang, Helin ; dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter for MPLS
> 
> This patch enables MPLSoUDP and MPLSoGRE cloud filter with replace cloud
> filter.
> 
> Signed-off-by: Beilei Xing 
> ---
>  drivers/net/i40e/i40e_ethdev.c |  44 +++--
>  drivers/net/i40e/i40e_ethdev.h |   9 +++-
>  drivers/net/i40e/i40e_flow.c   | 108
> +
>  3 files changed, 144 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 91bfd73..6044daf 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -1286,6 +1286,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
>   if (ret < 0)
>   goto err_init_fdir_filter_list;
> 
> + i40e_replace_mpls_l1_filter(pf);
> + i40e_replace_mpls_cloud_filter(pf);
> +
>   return 0;
> 
>  err_init_fdir_filter_list:
> @@ -6941,6 +6944,7 @@ i40e_dev_consistent_tunnel_filter_set(struct
> i40e_pf *pf,
>   struct i40e_tunnel_rule *tunnel_rule = &pf->tunnel;
>   struct i40e_tunnel_filter *tunnel, *node;
>   struct i40e_tunnel_filter check_filter; /* Check if filter exists */
> + uint32_t teid_le;
>   bool big_buffer = 0;
> 
>   cld_filter = rte_zmalloc("tunnel_filter", @@ -6989,6 +6993,28 @@
> i40e_dev_consistent_tunnel_filter_set(struct i40e_pf *pf,
>   case I40E_TUNNEL_TYPE_IP_IN_GRE:
>   tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_IP;
>   break;
> + case I40E_TUNNEL_TYPE_MPLSoUDP:
> + teid_le = rte_cpu_to_le_32(tunnel_filter->tenant_id);
> + pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD0] =
> + teid_le >> 4;
> + pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> + (teid_le & 0xF) << 12;
> + pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD2] =
> + 0x40;
> + big_buffer = 1;
> + tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoUDP;
> + break;
> + case I40E_TUNNEL_TYPE_MPLSoGRE:
> + teid_le = rte_cpu_to_le_32(tunnel_filter->tenant_id);
> + pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD0] =
> + teid_le >> 4;
> + pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> + (teid_le & 0xF) << 12;
> + pfilter-
> >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD2] =
> + 0x0;
> + big_buffer = 1;
> + tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoGRE;
> + break;
>   default:
>   /* Other tunnel types is not supported. */
>   PMD_DRV_LOG(ERR, "tunnel type is not supported."); @@ -
> 6996,11 +7022,19 @@ i40e_dev_consistent_tunnel_filter_set(struct i40e_pf
> *pf,
>   return -EINVAL;
>   }
> 
> - val = i40e_dev_get_filter_type(tunnel_filter->filter_type,
> -&pfilter->element.flags);
> - if (val < 0) {
> - rte_free(cld_filter);
> - return -EINVAL;
> + if (tunnel_filter->tunnel_type == I40E_TUNNEL_TYPE_MPLSoUDP)
> + pfilter->element.flags =
> + I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoUDP;
> + else if (tunnel_filter->tunnel_type ==
> I40E_TUNNEL_TYPE_MPLSoGRE)
> + pfilter->element.flags =
> + I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoGRE;
> + else {
> + val = i40e_dev_get_filter_type(tunnel_filter->filter_type,
> + &pfilter->element.flags);
> + if (val < 0) {
> + rte_free(cld_filter);
> + return -EINVAL;
> + }
>   }
> 
>   pfilter->element.flags |= rte_cpu_to_le_16( diff --git
> a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h index
> dd9d709..f305baa 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -499,8 +499,11 @@ struct i40e_ethertype_rule {
>  /* Tunnel filter number HW supports */
>  #define I40E_MAX_TUNNEL_FILTER_NUM 400
> 
> -#define I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoUDP 0x11 -#define
> I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoGRE 0x12
> +#define I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoUDP 8 #define
> +I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoGRE 9 #define
> +I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoUDP 0x11 #define
> +I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoGRE 0x12 #define
> +I40E_AQC_ADD_L1_FILTER_TEID_MPLS 0x11
> 
>  enum i40e_tunnel_iptype {
>   I40E_TUNNEL_IPTYPE_IPV4,
> @@ -963,6 +966,8 @@ enum i40e_status_code
> i40e_aq_remove_cloud_filters_big_buffer(
>  enum i40e_status_code i40e_aq_replace_cloud

Re: [dpdk-dev] [PATCH v2 00/18] net/fm10k: update base code.

[Ferruh Yigit]

On 3/8/2017 6:18 AM, Qi Zhang wrote:
> The patch set update to latest fm10k base code, there is no
> significent features be added, but include couple enhancement
> like clean up the logic for tlv attr parse, improve VF multi-bit 
> VLAN handle etc, also with some code/comment clean as well.
> 
> v2: 
>   - add base code information
>   - correct couple typo errors in commit log.
> 
> Qi Zhang (17):
>   net/fm10k/base: add a flag to indicate VF trust mode
>   net/fm10k/base: reset multicast mode when deleting lport
>   net/fm10k/base: expose macros needed by DPDK
>   net/fm10k/base: add error code
>   net/fm10k/base: clean up the logic
>   net/fm10k/base: add new item to lport msg attr
>   net/fm10k/base: update comment
>   net/fm10k/base: use different name for override bit
>   net/fm10k/base: update comment regarding reserved bits check
>   net/fm10k/base: improve VF's multi-bit VLAN update requests
>   net/fm10k/base: enable lport map request
>   net/fm10k/base: add macros for global interrupt
>   net/fm10k/base: don't stop reset
>   net/fm10k/base: add marcro for geneve tunnel
>   net/fm10k/base: improve re-map queues handle
>   net/fm10k/base: replace marcos
>   net/fm10k/base: request reset when mbx->state changes
>   net/fm10k/base: add base code driver information

Series applied to dpdk-next-net/master, thanks.

defined(DPDK_SUPPORT) removed from patch 3/18, which is useless and
looks like exists by mistake, please double check updated code.

Re: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter for MPLS

[Xing, Beilei]

Hi Bernard,

> -Original Message-
> From: Iremonger, Bernard
> Sent: Thursday, March 9, 2017 7:13 PM
> To: Xing, Beilei ; Wu, Jingjing 
> Cc: Zhang, Helin ; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter for MPLS
> 
> Hi Beilei,
> 
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Beilei Xing
> > Sent: Friday, March 3, 2017 9:44 AM
> > To: Wu, Jingjing 
> > Cc: Zhang, Helin ; dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter for MPLS
> >
> > This patch enables MPLSoUDP and MPLSoGRE cloud filter with replace
> > cloud filter.
> >
> > Signed-off-by: Beilei Xing 
> > ---
> >  drivers/net/i40e/i40e_ethdev.c |  44 +++--
> >  drivers/net/i40e/i40e_ethdev.h |   9 +++-
> >  drivers/net/i40e/i40e_flow.c   | 108
> > +
> >  3 files changed, 144 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 91bfd73..6044daf 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -1286,6 +1286,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
> > if (ret < 0)
> > goto err_init_fdir_filter_list;
> >
> > +   i40e_replace_mpls_l1_filter(pf);
> > +   i40e_replace_mpls_cloud_filter(pf);
> > +
> > return 0;
> >
> >  err_init_fdir_filter_list:
> > @@ -6941,6 +6944,7 @@ i40e_dev_consistent_tunnel_filter_set(struct
> > i40e_pf *pf,
> > struct i40e_tunnel_rule *tunnel_rule = &pf->tunnel;
> > struct i40e_tunnel_filter *tunnel, *node;
> > struct i40e_tunnel_filter check_filter; /* Check if filter exists */
> > +   uint32_t teid_le;
> > bool big_buffer = 0;
> >
> > cld_filter = rte_zmalloc("tunnel_filter", @@ -6989,6 +6993,28 @@
> > i40e_dev_consistent_tunnel_filter_set(struct i40e_pf *pf,
> > case I40E_TUNNEL_TYPE_IP_IN_GRE:
> > tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_IP;
> > break;
> > +   case I40E_TUNNEL_TYPE_MPLSoUDP:
> > +   teid_le = rte_cpu_to_le_32(tunnel_filter->tenant_id);
> > +   pfilter-
> > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD0] =
> > +   teid_le >> 4;
> > +   pfilter-
> > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> > +   (teid_le & 0xF) << 12;
> > +   pfilter-
> > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD2] =
> > +   0x40;
> > +   big_buffer = 1;
> > +   tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoUDP;
> > +   break;
> > +   case I40E_TUNNEL_TYPE_MPLSoGRE:
> > +   teid_le = rte_cpu_to_le_32(tunnel_filter->tenant_id);
> > +   pfilter-
> > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD0] =
> > +   teid_le >> 4;
> > +   pfilter-
> > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> > +   (teid_le & 0xF) << 12;
> > +   pfilter-
> > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD2] =
> > +   0x0;
> > +   big_buffer = 1;
> > +   tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoGRE;
> > +   break;
> > default:
> > /* Other tunnel types is not supported. */
> > PMD_DRV_LOG(ERR, "tunnel type is not supported."); @@ -
> > 6996,11 +7022,19 @@ i40e_dev_consistent_tunnel_filter_set(struct
> > i40e_pf *pf,
> > return -EINVAL;
> > }
> >
> > -   val = i40e_dev_get_filter_type(tunnel_filter->filter_type,
> > -  &pfilter->element.flags);
> > -   if (val < 0) {
> > -   rte_free(cld_filter);
> > -   return -EINVAL;
> > +   if (tunnel_filter->tunnel_type == I40E_TUNNEL_TYPE_MPLSoUDP)
> > +   pfilter->element.flags =
> > +   I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoUDP;
> > +   else if (tunnel_filter->tunnel_type ==
> > I40E_TUNNEL_TYPE_MPLSoGRE)
> > +   pfilter->element.flags =
> > +   I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoGRE;
> > +   else {
> > +   val = i40e_dev_get_filter_type(tunnel_filter->filter_type,
> > +   &pfilter->element.flags);
> > +   if (val < 0) {
> > +   rte_free(cld_filter);
> > +   return -EINVAL;
> > +   }
> > }
> >
> > pfilter->element.flags |= rte_cpu_to_le_16( diff --git
> > a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> > index dd9d709..f305baa 100644
> > --- a/drivers/net/i40e/i40e_ethdev.h
> > +++ b/drivers/net/i40e/i40e_ethdev.h
> > @@ -499,8 +499,11 @@ struct i40e_ethertype_rule {
> >  /* Tunnel filter number HW supports */  #define
> > I40E_MAX_TUNNEL_FILTER_NUM 400
> >
> > -#define I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoUDP 0x11 -#define
> > I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoGRE 0x12
> > +#define I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoUDP 8 #define
> > +I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoGRE 

Re: [dpdk-dev] ip_pipeline firewall customization

[Shyam Shrivastav]

Hi Cristian

Please see my comments inline (in blue)

>   b) Make required changes in pipeline_firewall_msg_req_add_handler if
> > portid is -1, that is table entry action to be .action =
> > RTE_PIPELINE_ACTION_DROP.
> >
>
> You don't really need to do this for implementing a drop action. You can
> simply create a SINK output port (which basically drops all the packets
> directed to it) and set this as the output port for all rules that drop
> packet.
>
> See ip_pipeline/config/firewall.cfg as example.
>

Yes dropping of packets can be achieved by creating a sink port. However we
need further processing to be done for a packet hitting a pass rule, and
best way for this processing is f_action_hit table handler.  In this
routine we can distinguish between drop/pass actions as
rte_pipeline_table_entry is passed to it, which also contains  portid but
no generic way to  distinguish between normal or sink port.
I think it would be value addition to allow this action in firewall
pipeline, please let me know your thoughts. As for our project looks like
we have to include this.


>
> > 2) I am registering a f_action_hit function for firewall table to perform
> > certain translations if action is pass (RTE_PIPELINE_ACTION_PORT).
>
> What type of actions are you performing? If generic enough, it would be
> interesting to add them to this pipeline, so I encourage you to contribute
> with ideas and code patches.
>
>
Probably it's not generic, involves vlan translation and mac swapping to
inject the "passed" packets back to the router to travel to next hop.
Router injects traffic for filtering to directly connected firewall port
with vlan tag say A , firewall in turn injects pass traffic with different
vlan tag say B back to the router which then does next hop processing.


> >
> > Is this a bug or am I missing something ?
> >
>
> You can make sure no ARP packets are received by the firewall pipeline by
> simply filtering all the ARP packets to a separate RXQ of the NIC port,
> which can be further handled by a separate function.
>
> See ip_pipeline/config/network_layers.cfg as example:
> [LINK0]
> arp_q = 4
>

There are two things here

1) ARP packets should not hit the ipv4 acl which looks like happening, have
not worked on "why part" for now , need to look at the
rte_table_acl_lookup->rte_acl_classify.

2) Yes we can filter ARP packets to a separate queue and to separate lcore
at link level, but need to decide whether its worth it. We are planning to
use ipv4 RSS (with 8 queues, lcores and processors) wherein all arp packets
would get filtered to queue 0 by default and then dropped by ACL.


Thanks and regards
Shyam

Re: [dpdk-dev] [PATCH v2 1/6] ethdev: add descriptor status API

[Andrew Rybchenko]

On 03/07/2017 06:59 PM, Olivier Matz wrote:

Introduce a new API to get the status of a descriptor.

For Rx, it is almost similar to rx_descriptor_done API, except it
differentiates "used" descriptors (which are hold by the driver and not
returned to the hardware).

For Tx, it is a new API.

The descriptor_done() API, and probably the rx_queue_count() API could
be replaced by this new API as soon as it is implemented on all PMDs.

Signed-off-by: Olivier Matz 
---
  doc/guides/nics/features/default.ini |   2 +
  lib/librte_ether/rte_ethdev.h| 125 +++
  2 files changed, 127 insertions(+)

diff --git a/doc/guides/nics/features/default.ini 
b/doc/guides/nics/features/default.ini
index 9e363ff..0e6a78d 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -49,6 +49,8 @@ Inner L3 checksum=
  Inner L4 checksum=
  Packet type parsing  =
  Timesync =
+Rx Descriptor Status =
+Tx Descriptor Status =
  Basic stats  =
  Extended stats   =
  Stats per queue  =
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 97f3e2d..904ecbe 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1179,6 +1179,12 @@ typedef uint32_t (*eth_rx_queue_count_t)(struct 
rte_eth_dev *dev,
  typedef int (*eth_rx_descriptor_done_t)(void *rxq, uint16_t offset);
  /**< @internal Check DD bit of specific RX descriptor */
  
+typedef int (*eth_rx_descriptor_status_t)(void *rxq, uint16_t offset);

+/**< @internal Check the status of a Rx descriptor */
+
+typedef int (*eth_tx_descriptor_status_t)(void *txq, uint16_t offset);
+/**< @internal Check the status of a Tx descriptor */
+
  typedef int (*eth_fw_version_get_t)(struct rte_eth_dev *dev,
 char *fw_version, size_t fw_size);
  /**< @internal Get firmware information of an Ethernet device. */
@@ -1483,6 +1489,10 @@ struct eth_dev_ops {
eth_queue_release_trx_queue_release; /**< Release RX queue. */
eth_rx_queue_count_t   rx_queue_count;/**< Get Rx queue count. */
eth_rx_descriptor_done_t   rx_descriptor_done; /**< Check rxd DD bit. */
+   eth_rx_descriptor_status_t rx_descriptor_status;
+   /**< Check the status of a Rx descriptor. */
+   eth_tx_descriptor_status_t tx_descriptor_status;
+   /**< Check the status of a Tx descriptor. */
eth_rx_enable_intr_t   rx_queue_intr_enable;  /**< Enable Rx queue 
interrupt. */
eth_rx_disable_intr_t  rx_queue_intr_disable; /**< Disable Rx queue 
interrupt. */
eth_tx_queue_setup_t   tx_queue_setup;/**< Set up device TX queue. 
*/
@@ -2768,6 +2778,121 @@ rte_eth_rx_descriptor_done(uint8_t port_id, uint16_t 
queue_id, uint16_t offset)
dev->data->rx_queues[queue_id], offset);
  }
  
+#define RTE_ETH_RX_DESC_AVAIL0 /**< Desc available for hw. */

+#define RTE_ETH_RX_DESC_DONE 1 /**< Desc done, filled by hw. */
+#define RTE_ETH_RX_DESC_UNAVAIL  2 /**< Desc used by driver or hw. */
+
+/**
+ * Check the status of a Rx descriptor in the queue
+ *
+ * It should be called in a similar context than the Rx function:
+ * - on a dataplane core
+ * - not concurrently on the same queue
+ *
+ * Since it's a dataplane function, no check is performed on port_id and
+ * queue_id. The caller must therefore ensure that the port is enabled
+ * and the queue is configured and running.
+ *
+ * Note: accessing to a random descriptor in the ring may trigger cache
+ * misses and have a performance impact.
+ *
+ * @param port_id
+ *  A valid port identifier of the Ethernet device which.
+ * @param queue_id
+ *  A valid Rx queue identifier on this port.
+ * @param offset
+ *  The offset of the descriptor starting from tail (0 is the next
+ *  packet to be received by the driver).
+ *
+ * @return
+ *  - (RTE_ETH_RX_DESC_AVAIL): Descriptor is available for the hardware to
+ *receive a packet.
+ *  - (RTE_ETH_RX_DESC_DONE): Descriptor is done, it is filled by hw, but
+ *not yet processed by the driver (i.e. in the receive queue).
+ *  - (RTE_ETH_RX_DESC_UNAVAIL): Descriptor is unavailable, either hold by
+ *the driver and not yet returned to hw, or reserved by the hw.
+ *  - (-EINVAL) bad descriptor offset.
+ *  - (-ENOTSUP) if the device does not support this function.
+ *  - (-ENODEV) bad port or queue (only if compiled with debug).
+ */
+static inline int
+rte_eth_rx_descriptor_status(uint8_t port_id, uint16_t queue_id,
+   uint16_t offset)
+{
+   struct rte_eth_dev *dev;
+   void *rxq;
+
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+   RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+#endif
+   dev = &rte_eth_devices[port_id];
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+   if (queue_id >= dev->data->nb_rx_queues)
+   return -ENODEV;
+#endif
+   RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_descriptor_status, -ENOTSUP);
+   rxq = dev->data->rx_

Re: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter for MPLS

[Iremonger, Bernard]

Hi Beilei,
 

> > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Beilei Xing
> > > Sent: Friday, March 3, 2017 9:44 AM
> > > To: Wu, Jingjing 
> > > Cc: Zhang, Helin ; dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter for
> > > MPLS
> > >
> > > This patch enables MPLSoUDP and MPLSoGRE cloud filter with replace
> > > cloud filter.
> > >
> > > Signed-off-by: Beilei Xing 
> > > ---
> > >  drivers/net/i40e/i40e_ethdev.c |  44 +++--
> > >  drivers/net/i40e/i40e_ethdev.h |   9 +++-
> > >  drivers/net/i40e/i40e_flow.c   | 108
> > > +
> > >  3 files changed, 144 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index 91bfd73..6044daf 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -1286,6 +1286,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
> > >   if (ret < 0)
> > >   goto err_init_fdir_filter_list;
> > >
> > > + i40e_replace_mpls_l1_filter(pf);
> > > + i40e_replace_mpls_cloud_filter(pf);
> > > +
> > >   return 0;
> > >
> > >  err_init_fdir_filter_list:
> > > @@ -6941,6 +6944,7 @@ i40e_dev_consistent_tunnel_filter_set(struct
> > > i40e_pf *pf,
> > >   struct i40e_tunnel_rule *tunnel_rule = &pf->tunnel;
> > >   struct i40e_tunnel_filter *tunnel, *node;
> > >   struct i40e_tunnel_filter check_filter; /* Check if filter exists
> > > */
> > > + uint32_t teid_le;
> > >   bool big_buffer = 0;
> > >
> > >   cld_filter = rte_zmalloc("tunnel_filter", @@ -6989,6 +6993,28 @@
> > > i40e_dev_consistent_tunnel_filter_set(struct i40e_pf *pf,
> > >   case I40E_TUNNEL_TYPE_IP_IN_GRE:
> > >   tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_IP;
> > >   break;
> > > + case I40E_TUNNEL_TYPE_MPLSoUDP:
> > > + teid_le = rte_cpu_to_le_32(tunnel_filter->tenant_id);
> > > + pfilter-
> > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD0] =
> > > + teid_le >> 4;
> > > + pfilter-
> > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> > > + (teid_le & 0xF) << 12;
> > > + pfilter-
> > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD2] =
> > > + 0x40;
> > > + big_buffer = 1;
> > > + tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoUDP;
> > > + break;
> > > + case I40E_TUNNEL_TYPE_MPLSoGRE:
> > > + teid_le = rte_cpu_to_le_32(tunnel_filter->tenant_id);
> > > + pfilter-
> > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD0] =
> > > + teid_le >> 4;
> > > + pfilter-
> > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> > > + (teid_le & 0xF) << 12;
> > > + pfilter-
> > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD2] =
> > > + 0x0;
> > > + big_buffer = 1;
> > > + tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoGRE;
> > > + break;
> > >   default:
> > >   /* Other tunnel types is not supported. */
> > >   PMD_DRV_LOG(ERR, "tunnel type is not supported."); @@ -
> > > 6996,11 +7022,19 @@ i40e_dev_consistent_tunnel_filter_set(struct
> > > i40e_pf *pf,
> > >   return -EINVAL;
> > >   }
> > >
> > > - val = i40e_dev_get_filter_type(tunnel_filter->filter_type,
> > > -&pfilter->element.flags);
> > > - if (val < 0) {
> > > - rte_free(cld_filter);
> > > - return -EINVAL;
> > > + if (tunnel_filter->tunnel_type == I40E_TUNNEL_TYPE_MPLSoUDP)
> > > + pfilter->element.flags =
> > > + I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoUDP;
> > > + else if (tunnel_filter->tunnel_type ==
> > > I40E_TUNNEL_TYPE_MPLSoGRE)
> > > + pfilter->element.flags =
> > > + I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoGRE;
> > > + else {
> > > + val = i40e_dev_get_filter_type(tunnel_filter->filter_type,
> > > + &pfilter->element.flags);
> > > + if (val < 0) {
> > > + rte_free(cld_filter);
> > > + return -EINVAL;
> > > + }
> > >   }
> > >
> > >   pfilter->element.flags |= rte_cpu_to_le_16( diff --git
> > > a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> > > index dd9d709..f305baa 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.h
> > > +++ b/drivers/net/i40e/i40e_ethdev.h
> > > @@ -499,8 +499,11 @@ struct i40e_ethertype_rule {
> > >  /* Tunnel filter number HW supports */  #define
> > > I40E_MAX_TUNNEL_FILTER_NUM 400
> > >
> > > -#define I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoUDP 0x11 -#define
> > > I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoGRE 0x12
> > > +#define I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoUDP 8 #define
> > > +I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoGRE 9 #define
> > > +I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoUDP 0x11 #define
> > > +I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoGRE 0x12 #define
> > > +I40E_AQC_ADD_L1_FILTER_TEID_MPLS 0x1

Re: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter for MPLS

[Xing, Beilei]

> -Original Message-
> From: Iremonger, Bernard
> Sent: Thursday, March 9, 2017 7:54 PM
> To: Xing, Beilei ; Wu, Jingjing 
> Cc: Zhang, Helin ; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter for MPLS
> 
> Hi Beilei,
>  
> 
> > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Beilei Xing
> > > > Sent: Friday, March 3, 2017 9:44 AM
> > > > To: Wu, Jingjing 
> > > > Cc: Zhang, Helin ; dev@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter for
> > > > MPLS
> > > >
> > > > This patch enables MPLSoUDP and MPLSoGRE cloud filter with replace
> > > > cloud filter.
> > > >
> > > > Signed-off-by: Beilei Xing 
> > > > ---
> > > >  drivers/net/i40e/i40e_ethdev.c |  44 +++--
> > > >  drivers/net/i40e/i40e_ethdev.h |   9 +++-
> > > >  drivers/net/i40e/i40e_flow.c   | 108
> > > > +
> > > >  3 files changed, 144 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > > b/drivers/net/i40e/i40e_ethdev.c index 91bfd73..6044daf 100644
> > > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > > @@ -1286,6 +1286,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
> > > > if (ret < 0)
> > > > goto err_init_fdir_filter_list;
> > > >
> > > > +   i40e_replace_mpls_l1_filter(pf);
> > > > +   i40e_replace_mpls_cloud_filter(pf);
> > > > +
> > > > return 0;
> > > >
> > > >  err_init_fdir_filter_list:
> > > > @@ -6941,6 +6944,7 @@ i40e_dev_consistent_tunnel_filter_set(struct
> > > > i40e_pf *pf,
> > > > struct i40e_tunnel_rule *tunnel_rule = &pf->tunnel;
> > > > struct i40e_tunnel_filter *tunnel, *node;
> > > > struct i40e_tunnel_filter check_filter; /* Check if filter
> > > > exists */
> > > > +   uint32_t teid_le;
> > > > bool big_buffer = 0;
> > > >
> > > > cld_filter = rte_zmalloc("tunnel_filter", @@ -6989,6 +6993,28 @@
> > > > i40e_dev_consistent_tunnel_filter_set(struct i40e_pf *pf,
> > > > case I40E_TUNNEL_TYPE_IP_IN_GRE:
> > > > tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_IP;
> > > > break;
> > > > +   case I40E_TUNNEL_TYPE_MPLSoUDP:
> > > > +   teid_le = rte_cpu_to_le_32(tunnel_filter->tenant_id);
> > > > +   pfilter-
> > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD0] =
> > > > +   teid_le >> 4;
> > > > +   pfilter-
> > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> > > > +   (teid_le & 0xF) << 12;
> > > > +   pfilter-
> > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD2] =
> > > > +   0x40;
> > > > +   big_buffer = 1;
> > > > +   tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoUDP;
> > > > +   break;
> > > > +   case I40E_TUNNEL_TYPE_MPLSoGRE:
> > > > +   teid_le = rte_cpu_to_le_32(tunnel_filter->tenant_id);
> > > > +   pfilter-
> > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD0] =
> > > > +   teid_le >> 4;
> > > > +   pfilter-
> > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> > > > +   (teid_le & 0xF) << 12;
> > > > +   pfilter-
> > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD2] =
> > > > +   0x0;
> > > > +   big_buffer = 1;
> > > > +   tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoGRE;
> > > > +   break;
> > > > default:
> > > > /* Other tunnel types is not supported. */
> > > > PMD_DRV_LOG(ERR, "tunnel type is not supported."); @@ -
> > > > 6996,11 +7022,19 @@ i40e_dev_consistent_tunnel_filter_set(struct
> > > > i40e_pf *pf,
> > > > return -EINVAL;
> > > > }
> > > >
> > > > -   val = i40e_dev_get_filter_type(tunnel_filter->filter_type,
> > > > -  &pfilter->element.flags);
> > > > -   if (val < 0) {
> > > > -   rte_free(cld_filter);
> > > > -   return -EINVAL;
> > > > +   if (tunnel_filter->tunnel_type == I40E_TUNNEL_TYPE_MPLSoUDP)
> > > > +   pfilter->element.flags =
> > > > +   I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoUDP;
> > > > +   else if (tunnel_filter->tunnel_type ==
> > > > I40E_TUNNEL_TYPE_MPLSoGRE)
> > > > +   pfilter->element.flags =
> > > > +   I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoGRE;
> > > > +   else {
> > > > +   val = 
> > > > i40e_dev_get_filter_type(tunnel_filter->filter_type,
> > > > +   
> > > > &pfilter->element.flags);
> > > > +   if (val < 0) {
> > > > +   rte_free(cld_filter);
> > > > +   return -EINVAL;
> > > > +  

Re: [dpdk-dev] [PATCH] mem: balanced allocation of hugepages

[Ilya Maximets]

On 08.03.2017 16:46, Sergio Gonzalez Monroy wrote:
> Hi Ilya,
> 
> I have done similar tests and as you already pointed out, 'numactl 
> --interleave' does not seem to work as expected.
> I have also checked that the issue can be reproduced with quota limit on 
> hugetlbfs mount point.
> 
> I would be inclined towards *adding libnuma as dependency* to DPDK to make 
> memory allocation a bit more reliable.
> 
> Currently at a high level regarding hugepages per numa node:
> 1) Try to map all free hugepages. The total number of mapped hugepages 
> depends if there were any limits, such as cgroups or quota in mount point.
> 2) Find out numa node of each hugepage.
> 3) Check if we have enough hugepages for requested memory in each numa 
> socket/node.
> 
> Using libnuma we could try to allocate hugepages per numa:
> 1) Try to map as many hugepages from numa 0.
> 2) Check if we have enough hugepages for requested memory in numa 0.
> 3) Try to map as many hugepages from numa 1.
> 4) Check if we have enough hugepages for requested memory in numa 1.
> 
> This approach would improve failing scenarios caused by limits but It would 
> still not fix issues regarding non-contiguous hugepages (worst case each 
> hugepage is a memseg).
> The non-contiguous hugepages issues are not as critical now that mempools can 
> span over multiple memsegs/hugepages, but it is still a problem for any other 
> library requiring big chunks of memory.
> 
> Potentially if we were to add an option such as 'iommu-only' when all devices 
> are bound to vfio-pci, we could have a reliable way to allocate hugepages by 
> just requesting the number of pages from each numa.
> 
> Thoughts?

Hi Sergio,

Thanks for your attention to this.

For now, as we have some issues with non-contiguous
hugepages, I'm thinking about following hybrid schema:
1) Allocate essential hugepages:
1.1) Allocate as many hugepages from numa N to
 only fit requested memory for this numa.
1.2) repeat 1.1 for all numa nodes.
2) Try to map all remaining free hugepages in a round-robin
   fashion like in this patch.
3) Sort pages and choose the most suitable.

This solution should decrease number of issues connected with
non-contiguous memory.

Best regards, Ilya Maximets.

> 
> On 06/03/2017 09:34, Ilya Maximets wrote:
>> Hi all.
>>
>> So, what about this change?
>>
>> Best regards, Ilya Maximets.
>>
>> On 16.02.2017 16:01, Ilya Maximets wrote:
>>> Currently EAL allocates hugepages one by one not paying
>>> attention from which NUMA node allocation was done.
>>>
>>> Such behaviour leads to allocation failure if number of
>>> available hugepages for application limited by cgroups
>>> or hugetlbfs and memory requested not only from the first
>>> socket.
>>>
>>> Example:
>>> # 90 x 1GB hugepages availavle in a system
>>>
>>> cgcreate -g hugetlb:/test
>>> # Limit to 32GB of hugepages
>>> cgset -r hugetlb.1GB.limit_in_bytes=34359738368 test
>>> # Request 4GB from each of 2 sockets
>>> cgexec -g hugetlb:test testpmd --socket-mem=4096,4096 ...
>>>
>>> EAL: SIGBUS: Cannot mmap more hugepages of size 1024 MB
>>> EAL: 32 not 90 hugepages of size 1024 MB allocated
>>> EAL: Not enough memory available on socket 1!
>>>  Requested: 4096MB, available: 0MB
>>> PANIC in rte_eal_init():
>>> Cannot init memory
>>>
>>> This happens beacause all allocated pages are
>>> on socket 0.
>>>
>>> Fix this issue by setting mempolicy MPOL_PREFERRED for each
>>> hugepage to one of requested nodes in a round-robin fashion.
>>> In this case all allocated pages will be fairly distributed
>>> between all requested nodes.
>>>
>>> New config option RTE_LIBRTE_EAL_NUMA_AWARE_HUGEPAGES
>>> introduced and disabled by default because of external
>>> dependency from libnuma.
>>>
>>> Cc: 
>>> Fixes: 77988fc08dc5 ("mem: fix allocating all free hugepages")
>>>
>>> Signed-off-by: Ilya Maximets 
>>> ---
>>>   config/common_base   |  1 +
>>>   lib/librte_eal/Makefile  |  4 ++
>>>   lib/librte_eal/linuxapp/eal/eal_memory.c | 66 
>>> 
>>>   mk/rte.app.mk|  3 ++
>>>   4 files changed, 74 insertions(+)
>>>
> 
> 
> 
> 

[dpdk-dev] [PATCH v2 0/6] librte_cfgfile enhancements

[Allain Legacy]

This patchset includes some minor enhancements that we have developped for
our DPDK application.  We would like to contribute them upstream to help
ease adoption of the DPDK by anyone looking for this type of
functionality.  The commit logs on each patch should be self-sufficient in
explaining the intent and purpose.

v2:
* Added unit tests for the cfgfile library in the initial patch of the
  series and then added additional tests in subsequent patches where
  appropriate.  These will not run unless the following config parameter is
  set and additional packages are installed (e.g., libarchive-dev):
CONFIG_RTE_APP_TEST_RESOURCE_TAR=y
* Reworked the configurable comment character patch to allow specifying a
  different character at runtime rather than build time.  Used a separate
  API to avoid affecting existing users or users that choose not to
  leverage the extended API.  Used a "parameters" structure to pass
  additional arguments rather than adding more arguments to the function to
  allow expansion in the future with minimal impact on existing users.
* Dropped the patch to initialize the cfg structure because the segfault
  that this was trying to address was already fixed by 2 earlier commits
  which we did not have in our development environment.  I realized this
  while trying to add unit tests to catch the segfault case.
* Fixed the doxygen comments related to the RTE_CFG_GLOBAL_SECTION patch
* Added an additional patch to allow parsing a key with an empty value
  (i.e., "key=").  I realized that I had forgotten to include this in my
  first patchset.

Allain Legacy (5):
  test: basic unit tests for cfgfile
  cfgfile: add support for unamed global section
  cfgfile: configurable comment character
  cfgfile: use strnlen to constrain memchr search
  cfgfile: add support for empty value string

Joseph Richard (1):
  cfgfile: increase local buffer size for max name and value

 config/common_base   |  1 +
 lib/librte_cfgfile/rte_cfgfile.c | 28 +++-
 lib/librte_cfgfile/rte_cfgfile.h |  6 ++
 3 files changed, 30 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v2 1/6] test: basic unit tests for cfgfile

[Allain Legacy]

This commit adds the basic infrastructure for the cfgfile library unit
tests.  It includes success path tests for the most commonly used APIs.
More unit tests will be added later.

Signed-off-by: Allain Legacy 
---
 MAINTAINERS   |   1 +
 test/test/Makefile|   2 +
 test/test/test_cfgfile.c  | 210 ++
 test/test/test_cfgfiles/etc/empty.ini |   0
 test/test/test_cfgfiles/etc/invalid_key_value.ini |   3 +
 test/test/test_cfgfiles/etc/invalid_section.ini   |   3 +
 test/test/test_cfgfiles/etc/line_too_long.ini |   3 +
 test/test/test_cfgfiles/etc/missing_section.ini   |   2 +
 test/test/test_cfgfiles/etc/sample1.ini   |  12 ++
 9 files changed, 236 insertions(+)
 create mode 100644 test/test/test_cfgfile.c
 create mode 100644 test/test/test_cfgfiles/etc/empty.ini
 create mode 100644 test/test/test_cfgfiles/etc/invalid_key_value.ini
 create mode 100644 test/test/test_cfgfiles/etc/invalid_section.ini
 create mode 100644 test/test/test_cfgfiles/etc/line_too_long.ini
 create mode 100644 test/test/test_cfgfiles/etc/missing_section.ini
 create mode 100644 test/test/test_cfgfiles/etc/sample1.ini

diff --git a/MAINTAINERS b/MAINTAINERS
index 5030c1c..16d5dd8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -598,6 +598,7 @@ Other libraries
 Configuration file
 M: Cristian Dumitrescu 
 F: lib/librte_cfgfile/
+F: test/test/test_cfgfile*
 
 Interactive command line
 M: Olivier Matz 
diff --git a/test/test/Makefile b/test/test/Makefile
index 1a5e03d..622e795 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -85,6 +85,8 @@ $(eval $(call 
linked_resource,test_resource_c,test_resource.res))
 $(eval $(call linked_tar_resource,test_resource_tar,test_resource.c))
 SRCS-$(CONFIG_RTE_APP_TEST_RESOURCE_TAR) += test_pci.c
 $(eval $(call linked_tar_resource,test_pci_sysfs,test_pci_sysfs))
+SRCS-$(CONFIG_RTE_APP_TEST_RESOURCE_TAR) += test_cfgfile.c
+$(eval $(call linked_tar_resource,test_cfgfiles,test_cfgfiles))
 SRCS-y += test_prefetch.c
 SRCS-y += test_byteorder.c
 SRCS-y += test_per_lcore.c
diff --git a/test/test/test_cfgfile.c b/test/test/test_cfgfile.c
new file mode 100644
index 000..fae2e0a
--- /dev/null
+++ b/test/test/test_cfgfile.c
@@ -0,0 +1,210 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2017 Wind River Systems Inc. All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in
+ *   the documentation and/or other materials provided with the
+ *   distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ *   contributors may be used to endorse or promote products derived
+ *   from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "test.h"
+#include "resource.h"
+
+
+#define CFG_FILES_ETC "test_cfgfiles/etc"
+
+REGISTER_LINKED_RESOURCE(test_cfgfiles);
+
+static int
+test_cfgfile_setup(void)
+{
+   const struct resource *r;
+   int ret;
+
+   r = resource_find("test_cfgfiles");
+   TEST_ASSERT_NOT_NULL(r, "missing resource test_cfgfiles");
+
+   ret = resource_untar(r);
+   TEST_ASSERT_SUCCESS(ret, "failed to untar %s", r->name);
+
+   return 0;
+}
+
+static int
+test_cfgfile_cleanup(void)
+{
+   const struct resource *r;
+   int ret;
+
+   r = resource_find("test_cfgfiles");
+   TEST_ASSERT_NOT_NULL(r, "missing resource test_cfgfiles");
+
+   ret = resource_rm_by_tar(r);
+   TEST_ASSERT_SUCCESS(ret, "Failed to delete resource %s", r->name);
+
+   return 0;
+}
+
+static int
+_test_cfgfile_sample(struct rte_cfgfile *cfgfile)
+{
+   const char *value;
+   int ret;
+
+  

[dpdk-dev] [PATCH v2 3/6] cfgfile: configurable comment character

[Allain Legacy]

The current cfgfile comment character is hardcoded to ';'.  This commit a
new API to allow the user to specify which comment character to use while
parsing the file.

This is to ease adoption by applications that have an existing
configuration file which may use a different comment character.  For
instance, an application may already have a configuration file that uses
the '#' as the comment character.

The approach of using a new API with an extensible parameters structure was
used rather than simply adding a new argument to the existing API to allow
for additional arguments to be introduced in the future.

Signed-off-by: Allain Legacy 
---
 lib/librte_cfgfile/rte_cfgfile.c| 26 ++---
 lib/librte_cfgfile/rte_cfgfile.h| 34 +
 test/test/test_cfgfile.c| 29 
 test/test/test_cfgfiles/etc/sample2.ini | 12 
 4 files changed, 98 insertions(+), 3 deletions(-)
 create mode 100644 test/test/test_cfgfiles/etc/sample2.ini

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 832fea8..7ecab22 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -85,9 +85,29 @@ struct rte_cfgfile {
return newlen;
 }
 
+void
+rte_cfgfile_init_parameters(struct rte_cfgfile_parameters *params)
+{
+   memset(params, 0, sizeof(*params));
+   params->comment_character = CFG_DEFAULT_COMMENT_CHARACTER;
+}
+
 struct rte_cfgfile *
 rte_cfgfile_load(const char *filename, int flags)
 {
+   struct rte_cfgfile_parameters params;
+
+   /* setup default parameter are add specified flags */
+   rte_cfgfile_init_parameters(¶ms);
+   params.flags |= flags;
+
+   return rte_cfgfile_load_with_params(filename, ¶ms);
+}
+
+struct rte_cfgfile *
+rte_cfgfile_load_with_params(const char *filename,
+const struct rte_cfgfile_parameters *params)
+{
int allocated_sections = CFG_ALLOC_SECTION_BATCH;
int allocated_entries = 0;
int curr_section = -1;
@@ -107,7 +127,7 @@ struct rte_cfgfile *
 
memset(cfg->sections, 0, sizeof(cfg->sections[0]) * allocated_sections);
 
-   if (flags & CFG_FLAG_GLOBAL_SECTION) {
+   if (params->flags & CFG_FLAG_GLOBAL_SECTION) {
curr_section = 0;
allocated_entries = CFG_ALLOC_ENTRY_BATCH;
cfg->sections[curr_section] = malloc(
@@ -132,7 +152,7 @@ struct rte_cfgfile *
"Check if line too long\n", lineno);
goto error1;
}
-   pos = memchr(buffer, ';', sizeof(buffer));
+   pos = memchr(buffer, params->comment_character, sizeof(buffer));
if (pos != NULL) {
*pos = '\0';
len = pos -  buffer;
@@ -242,7 +262,7 @@ struct rte_cfgfile *
}
}
fclose(f);
-   cfg->flags = flags;
+   cfg->flags = params->flags;
cfg->num_sections = curr_section + 1;
/* curr_section will still be -1 if we have an empty file */
if (curr_section >= 0)
diff --git a/lib/librte_cfgfile/rte_cfgfile.h b/lib/librte_cfgfile/rte_cfgfile.h
index 0e805c2..069bbd4 100644
--- a/lib/librte_cfgfile/rte_cfgfile.h
+++ b/lib/librte_cfgfile/rte_cfgfile.h
@@ -66,6 +66,12 @@ struct rte_cfgfile_entry {
char value[CFG_VALUE_LEN]; /**< Value */
 };
 
+/** Configuration file operation optional arguments */
+struct rte_cfgfile_parameters {
+   int flags; /**< Config file flags */
+   char comment_character; /**< Config file comment character */
+};
+
 /**@{ cfgfile load operation flags */
 /**
  * Indicates that the file supports key value entries before the first defined
@@ -74,6 +80,17 @@ struct rte_cfgfile_entry {
 #define CFG_FLAG_GLOBAL_SECTION (1 << 0)
 /**@} */
 
+/** Defines the default comment character used for parsing config files. */
+#define CFG_DEFAULT_COMMENT_CHARACTER ';'
+
+/**
+ * Initialize config file optional parameters to default values.
+ *
+ * @param params
+ *   parameters to be initialized
+ */
+void rte_cfgfile_init_parameters(struct rte_cfgfile_parameters *params);
+
 /**
 * Open config file
 *
@@ -87,6 +104,23 @@ struct rte_cfgfile_entry {
 struct rte_cfgfile *rte_cfgfile_load(const char *filename, int flags);
 
 /**
+ * Open config file with specified optional parameters.  Use @see
+ * rte_cfgfile_init_parameters to setup the default parameters.
+ *
+ * @param filename
+ *   Config file name
+ * @param params
+ *   Config file flags
+ * @return
+ *   Handle to configuration file on success, NULL otherwise
+ * @param
+ *
+ */
+struct rte_cfgfile *rte_cfgfile_load_with_params(const char *filename,
+   const struct rte_cfgfile_parameters *params);
+
+
+/**
 * Get number of sections in config file
 *
 * @param cfg
diff --git a/test/test/test_cfgfile.c b/test/test/test_cfgfile.c
index dd7afae..

[dpdk-dev] [PATCH v2 2/6] cfgfile: add support for unamed global section

[Allain Legacy]

The current implementation of the cfgfile library requires that all
key=value pairs be within [SECTION] definitions.  The ini file standard
allows for key=value pairs in an unnamed section.

   https://en.wikipedia.org/wiki/INI_file#Global_properties

This commit adds the capability of parsing key=value pairs from such an
unnamed section. The CFG_FLAG_GLOBAL_SECTION flag must be passed to the
rte_cfgfile_load() API to enable this functionality.  Any key=value pairs
found before the first section can be accessed in the section named
"GLOBAL".

Signed-off-by: Allain Legacy 
---
 lib/librte_cfgfile/rte_cfgfile.c | 16 
 lib/librte_cfgfile/rte_cfgfile.h | 10 +-
 test/test/test_cfgfile.c | 33 +
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 829109a..832fea8 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -107,6 +107,22 @@ struct rte_cfgfile *
 
memset(cfg->sections, 0, sizeof(cfg->sections[0]) * allocated_sections);
 
+   if (flags & CFG_FLAG_GLOBAL_SECTION) {
+   curr_section = 0;
+   allocated_entries = CFG_ALLOC_ENTRY_BATCH;
+   cfg->sections[curr_section] = malloc(
+   sizeof(*cfg->sections[0]) +
+   sizeof(cfg->sections[0]->entries[0]) *
+   allocated_entries);
+   if (cfg->sections[curr_section] == NULL) {
+   printf("Error - no memory for global section\n");
+   goto error1;
+   }
+
+   snprintf(cfg->sections[curr_section]->name,
+sizeof(cfg->sections[0]->name), "GLOBAL");
+   }
+
while (fgets(buffer, sizeof(buffer), f) != NULL) {
char *pos = NULL;
size_t len = strnlen(buffer, sizeof(buffer));
diff --git a/lib/librte_cfgfile/rte_cfgfile.h b/lib/librte_cfgfile/rte_cfgfile.h
index b40e6a1..0e805c2 100644
--- a/lib/librte_cfgfile/rte_cfgfile.h
+++ b/lib/librte_cfgfile/rte_cfgfile.h
@@ -66,13 +66,21 @@ struct rte_cfgfile_entry {
char value[CFG_VALUE_LEN]; /**< Value */
 };
 
+/**@{ cfgfile load operation flags */
+/**
+ * Indicates that the file supports key value entries before the first defined
+ * section.  These entries can be accessed in the "GLOBAL" section.
+ */
+#define CFG_FLAG_GLOBAL_SECTION (1 << 0)
+/**@} */
+
 /**
 * Open config file
 *
 * @param filename
 *   Config file name
 * @param flags
-*   Config file flags, Reserved for future use. Must be set to 0.
+*   Config file flags
 * @return
 *   Handle to configuration file on success, NULL otherwise
 */
diff --git a/test/test/test_cfgfile.c b/test/test/test_cfgfile.c
index fae2e0a..dd7afae 100644
--- a/test/test/test_cfgfile.c
+++ b/test/test/test_cfgfile.c
@@ -163,6 +163,36 @@
 }
 
 static int
+test_cfgfile_global_properties(void)
+{
+   struct rte_cfgfile *cfgfile;
+   const char *value;
+   int ret;
+
+   cfgfile = rte_cfgfile_load(CFG_FILES_ETC "/missing_section.ini",
+  CFG_FLAG_GLOBAL_SECTION);
+   TEST_ASSERT_NOT_NULL(cfgfile, "Expected failured did not occur");
+
+   ret = rte_cfgfile_num_sections(cfgfile, NULL, 0);
+   TEST_ASSERT(ret == 1, "Unexpected number of sections: %d", ret);
+
+   ret = rte_cfgfile_has_section(cfgfile, "GLOBAL");
+   TEST_ASSERT(ret, "global section missing");
+
+   ret = rte_cfgfile_section_num_entries(cfgfile, "GLOBAL");
+   TEST_ASSERT(ret == 1, "GLOBAL unexpected number of entries: %d", ret);
+
+   value = rte_cfgfile_get_entry(cfgfile, "GLOBAL", "key");
+   TEST_ASSERT(strcmp("value", value) == 0,
+   "key unexpected value: %s", value);
+
+   ret = rte_cfgfile_close(cfgfile);
+   TEST_ASSERT_SUCCESS(ret, "Failed to close cfgfile");
+
+   return 0;
+}
+
+static int
 test_cfgfile_empty_file(void)
 {
struct rte_cfgfile *cfgfile;
@@ -198,6 +228,9 @@
if (test_cfgfile_missing_section())
return -1;
 
+   if (test_cfgfile_global_properties())
+   return -1;
+
if (test_cfgfile_empty_file())
return -1;
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] doc: add details on requirements for patch ack and merge

[Thomas Monjalon]

2017-02-21 16:16, Mcnamara, John:
> From: Bruce Richardson
> > 
> > Add to the contributors guide the requirements and guidelines for getting
> > patches acked and merged. It details at what point the review comments and
> > the ack's need to be received in order to have a given patch merged into a
> > release.
> > 
> > These guidelines are as agreed by the DPDK technical board at the meeting
> > held on 2017-02-15.
> > 
> > Signed-off-by: Bruce Richardson 
> 
> Minor nit. I'd prefer the first word of the bullet items to be capitalized
> as a sentence. Apart from that:
> 
> Acked-by: John McNamara 

Applied with capitalization fixed and s/relevent/relevant/.
Thanks

[dpdk-dev] [PATCH v2 6/6] cfgfile: add support for empty value string

[Allain Legacy]

This commit adds support to the cfgfile library for parsing a key=value
line that has no value string specified (e.g., "key=").  This can be used
to override a configuration attribute that has a default value or default
list of values to set it back to an undefined value to disable
functionality.

Signed-off-by: Allain Legacy 
---
 lib/librte_cfgfile/rte_cfgfile.c| 11 ++-
 test/test/test_cfgfile.c| 17 +
 test/test/test_cfgfiles/etc/sample1.ini |  2 +-
 test/test/test_cfgfiles/etc/sample2.ini |  2 +-
 4 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 35c81e4..d3022b9 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -219,11 +219,12 @@ struct rte_cfgfile *
 
struct rte_cfgfile_section *sect =
cfg->sections[curr_section];
-   char *split[2];
-   if (rte_strsplit(buffer, sizeof(buffer), split, 2, '=')
-   != 2) {
+   int n;
+   char *split[2] = {NULL};
+   n = rte_strsplit(buffer, sizeof(buffer), split, 2, '=');
+   if ((n < 1) || (n > 2)) {
printf("Error at line %d - cannot split "
-   "string\n", lineno);
+  "string, n=%d\n", lineno, n);
goto error1;
}
 
@@ -254,7 +255,7 @@ struct rte_cfgfile *
snprintf(entry->name, sizeof(entry->name), "%s",
split[0]);
snprintf(entry->value, sizeof(entry->value), "%s",
-   split[1]);
+split[1] ? split[1] : "");
_strip(entry->name, strnlen(entry->name,
sizeof(entry->name)));
_strip(entry->value, strnlen(entry->value,
diff --git a/test/test/test_cfgfile.c b/test/test/test_cfgfile.c
index eab8ccc..d8214d1 100644
--- a/test/test/test_cfgfile.c
+++ b/test/test/test_cfgfile.c
@@ -105,8 +105,7 @@
"key2 unexpected value: %s", value);
 
value = rte_cfgfile_get_entry(cfgfile, "section2", "key3");
-   TEST_ASSERT(strcmp("value3", value) == 0,
-   "key3 unexpected value: %s", value);
+   TEST_ASSERT(strlen(value) == 0, "key3 unexpected value: %s", value);
 
return 0;
 }
@@ -167,17 +166,6 @@
 }
 
 static int
-test_cfgfile_invalid_key_value_pair(void)
-{
-   struct rte_cfgfile *cfgfile;
-
-   cfgfile = rte_cfgfile_load(CFG_FILES_ETC "/invalid_key_value.ini", 0);
-   TEST_ASSERT_NULL(cfgfile, "Expected failured did not occur");
-
-   return 0;
-}
-
-static int
 test_cfgfile_missing_section(void)
 {
struct rte_cfgfile *cfgfile;
@@ -251,9 +239,6 @@
if (test_cfgfile_invalid_section_header())
return -1;
 
-   if (test_cfgfile_invalid_key_value_pair())
-   return -1;
-
if (test_cfgfile_missing_section())
return -1;
 
diff --git a/test/test/test_cfgfiles/etc/sample1.ini 
b/test/test/test_cfgfiles/etc/sample1.ini
index aef91c2..5b001d3 100644
--- a/test/test/test_cfgfiles/etc/sample1.ini
+++ b/test/test/test_cfgfiles/etc/sample1.ini
@@ -8,5 +8,5 @@ key1=value1
 ; this is section 2
 ;key1=value1
 key2=value2
-key3=value3 ; this is key3
+key3= ; this is key3
 ignore-missing-separator
diff --git a/test/test/test_cfgfiles/etc/sample2.ini 
b/test/test/test_cfgfiles/etc/sample2.ini
index 21075e9..8fee5a7 100644
--- a/test/test/test_cfgfiles/etc/sample2.ini
+++ b/test/test/test_cfgfiles/etc/sample2.ini
@@ -8,5 +8,5 @@ key1=value1
 # this is section 2
 #key1=value1
 key2=value2
-key3=value3 # this is key3
+key3= # this is key3
 ignore-missing-separator
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 5/6] cfgfile: increase local buffer size for max name and value

[Allain Legacy]

From: Joseph Richard 

When parsing a ini file with a "key = value" line that has both "key" and
"value" sized to the maximum allowed length causes a parsing failure.  The
internal "buffer" variable should be sized at least as large as the maximum
for both fields.  This commit updates the local array to be sized to hold
the max name, max value, " = ", and the nul terminator.

Signed-off-by: Allain Legacy 
---
 lib/librte_cfgfile/rte_cfgfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 416b3b6..35c81e4 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -112,7 +112,7 @@ struct rte_cfgfile *
int allocated_entries = 0;
int curr_section = -1;
int curr_entry = -1;
-   char buffer[256] = {0};
+   char buffer[CFG_NAME_LEN + CFG_VALUE_LEN + 4] = {0};
int lineno = 0;
struct rte_cfgfile *cfg = NULL;
 
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 4/6] cfgfile: use strnlen to constrain memchr search

[Allain Legacy]

The call to memchr() uses the absolute length of the string buffer instead
of the actual length of the string returned by fgets().  This causes the
search to go beyond the '\n' character and find ';' characters in random
garbage on the stack.  This then causes the 'len' variable to be updated
and the subsequent search for the '=' character to potentially find one
beyond the first newline character.

Since this bug relies on ';' and '=' characters appearing in random places
in the 'buffer' variable it is intermittently reproducible at best.

Signed-off-by: Allain Legacy 
---
 lib/librte_cfgfile/rte_cfgfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 7ecab22..416b3b6 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -152,7 +152,7 @@ struct rte_cfgfile *
"Check if line too long\n", lineno);
goto error1;
}
-   pos = memchr(buffer, params->comment_character, sizeof(buffer));
+   pos = memchr(buffer, params->comment_character, len);
if (pos != NULL) {
*pos = '\0';
len = pos -  buffer;
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 0/4] net/bonding: bonding and LACP fixes

[Thomas Monjalon]

2017-02-08 18:14, Thomas Monjalon:
> 2016-08-01 16:42, Robert Sanford:
> > In this patch series, we fix two bonding driver bugs and
> > enhance testpmd so that bonding mode 4 (LACP) ports remain
> > operational even when idle.
> 
> These patches are blocked because we are waiting a v2.
> 
> Waiting so long for a fix without having any feedback makes me think
> that either the bug is not so important or the PMD is not used.

Last call: anyone interested in this series?

Re: [dpdk-dev] [PATCH] vhost: add back support for concurrent enqueue

[Thomas Monjalon]

2016-08-23 15:42, Rich Lane:
> On Mon, Aug 22, 2016 at 7:16 AM, Yuanhan Liu 
> wrote:
> > On Thu, Aug 18, 2016 at 11:27:06AM -0700, Rich Lane wrote:
> > > I could also add back concurrent enqueue support for mergeable RX, but I
> > > was hoping to avoid that since the mergeable codepath is already complex
> > > and wouldn't be used in high performance deployments.
> >
> > Another note is that, you might also have noticed, Zhihong made a patch
> > set [0] to optimize the enqueue code path (mainly on mergeable path). It
> > basically does a rewrite from scatch, which removes the desc buf
> > reservation,
> > meaning it would be harder to do concurrent enqueue support based on that.
> >
> > [0]: Aug 19 Zhihong Wang(  68) ├─>[PATCH v3 0/5] vhost: optimize
> > enqueue
> 
> Yes, I'd noticed that these patches would conflict. Once the vhost-cuse
> removal and
> Zhihong's patches have merged I'll send a new version.

What is the status of this feature?

Re: [dpdk-dev] [PATCH 5/5] cfgfile: increase local buffer size for max name and value

[Wiles, Keith]

> On Mar 2, 2017, at 1:29 PM, Allain Legacy  wrote:
> 
> From: Joseph Richard 
> 
> When parsing a ini file with a "key = value" line that has both "key" and
> "value" sized to the maximum allowed length causes a parsing failure.  The
> internal "buffer" variable should be sized at least as large as the maximum
> for both fields.  This commit updates the local array to be sized to hold
> the max name, max value, " = ", and the nul terminator.
> 
> Signed-off-by: Allain Legacy 
> ---
> lib/librte_cfgfile/rte_cfgfile.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cfgfile/rte_cfgfile.c 
> b/lib/librte_cfgfile/rte_cfgfile.c
> index 28956ea..107d637 100644
> --- a/lib/librte_cfgfile/rte_cfgfile.c
> +++ b/lib/librte_cfgfile/rte_cfgfile.c
> @@ -92,7 +92,7 @@ struct rte_cfgfile *
>   int allocated_entries = 0;
>   int curr_section = -1;
>   int curr_entry = -1;
> - char buffer[256] = {0};
> + char buffer[CFG_NAME_LEN + CFG_VALUE_LEN + 4] = {0};

Would this change still cause a failure and memory over write if the user 
decides to have very large string. Does the code check the lengths to make sure 
they are valid and return error?

If the code is testing the size and make sure a memory over write does not 
happen, then I am OK with acking this patch. 

>   int lineno = 0;
>   size_t size;
>   struct rte_cfgfile *cfg = NULL;
> -- 
> 1.8.3.1
> 

Regards,
Keith

Re: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter for MPLS

[Iremonger, Bernard]

Hi Beilei,

 
> > > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Beilei Xing
> > > > > Sent: Friday, March 3, 2017 9:44 AM
> > > > > To: Wu, Jingjing 
> > > > > Cc: Zhang, Helin ; dev@dpdk.org
> > > > > Subject: [dpdk-dev] [PATCH 3/3] net/i40e: enable cloud filter
> > > > > for MPLS
> > > > >
> > > > > This patch enables MPLSoUDP and MPLSoGRE cloud filter with
> > > > > replace cloud filter.
> > > > >
> > > > > Signed-off-by: Beilei Xing 
> > > > > ---
> > > > >  drivers/net/i40e/i40e_ethdev.c |  44 +++--
> > > > >  drivers/net/i40e/i40e_ethdev.h |   9 +++-
> > > > >  drivers/net/i40e/i40e_flow.c   | 108
> > > > > +
> > > > >  3 files changed, 144 insertions(+), 17 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > > > b/drivers/net/i40e/i40e_ethdev.c index 91bfd73..6044daf 100644
> > > > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > > > @@ -1286,6 +1286,9 @@ eth_i40e_dev_init(struct rte_eth_dev
> *dev)
> > > > >   if (ret < 0)
> > > > >   goto err_init_fdir_filter_list;
> > > > >
> > > > > + i40e_replace_mpls_l1_filter(pf);
> > > > > + i40e_replace_mpls_cloud_filter(pf);
> > > > > +
> > > > >   return 0;
> > > > >
> > > > >  err_init_fdir_filter_list:
> > > > > @@ -6941,6 +6944,7 @@
> > > > > i40e_dev_consistent_tunnel_filter_set(struct
> > > > > i40e_pf *pf,
> > > > >   struct i40e_tunnel_rule *tunnel_rule = &pf->tunnel;
> > > > >   struct i40e_tunnel_filter *tunnel, *node;
> > > > >   struct i40e_tunnel_filter check_filter; /* Check if filter
> > > > > exists */
> > > > > + uint32_t teid_le;
> > > > >   bool big_buffer = 0;
> > > > >
> > > > >   cld_filter = rte_zmalloc("tunnel_filter", @@ -6989,6 +6993,28
> > > > > @@ i40e_dev_consistent_tunnel_filter_set(struct i40e_pf *pf,
> > > > >   case I40E_TUNNEL_TYPE_IP_IN_GRE:
> > > > >   tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_IP;
> > > > >   break;
> > > > > + case I40E_TUNNEL_TYPE_MPLSoUDP:
> > > > > + teid_le = rte_cpu_to_le_32(tunnel_filter-
> >tenant_id);
> > > > > + pfilter-
> > > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD0] =
> > > > > + teid_le >> 4;
> > > > > + pfilter-
> > > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> > > > > + (teid_le & 0xF) << 12;
> > > > > + pfilter-
> > > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD2] =
> > > > > + 0x40;
> > > > > + big_buffer = 1;
> > > > > + tun_type =
> I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoUDP;
> > > > > + break;
> > > > > + case I40E_TUNNEL_TYPE_MPLSoGRE:
> > > > > + teid_le = rte_cpu_to_le_32(tunnel_filter-
> >tenant_id);
> > > > > + pfilter-
> > > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD0] =
> > > > > + teid_le >> 4;
> > > > > + pfilter-
> > > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD1] =
> > > > > + (teid_le & 0xF) << 12;
> > > > > + pfilter-
> > > > > >general_fields[I40E_AQC_ADD_CLOUD_FV_FLU_0X11_WORD2] =
> > > > > + 0x0;
> > > > > + big_buffer = 1;
> > > > > + tun_type =
> I40E_AQC_ADD_CLOUD_TNL_TYPE_MPLSoGRE;
> > > > > + break;
> > > > >   default:
> > > > >   /* Other tunnel types is not supported. */
> > > > >   PMD_DRV_LOG(ERR, "tunnel type is not
> supported."); @@ -
> > > > > 6996,11 +7022,19 @@ i40e_dev_consistent_tunnel_filter_set(struct
> > > > > i40e_pf *pf,
> > > > >   return -EINVAL;
> > > > >   }
> > > > >
> > > > > - val = i40e_dev_get_filter_type(tunnel_filter->filter_type,
> > > > > -&pfilter->element.flags);
> > > > > - if (val < 0) {
> > > > > - rte_free(cld_filter);
> > > > > - return -EINVAL;
> > > > > + if (tunnel_filter->tunnel_type ==
> I40E_TUNNEL_TYPE_MPLSoUDP)
> > > > > + pfilter->element.flags =
> > > > > +
>   I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoUDP;
> > > > > + else if (tunnel_filter->tunnel_type ==
> > > > > I40E_TUNNEL_TYPE_MPLSoGRE)
> > > > > + pfilter->element.flags =
> > > > > +
>   I40E_AQC_ADD_CLOUD_FILTER_TEID_MPLSoGRE;
> > > > > + else {
> > > > > + val = i40e_dev_get_filter_type(tunnel_filter-
> >filter_type,
> > > > > + &pfilter-
> >element.flags);
> > > > > + if (val < 0) {
> > > > > + rte_free(cld_filter);
> > > > > + return -EINVAL;
> > > > > + }
> > > > >   }
> > > > >
> > > > >   pfilter->element.flags |= rte_cpu_to_le_16( diff --git
> > > > > a/drivers/net/i40e/i40e_ethdev.h
> > > > > b/drivers/net/i40e/i

Re: [dpdk-dev] [PATCH v3 13/16] net/avp: device statistics operations

[Legacy, Allain]

> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Memset here is unnecessary since only caller is rte_eth_stats_get() which
> already did memset
> 
Ok.  Will remove.

Re: [dpdk-dev] [PATCH v3 15/16] net/avp: device start and stop operations

[Legacy, Allain]

> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> 
> The second goto is unnecessary.
Ok.  Will remove.

Re: [dpdk-dev] ip_pipeline firewall customization

[Shyam Shrivastav]

My mistake, arp packets are hit by

pipeline>p 1 firewall add priority 1 ipv4 0.0.0.0 0 0.0.0.0 0 0 65535 0
65535 0 0 port 0

ACL as anything and rightly so gets matched for above fields. Only way is
to somehow avoid ARP packets being input to table itself, and yes one way
is to direct them to a separate queue .Another is  explicitly filtering
them before or after  firewall table look up, firewall table is supposed to
be only for ipv4/ipv6 packets have to work/think more on this

On Thu, Mar 9, 2017 at 5:13 PM, Shyam Shrivastav  wrote:

> Hi Cristian
>
> Please see my comments inline (in blue)
>
> >   b) Make required changes in pipeline_firewall_msg_req_add_handler
>> if
>> > portid is -1, that is table entry action to be .action =
>> > RTE_PIPELINE_ACTION_DROP.
>> >
>>
>> You don't really need to do this for implementing a drop action. You can
>> simply create a SINK output port (which basically drops all the packets
>> directed to it) and set this as the output port for all rules that drop
>> packet.
>>
>> See ip_pipeline/config/firewall.cfg as example.
>>
>
> Yes dropping of packets can be achieved by creating a sink port. However
> we need further processing to be done for a packet hitting a pass rule, and
> best way for this processing is f_action_hit table handler.  In this
> routine we can distinguish between drop/pass actions as
> rte_pipeline_table_entry is passed to it, which also contains  portid but
> no generic way to  distinguish between normal or sink port.
> I think it would be value addition to allow this action in firewall
> pipeline, please let me know your thoughts. As for our project looks like
> we have to include this.
>
>
>>
>> > 2) I am registering a f_action_hit function for firewall table to
>> perform
>> > certain translations if action is pass (RTE_PIPELINE_ACTION_PORT).
>>
>> What type of actions are you performing? If generic enough, it would be
>> interesting to add them to this pipeline, so I encourage you to contribute
>> with ideas and code patches.
>>
>>
> Probably it's not generic, involves vlan translation and mac swapping to
> inject the "passed" packets back to the router to travel to next hop.
> Router injects traffic for filtering to directly connected firewall port
> with vlan tag say A , firewall in turn injects pass traffic with different
> vlan tag say B back to the router which then does next hop processing.
>
>
>> >
>> > Is this a bug or am I missing something ?
>> >
>>
>> You can make sure no ARP packets are received by the firewall pipeline by
>> simply filtering all the ARP packets to a separate RXQ of the NIC port,
>> which can be further handled by a separate function.
>>
>> See ip_pipeline/config/network_layers.cfg as example:
>> [LINK0]
>> arp_q = 4
>>
>
> There are two things here
>
> 1) ARP packets should not hit the ipv4 acl which looks like happening,
> have not worked on "why part" for now , need to look at the
> rte_table_acl_lookup->rte_acl_classify.
>
> 2) Yes we can filter ARP packets to a separate queue and to separate lcore
> at link level, but need to decide whether its worth it. We are planning to
> use ipv4 RSS (with 8 queues, lcores and processors) wherein all arp packets
> would get filtered to queue 0 by default and then dropped by ACL.
>
>
> Thanks and regards
> Shyam
>
>
>

Re: [dpdk-dev] [PATCH v3 08/16] net/avp: device initialization

[Legacy, Allain]

> -Original Message-
> From: Chas Williams [mailto:3ch...@gmail.com]
> I don't see the other side of this to unregister the callback.  It's also a 
> bit
> confusing with this here and the other parts in part 15.  It looks like you
> enable the interrupts on .dev_create but disable on .dev_stop?
> If that's the case, you likely want to just do the setup here and the enable 
> in
> .dev_start.
Agreed.  This is not symmetric.  I will setup in create, enable in start, 
disable in stop.

Re: [dpdk-dev] [PATCH v3 1/6] net/tap: add MAC address management ops

[Ferruh Yigit]

On 3/7/2017 4:31 PM, Pascal Mazon wrote:
> Set a random MAC address when probing the device, as to not leave an
> empty MAC in pmd->eth_addr.
> 
> This MAC will be set on the tap netdevice as soon as it's been created
> using tun_alloc(). As the tap_mac_add() function depend on the fd in
> the first rxq, move code from tun_alloc() to tap_setup_queue(),
> after it's been set.
> 
> Signed-off-by: Pascal Mazon 
> ---
>  doc/guides/nics/features/tap.ini |  1 +
>  drivers/net/tap/rte_eth_tap.c| 97 
> ++--
>  2 files changed, 85 insertions(+), 13 deletions(-)
> 
> diff --git a/doc/guides/nics/features/tap.ini 
> b/doc/guides/nics/features/tap.ini
> index f4aca6921ddc..d9b47a003654 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -9,6 +9,7 @@ Jumbo frame  = Y
>  Promiscuous mode = Y
>  Allmulticast mode= Y
>  Basic stats  = Y
> +Unicast MAC filter   = Y
>  Other kdrv   = Y
>  ARMv7= Y
>  ARMv8= Y
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index ece3a5fcc897..1e46ee36efa2 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -63,6 +63,8 @@
>  #define RTE_PMD_TAP_MAX_QUEUES   1
>  #endif
>  
> +#define RTE_PMD_TAP_MAX_MAC_ADDRS 1

mac_addr_add and mac_addr_remove not really supported, because only one
MAC is supported. For mac_addr_add() all indexes other than 0 will give
an error. So only mac_addr_set is supported.

For this case what is the benefit of implementing these functions and
claim support, instead of just leaving mac_addr_add and mac_addr_remove
NULL?


<...>

> + if (qid == 0) {
> + /*
> +  * tap_setup_queue() is called for both tx and rx.
> +  * Let's use dev->data->r/tx_queues[qid] to determine if init
> +  * has already been done.
> +  */
> + if (dev->data->rx_queues[qid] && dev->data->tx_queues[qid])
> + return fd;
> +
> + tap_mac_set(dev, &pmd->eth_addr);

What is the reason of changing behavior here?

Tap devices assigned random MAC by kernel, and previous implementation
was reading that value and using it for DPDK.

Now kernel assigns a random MAC, and DPDK overwrites it another random
MAC, previous implementation was simpler I think.

It is OK to move this code tap_setup_queue(), I just missed the benefit
of overwriting with DPDK random MAC?

<...>

Re: [dpdk-dev] [PATCH v3 2/6] net/tap: add speed capabilities

[Ferruh Yigit]

On 3/7/2017 4:31 PM, Pascal Mazon wrote:
> Tap PMD is flexible, it supports any speed.
> 
> Signed-off-by: Pascal Mazon 
> ---
>  doc/guides/nics/features/tap.ini |  1 +
>  drivers/net/tap/rte_eth_tap.c| 35 +++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/doc/guides/nics/features/tap.ini 
> b/doc/guides/nics/features/tap.ini
> index d9b47a003654..dad5a0561087 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -9,6 +9,7 @@ Jumbo frame  = Y
>  Promiscuous mode = Y
>  Allmulticast mode= Y
>  Basic stats  = Y
> +Speed capabilities   = Y
>  Unicast MAC filter   = Y
>  Other kdrv   = Y
>  ARMv7= Y
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 1e46ee36efa2..ef525a3f0826 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
>   return 0;
>  }
>  
> +static uint32_t
> +tap_dev_speed_capa(void)
> +{
> + uint32_t speed = pmd_link.link_speed;

link_speed is already hardcoded into PMD, so there is nothing to detect
here. Would it be different if PMD directly return pmd_link.link_speed?

> + uint32_t capa = 0;
> +
> + if (speed >= ETH_SPEED_NUM_10M)
> + capa |= ETH_LINK_SPEED_10M;
> + if (speed >= ETH_SPEED_NUM_100M)
> + capa |= ETH_LINK_SPEED_100M;
> + if (speed >= ETH_SPEED_NUM_1G)
> + capa |= ETH_LINK_SPEED_1G;
> + if (speed >= ETH_SPEED_NUM_5G)
> + capa |= ETH_LINK_SPEED_2_5G;
> + if (speed >= ETH_SPEED_NUM_5G)
> + capa |= ETH_LINK_SPEED_5G;
> + if (speed >= ETH_SPEED_NUM_10G)
> + capa |= ETH_LINK_SPEED_10G;
> + if (speed >= ETH_SPEED_NUM_20G)
> + capa |= ETH_LINK_SPEED_20G;
> + if (speed >= ETH_SPEED_NUM_25G)
> + capa |= ETH_LINK_SPEED_25G;
> + if (speed >= ETH_SPEED_NUM_40G)
> + capa |= ETH_LINK_SPEED_40G;
> + if (speed >= ETH_SPEED_NUM_50G)
> + capa |= ETH_LINK_SPEED_50G;
> + if (speed >= ETH_SPEED_NUM_56G)
> + capa |= ETH_LINK_SPEED_56G;
> + if (speed >= ETH_SPEED_NUM_100G)
> + capa |= ETH_LINK_SPEED_100G;

I would prefer switch here [1], but functionally both are same, it is
your call.

[1]
switch (speed) {
case ETH_SPEED_NUM_100G:
capa |= ETH_LINK_SPEED_100G
/* fallthrough */
case ETH_SPEED_NUM_56G:
capa |= ETH_LINK_SPEED_56G
/* fallthrough */
...
};


> +
> + return capa;
> +}
> +
>  static void
>  tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>  {
> @@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct 
> rte_eth_dev_info *dev_info)
>   dev_info->max_tx_queues = internals->nb_queues;
>   dev_info->min_rx_bufsize = 0;
>   dev_info->pci_dev = NULL;
> + dev_info->speed_capa = tap_dev_speed_capa();
>  }
>  
>  static void
> 

Re: [dpdk-dev] [PATCH v3 5/6] net/tap: add packet type management

[Ferruh Yigit]

On 3/7/2017 4:31 PM, Pascal Mazon wrote:
> Advertize RTE_PTYPE_UNKNOWN since tap does not report any packet type.
> 
> Signed-off-by: Pascal Mazon 
> ---
>  doc/guides/nics/features/tap.ini |  1 +
>  drivers/net/tap/rte_eth_tap.c| 15 +++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/doc/guides/nics/features/tap.ini 
> b/doc/guides/nics/features/tap.ini
> index 6aa11874e2bc..7f3f4d661dd7 100644
> --- a/doc/guides/nics/features/tap.ini
> +++ b/doc/guides/nics/features/tap.ini
> @@ -13,6 +13,7 @@ MTU update   = Y
>  Multicast MAC filter = Y
>  Speed capabilities   = Y
>  Unicast MAC filter   = Y
> +Packet type parsing  = Y
>  Other kdrv   = Y
>  ARMv7= Y
>  ARMv8= Y
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index d76f1dc83b03..edb5d2a82f12 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -216,6 +217,8 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, 
> uint16_t nb_pkts)
>   mbuf->data_len = len;
>   mbuf->pkt_len = len;
>   mbuf->port = rxq->in_port;
> + mbuf->packet_type = rte_net_get_ptype(mbuf, NULL,
> +   RTE_PTYPE_ALL_MASK);

Isn't PMD become inconsistent with this update. It reports
RTE_PTYPE_UNKNOWN, but sets mbuf->packet_type with various ptype.

Do we want software packet type parsing in PMD level? Any change some
users may not interested with this data at all.

>  
>   /* account for the receive frame */
>   bufs[num_rx++] = mbuf;
> @@ -769,6 +772,17 @@ tap_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>   return 0;
>  }
>  
> +static const uint32_t*
> +tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
> +{
> + static const uint32_t ptypes[] = {
> + RTE_PTYPE_UNKNOWN,
> +
> + };
> +
> + return ptypes;
> +}
> +
>  static const struct eth_dev_ops ops = {
>   .dev_start  = tap_dev_start,
>   .dev_stop   = tap_dev_stop,
> @@ -793,6 +807,7 @@ static const struct eth_dev_ops ops = {
>   .mtu_set= tap_mtu_set,
>   .stats_get  = tap_stats_get,
>   .stats_reset= tap_stats_reset,
> + .dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
>  };
>  
>  static int
> 

Re: [dpdk-dev] [PATCH v3 2/6] net/tap: add speed capabilities

[Wiles, Keith]

> On Mar 9, 2017, at 8:18 AM, Yigit, Ferruh  wrote:
> 
> On 3/7/2017 4:31 PM, Pascal Mazon wrote:
>> Tap PMD is flexible, it supports any speed.
>> 
>> Signed-off-by: Pascal Mazon 
>> ---
>> doc/guides/nics/features/tap.ini |  1 +
>> drivers/net/tap/rte_eth_tap.c| 35 +++
>> 2 files changed, 36 insertions(+)
>> 
>> diff --git a/doc/guides/nics/features/tap.ini 
>> b/doc/guides/nics/features/tap.ini
>> index d9b47a003654..dad5a0561087 100644
>> --- a/doc/guides/nics/features/tap.ini
>> +++ b/doc/guides/nics/features/tap.ini
>> @@ -9,6 +9,7 @@ Jumbo frame  = Y
>> Promiscuous mode = Y
>> Allmulticast mode= Y
>> Basic stats  = Y
>> +Speed capabilities   = Y
>> Unicast MAC filter   = Y
>> Other kdrv   = Y
>> ARMv7= Y
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index 1e46ee36efa2..ef525a3f0826 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
>>  return 0;
>> }
>> 
>> +static uint32_t
>> +tap_dev_speed_capa(void)
>> +{
>> +uint32_t speed = pmd_link.link_speed;
> 
> link_speed is already hardcoded into PMD, so there is nothing to detect
> here. Would it be different if PMD directly return pmd_link.link_speed?

The link speed is passed into the PMD via the command line, which means it can 
change per run.
> 
>> +uint32_t capa = 0;
>> +
>> +if (speed >= ETH_SPEED_NUM_10M)
>> +capa |= ETH_LINK_SPEED_10M;
>> +if (speed >= ETH_SPEED_NUM_100M)
>> +capa |= ETH_LINK_SPEED_100M;
>> +if (speed >= ETH_SPEED_NUM_1G)
>> +capa |= ETH_LINK_SPEED_1G;
>> +if (speed >= ETH_SPEED_NUM_5G)
>> +capa |= ETH_LINK_SPEED_2_5G;
>> +if (speed >= ETH_SPEED_NUM_5G)
>> +capa |= ETH_LINK_SPEED_5G;
>> +if (speed >= ETH_SPEED_NUM_10G)
>> +capa |= ETH_LINK_SPEED_10G;
>> +if (speed >= ETH_SPEED_NUM_20G)
>> +capa |= ETH_LINK_SPEED_20G;
>> +if (speed >= ETH_SPEED_NUM_25G)
>> +capa |= ETH_LINK_SPEED_25G;
>> +if (speed >= ETH_SPEED_NUM_40G)
>> +capa |= ETH_LINK_SPEED_40G;
>> +if (speed >= ETH_SPEED_NUM_50G)
>> +capa |= ETH_LINK_SPEED_50G;
>> +if (speed >= ETH_SPEED_NUM_56G)
>> +capa |= ETH_LINK_SPEED_56G;
>> +if (speed >= ETH_SPEED_NUM_100G)
>> +capa |= ETH_LINK_SPEED_100G;
> 
> I would prefer switch here [1], but functionally both are same, it is
> your call.
> 
> [1]
> switch (speed) {
> case ETH_SPEED_NUM_100G:
>   capa |= ETH_LINK_SPEED_100G
>   /* fallthrough */
> case ETH_SPEED_NUM_56G:
>   capa |= ETH_LINK_SPEED_56G
>   /* fallthrough */
> ...
> };
> 
> 
>> +
>> +return capa;
>> +}
>> +
>> static void
>> tap_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>> {
>> @@ -363,6 +397,7 @@ tap_dev_info(struct rte_eth_dev *dev, struct 
>> rte_eth_dev_info *dev_info)
>>  dev_info->max_tx_queues = internals->nb_queues;
>>  dev_info->min_rx_bufsize = 0;
>>  dev_info->pci_dev = NULL;
>> +dev_info->speed_capa = tap_dev_speed_capa();
>> }
>> 
>> static void

Regards,
Keith

Re: [dpdk-dev] [RFC PATCH] net/virtio: Align Virtio-net header on cache line in receive path

[Maxime Coquelin]

On 03/08/2017 07:01 AM, Yao, Lei A wrote:




-Original Message-
From: Maxime Coquelin [mailto:maxime.coque...@redhat.com]
Sent: Monday, March 6, 2017 10:11 PM
To: Yuanhan Liu 
Cc: Liang, Cunming ; Tan, Jianfeng
; dev@dpdk.org; Wang, Zhihong
; Yao, Lei A 
Subject: Re: [RFC PATCH] net/virtio: Align Virtio-net header on cache line in
receive path



On 03/06/2017 09:46 AM, Yuanhan Liu wrote:

On Wed, Mar 01, 2017 at 08:36:24AM +0100, Maxime Coquelin wrote:



On 02/23/2017 06:49 AM, Yuanhan Liu wrote:

On Wed, Feb 22, 2017 at 10:36:36AM +0100, Maxime Coquelin wrote:



On 02/22/2017 02:37 AM, Yuanhan Liu wrote:

On Tue, Feb 21, 2017 at 06:32:43PM +0100, Maxime Coquelin wrote:

This patch aligns the Virtio-net header on a cache-line boundary to
optimize cache utilization, as it puts the Virtio-net header (which
is always accessed) on the same cache line as the packet header.

For example with an application that forwards packets at L2 level,
a single cache-line will be accessed with this patch, instead of
two before.


I'm assuming you were testing pkt size <= (64 - hdr_size)?


No, I tested with 64 bytes packets only.


Oh, my bad, I overlooked it. While you were saying "a single cache
line", I was thinking putting the virtio net hdr and the "whole"
packet data in single cache line, which is not possible for pkt
size 64B.


I run some more tests this morning with different packet sizes,
and also with changing the mbuf size on guest side to have multi-
buffers packets:

+---+++-+
| Txpkt | Rxmbuf | v17.02 | v17.02 + vnet hdr align |
+---+++-+
|64 |   2048 |  11.05 |   11.78 |
|   128 |   2048 |  10.66 |   11.48 |
|   256 |   2048 |  10.47 |   11.21 |
|   512 |   2048 |  10.22 |   10.88 |
|  1024 |   2048 |   7.65 |7.84 |
|  1500 |   2048 |   6.25 |6.45 |
|  2000 |   2048 |   5.31 |5.43 |
|  2048 |   2048 |   5.32 |4.25 |
|  1500 |512 |   3.89 |3.98 |
|  2048 |512 |   1.96 |2.02 |
+---+++-+


Could you share more info, say is it a PVP test? Is mergeable on?
What's the fwd mode?


No, this is not PVP benchmark, I have neither another server nor a packet
generator connected to my Haswell machine back-to-back.

This is simple micro-benchmark, vhost PMD in txonly, Virtio PMD in
rxonly. In this configuration, mergeable is ON and no offload disabled
in QEMU cmdline.


Okay, I see. So the boost, as you have stated, comes from saving two
cache line access to one. Before that, vhost write 2 cache lines,
while the virtio pmd reads 2 cache lines: one for reading the header,
another one for reading the ether header, for updating xstats (there
is no ether access in the fwd mode you tested).


That's why I would be interested in more testing on recent hardware
with PVP benchmark. Is it something that could be run in Intel lab?


I think Yao Lei could help on that? But as stated, I think it may
break the performance for bit packets. And I also won't expect big
boost even for 64B in PVP test, judging that it's only 6% boost in
micro bechmarking.

That would be great.
Note that on SandyBridge, on which I see a drop in perf with
microbenchmark, I get a 4% gain on PVP benchmark. So on recent hardware
that show a gain on microbenchmark, I'm curious of the gain with PVP
bench.


Hi, Maxime, Yuanhan

I have execute the PVP and loopback performance test on my Ivy bridge server.
OS:Ubutnu16.04
CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
Kernal:  4.4.0
gcc : 5.4.0
I use MAC forward for test.

Performance base is commit f5472703c0bdfc29c46fc4b2ca445bce3dc08c9f,
"eal: optimize aligned memcpy on x86".
I can see big performance drop on Mergeable and no-mergeable path
after apply this patch
Mergebale Path loopback test
packet size Performance compare 
64  -21.76%
128 -17.79%
260 -20.25%
520 -14.80%
1024-9.34%
1500-6.16%

No-mergeable  Path loopback test
packet size 
64  -13.72%
128 -10.35%
260 -16.40%
520 -14.78%
1024-10.48%
1500-6.91%

Mergeable Path PVP test 
packet size Performance compare 
64 -16.33%

No-mergeable Path PVP test  
packet size 
64 -8.69%


Thanks Yao for the testing.
I'm surprised of the PVP results as even on SandyBridg

Re: [dpdk-dev] [PATCH v3 08/16] net/avp: device initialization

[Legacy, Allain]

> -Original Message-
> From: Legacy, Allain
> > From: Chas Williams [mailto:3ch...@gmail.com]
> > I don't see the other side of this to unregister the callback.  It's also a 
> > bit
> > confusing with this here and the other parts in part 15.  It looks like you
> > enable the interrupts on .dev_create but disable on .dev_stop?
> > If that's the case, you likely want to just do the setup here and the enable
> in
> > .dev_start.
> Agreed.  This is not symmetric.  I will setup in create, enable in start, 
> disable
> in stop.
I was mistaken.  The interrupts need to be setup and enabled when the device 
is created, and then only disabled if the device is closed in preparation for 
removal.  The start/stop functions should not be involved.

So:

avp_dev_create(), will do:
avp_dev_setup_interrupts()
avp_enable_interrupts()

and

avp_dev_close(), will do:
avp_disable_interrupts().

Currently, avp_dev_create() and avp_dev_close() are in separate patches.  
 I'll see how much trouble it would be to move the dev_close() to be in the 
same patch as the dev_create() but there is little value in doing that since 
functionally there is no net effect.  If the effort is too high I will be 
inclined 
to leave it as is. 

Re: [dpdk-dev] [PATCH 5/5] cfgfile: increase local buffer size for max name and value

[Legacy, Allain]

> -Original Message-
> From: Wiles, Keith [mailto:keith.wi...@intel.com]
> Sent: Thursday, March 09, 2017 8:46 AM
> Would this change still cause a failure and memory over write if the user
> decides to have very large string. Does the code check the lengths to make
> sure they are valid and return error?
> 

The fgets() is bounded by the size of the buffer and the subsequent validation 
will raise an error if no newline was detected within the buffer therefore an 
overly long line will result in a failure.  I have added a test case in the v2 
patchset in which I have added a unit test framework for this library.

while (fgets(buffer, sizeof(buffer), f) != NULL) {
char *pos = NULL;
size_t len = strnlen(buffer, sizeof(buffer));
lineno++;
if ((len >= sizeof(buffer) - 1) && (buffer[len-1] != '\n')) {
printf("Error line %d - no \\n found on string. "
"Check if line too long\n", lineno);
goto error1;
}

Does that satisfy your concern and qualify for you Ack?

Re: [dpdk-dev] [PATCH 5/5] cfgfile: increase local buffer size for max name and value

[Wiles, Keith]

> On Mar 9, 2017, at 9:16 AM, Legacy, Allain  
> wrote:
> 
>> -Original Message-
>> From: Wiles, Keith [mailto:keith.wi...@intel.com]
>> Sent: Thursday, March 09, 2017 8:46 AM
>> Would this change still cause a failure and memory over write if the user
>> decides to have very large string. Does the code check the lengths to make
>> sure they are valid and return error?
>> 
> 
> The fgets() is bounded by the size of the buffer and the subsequent 
> validation will raise an error if no newline was detected within the buffer 
> therefore an overly long line will result in a failure.  I have added a test 
> case in the v2 patchset in which I have added a unit test framework for this 
> library.
> 
>   while (fgets(buffer, sizeof(buffer), f) != NULL) {
>   char *pos = NULL;
>   size_t len = strnlen(buffer, sizeof(buffer));
>   lineno++;
>   if ((len >= sizeof(buffer) - 1) && (buffer[len-1] != '\n')) {
>   printf("Error line %d - no \\n found on string. "
>   "Check if line too long\n", lineno);
>   goto error1;
>   }
> 
> Does that satisfy your concern and qualify for you Ack?

Acked-by: Keith Wiles 

Regards,
Keith

Re: [dpdk-dev] [PATCH v3 1/4] net/tap: move private elements to external header

[Ferruh Yigit]

On 3/7/2017 4:35 PM, Pascal Mazon wrote:
> In the next patch, access to struct pmd_internals will be necessary in
> tap_flow.c to store the flows.
> 
> Signed-off-by: Pascal Mazon 
> Acked-by: Olga Shern 
> ---
>  drivers/net/tap/Makefile  |  1 +
>  drivers/net/tap/rte_eth_tap.c | 34 ++--
>  drivers/net/tap/tap.h | 73 
> +++

tap.h is a generic name, I think rte_eth_tap.h fits better here.

<...>

Re: [dpdk-dev] [PATCH v3 3/4] net/tap: add netlink back-end for flow API

[Ferruh Yigit]

On 3/7/2017 4:35 PM, Pascal Mazon wrote:
> Each kernel netdevice may have queueing disciplines set for it, which
> determine how to handle the packet (mostly on egress). That's part of
> the TC (Traffic Control) mechanism.

This is nice.
qdisc is egress part of the network stack right, is there any ingress
part of it?

> 
> Through TC, it is possible to set filter rules that match specific
> packets, and act according to what is in the rule. This is a perfect
> candidate to implement the flow API for the tap PMD, as it has an
> associated kernel netdevice automatically.
> 
> Each flow API rule will be translated into its TC counterpart.

What can be use cases here?

> 
> To leverage TC, it is necessary to communicate with the kernel using
> netlink. This patch introduces a library to help that communication.
> 

What do you think implementing these out of tap PMD? These can be used
by KNI too.

> Inside netlink.c, functions are generic for any netlink messaging.
> Inside tcmsgs.c, functions are specific to deal with TC rules.
> 
> Signed-off-by: Pascal Mazon 
> Acked-by: Olga Shern 
<...>

Re: [dpdk-dev] [PATCH 05/11] net/sfc: add flow API filters support

[Andrew Rybchenko]

On 03/07/2017 04:21 PM, Ferruh Yigit wrote:

On 3/2/2017 4:03 PM, Andrew Rybchenko wrote:

From: Roman Zhukov 

Only pattern items VOID, ETH and actions VOID, QUEUE is now
supported.

Signed-off-by: Roman Zhukov 
Signed-off-by: Andrew Rybchenko 

<...>


diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
new file mode 100644
index 000..0590756
--- /dev/null
+++ b/drivers/net/sfc/sfc_flow.c
@@ -0,0 +1,693 @@
+/*-
+ * Copyright (c) 2017 Solarflare Communications Inc.
+ * All rights reserved.

Missing "BSD LICENSE" line. This is same for all new files.
I think this discussed before, I don't know if that line is mandatory,
but can be good to have at least to be consistent to rest of the codes.


Since it is common for all our files, let me address is separately.

Thanks,
Andrew.

[dpdk-dev] [PATCH v2 04/11] net/sfc: provide a way to check if filter is supported

[Andrew Rybchenko]

The information is obtained from firmware on attach. It may
change after MC reboot (firmware version or variant change).
Cache should be refreshed after MC reboot when it is handled
properly (not yet).

Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
Reviewed-by: Andy Moreton 
Reviewed-by: Robert Stonehouse 
---
 drivers/net/sfc/Makefile |   1 +
 drivers/net/sfc/sfc.c|   7 +++
 drivers/net/sfc/sfc.h|   3 +
 drivers/net/sfc/sfc_filter.c | 135 +++
 drivers/net/sfc/sfc_filter.h |  56 ++
 5 files changed, 202 insertions(+)
 create mode 100644 drivers/net/sfc/sfc_filter.c
 create mode 100644 drivers/net/sfc/sfc_filter.h

diff --git a/drivers/net/sfc/Makefile b/drivers/net/sfc/Makefile
index 619a0ed..2b7ede8 100644
--- a/drivers/net/sfc/Makefile
+++ b/drivers/net/sfc/Makefile
@@ -90,6 +90,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_port.c
 SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_rx.c
 SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_tx.c
 SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_tso.c
+SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_filter.c
 
 VPATH += $(SRCDIR)/base
 
diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index 03ecec2..6fd8bf1 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -633,6 +633,10 @@ sfc_attach(struct sfc_adapter *sa)
if (rc != 0)
goto fail_set_rss_defaults;
 
+   rc = sfc_filter_attach(sa);
+   if (rc != 0)
+   goto fail_filter_attach;
+
sfc_log_init(sa, "fini nic");
efx_nic_fini(enp);
 
@@ -641,6 +645,7 @@ sfc_attach(struct sfc_adapter *sa)
sfc_log_init(sa, "done");
return 0;
 
+fail_filter_attach:
 fail_set_rss_defaults:
sfc_intr_detach(sa);
 
@@ -678,6 +683,8 @@ sfc_detach(struct sfc_adapter *sa)
 
SFC_ASSERT(sfc_adapter_is_locked(sa));
 
+   sfc_filter_detach(sa);
+
sfc_intr_detach(sa);
 
sfc_log_init(sa, "unprobe nic");
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 655328f..b782360 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -38,6 +38,8 @@
 
 #include "efx.h"
 
+#include "sfc_filter.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -180,6 +182,7 @@ struct sfc_adapter {
struct sfc_mcdi mcdi;
struct sfc_intr intr;
struct sfc_port port;
+   struct sfc_filter   filter;
 
unsigned intrxq_max;
unsigned inttxq_max;
diff --git a/drivers/net/sfc/sfc_filter.c b/drivers/net/sfc/sfc_filter.c
new file mode 100644
index 000..98435e9
--- /dev/null
+++ b/drivers/net/sfc/sfc_filter.c
@@ -0,0 +1,135 @@
+/*-
+ * Copyright (c) 2017 Solarflare Communications Inc.
+ * All rights reserved.
+ *
+ * This software was jointly developed between OKTET Labs (under contract
+ * for Solarflare) and Solarflare Communications, Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *this list of conditions and the following disclaimer in the documentation
+ *and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+ * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
+ * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include 
+
+#include "efx.h"
+
+#include "sfc.h"
+#include "sfc_log.h"
+
+boolean_t
+sfc_filter_is_match_supported(struct sfc_adapter *sa, uint32_t match)
+{
+   struct sfc_filter *filter = &sa->filter;
+   size_t i;
+
+   for (i = 0; i < filter->supported_match_num; ++i) {
+   if (match == filter->supported_match[i])
+   return B_TRUE;
+   }
+
+   return B_FALSE;
+}
+
+static int
+sfc_filter_cache_match_supported(struct sfc_adapter *sa)
+{
+   struct sfc_filter *filter = &sa->filter;
+   size_t num = filter->supported_match_num;
+   uint32_t *buf = filter->supported_match;
+   unsigned int retry;
+ 

[dpdk-dev] [PATCH v2 01/11] net/sfc/base: split local MAC I/G back into separate flags

[Andrew Rybchenko]

From: Mark Spender 

The flag EFX_FILTER_MATCH_LOC_MAC_IG to represent filtering on the
individual/group bit of the MAC address (with the two cases being
distingusished by the MAC address in the filter specification) was
introduced to mirror the Linux driver filtering code, but the
implementations are different enough anyway that it isn't of much
value.

Having separate flags for unknown unicast and multicast simplifies
the code and allows the set of flags to match those used by MCDI.

It will also makes it easier to report whether these filters are
supported.

In the MCDI definitions, the unknown multicast and unicast flags
have the values 0x4000 and 0x8000 respectively, and so using
the same values for simplicity requires 32 bits in the filter
specification to store the flags. This means the structure is now
a little bigger than 64 bytes, but filters are not often used on
critical paths so this shouldn't have much impact - on Linux they
are also bigger than they used to be.

Signed-off-by: Mark Spender 
Signed-off-by: Andrew Rybchenko 
---
 drivers/net/sfc/base/ef10_filter.c | 48 --
 drivers/net/sfc/base/efx.h | 10 
 drivers/net/sfc/base/efx_filter.c  |  8 ++-
 3 files changed, 12 insertions(+), 54 deletions(-)

diff --git a/drivers/net/sfc/base/ef10_filter.c 
b/drivers/net/sfc/base/ef10_filter.c
index c161977..f520147 100644
--- a/drivers/net/sfc/base/ef10_filter.c
+++ b/drivers/net/sfc/base/ef10_filter.c
@@ -142,6 +142,10 @@ ef10_filter_init(
MATCH_MASK(MC_CMD_FILTER_OP_IN_MATCH_OUTER_VLAN));
EFX_STATIC_ASSERT(EFX_FILTER_MATCH_IP_PROTO ==
MATCH_MASK(MC_CMD_FILTER_OP_IN_MATCH_IP_PROTO));
+   EFX_STATIC_ASSERT(EFX_FILTER_MATCH_UNKNOWN_MCAST_DST ==
+   MATCH_MASK(MC_CMD_FILTER_OP_IN_MATCH_UNKNOWN_MCAST_DST));
+   EFX_STATIC_ASSERT((uint32_t)EFX_FILTER_MATCH_UNKNOWN_UCAST_DST ==
+   MATCH_MASK(MC_CMD_FILTER_OP_IN_MATCH_UNKNOWN_UCAST_DST));
 #undef MATCH_MASK
 
EFSYS_KMEM_ALLOC(enp->en_esip, sizeof (ef10_filter_table_t), eftp);
@@ -184,7 +188,6 @@ efx_mcdi_filter_op_add(
efx_mcdi_req_t req;
uint8_t payload[MAX(MC_CMD_FILTER_OP_IN_LEN,
MC_CMD_FILTER_OP_OUT_LEN)];
-   uint32_t match_fields = 0;
efx_rc_t rc;
 
memset(payload, 0, sizeof (payload));
@@ -211,26 +214,10 @@ efx_mcdi_filter_op_add(
goto fail1;
}
 
-   if (spec->efs_match_flags & EFX_FILTER_MATCH_LOC_MAC_IG) {
-   /*
-* The LOC_MAC_IG match flag can represent unknown unicast
-*  or multicast filters - use the MAC address to distinguish
-*  them.
-*/
-   if (EFX_MAC_ADDR_IS_MULTICAST(spec->efs_loc_mac))
-   match_fields |= 1U <<
-   MC_CMD_FILTER_OP_IN_MATCH_UNKNOWN_MCAST_DST_LBN;
-   else
-   match_fields |= 1U <<
-   MC_CMD_FILTER_OP_IN_MATCH_UNKNOWN_UCAST_DST_LBN;
-   }
-
-   match_fields |= spec->efs_match_flags & (~EFX_FILTER_MATCH_LOC_MAC_IG);
-
MCDI_IN_SET_DWORD(req, FILTER_OP_IN_PORT_ID,
EVB_PORT_ID_ASSIGNED);
MCDI_IN_SET_DWORD(req, FILTER_OP_IN_MATCH_FIELDS,
-   match_fields);
+   spec->efs_match_flags);
MCDI_IN_SET_DWORD(req, FILTER_OP_IN_RX_DEST,
MC_CMD_FILTER_OP_IN_RX_DEST_HOST);
MCDI_IN_SET_DWORD(req, FILTER_OP_IN_RX_QUEUE,
@@ -889,9 +876,6 @@ efx_mcdi_get_parser_disp_info(
uint8_t payload[MAX(MC_CMD_GET_PARSER_DISP_INFO_IN_LEN,
MC_CMD_GET_PARSER_DISP_INFO_OUT_LENMAX)];
efx_rc_t rc;
-   uint32_t i;
-   boolean_t support_unknown_ucast = B_FALSE;
-   boolean_t support_unknown_mcast = B_FALSE;
 
(void) memset(payload, 0, sizeof (payload));
req.emr_cmd = MC_CMD_GET_PARSER_DISP_INFO;
@@ -927,28 +911,6 @@ efx_mcdi_get_parser_disp_info(
EFX_STATIC_ASSERT(sizeof (uint32_t) ==
MC_CMD_GET_PARSER_DISP_INFO_OUT_SUPPORTED_MATCHES_LEN);
 
-   /*
-* Remove UNKNOWN UCAST and MCAST flags, and if both are present, change
-* the lower priority one to LOC_MAC_IG.
-*/
-   for (i = 0; i < *length; i++) {
-   if (list[i] & MC_CMD_FILTER_OP_IN_MATCH_UNKNOWN_UCAST_DST_LBN) {
-   list[i] &=
-   (~MC_CMD_FILTER_OP_IN_MATCH_UNKNOWN_UCAST_DST_LBN);
-   support_unknown_ucast = B_TRUE;
-   }
-   if (list[i] & MC_CMD_FILTER_OP_IN_MATCH_UNKNOWN_MCAST_DST_LBN) {
-   list[i] &=
-   (~MC_CMD_FILTER_OP_IN_MATCH_UNKNOWN_MCAST_DST_LBN);
-   support_unknown_mcast = B_TRUE;
-   }
-
-   if (support_unknown_ucast && support_unknown_mcast) {
-   list[i] &= EFX_FILTER_MATCH_LOC_

[dpdk-dev] [PATCH v2 02/11] net/sfc/base: improve API to get supported filter matches

[Andrew Rybchenko]

From: Mark Spender 

The previous API had various problems, including the length of the
caller provided buffer not being specified, no means being available
to discover how big the buffer needs to be, and a lack of clarity of
what the resulting list contains.

To improve it:
- add the buffer length as a parameter
- if the provided buffer is too short, fail with ENOSPC and return
  the required length
- ensure that the list contents are valid and add comments describing it

It is safe to change this API as, unsuprisingly, it has no users.

Signed-off-by: Mark Spender 
Signed-off-by: Andrew Rybchenko 
---
 drivers/net/sfc/base/ef10_filter.c | 90 +++---
 drivers/net/sfc/base/ef10_impl.h   |  7 +--
 drivers/net/sfc/base/efx.h |  7 +--
 drivers/net/sfc/base/efx_filter.c  | 68 +++-
 drivers/net/sfc/base/efx_impl.h|  3 +-
 5 files changed, 132 insertions(+), 43 deletions(-)

diff --git a/drivers/net/sfc/base/ef10_filter.c 
b/drivers/net/sfc/base/ef10_filter.c
index f520147..695bb84 100644
--- a/drivers/net/sfc/base/ef10_filter.c
+++ b/drivers/net/sfc/base/ef10_filter.c
@@ -868,13 +868,16 @@ ef10_filter_delete(
 
 static __checkReturn   efx_rc_t
 efx_mcdi_get_parser_disp_info(
-   __inefx_nic_t *enp,
-   __out   uint32_t *list,
-   __out   size_t *length)
+   __inefx_nic_t *enp,
+   __out_ecount(buffer_length) uint32_t *buffer,
+   __insize_t buffer_length,
+   __out   size_t *list_lengthp)
 {
efx_mcdi_req_t req;
uint8_t payload[MAX(MC_CMD_GET_PARSER_DISP_INFO_IN_LEN,
MC_CMD_GET_PARSER_DISP_INFO_OUT_LENMAX)];
+   size_t matches_count;
+   size_t list_size;
efx_rc_t rc;
 
(void) memset(payload, 0, sizeof (payload));
@@ -894,25 +897,41 @@ efx_mcdi_get_parser_disp_info(
goto fail1;
}
 
-   *length = MCDI_OUT_DWORD(req,
+   matches_count = MCDI_OUT_DWORD(req,
GET_PARSER_DISP_INFO_OUT_NUM_SUPPORTED_MATCHES);
 
if (req.emr_out_length_used <
-   MC_CMD_GET_PARSER_DISP_INFO_OUT_LEN(*length)) {
+   MC_CMD_GET_PARSER_DISP_INFO_OUT_LEN(matches_count)) {
rc = EMSGSIZE;
goto fail2;
}
 
-   memcpy(list,
-   MCDI_OUT2(req,
-   uint32_t,
-   GET_PARSER_DISP_INFO_OUT_SUPPORTED_MATCHES),
-   (*length) * sizeof (uint32_t));
+   *list_lengthp = matches_count;
+
+   if (buffer_length < matches_count) {
+   rc = ENOSPC;
+   goto fail3;
+   }
+
+   /*
+* Check that the elements in the list in the MCDI response are the size
+* we expect, so we can just copy them directly. Any conversion of the
+* flags is handled by the caller.
+*/
EFX_STATIC_ASSERT(sizeof (uint32_t) ==
MC_CMD_GET_PARSER_DISP_INFO_OUT_SUPPORTED_MATCHES_LEN);
 
+   list_size = matches_count *
+   MC_CMD_GET_PARSER_DISP_INFO_OUT_SUPPORTED_MATCHES_LEN;
+   memcpy(buffer,
+   MCDI_OUT2(req, uint32_t,
+   GET_PARSER_DISP_INFO_OUT_SUPPORTED_MATCHES),
+   list_size);
+
return (0);
 
+fail3:
+   EFSYS_PROBE(fail3);
 fail2:
EFSYS_PROBE(fail2);
 fail1:
@@ -923,14 +942,55 @@ efx_mcdi_get_parser_disp_info(
 
__checkReturn   efx_rc_t
 ef10_filter_supported_filters(
-   __inefx_nic_t *enp,
-   __out   uint32_t *list,
-   __out   size_t *length)
+   __inefx_nic_t *enp,
+   __out_ecount(buffer_length) uint32_t *buffer,
+   __insize_t buffer_length,
+   __out   size_t *list_lengthp)
 {
-   efx_rc_t rc;
 
-   if ((rc = efx_mcdi_get_parser_disp_info(enp, list, length)) != 0)
+   size_t mcdi_list_length;
+   size_t list_length;
+   uint32_t i;
+   efx_rc_t rc;
+   uint32_t all_filter_flags =
+   (EFX_FILTER_MATCH_REM_HOST | EFX_FILTER_MATCH_LOC_HOST |
+   EFX_FILTER_MATCH_REM_MAC | EFX_FILTER_MATCH_REM_PORT |
+   EFX_FILTER_MATCH_LOC_MAC | EFX_FILTER_MATCH_LOC_PORT |
+   EFX_FILTER_MATCH_ETHER_TYPE | EFX_FILTER_MATCH_INNER_VID |
+   EFX_FILTER_MATCH_OUTER_VID | EFX_FILTER_MATCH_IP_PROTO |
+   EFX_FILTER_MATCH_UNKNOWN_MCAST_DST |
+   EFX_FILTER_MATCH_UNKNOWN_UCAST_DST);
+
+   rc = efx_mcdi_get_parser_disp_info(enp, buffer, buffer_length,
+   &mcdi_list_length);
+   if (rc != 0) {
+   if (rc == ENOSPC) {
+   /* Pass through mcdi_list_length for the list length */
+   *list_lengthp = mcdi_list_length;
+   }
goto fail1;
+   }
+
+   /*
+* The static asse

[dpdk-dev] [PATCH v2 03/11] net/sfc: implement dummy filter control callback

[Andrew Rybchenko]

Just log error for all filter types and return no support indication.

Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
Reviewed-by: Andy Moreton 
Reviewed-by: Robert Stonehouse 
---
 drivers/net/sfc/sfc_ethdev.c | 49 
 1 file changed, 49 insertions(+)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index cac01ac..e696dd2 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1208,6 +1208,54 @@ sfc_dev_rss_reta_update(struct rte_eth_dev *dev,
 }
 #endif
 
+static int
+sfc_dev_filter_ctrl(struct rte_eth_dev *dev, enum rte_filter_type filter_type,
+   __rte_unused enum rte_filter_op filter_op,
+   __rte_unused void *arg)
+{
+   struct sfc_adapter *sa = dev->data->dev_private;
+   int rc = ENOTSUP;
+
+   sfc_log_init(sa, "entry");
+
+   switch (filter_type) {
+   case RTE_ETH_FILTER_NONE:
+   sfc_err(sa, "Global filters configuration not supported");
+   break;
+   case RTE_ETH_FILTER_MACVLAN:
+   sfc_err(sa, "MACVLAN filters not supported");
+   break;
+   case RTE_ETH_FILTER_ETHERTYPE:
+   sfc_err(sa, "EtherType filters not supported");
+   break;
+   case RTE_ETH_FILTER_FLEXIBLE:
+   sfc_err(sa, "Flexible filters not supported");
+   break;
+   case RTE_ETH_FILTER_SYN:
+   sfc_err(sa, "SYN filters not supported");
+   break;
+   case RTE_ETH_FILTER_NTUPLE:
+   sfc_err(sa, "NTUPLE filters not supported");
+   break;
+   case RTE_ETH_FILTER_TUNNEL:
+   sfc_err(sa, "Tunnel filters not supported");
+   break;
+   case RTE_ETH_FILTER_FDIR:
+   sfc_err(sa, "Flow Director filters not supported");
+   break;
+   case RTE_ETH_FILTER_HASH:
+   sfc_err(sa, "Hash filters not supported");
+   break;
+   default:
+   sfc_err(sa, "Unknown filter type %u", filter_type);
+   break;
+   }
+
+   sfc_log_init(sa, "exit: %d", -rc);
+   SFC_ASSERT(rc >= 0);
+   return -rc;
+}
+
 static const struct eth_dev_ops sfc_eth_dev_ops = {
.dev_configure  = sfc_dev_configure,
.dev_start  = sfc_dev_start,
@@ -1247,6 +1295,7 @@ static const struct eth_dev_ops sfc_eth_dev_ops = {
.rss_hash_update= sfc_dev_rss_hash_update,
.rss_hash_conf_get  = sfc_dev_rss_hash_conf_get,
 #endif
+   .filter_ctrl= sfc_dev_filter_ctrl,
.set_mc_addr_list   = sfc_set_mc_addr_list,
.rxq_info_get   = sfc_rx_queue_info_get,
.txq_info_get   = sfc_tx_queue_info_get,
-- 
2.9.3

[dpdk-dev] [PATCH v2 00/11] Support flow API in Solarflare PMD

[Andrew Rybchenko]

Support simple queue destination flow API filters in Solarflare
libefx-based PMD including:
 - Ethernet source/destination, EtherType exact matching
 - VLAN ID exact matching including double-tagging
 - IPv4/6 source/destination and IP protocol exact matching
 - TCP/UDP source/destination exact matching

Supported combinations of fields mentioned above depend on
firmware (including running variant) and correctly processed by
validate callback.

v1 -> v2:
 - fix spelling
 - add comments to protocol parsers
 - add release notes
 - add l3 and l4 layer enum items on demand

Andrew Rybchenko (2):
  net/sfc: implement dummy filter control callback
  net/sfc: provide a way to check if filter is supported

Mark Spender (2):
  net/sfc/base: split local MAC I/G back into separate flags
  net/sfc/base: improve API to get supported filter matches

Roman Zhukov (7):
  net/sfc: add flow API filters support
  net/sfc: add VLAN in flow API filters support
  net/sfc: add IPV4 in flow API filters support
  net/sfc: add IPV6 in flow API filters support
  net/sfc: add TCP in flow API filters support
  net/sfc: add UDP in flow API filters support
  net/sfc: add unknown unicast/multicast match in flow API

 doc/guides/nics/features/sfc_efx.ini   |1 +
 doc/guides/nics/sfc_efx.rst|   45 ++
 doc/guides/rel_notes/release_17_05.rst |4 +
 drivers/net/sfc/Makefile   |2 +
 drivers/net/sfc/base/ef10_filter.c |  134 ++--
 drivers/net/sfc/base/ef10_impl.h   |7 +-
 drivers/net/sfc/base/efx.h |   17 +-
 drivers/net/sfc/base/efx_filter.c  |   76 +-
 drivers/net/sfc/base/efx_impl.h|3 +-
 drivers/net/sfc/sfc.c  |   18 +
 drivers/net/sfc/sfc.h  |3 +
 drivers/net/sfc/sfc_ethdev.c   |   59 +-
 drivers/net/sfc/sfc_filter.c   |  135 
 drivers/net/sfc/sfc_filter.h   |   60 ++
 drivers/net/sfc/sfc_flow.c | 1181 
 drivers/net/sfc/sfc_flow.h |   62 ++
 16 files changed, 1711 insertions(+), 96 deletions(-)
 create mode 100644 drivers/net/sfc/sfc_filter.c
 create mode 100644 drivers/net/sfc/sfc_filter.h
 create mode 100644 drivers/net/sfc/sfc_flow.c
 create mode 100644 drivers/net/sfc/sfc_flow.h

-- 
2.9.3

[dpdk-dev] [PATCH v2 09/11] net/sfc: add TCP in flow API filters support

[Andrew Rybchenko]

From: Roman Zhukov 

Exact match of source and destination ports is supported by parser.
IP protocol match is enforced to TCP.

Signed-off-by: Roman Zhukov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
---
 doc/guides/nics/sfc_efx.rst |  2 +
 drivers/net/sfc/sfc_flow.c  | 89 +
 2 files changed, 91 insertions(+)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index a4ea162..7e00fdc 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -137,6 +137,8 @@ Supported pattern items:
 - IPV6 (exact match of source/destination addresses,
   IP transport protocol)
 
+- TCP (exact match of source/destination ports)
+
 Supported actions:
 
 - VOID
diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
index 1079ca4..38e6b73 100644
--- a/drivers/net/sfc/sfc_flow.c
+++ b/drivers/net/sfc/sfc_flow.c
@@ -56,6 +56,7 @@ enum sfc_flow_item_layers {
SFC_FLOW_ITEM_START_LAYER,
SFC_FLOW_ITEM_L2,
SFC_FLOW_ITEM_L3,
+   SFC_FLOW_ITEM_L4,
 };
 
 typedef int (sfc_flow_item_parse)(const struct rte_flow_item *item,
@@ -74,6 +75,7 @@ static sfc_flow_item_parse sfc_flow_parse_eth;
 static sfc_flow_item_parse sfc_flow_parse_vlan;
 static sfc_flow_item_parse sfc_flow_parse_ipv4;
 static sfc_flow_item_parse sfc_flow_parse_ipv6;
+static sfc_flow_item_parse sfc_flow_parse_tcp;
 
 static boolean_t
 sfc_flow_is_zero(const uint8_t *buf, unsigned int size)
@@ -538,6 +540,87 @@ sfc_flow_parse_ipv6(const struct rte_flow_item *item,
return -rte_errno;
 }
 
+/**
+ * Convert TCP item to EFX filter specification.
+ *
+ * @param item[in]
+ *   Item specification. Only source and destination ports fields
+ *   are supported. If the mask is NULL, default mask will be used.
+ *   Ranging is not supported.
+ * @param efx_spec[in, out]
+ *   EFX filter specification to update.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ */
+static int
+sfc_flow_parse_tcp(const struct rte_flow_item *item,
+  efx_filter_spec_t *efx_spec,
+  struct rte_flow_error *error)
+{
+   int rc;
+   const struct rte_flow_item_tcp *spec = NULL;
+   const struct rte_flow_item_tcp *mask = NULL;
+   const struct rte_flow_item_tcp supp_mask = {
+   .hdr = {
+   .src_port = 0x,
+   .dst_port = 0x,
+   }
+   };
+
+   rc = sfc_flow_parse_init(item,
+(const void **)&spec,
+(const void **)&mask,
+&supp_mask,
+&rte_flow_item_tcp_mask,
+sizeof(struct rte_flow_item_tcp),
+error);
+   if (rc != 0)
+   return rc;
+
+   /*
+* Filtering by TCP source and destination ports requires
+* the appropriate IP_PROTO in hardware filters
+*/
+   if (!(efx_spec->efs_match_flags & EFX_FILTER_MATCH_IP_PROTO)) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_IP_PROTO;
+   efx_spec->efs_ip_proto = EFX_IPPROTO_TCP;
+   } else if (efx_spec->efs_ip_proto != EFX_IPPROTO_TCP) {
+   rte_flow_error_set(error, EINVAL,
+   RTE_FLOW_ERROR_TYPE_ITEM, item,
+   "IP proto in pattern with TCP item should be 
appropriate");
+   return -rte_errno;
+   }
+
+   if (spec == NULL)
+   return 0;
+
+   /*
+* Source and destination ports are in big-endian byte order in item and
+* in little-endian in efx_spec, so byte swap is used
+*/
+   if (mask->hdr.src_port == supp_mask.hdr.src_port) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_REM_PORT;
+   efx_spec->efs_rem_port = rte_bswap16(spec->hdr.src_port);
+   } else if (mask->hdr.src_port != 0) {
+   goto fail_bad_mask;
+   }
+
+   if (mask->hdr.dst_port == supp_mask.hdr.dst_port) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_LOC_PORT;
+   efx_spec->efs_loc_port = rte_bswap16(spec->hdr.dst_port);
+   } else if (mask->hdr.dst_port != 0) {
+   goto fail_bad_mask;
+   }
+
+   return 0;
+
+fail_bad_mask:
+   rte_flow_error_set(error, EINVAL,
+  RTE_FLOW_ERROR_TYPE_ITEM, item,
+  "Bad mask in the TCP pattern item");
+   return -rte_errno;
+}
+
 static const struct sfc_flow_item sfc_flow_items[] = {
{
.type = RTE_FLOW_ITEM_TYPE_VOID,
@@ -569,6 +652,12 @@ static const struct sfc_flow_item sfc_flow_items[] = {
.layer = SFC_FLOW_ITEM_L3,
.parse = sfc_flow_parse_ipv6,
},
+   {
+   .type = RTE_FLOW_ITEM_TYPE_TCP,
+   .prev_layer = SFC_FLOW_ITEM_L3,
+   .l

[dpdk-dev] [PATCH v2 05/11] net/sfc: add flow API filters support

[Andrew Rybchenko]

From: Roman Zhukov 

Only pattern items VOID, ETH and actions VOID, QUEUE is now
supported.

Signed-off-by: Roman Zhukov 
Signed-off-by: Andrew Rybchenko 
---
 doc/guides/nics/features/sfc_efx.ini   |   1 +
 doc/guides/nics/sfc_efx.rst|  24 ++
 doc/guides/rel_notes/release_17_05.rst |   4 +
 drivers/net/sfc/Makefile   |   1 +
 drivers/net/sfc/sfc.c  |  11 +
 drivers/net/sfc/sfc_ethdev.c   |  14 +-
 drivers/net/sfc/sfc_filter.h   |   4 +
 drivers/net/sfc/sfc_flow.c | 704 +
 drivers/net/sfc/sfc_flow.h |  62 +++
 9 files changed, 822 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/sfc/sfc_flow.c
 create mode 100644 drivers/net/sfc/sfc_flow.h

diff --git a/doc/guides/nics/features/sfc_efx.ini 
b/doc/guides/nics/features/sfc_efx.ini
index 3a15baa..bb60ad6 100644
--- a/doc/guides/nics/features/sfc_efx.ini
+++ b/doc/guides/nics/features/sfc_efx.ini
@@ -19,6 +19,7 @@ RSS hash = Y
 RSS key update   = Y
 RSS reta update  = Y
 Flow control = Y
+Flow API = Y
 VLAN offload = P
 L3 checksum offload  = Y
 L4 checksum offload  = Y
diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 0a05a0a..f2e410f 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -81,6 +81,8 @@ SFC EFX PMD has support for:
 
 - Transmit VLAN insertion (if running firmware variant supports it)
 
+- Flow API
+
 
 Non-supported Features
 --
@@ -114,6 +116,28 @@ required in the receive buffer.
 It should be taken into account when mbuf pool for receive is created.
 
 
+Flow API support
+
+
+Supported attributes:
+
+- Ingress
+
+Supported pattern items:
+
+- VOID
+
+- ETH (exact match of source/destination addresses, EtherType)
+
+Supported actions:
+
+- VOID
+
+- QUEUE
+
+Validating flow rules depends on the firmware variant.
+
+
 Supported NICs
 --
 
diff --git a/doc/guides/rel_notes/release_17_05.rst 
b/doc/guides/rel_notes/release_17_05.rst
index 123eeb4..3ab4a9b 100644
--- a/doc/guides/rel_notes/release_17_05.rst
+++ b/doc/guides/rel_notes/release_17_05.rst
@@ -64,6 +64,10 @@ New Features
   performance enhancements viz. configurable TX data ring, Receive
   Data Ring, ability to register memory regions.
 
+* **Updated the sfc_efx driver.**
+
+  * Generic flow API support for Ethernet, VLAN, IPv4, IPv6, UDP and TCP
+pattern items with QUEUE action for ingress traffic.
 
 Resolved Issues
 ---
diff --git a/drivers/net/sfc/Makefile b/drivers/net/sfc/Makefile
index 2b7ede8..e1ee04c 100644
--- a/drivers/net/sfc/Makefile
+++ b/drivers/net/sfc/Makefile
@@ -91,6 +91,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_rx.c
 SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_tx.c
 SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_tso.c
 SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_filter.c
+SRCS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc_flow.c
 
 VPATH += $(SRCDIR)/base
 
diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index 6fd8bf1..3e419b6 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -320,10 +320,17 @@ sfc_start(struct sfc_adapter *sa)
if (rc != 0)
goto fail_tx_start;
 
+   rc = sfc_flow_start(sa);
+   if (rc != 0)
+   goto fail_flows_insert;
+
sa->state = SFC_ADAPTER_STARTED;
sfc_log_init(sa, "done");
return 0;
 
+fail_flows_insert:
+   sfc_tx_stop(sa);
+
 fail_tx_start:
sfc_rx_stop(sa);
 
@@ -368,6 +375,7 @@ sfc_stop(struct sfc_adapter *sa)
 
sa->state = SFC_ADAPTER_STOPPING;
 
+   sfc_flow_stop(sa);
sfc_tx_stop(sa);
sfc_rx_stop(sa);
sfc_port_stop(sa);
@@ -640,6 +648,8 @@ sfc_attach(struct sfc_adapter *sa)
sfc_log_init(sa, "fini nic");
efx_nic_fini(enp);
 
+   sfc_flow_init(sa);
+
sa->state = SFC_ADAPTER_INITIALIZED;
 
sfc_log_init(sa, "done");
@@ -698,5 +708,6 @@ sfc_detach(struct sfc_adapter *sa)
 
sfc_mem_bar_fini(sa);
 
+   sfc_flow_fini(sa);
sa->state = SFC_ADAPTER_UNINITIALIZED;
 }
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index e696dd2..5297159 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -40,7 +40,7 @@
 #include "sfc_ev.h"
 #include "sfc_rx.h"
 #include "sfc_tx.h"
-
+#include "sfc_flow.h"
 
 static void
 sfc_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
@@ -1210,8 +1210,8 @@ sfc_dev_rss_reta_update(struct rte_eth_dev *dev,
 
 static int
 sfc_dev_filter_ctrl(struct rte_eth_dev *dev, enum rte_filter_type filter_type,
-   __rte_unused enum rte_filter_op filter_op,
-   __rte_unused void *arg)
+   enum rte_filter_op filter_op,
+   void *arg)
 {
struct sfc_adapter *sa = dev->data->dev_private;
int rc = ENOTSUP;
@@ -1

[dpdk-dev] [PATCH v2 11/11] net/sfc: add unknown unicast/multicast match in flow API

[Andrew Rybchenko]

From: Roman Zhukov 

Support individual/group destination address match (unknown unicast
and all-multicast correspondingly in terms of firmware).

Signed-off-by: Roman Zhukov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
---
 doc/guides/nics/sfc_efx.rst | 11 ++-
 drivers/net/sfc/sfc_flow.c  | 17 +++--
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 4d91b1d..8229d7d 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -127,7 +127,8 @@ Supported pattern items:
 
 - VOID
 
-- ETH (exact match of source/destination addresses, EtherType)
+- ETH (exact match of source/destination addresses, individual/group match
+  of destination address, EtherType)
 
 - VLAN (exact match of VID, double-tagging is supported)
 
@@ -149,6 +150,14 @@ Supported actions:
 
 Validating flow rules depends on the firmware variant.
 
+Ethernet destinaton individual/group match
+~~
+
+Ethernet item supports I/G matching, if only the corresponding bit is set
+in the mask of destination address. If destinaton address in the spec is
+multicast, it matches all multicast (and broadcast) packets, oherwise it
+matches unicast packets that are not filtered by other flow rules.
+
 
 Supported NICs
 --
diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
index ae36934..fe352c0 100644
--- a/drivers/net/sfc/sfc_flow.c
+++ b/drivers/net/sfc/sfc_flow.c
@@ -205,8 +205,10 @@ sfc_flow_parse_void(__rte_unused const struct 
rte_flow_item *item,
  *
  * @param item[in]
  *   Item specification. Only source and destination addresses and
- *   Ethernet type fields are supported. If the mask is NULL, default
- *   mask will be used. Ranging is not supported.
+ *   Ethernet type fields are supported. In addition to full and
+ *   empty masks of destination address, individual/group mask is
+ *   also supported. If the mask is NULL, default mask will be used.
+ *   Ranging is not supported.
  * @param efx_spec[in, out]
  *   EFX filter specification to update.
  * @param[out] error
@@ -225,6 +227,9 @@ sfc_flow_parse_eth(const struct rte_flow_item *item,
.src.addr_bytes = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff },
.type = 0x,
};
+   const uint8_t ig_mask[EFX_MAC_ADDR_LEN] = {
+   0x01, 0x00, 0x00, 0x00, 0x00, 0x00
+   };
 
rc = sfc_flow_parse_init(item,
 (const void **)&spec,
@@ -244,6 +249,14 @@ sfc_flow_parse_eth(const struct rte_flow_item *item,
efx_spec->efs_match_flags |= EFX_FILTER_MATCH_LOC_MAC;
rte_memcpy(efx_spec->efs_loc_mac, spec->dst.addr_bytes,
   EFX_MAC_ADDR_LEN);
+   } else if (memcmp(mask->dst.addr_bytes, ig_mask,
+ EFX_MAC_ADDR_LEN) == 0) {
+   if (is_unicast_ether_addr(&spec->dst))
+   efx_spec->efs_match_flags |=
+   EFX_FILTER_MATCH_UNKNOWN_UCAST_DST;
+   else
+   efx_spec->efs_match_flags |=
+   EFX_FILTER_MATCH_UNKNOWN_MCAST_DST;
} else if (!is_zero_ether_addr(&mask->dst)) {
goto fail_bad_mask;
}
-- 
2.9.3

[dpdk-dev] [PATCH v2 07/11] net/sfc: add IPV4 in flow API filters support

[Andrew Rybchenko]

From: Roman Zhukov 

Exact match of IP protocol, source and destination
addresses is supported by parser.
EtherType match is enforced to IPv4 EtherType.

Signed-off-by: Roman Zhukov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
---
 doc/guides/nics/sfc_efx.rst |  3 ++
 drivers/net/sfc/sfc_flow.c  | 98 +
 2 files changed, 101 insertions(+)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 71dc99f..12ac308 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -131,6 +131,9 @@ Supported pattern items:
 
 - VLAN (exact match of VID, double-tagging is supported)
 
+- IPV4 (exact match of source/destination addresses,
+  IP transport protocol)
+
 Supported actions:
 
 - VOID
diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
index 70d926f..05be5a5 100644
--- a/drivers/net/sfc/sfc_flow.c
+++ b/drivers/net/sfc/sfc_flow.c
@@ -55,6 +55,7 @@ enum sfc_flow_item_layers {
SFC_FLOW_ITEM_ANY_LAYER,
SFC_FLOW_ITEM_START_LAYER,
SFC_FLOW_ITEM_L2,
+   SFC_FLOW_ITEM_L3,
 };
 
 typedef int (sfc_flow_item_parse)(const struct rte_flow_item *item,
@@ -71,6 +72,7 @@ struct sfc_flow_item {
 static sfc_flow_item_parse sfc_flow_parse_void;
 static sfc_flow_item_parse sfc_flow_parse_eth;
 static sfc_flow_item_parse sfc_flow_parse_vlan;
+static sfc_flow_item_parse sfc_flow_parse_ipv4;
 
 static boolean_t
 sfc_flow_is_zero(const uint8_t *buf, unsigned int size)
@@ -337,6 +339,96 @@ sfc_flow_parse_vlan(const struct rte_flow_item *item,
return 0;
 }
 
+/**
+ * Convert IPv4 item to EFX filter specification.
+ *
+ * @param item[in]
+ *   Item specification. Only source and destination addresses and
+ *   protocol fields are supported. If the mask is NULL, default
+ *   mask will be used. Ranging is not supported.
+ * @param efx_spec[in, out]
+ *   EFX filter specification to update.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ */
+static int
+sfc_flow_parse_ipv4(const struct rte_flow_item *item,
+   efx_filter_spec_t *efx_spec,
+   struct rte_flow_error *error)
+{
+   int rc;
+   const struct rte_flow_item_ipv4 *spec = NULL;
+   const struct rte_flow_item_ipv4 *mask = NULL;
+   const uint16_t ether_type_ipv4 = rte_cpu_to_le_16(EFX_ETHER_TYPE_IPV4);
+   const struct rte_flow_item_ipv4 supp_mask = {
+   .hdr = {
+   .src_addr = 0x,
+   .dst_addr = 0x,
+   .next_proto_id = 0xff,
+   }
+   };
+
+   rc = sfc_flow_parse_init(item,
+(const void **)&spec,
+(const void **)&mask,
+&supp_mask,
+&rte_flow_item_ipv4_mask,
+sizeof(struct rte_flow_item_ipv4),
+error);
+   if (rc != 0)
+   return rc;
+
+   /*
+* Filtering by IPv4 source and destination addresses requires
+* the appropriate ETHER_TYPE in hardware filters
+*/
+   if (!(efx_spec->efs_match_flags & EFX_FILTER_MATCH_ETHER_TYPE)) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_ETHER_TYPE;
+   efx_spec->efs_ether_type = ether_type_ipv4;
+   } else if (efx_spec->efs_ether_type != ether_type_ipv4) {
+   rte_flow_error_set(error, EINVAL,
+   RTE_FLOW_ERROR_TYPE_ITEM, item,
+   "Ethertype in pattern with IPV4 item should be 
appropriate");
+   return -rte_errno;
+   }
+
+   if (spec == NULL)
+   return 0;
+
+   /*
+* IPv4 addresses are in big-endian byte order in item and in
+* efx_spec
+*/
+   if (mask->hdr.src_addr == supp_mask.hdr.src_addr) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_REM_HOST;
+   efx_spec->efs_rem_host.eo_u32[0] = spec->hdr.src_addr;
+   } else if (mask->hdr.src_addr != 0) {
+   goto fail_bad_mask;
+   }
+
+   if (mask->hdr.dst_addr == supp_mask.hdr.dst_addr) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_LOC_HOST;
+   efx_spec->efs_loc_host.eo_u32[0] = spec->hdr.dst_addr;
+   } else if (mask->hdr.dst_addr != 0) {
+   goto fail_bad_mask;
+   }
+
+   if (mask->hdr.next_proto_id == supp_mask.hdr.next_proto_id) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_IP_PROTO;
+   efx_spec->efs_ip_proto = spec->hdr.next_proto_id;
+   } else if (mask->hdr.next_proto_id != 0) {
+   goto fail_bad_mask;
+   }
+
+   return 0;
+
+fail_bad_mask:
+   rte_flow_error_set(error, EINVAL,
+  RTE_FLOW_ERROR_TYPE_ITEM, item,
+  "Bad mask in the IPV4 pattern item");
+  

[dpdk-dev] [PATCH v2 10/11] net/sfc: add UDP in flow API filters support

[Andrew Rybchenko]

From: Roman Zhukov 

Exact match of source and destination ports is supported by parser.
IP protocol match is enforced to UDP.

Signed-off-by: Roman Zhukov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
---
 doc/guides/nics/sfc_efx.rst |  2 ++
 drivers/net/sfc/sfc_flow.c  | 88 +
 2 files changed, 90 insertions(+)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 7e00fdc..4d91b1d 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -139,6 +139,8 @@ Supported pattern items:
 
 - TCP (exact match of source/destination ports)
 
+- UDP (exact match of source/destination ports)
+
 Supported actions:
 
 - VOID
diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
index 38e6b73..ae36934 100644
--- a/drivers/net/sfc/sfc_flow.c
+++ b/drivers/net/sfc/sfc_flow.c
@@ -76,6 +76,7 @@ static sfc_flow_item_parse sfc_flow_parse_vlan;
 static sfc_flow_item_parse sfc_flow_parse_ipv4;
 static sfc_flow_item_parse sfc_flow_parse_ipv6;
 static sfc_flow_item_parse sfc_flow_parse_tcp;
+static sfc_flow_item_parse sfc_flow_parse_udp;
 
 static boolean_t
 sfc_flow_is_zero(const uint8_t *buf, unsigned int size)
@@ -621,6 +622,87 @@ sfc_flow_parse_tcp(const struct rte_flow_item *item,
return -rte_errno;
 }
 
+/**
+ * Convert UDP item to EFX filter specification.
+ *
+ * @param item[in]
+ *   Item specification. Only source and destination ports fields
+ *   are supported. If the mask is NULL, default mask will be used.
+ *   Ranging is not supported.
+ * @param efx_spec[in, out]
+ *   EFX filter specification to update.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ */
+static int
+sfc_flow_parse_udp(const struct rte_flow_item *item,
+  efx_filter_spec_t *efx_spec,
+  struct rte_flow_error *error)
+{
+   int rc;
+   const struct rte_flow_item_udp *spec = NULL;
+   const struct rte_flow_item_udp *mask = NULL;
+   const struct rte_flow_item_udp supp_mask = {
+   .hdr = {
+   .src_port = 0x,
+   .dst_port = 0x,
+   }
+   };
+
+   rc = sfc_flow_parse_init(item,
+(const void **)&spec,
+(const void **)&mask,
+&supp_mask,
+&rte_flow_item_udp_mask,
+sizeof(struct rte_flow_item_udp),
+error);
+   if (rc != 0)
+   return rc;
+
+   /*
+* Filtering by UDP source and destination ports requires
+* the appropriate IP_PROTO in hardware filters
+*/
+   if (!(efx_spec->efs_match_flags & EFX_FILTER_MATCH_IP_PROTO)) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_IP_PROTO;
+   efx_spec->efs_ip_proto = EFX_IPPROTO_UDP;
+   } else if (efx_spec->efs_ip_proto != EFX_IPPROTO_UDP) {
+   rte_flow_error_set(error, EINVAL,
+   RTE_FLOW_ERROR_TYPE_ITEM, item,
+   "IP proto in pattern with UDP item should be 
appropriate");
+   return -rte_errno;
+   }
+
+   if (spec == NULL)
+   return 0;
+
+   /*
+* Source and destination ports are in big-endian byte order in item and
+* in little-endian in efx_spec, so byte swap is used
+*/
+   if (mask->hdr.src_port == supp_mask.hdr.src_port) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_REM_PORT;
+   efx_spec->efs_rem_port = rte_bswap16(spec->hdr.src_port);
+   } else if (mask->hdr.src_port != 0) {
+   goto fail_bad_mask;
+   }
+
+   if (mask->hdr.dst_port == supp_mask.hdr.dst_port) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_LOC_PORT;
+   efx_spec->efs_loc_port = rte_bswap16(spec->hdr.dst_port);
+   } else if (mask->hdr.dst_port != 0) {
+   goto fail_bad_mask;
+   }
+
+   return 0;
+
+fail_bad_mask:
+   rte_flow_error_set(error, EINVAL,
+  RTE_FLOW_ERROR_TYPE_ITEM, item,
+  "Bad mask in the UDP pattern item");
+   return -rte_errno;
+}
+
 static const struct sfc_flow_item sfc_flow_items[] = {
{
.type = RTE_FLOW_ITEM_TYPE_VOID,
@@ -658,6 +740,12 @@ static const struct sfc_flow_item sfc_flow_items[] = {
.layer = SFC_FLOW_ITEM_L4,
.parse = sfc_flow_parse_tcp,
},
+   {
+   .type = RTE_FLOW_ITEM_TYPE_UDP,
+   .prev_layer = SFC_FLOW_ITEM_L3,
+   .layer = SFC_FLOW_ITEM_L4,
+   .parse = sfc_flow_parse_udp,
+   },
 };
 
 /*
-- 
2.9.3

[dpdk-dev] [PATCH v2 06/11] net/sfc: add VLAN in flow API filters support

[Andrew Rybchenko]

From: Roman Zhukov 

Exact match of VLAN ID bits is supported only and required in VLAN item.
Mask to match VLAN ID bits only is required, default mask to match entire
TCI is not supported.

Signed-off-by: Roman Zhukov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
---
 doc/guides/nics/sfc_efx.rst |  2 ++
 drivers/net/sfc/sfc_flow.c  | 74 +
 2 files changed, 76 insertions(+)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index f2e410f..71dc99f 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -129,6 +129,8 @@ Supported pattern items:
 
 - ETH (exact match of source/destination addresses, EtherType)
 
+- VLAN (exact match of VID, double-tagging is supported)
+
 Supported actions:
 
 - VOID
diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
index 6b20bae..70d926f 100644
--- a/drivers/net/sfc/sfc_flow.c
+++ b/drivers/net/sfc/sfc_flow.c
@@ -70,6 +70,7 @@ struct sfc_flow_item {
 
 static sfc_flow_item_parse sfc_flow_parse_void;
 static sfc_flow_item_parse sfc_flow_parse_eth;
+static sfc_flow_item_parse sfc_flow_parse_vlan;
 
 static boolean_t
 sfc_flow_is_zero(const uint8_t *buf, unsigned int size)
@@ -269,6 +270,73 @@ sfc_flow_parse_eth(const struct rte_flow_item *item,
return -rte_errno;
 }
 
+/**
+ * Convert VLAN item to EFX filter specification.
+ *
+ * @param item[in]
+ *   Item specification. Only VID field is supported.
+ *   The mask can not be NULL. Ranging is not supported.
+ * @param efx_spec[in, out]
+ *   EFX filter specification to update.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ */
+static int
+sfc_flow_parse_vlan(const struct rte_flow_item *item,
+   efx_filter_spec_t *efx_spec,
+   struct rte_flow_error *error)
+{
+   int rc;
+   uint16_t vid;
+   const struct rte_flow_item_vlan *spec = NULL;
+   const struct rte_flow_item_vlan *mask = NULL;
+   const struct rte_flow_item_vlan supp_mask = {
+   .tci = rte_cpu_to_be_16(ETH_VLAN_ID_MAX),
+   };
+
+   rc = sfc_flow_parse_init(item,
+(const void **)&spec,
+(const void **)&mask,
+&supp_mask,
+NULL,
+sizeof(struct rte_flow_item_vlan),
+error);
+   if (rc != 0)
+   return rc;
+
+   /*
+* VID is in big-endian byte order in item and
+* in little-endian in efx_spec, so byte swap is used.
+* If two VLAN items are included, the first matches
+* the outer tag and the next matches the inner tag.
+*/
+   if (mask->tci == supp_mask.tci) {
+   vid = rte_bswap16(spec->tci);
+
+   if (!(efx_spec->efs_match_flags &
+ EFX_FILTER_MATCH_OUTER_VID)) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_OUTER_VID;
+   efx_spec->efs_outer_vid = vid;
+   } else if (!(efx_spec->efs_match_flags &
+EFX_FILTER_MATCH_INNER_VID)) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_INNER_VID;
+   efx_spec->efs_inner_vid = vid;
+   } else {
+   rte_flow_error_set(error, EINVAL,
+  RTE_FLOW_ERROR_TYPE_ITEM, item,
+  "More than two VLAN items");
+   return -rte_errno;
+   }
+   } else {
+   rte_flow_error_set(error, EINVAL,
+  RTE_FLOW_ERROR_TYPE_ITEM, item,
+  "VLAN ID in TCI match is required");
+   return -rte_errno;
+   }
+
+   return 0;
+}
+
 static const struct sfc_flow_item sfc_flow_items[] = {
{
.type = RTE_FLOW_ITEM_TYPE_VOID,
@@ -282,6 +350,12 @@ static const struct sfc_flow_item sfc_flow_items[] = {
.layer = SFC_FLOW_ITEM_L2,
.parse = sfc_flow_parse_eth,
},
+   {
+   .type = RTE_FLOW_ITEM_TYPE_VLAN,
+   .prev_layer = SFC_FLOW_ITEM_L2,
+   .layer = SFC_FLOW_ITEM_L2,
+   .parse = sfc_flow_parse_vlan,
+   },
 };
 
 /*
-- 
2.9.3

[dpdk-dev] [PATCH v2 08/11] net/sfc: add IPV6 in flow API filters support

[Andrew Rybchenko]

From: Roman Zhukov 

Exact match of IP protocol, source and destination
addresses is supported by parser.
EtherType match is enforced to IPv6 EtherType.

Signed-off-by: Roman Zhukov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
---
 doc/guides/nics/sfc_efx.rst |   3 ++
 drivers/net/sfc/sfc_flow.c  | 115 
 2 files changed, 118 insertions(+)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 12ac308..a4ea162 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -134,6 +134,9 @@ Supported pattern items:
 - IPV4 (exact match of source/destination addresses,
   IP transport protocol)
 
+- IPV6 (exact match of source/destination addresses,
+  IP transport protocol)
+
 Supported actions:
 
 - VOID
diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
index 05be5a5..1079ca4 100644
--- a/drivers/net/sfc/sfc_flow.c
+++ b/drivers/net/sfc/sfc_flow.c
@@ -73,6 +73,7 @@ static sfc_flow_item_parse sfc_flow_parse_void;
 static sfc_flow_item_parse sfc_flow_parse_eth;
 static sfc_flow_item_parse sfc_flow_parse_vlan;
 static sfc_flow_item_parse sfc_flow_parse_ipv4;
+static sfc_flow_item_parse sfc_flow_parse_ipv6;
 
 static boolean_t
 sfc_flow_is_zero(const uint8_t *buf, unsigned int size)
@@ -429,6 +430,114 @@ sfc_flow_parse_ipv4(const struct rte_flow_item *item,
return -rte_errno;
 }
 
+/**
+ * Convert IPv6 item to EFX filter specification.
+ *
+ * @param item[in]
+ *   Item specification. Only source and destination addresses and
+ *   next header fields are supported. If the mask is NULL, default
+ *   mask will be used. Ranging is not supported.
+ * @param efx_spec[in, out]
+ *   EFX filter specification to update.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ */
+static int
+sfc_flow_parse_ipv6(const struct rte_flow_item *item,
+   efx_filter_spec_t *efx_spec,
+   struct rte_flow_error *error)
+{
+   int rc;
+   const struct rte_flow_item_ipv6 *spec = NULL;
+   const struct rte_flow_item_ipv6 *mask = NULL;
+   const uint16_t ether_type_ipv6 = rte_cpu_to_le_16(EFX_ETHER_TYPE_IPV6);
+   const struct rte_flow_item_ipv6 supp_mask = {
+   .hdr = {
+   .src_addr = { 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff },
+   .dst_addr = { 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff },
+   .proto = 0xff,
+   }
+   };
+
+   rc = sfc_flow_parse_init(item,
+(const void **)&spec,
+(const void **)&mask,
+&supp_mask,
+&rte_flow_item_ipv6_mask,
+sizeof(struct rte_flow_item_ipv6),
+error);
+   if (rc != 0)
+   return rc;
+
+   /*
+* Filtering by IPv6 source and destination addresses requires
+* the appropriate ETHER_TYPE in hardware filters
+*/
+   if (!(efx_spec->efs_match_flags & EFX_FILTER_MATCH_ETHER_TYPE)) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_ETHER_TYPE;
+   efx_spec->efs_ether_type = ether_type_ipv6;
+   } else if (efx_spec->efs_ether_type != ether_type_ipv6) {
+   rte_flow_error_set(error, EINVAL,
+   RTE_FLOW_ERROR_TYPE_ITEM, item,
+   "Ethertype in pattern with IPV6 item should be 
appropriate");
+   return -rte_errno;
+   }
+
+   if (spec == NULL)
+   return 0;
+
+   /*
+* IPv6 addresses are in big-endian byte order in item and in
+* efx_spec
+*/
+   if (memcmp(mask->hdr.src_addr, supp_mask.hdr.src_addr,
+  sizeof(mask->hdr.src_addr)) == 0) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_REM_HOST;
+
+   RTE_BUILD_BUG_ON(sizeof(efx_spec->efs_rem_host) !=
+sizeof(spec->hdr.src_addr));
+   rte_memcpy(&efx_spec->efs_rem_host, spec->hdr.src_addr,
+  sizeof(efx_spec->efs_rem_host));
+   } else if (!sfc_flow_is_zero(mask->hdr.src_addr,
+sizeof(mask->hdr.src_addr))) {
+   goto fail_bad_mask;
+   }
+
+   if (memcmp(mask->hdr.dst_addr, supp_mask.hdr.dst_addr,
+  sizeof(mask->hdr.dst_addr)) == 0) {
+   efx_spec->efs_match_flags |= EFX_FILTER_MATCH_LOC_HOST;
+
+   RTE_BUILD_BUG_ON(sizeof(efx_spec->efs_loc_host) !=
+  

Re: [dpdk-dev] [PATCH v2] lpm: extend IPv6 next hop field

[Thomas Monjalon]

2017-02-22 12:02, Bruce Richardson:
> On Tue, Feb 21, 2017 at 04:46:39PM +0200, Vladyslav Buslov wrote:
> > This patch extend next_hop field from 8-bits to 21-bits in LPM library
> > for IPv6.
> > 
> > Added versioning symbols to functions and updated
> > library and applications that have a dependency on LPM library.
> > 
> > Signed-off-by: Vladyslav Buslov 
> Acked-by: Bruce Richardson 

There is a compilation error in examples/l3fwd/l3fwd_lpm.h.
The ports/hops must be upgraded to 32-bit in this file.

Re: [dpdk-dev] [PATCH] eal: fix bug in x86 cmpset

[Thomas Monjalon]

2017-02-10 08:46, Stephen Hemminger:
> On Fri, 10 Feb 2017 11:53:06 +0100
> Thomas Monjalon  wrote:
> 
> > 2017-02-10 10:39, Hunt, David:
> > > 
> > > On 9/2/2017 4:53 PM, Thomas Monjalon wrote:  
> > > > 2016-11-06 22:09, Thomas Monjalon:  
> > > >> 2016-09-29 18:34, Thomas Monjalon:  
> > > >>> 2016-09-30 02:54, Nikhil Rao:  
> > >  The original code used movl instead of xchgl, this caused
> > >  rte_atomic64_cmpset to use ebx as the lower dword of the source
> > >  to cmpxchg8b instead of the lower dword of function argument "src".  
> > > >>> Could you please start the explanation with a statement of
> > > >>> what is wrong from an user point of view?
> > > >>> It could help to understand how severe it is.  
> > > >> Please, we need a clear explanation of the bug, and an 
> > > >> acknowledgement.  
> > > > Should we close this bug?  
> > > 
> > > I took a few minutes to look at this, and the issue can easily be 
> > > reproduced with a small snippet of code.
> > > With the 'mov', the lower dword of the result is incorrect. This is 
> > > resolved by using 'xchgl'.
> > > 
> > > void main()
> > > {
> > >  uint64_t a = 0xff00ff;
> > > 
> > >  rte_atomic64_cmpset( &a, 0xff00ff, 0xfa00fa);
> > >  printf("0x%lx\n", a);
> > > }
> > > 
> > > When using 'mov', the result is 0xfa
> > > When using 'xchgl', the result is 0xfa00fa, as expected.  
> > 
> > This operation is used a lot in drivers for link status.
> > 
> > I think we need to clearly explain what was the consequence of this bug.
> 
> 
> A bigger issue is why there are a huge number of copies of the same link code
> in drivers. Definitely should be common code.  Also why is cmpset used here
> when a simple atomic_set would work as well for what was intended.


I'm surprised that there is no progress on this issue.

Re: [dpdk-dev] [ovs-dev] [PATCH] selinux: Allow creating tap devices.

[Aaron Conole]

Aaron Conole  writes:

> Daniele Di Proietto  writes:
>
>> On 26/01/2017 12:35, "Ansis Atteka"  wrote:
>>>
>>>
>>>On 26 January 2017 at 21:24, Aaron Conole 
>>> wrote:
>>>
>>>Daniele Di Proietto  writes:
>>>
 On 25/01/2017 00:01, "Ansis Atteka"  wrote:

>On Jan 25, 2017 4:22 AM, "Daniele Di Proietto"  
>wrote:
>
>Current SELinux policy in RHEL and Fedora doesn't allow the creation of
>TAP devices.
>
>A tap device is used by dpif-netdev to create internal devices.
>
>Without this patch, adding any bridge backed by the userspace datapath
>would fail.
>
>This doesn't mean that we can run Open vSwitch with DPDK under SELinux
>yet, but at least we can use the userspace datapath.
>
>Signed-off-by: Daniele Di Proietto 
>>>
>>>I just noticed this, sorry for jumping in late.
>>>
>Acked-by: Ansis Atteka 
>
>
>I saw that other open source projects like OpenVPN use rw_file_perms
> shortcut macro. Not sure how relevant that is for OVS but that macro
> expands to a little more function calls than what you have
> below. Maybe we don't need it, if what you have
> just worked.

 Thanks a lot for the review.

 I cooked this up using audit2allow and I tested it on fedora 25.  I'm
 now able to create and delete userspace bridges, without any further
 complaints from selinux
>>>
>>>I have the following openvswitch-custom.te that did work to run
>>>ovs+dpdk under selinux and pass traffic:
>>>
>>>
>>>Thanks for posting this. I think that this is really helpful to
>>> gather all necessary OVS+DPDK rules from different sources to make
>>> sure that nothing is missed.
>> +1, thanks a lot
>>> 
>>>
>>>
>>> 8< 
>>>
>>>require {
>>>type openvswitch_t;
>>>type openvswitch_tmp_t;
>>>type openvswitch_var_run_t;
>>>type ifconfig_exec_t;
>>>type hostname_exec_t;
>>>type vfio_device_t;
>>>type kernel_t;
>>>type tun_tap_device_t;
>>>type hugetlbfs_t;
>>>type init_t;
>>>class netlink_socket { setopt getopt create connect getattr write 
>>> read };
>>>class file { write getattr read open execute execute_no_trans create 
>>> unlink };
>>>class chr_file { write getattr read open ioctl };
>>>class unix_stream_socket { write getattr read connectto connect 
>>> setopt getopt sendto accept bind recvfrom acceptfrom };
>>>class dir { write remove_name add_name lock read };
>>>}
>>>
>>>#= openvswitch_t ==
>>>allow openvswitch_t self:netlink_socket { setopt getopt create connect 
>>>getattr write read };
>>>allow openvswitch_t hostname_exec_t:file { read getattr open execute 
>>>execute_no_trans };
>>>allow openvswitch_t ifconfig_exec_t:file { read getattr open execute 
>>>execute_no_trans };
>>>allow openvswitch_t openvswitch_tmp_t:file { execute execute_no_trans };
>>>allow openvswitch_t openvswitch_tmp_t:unix_stream_socket { write getattr 
>>>read connectto connect setopt getopt sendto accept bind recvfrom acceptfrom 
>>>};
>>>allow openvswitch_t vfio_device_t:chr_file { read write open ioctl getattr };
>>>allow openvswitch_t tun_tap_device_t:chr_file { read write getattr open 
>>>ioctl };
>>>allow openvswitch_t hugetlbfs_t:dir { write remove_name add_name lock read };
>>>allow openvswitch_t hugetlbfs_t:file { create unlink };
>>>allow openvswitch_t kernel_t:unix_stream_socket { write getattr read 
>>>connectto connect setopt getopt sendto accept bind recvfrom acceptfrom };
>>>allow openvswitch_t init_t:file { read open };
>>>
>>> >8 
>>>
>>>You'll note that this change gives the openvswitch complete access to
>>>hugetlbfs label, which might be the biggest scary part.
>>>
>>>
>>>There is also option to use SELinux switches that allow to activate only 
>>>subset of SElinux rules on a "per OVS feature basis" if there is risk that 
>>>because of DPDK whitelise we could be unconditionally loosening up SElinux 
>>>policy too much for non-DPDK
>>> cases. See [https://wiki.centos.org/TipsAndTricks/SelinuxBooleans] for more 
>>> details.
>> Ok, so perhaps we should require tun_tap_device_t permissions only if
>> we enable userspace support with a boolean.
>> I just posted this piece because the corresponding code is in
>> openvswitch source tree.
>> The rest of the permissions (hugepages, vfio) are required because of
>> code that's in the dpdk library.  Is there a way to put these in DPDK
>> and then just call a macro here, like
>> dpdk_perms(openvswitch_t)
>
> Below is an example of the macro:
>
>  8< 
>
> define(`dpdk_perms', `
>   gen_require(`
>   type vfio_device_t;
>   type kernel_t;
>   type hugetlbfs_t;
>   class file { write getattr read open execute execute_no_trans
>   create unlink };
> 

Re: [dpdk-dev] [PATCH v5 1/3] ethdev: new API to free consumed buffers in Tx ring

[Billy McFall]

On Tue, Mar 7, 2017 at 11:03 AM, Thomas Monjalon 
wrote:

> 2017-03-07 09:29, Billy McFall:
> > On Mon, Feb 27, 2017 at 8:48 AM, Thomas Monjalon <
> thomas.monja...@6wind.com>
> > wrote:
> > > I think you could use rte_errno (while keeping negative return codes).
> > >
> >
> > I can do that if you want, but if I understand your comment, it will make
> > the implementation of the function not as clean. I cannot use the
> existing
> > RTE_ETH_VALID_PORTID_OR_ERR_RET(..) and RTE_FUNC_PTR_OR_ERR_RET(..)
> MACROs
> > because they are handling the return on error. Or am I missing something?
>
> Yes. Maybe we need new macros for basic error management with rte_errno.
>
> Looking at the code. Do you think we need new MACROs or just set rte_errno
in the existing? What would be the down side to setting rte_errno for all
the existing calls?

Looks like RTE_ETH_VALID_PORTID_OR_ERR_RET(..) is being called ~135 times.
Most calls are with retval set to either -ENODEV or -EINVAL. A few
instances of 0 and -1, but not many.

Looks like RTE_FUNC_PTR_OR_ERR_RET(..) is being called ~100 times. Most
calls are with retval set to -ENOTSUP. A few instances of 0, but not many.

I was thinking:
/* Macros to check for valid port */
#define RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, retval) do { \
if (!rte_eth_dev_is_valid_port(port_id)) { \
RTE_PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id); \
+ if (retval < 0) \
+ rte_errno = -retval; \
return retval; \
} \
} while (0)

Thanks,
Billy

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/sfc: set multicast address list in started state only

[Ferruh Yigit]

On 3/2/2017 2:40 PM, Andrew Rybchenko wrote:
> From: Ivan Malov 
> 
> According to 'libefx' API requirements, one is allowed to
> apply multicast address list to the port in started state
> only, otherwise the new array should be copied to a local
> storage in order to be applied during the next port start
> 
> Fixes: 0fa0070e4391 ("net/sfc: support multicast addresses list controls")
> Coverity issue: 141296
> 
> Fixes: e9ddf37a507d ("net/sfc: fix setting empty multicast list")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Ivan Malov 
> Signed-off-by: Andrew Rybchenko 
> Reviewed-by: Andrew Lee 
> Reviewed-by: Andy Moreton 

Hi Andrew,

This patch does not apply clean to next-net because of other sfc patches
get first. Can you please rebase the patch to latest next-net?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v3 2/6] net/tap: add speed capabilities

[Ferruh Yigit]

On 3/9/2017 2:36 PM, Wiles, Keith wrote:
> 
>> On Mar 9, 2017, at 8:18 AM, Yigit, Ferruh  wrote:
>>
>> On 3/7/2017 4:31 PM, Pascal Mazon wrote:
>>> Tap PMD is flexible, it supports any speed.
>>>
>>> Signed-off-by: Pascal Mazon 
>>> ---
>>> doc/guides/nics/features/tap.ini |  1 +
>>> drivers/net/tap/rte_eth_tap.c| 35 +++
>>> 2 files changed, 36 insertions(+)
>>>
>>> diff --git a/doc/guides/nics/features/tap.ini 
>>> b/doc/guides/nics/features/tap.ini
>>> index d9b47a003654..dad5a0561087 100644
>>> --- a/doc/guides/nics/features/tap.ini
>>> +++ b/doc/guides/nics/features/tap.ini
>>> @@ -9,6 +9,7 @@ Jumbo frame  = Y
>>> Promiscuous mode = Y
>>> Allmulticast mode= Y
>>> Basic stats  = Y
>>> +Speed capabilities   = Y
>>> Unicast MAC filter   = Y
>>> Other kdrv   = Y
>>> ARMv7= Y
>>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>>> index 1e46ee36efa2..ef525a3f0826 100644
>>> --- a/drivers/net/tap/rte_eth_tap.c
>>> +++ b/drivers/net/tap/rte_eth_tap.c
>>> @@ -351,6 +351,40 @@ tap_dev_configure(struct rte_eth_dev *dev __rte_unused)
>>> return 0;
>>> }
>>>
>>> +static uint32_t
>>> +tap_dev_speed_capa(void)
>>> +{
>>> +   uint32_t speed = pmd_link.link_speed;
>>
>> link_speed is already hardcoded into PMD, so there is nothing to detect
>> here. Would it be different if PMD directly return pmd_link.link_speed?
> 
> The link speed is passed into the PMD via the command line, which means it 
> can change per run.

Right, I missed that.

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/mlx5: fix TSO TCP checksum flag

[Ferruh Yigit]

On 3/9/2017 7:17 AM, Nélio Laranjeiro wrote:
> On Tue, Mar 07, 2017 at 01:47:51PM +0200, Shahaf Shuler wrote:
>> Since PKT_TX_TCP_SEG implies PKT_TX_TCP_CKSUM, the PMD must force this
>> flag.
>> The fix applied for both tunneled and non-tunneled packets.
>>
>> Fixes: 19c5dc66b851 ("net/mlx5: add hardware TSO support")
>> Fixes: 751f56489e31 ("net/mlx5: add hardware TSO support for VXLAN and GRE")

>>
>> Signed-off-by: Shahaf Shuler 

> Acked-by: Nelio Laranjeiro 

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH v4] eal: Support running as unprivileged user

[Thomas Monjalon]

2017-02-17 14:59, Sergio Gonzalez Monroy:
> On 31/01/2017 17:44, Ben Walker wrote:
> > For Linux kernel 4.0 and newer, the ability to obtain
> > physical page frame numbers for unprivileged users from
> > /proc/self/pagemap was removed. Instead, when an IOMMU
> > is present, simply choose our own DMA addresses instead.
> >
> > Signed-off-by: Ben Walker 
> > ---
> >   lib/librte_eal/common/eal_private.h  | 12 +
> >   lib/librte_eal/linuxapp/eal/eal_memory.c | 75 
> > +++-
> >   lib/librte_eal/linuxapp/eal/eal_pci.c|  6 ++-
> >   3 files changed, 71 insertions(+), 22 deletions(-)
> 
> Acked-by: Sergio Gonzalez Monroy 

Applied, thanks

Re: [dpdk-dev] [PATCH v4] eal: Support running as unprivileged user

[Thomas Monjalon]

2017-02-17 11:28, Stephen Hemminger:
> On Tue, 31 Jan 2017 10:44:53 -0700
> Ben Walker  wrote:
> 
> > +   if (physaddr == RTE_BAD_PHYS_ADDR) {
> > RTE_LOG(ERR, EAL,
> > -   "Cannot open /proc/self/pagemap: %s. "
> > -   "virt2phys address translation will not work\n",
> > +   "Cannot obtain physical addresses: %s. "
> > +   "Only vfio will function.\n",
> 
> Please don't split a single error message across multiple lines. It makes
> it harder for user to find the source code lines with simple grep.
> Better to just have one long line for format string, or better yet be less 
> wordy.
> 
> Yes, the existing DPDK code has lots of these issues.

You're right.
Here the split is acceptable as there is strerror(errno) in the middle.

[dpdk-dev] [PATCH v2] net/sfc: set multicast address list in started state only

[Andrew Rybchenko]

From: Ivan Malov 

According to 'libefx' API requirements, one is allowed to
apply multicast address list to the port in started state
only, otherwise the new array should be copied to a local
storage in order to be applied during the next port start

Fixes: 0fa0070e4391 ("net/sfc: support multicast addresses list controls")
Coverity issue: 141296

Fixes: e9ddf37a507d ("net/sfc: fix setting empty multicast list")
Cc: sta...@dpdk.org

Signed-off-by: Ivan Malov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
Reviewed-by: Andy Moreton 
---
 drivers/net/sfc/sfc.h|  4 
 drivers/net/sfc/sfc_ethdev.c | 35 ---
 drivers/net/sfc/sfc_port.c   | 19 +++
 3 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 655328f..0b3b318 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -150,6 +150,10 @@ struct sfc_port {
boolean_t   promisc;
boolean_t   allmulti;
 
+   unsigned intmax_mcast_addrs;
+   unsigned intnb_mcast_addrs;
+   uint8_t *mcast_addrs;
+
rte_spinlock_t  mac_stats_lock;
uint64_t*mac_stats_buf;
efsys_mem_t mac_stats_dma_mem;
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index cac01ac..135f3da 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -826,36 +826,33 @@ sfc_set_mc_addr_list(struct rte_eth_dev *dev, struct 
ether_addr *mc_addr_set,
 uint32_t nb_mc_addr)
 {
struct sfc_adapter *sa = dev->data->dev_private;
-   uint8_t *mc_addrs_p = NULL;
+   struct sfc_port *port = &sa->port;
+   uint8_t *mc_addrs = port->mcast_addrs;
int rc;
unsigned int i;
 
-   if (nb_mc_addr > EFX_MAC_MULTICAST_LIST_MAX) {
+   if (mc_addrs == NULL)
+   return -ENOBUFS;
+
+   if (nb_mc_addr > port->max_mcast_addrs) {
sfc_err(sa, "too many multicast addresses: %u > %u",
-nb_mc_addr, EFX_MAC_MULTICAST_LIST_MAX);
+nb_mc_addr, port->max_mcast_addrs);
return -EINVAL;
}
 
-   if (nb_mc_addr != 0) {
-   uint8_t *mc_addrs;
-
-   mc_addrs_p = rte_calloc("mc-addrs", nb_mc_addr,
-   EFX_MAC_ADDR_LEN, 0);
-   if (mc_addrs_p == NULL)
-   return -ENOMEM;
-
-   mc_addrs = mc_addrs_p;
-   for (i = 0; i < nb_mc_addr; ++i) {
-   (void)rte_memcpy(mc_addrs, mc_addr_set[i].addr_bytes,
-EFX_MAC_ADDR_LEN);
-   mc_addrs += EFX_MAC_ADDR_LEN;
-   }
+   for (i = 0; i < nb_mc_addr; ++i) {
+   (void)rte_memcpy(mc_addrs, mc_addr_set[i].addr_bytes,
+EFX_MAC_ADDR_LEN);
+   mc_addrs += EFX_MAC_ADDR_LEN;
}
 
-   rc = efx_mac_multicast_list_set(sa->nic, mc_addrs_p, nb_mc_addr);
+   port->nb_mcast_addrs = nb_mc_addr;
 
-   rte_free(mc_addrs_p);
+   if (sa->state != SFC_ADAPTER_STARTED)
+   return 0;
 
+   rc = efx_mac_multicast_list_set(sa->nic, port->mcast_addrs,
+   port->nb_mcast_addrs);
if (rc != 0)
sfc_err(sa, "cannot set multicast address list (rc = %u)", rc);
 
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index e2f5043..e77848d 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -153,6 +153,12 @@ sfc_port_start(struct sfc_adapter *sa)
if (rc != 0)
goto fail_mac_filter_set;
 
+   sfc_log_init(sa, "set multicast address list");
+   rc = efx_mac_multicast_list_set(sa->nic, port->mcast_addrs,
+   port->nb_mcast_addrs);
+   if (rc != 0)
+   goto fail_mcast_address_list_set;
+
if (port->mac_stats_reset_pending) {
rc = sfc_port_reset_mac_stats(sa);
if (rc != 0)
@@ -196,6 +202,7 @@ sfc_port_start(struct sfc_adapter *sa)
 0, B_FALSE);
 
 fail_mac_stats_periodic:
+fail_mcast_address_list_set:
 fail_mac_filter_set:
 fail_mac_addr_set:
 fail_mac_pdu_set:
@@ -245,6 +252,17 @@ sfc_port_init(struct sfc_adapter *sa)
else
port->pdu = EFX_MAC_PDU(dev_data->mtu);
 
+   port->max_mcast_addrs = EFX_MAC_MULTICAST_LIST_MAX;
+   port->nb_mcast_addrs = 0;
+   port->mcast_addrs = rte_calloc_socket("mcast_addr_list_buf",
+ port->max_mcast_addrs,
+ EFX_MAC_ADDR_LEN, 0,
+ sa->socket_id

[dpdk-dev] [PATCH v11 1/7] lib: add information metrics library

[Remy Horton]

This patch adds a new information metrics library. This Metrics
library implements a mechanism by which producers can publish
numeric information for later querying by consumers. Metrics
themselves are statistics that are not generated by PMDs, and
hence are not reported via ethdev extended statistics.

Metric information is populated using a push model, where
producers update the values contained within the metric
library by calling an update function on the relevant metrics.
Consumers receive metric information by querying the central
metric data, which is held in shared memory.

Signed-off-by: Remy Horton 
---
 MAINTAINERS|   4 +
 config/common_base |   5 +
 doc/api/doxy-api-index.md  |   1 +
 doc/api/doxy-api.conf  |   1 +
 doc/guides/prog_guide/index.rst|   1 +
 doc/guides/prog_guide/metrics_lib.rst  | 180 +
 doc/guides/rel_notes/release_17_02.rst |   1 +
 doc/guides/rel_notes/release_17_05.rst |   8 +
 lib/Makefile   |   1 +
 lib/librte_metrics/Makefile|  51 +
 lib/librte_metrics/rte_metrics.c   | 299 +
 lib/librte_metrics/rte_metrics.h   | 240 +++
 lib/librte_metrics/rte_metrics_version.map |  13 ++
 mk/rte.app.mk  |   2 +
 14 files changed, 807 insertions(+)
 create mode 100644 doc/guides/prog_guide/metrics_lib.rst
 create mode 100644 lib/librte_metrics/Makefile
 create mode 100644 lib/librte_metrics/rte_metrics.c
 create mode 100644 lib/librte_metrics/rte_metrics.h
 create mode 100644 lib/librte_metrics/rte_metrics_version.map

diff --git a/MAINTAINERS b/MAINTAINERS
index 5030c1c..66478f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -635,6 +635,10 @@ F: lib/librte_jobstats/
 F: examples/l2fwd-jobstats/
 F: doc/guides/sample_app_ug/l2_forward_job_stats.rst
 
+Metrics
+M: Remy Horton 
+F: lib/librte_metrics/
+
 
 Test Applications
 -
diff --git a/config/common_base b/config/common_base
index aeee13e..cea055f 100644
--- a/config/common_base
+++ b/config/common_base
@@ -501,6 +501,11 @@ CONFIG_RTE_LIBRTE_EFD=y
 CONFIG_RTE_LIBRTE_JOBSTATS=y
 
 #
+# Compile the device metrics library
+#
+CONFIG_RTE_LIBRTE_METRICS=y
+
+#
 # Compile librte_lpm
 #
 CONFIG_RTE_LIBRTE_LPM=y
diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index eb39f69..26a26b7 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -156,4 +156,5 @@ There are many libraries, so their headers may be grouped 
by topics:
   [common] (@ref rte_common.h),
   [ABI compat] (@ref rte_compat.h),
   [keepalive]  (@ref rte_keepalive.h),
+  [device metrics] (@ref rte_metrics.h),
   [version](@ref rte_version.h)
diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf
index fdcf13c..fbbcf8e 100644
--- a/doc/api/doxy-api.conf
+++ b/doc/api/doxy-api.conf
@@ -52,6 +52,7 @@ INPUT   = doc/api/doxy-api-index.md \
   lib/librte_mbuf \
   lib/librte_mempool \
   lib/librte_meter \
+  lib/librte_metrics \
   lib/librte_net \
   lib/librte_pdump \
   lib/librte_pipeline \
diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
index 77f427e..2a69844 100644
--- a/doc/guides/prog_guide/index.rst
+++ b/doc/guides/prog_guide/index.rst
@@ -62,6 +62,7 @@ Programmer's Guide
 packet_classif_access_ctrl
 packet_framework
 vhost_lib
+metrics_lib
 port_hotplug_framework
 source_org
 dev_kit_build_system
diff --git a/doc/guides/prog_guide/metrics_lib.rst 
b/doc/guides/prog_guide/metrics_lib.rst
new file mode 100644
index 000..87f806d
--- /dev/null
+++ b/doc/guides/prog_guide/metrics_lib.rst
@@ -0,0 +1,180 @@
+..  BSD LICENSE
+Copyright(c) 2017 Intel Corporation. All rights reserved.
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+* Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+* Redistributions in binary form must reproduce the above copyright
+notice, this list of conditions and the following disclaimer in
+the documentation and/or other materials provided with the
+distribution.
+* Neither the name of Intel Corporation nor the names of its
+contributors may be used to endorse or promote products derived
+from this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMIT

[dpdk-dev] [PATCH v11 2/7] app/proc_info: add metrics displaying

[Remy Horton]

From: Reshma Pattan 

Modify the dpdk-procinfo process to display the newly added metrics.
Added new command line option "--metrics" to display metrics.

Signed-off-by: Reshma Pattan 
Signed-off-by: Remy Horton 
---
 app/proc_info/main.c | 79 +++-
 1 file changed, 78 insertions(+), 1 deletion(-)

diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index 2c56d10..f513669 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -1,7 +1,7 @@
 /*
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -57,6 +57,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Maximum long option length for option parsing. */
 #define MAX_LONG_OPT_SZ 64
@@ -68,6 +69,8 @@ static uint32_t enabled_port_mask;
 static uint32_t enable_stats;
 /**< Enable xstats. */
 static uint32_t enable_xstats;
+/**< Enable metrics. */
+static uint32_t enable_metrics;
 /**< Enable stats reset. */
 static uint32_t reset_stats;
 /**< Enable xstats reset. */
@@ -85,6 +88,8 @@ proc_info_usage(const char *prgname)
"  --stats: to display port statistics, enabled by default\n"
"  --xstats: to display extended port statistics, disabled by "
"default\n"
+   "  --metrics: to display derived metrics of the ports, disabled 
by "
+   "default\n"
"  --stats-reset: to reset port statistics\n"
"  --xstats-reset: to reset port extended statistics\n",
prgname);
@@ -127,6 +132,7 @@ proc_info_parse_args(int argc, char **argv)
{"stats", 0, NULL, 0},
{"stats-reset", 0, NULL, 0},
{"xstats", 0, NULL, 0},
+   {"metrics", 0, NULL, 0},
{"xstats-reset", 0, NULL, 0},
{NULL, 0, 0, 0}
};
@@ -159,6 +165,10 @@ proc_info_parse_args(int argc, char **argv)
else if (!strncmp(long_option[option_index].name, 
"xstats",
MAX_LONG_OPT_SZ))
enable_xstats = 1;
+   else if (!strncmp(long_option[option_index].name,
+   "metrics",
+   MAX_LONG_OPT_SZ))
+   enable_metrics = 1;
/* Reset stats */
if (!strncmp(long_option[option_index].name, 
"stats-reset",
MAX_LONG_OPT_SZ))
@@ -301,6 +311,67 @@ nic_xstats_clear(uint8_t port_id)
printf("\n  NIC extended statistics for port %d cleared\n", port_id);
 }
 
+static void
+metrics_display(int port_id)
+{
+   struct rte_metric_value *metrics;
+   struct rte_metric_name *names;
+   int len, ret;
+   static const char *nic_stats_border = "";
+
+   len = rte_metrics_get_names(NULL, 0);
+   if (len < 0) {
+   printf("Cannot get metrics count\n");
+   return;
+   }
+   if (len == 0) {
+   printf("No metrics to display (none have been registered)\n");
+   return;
+   }
+
+   metrics = rte_malloc("proc_info_metrics",
+   sizeof(struct rte_metric_value) * len, 0);
+   if (metrics == NULL) {
+   printf("Cannot allocate memory for metrics\n");
+   return;
+   }
+
+   names =  rte_malloc(NULL, sizeof(struct rte_metric_name) * len, 0);
+   if (names == NULL) {
+   printf("Cannot allocate memory for metrcis names\n");
+   rte_free(metrics);
+   return;
+   }
+
+   if (len != rte_metrics_get_names(names, len)) {
+   printf("Cannot get metrics names\n");
+   rte_free(metrics);
+   rte_free(names);
+   return;
+   }
+
+   if (port_id == RTE_METRICS_GLOBAL)
+   printf("## Non port specific metrics  #\n");
+   else
+   printf("## metrics for port %-2d #\n", port_id);
+   printf("%s\n", nic_stats_border);
+   ret = rte_metrics_get_values(port_id, metrics, len);
+   if (ret < 0 || ret > len) {
+   printf("Cannot get metrics values\n");
+   rte_free(metrics);
+   rte_free(names);
+   return;
+   }
+
+   int i;
+   for (i = 0; i < len; i++)
+   printf("%s: %"PRIu64"\n", names[i].name, metrics[i].value);
+
+   printf("%s\n", nic_stats_border);
+   rte_free(metrics);
+   rte_free(names);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -360,8 +431,14 @@ main(int argc, char **argv)
nic

[dpdk-dev] [PATCH v11 0/7] Expanded statistics reporting

[Remy Horton]

This patchset consists of three libraries: A Metrics library for
distributing device information, a library that calculates bit-rate
statistics, and a library that calculates latency statistics. The
latter two libraries make use of the first library.

Metrics Library
---
The Metrics library implements a mechanism by which producers can
publish numeric information for later querying by consumers.
In practice producers will typically be other libraries or
primary processes, whereas consumers will typically be applications.

Metrics themselves are statistics that are not generated by PMDs.
Metric information is populated using a push model, where producers
update the values contained within the metric library by calling an
update function on the relevant metrics. Consumers receive metric
information by querying the central metric data, which is held
in shared memory so it can also be accessed by secondary processes.

For each metric, a separate value is maintained for each port id,
and when publishing metric values the producers need to specify
which port is being updated. In addition there is a special id
RTE_METRICS_GLOBAL that is intended for global statistics that are
not associated with any individual device. Since the metrics
library is self-contained, the only restriction on port numbers is
that they are less than RTE_MAX_ETHPORTS - there is no requirement
for the ports to actually exist.

Metrics must first be registered, which is the way producers declare
the names of the metrics they will be publishing. Registration can
either be done individually, or as a group as a metric set. The
intention is for all metrics in a set to be updated in one go,
although it is also possible for metrics within a set to be updated
individually. It is up to the producers to update metrics as required.

Bit-rate statistics library
---
The bit-rate library calculates the mean, the exponentially-weighted
moving average, and peak bit-rates for each active port (i.e. network
device). These statistics are then reported via the metrics library
using the following names: mean_bits_in, mean_bits_out, ewma_bits_in,
ewma_bits_out, peak_bits_in, and peak_bits_out. The sampling window 
used for calculation is decided by the application requiring the statistics.

Latency statistics library
--
The latency statistics library calculates the port-to-port latency of
packet processing by a DPDK application, reporting the minimum, average,
and maximum nano-seconds that packet processing takes, as well as the
jitter in processing delay. These statistics are then reported via the
metrics library using the following names: min_latency_ns, avg_latency_ns,
mac_latency_ns, and jitter_ns.


For more details on the metrics library and the Bit-rate and Latency
components see the Programmer's Guide updates in the patchset.
--
v11 changes:
* Rebased
* .map references to 17.02 changed to 17.05
* Release ntoes moved to release_17_05.rst
* Bit-rate library now also gives unfiltered average

v10 changes:
* Rebased
* Relocated some config-related directives.
* Removed incorrect capitalisations in API docs.
* Formatting & detail corrections in release notes.
* Moved around struct member descriptions.
* Rewritten rte_metrics.h file description.
* Rewritten description of RTE_METRICS_GLOBAL.
* Used 'producers' and 'consumers' as terms.
* Removed markup (bold text) in Doxygen tags.
* Added programming guide section.

v9 changes:
* Updated .map files to reflect function changes in v8
* Fixed rte_malloc() of zero bytes in proc_info when no metrics exist
* Fixed rte_metrics_init not being called explicitly in testpmd

v8 changes:
* Release notes correction
* Updated copyright years
* rte_metric_init() takes socket id & must be explicitly called
* rte_metrics_reg_metric renamed to rte_metrics_reg_name()
* rte_metrics_update_metric() renamed to rte_metrics_update_value()
* Doxygen updates
* Changed malloc()/free() to rte_malloc()/rte_free()
* Removed redundant memset()
* rte_stats_bitrates_s renamed to rte_stats_bitrates_s
* Split mbuf change to own patch for visibility
* CYCLES_PER_NS now a static inline function
* latency: "hidden" pthread creation now has polling API instead.
* Struct declarations and variable definitions cleaned up
* Double initialization of the latency library now returns -EEXIST
* MAINTAINERS entry for layenctstats in correct section

v7 changes:
* RTE_METRICS_NONPORT renamed to RTE_METRICS_GLOBAL
* Multiple changes to rte_metrics.h doxygen documentation
* Split apart latency patch into lib, test-pmd, & proc_info parts
* Reordered patches by functionality
* Insufficent capacity return value changed from -ERANGE to actual size
* Cache alignment in bitrate library
* Tightened up const usage to avoid STATIC_CONST_CHAR_ARRAY warning
* Reshma reinstated as author for (now split) latency patch
* Rebase to master

v6 changes:
* Metrics display now has "Non port specific" rather than "port -

[dpdk-dev] [PATCH v11 3/7] lib: add bitrate statistics library

[Remy Horton]

This patch adds a library that calculates peak and average data-rate
statistics. For ethernet devices. These statistics are reported using
the metrics library.

Signed-off-by: Remy Horton 
---
 MAINTAINERS|   4 +
 config/common_base |   5 +
 doc/api/doxy-api-index.md  |   1 +
 doc/api/doxy-api.conf  |   1 +
 doc/guides/prog_guide/metrics_lib.rst  |  65 ++
 doc/guides/rel_notes/release_17_02.rst |   1 +
 doc/guides/rel_notes/release_17_05.rst |   5 +
 lib/Makefile   |   1 +
 lib/librte_bitratestats/Makefile   |  53 
 lib/librte_bitratestats/rte_bitrate.c  | 141 +
 lib/librte_bitratestats/rte_bitrate.h  |  80 
 .../rte_bitratestats_version.map   |   9 ++
 mk/rte.app.mk  |   1 +
 13 files changed, 367 insertions(+)
 create mode 100644 lib/librte_bitratestats/Makefile
 create mode 100644 lib/librte_bitratestats/rte_bitrate.c
 create mode 100644 lib/librte_bitratestats/rte_bitrate.h
 create mode 100644 lib/librte_bitratestats/rte_bitratestats_version.map

diff --git a/MAINTAINERS b/MAINTAINERS
index 66478f3..8abf4fd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -639,6 +639,10 @@ Metrics
 M: Remy Horton 
 F: lib/librte_metrics/
 
+Bit-rate statistica
+M: Remy Horton 
+F: lib/librte_bitratestats/
+
 
 Test Applications
 -
diff --git a/config/common_base b/config/common_base
index cea055f..d700ee0 100644
--- a/config/common_base
+++ b/config/common_base
@@ -630,3 +630,8 @@ CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS=n
 # Compile the crypto performance application
 #
 CONFIG_RTE_APP_CRYPTO_PERF=y
+
+#
+# Compile the bitrate statistics library
+#
+CONFIG_RTE_LIBRTE_BITRATE=y
diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index 26a26b7..8492bce 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -157,4 +157,5 @@ There are many libraries, so their headers may be grouped 
by topics:
   [ABI compat] (@ref rte_compat.h),
   [keepalive]  (@ref rte_keepalive.h),
   [device metrics] (@ref rte_metrics.h),
+  [bitrate statistics] (@ref rte_bitrate.h),
   [version](@ref rte_version.h)
diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf
index fbbcf8e..c4b3b68 100644
--- a/doc/api/doxy-api.conf
+++ b/doc/api/doxy-api.conf
@@ -36,6 +36,7 @@ INPUT   = doc/api/doxy-api-index.md \
   lib/librte_eal/common/include \
   lib/librte_eal/common/include/generic \
   lib/librte_acl \
+  lib/librte_bitratestats \
   lib/librte_cfgfile \
   lib/librte_cmdline \
   lib/librte_compat \
diff --git a/doc/guides/prog_guide/metrics_lib.rst 
b/doc/guides/prog_guide/metrics_lib.rst
index 87f806d..1c2a28f 100644
--- a/doc/guides/prog_guide/metrics_lib.rst
+++ b/doc/guides/prog_guide/metrics_lib.rst
@@ -178,3 +178,68 @@ print out all metrics for a given port:
 free(metrics);
 free(names);
 }
+
+
+Bit-rate statistics library
+---
+
+The bit-rate library calculates the exponentially-weighted moving
+average and peak bit-rates for each active port (i.e. network device).
+These statistics are reported via the metrics library using the
+following names:
+
+- ``mean_bits_in``: Average inbound bit-rate
+- ``mean_bits_out``:  Average outbound bit-rate
+- ``ewma_bits_in``: Average inbound bit-rate (EWMA smoothed)
+- ``ewma_bits_out``:  Average outbound bit-rate (EWMA smoothed)
+- ``peak_bits_in``:  Peak inbound bit-rate
+- ``peak_bits_out``:  Peak outbound bit-rate
+
+Once initialised and clocked at the appropriate frequency, these
+statistics can be obtained by querying the metrics library.
+
+Initialization
+~~
+
+Before it is used the bit-rate statistics library has to be initialised
+by calling ``rte_stats_bitrate_create()``, which will return a bit-rate
+calculation object. Since the bit-rate library uses the metrics library
+to report the calculated statistics, the bit-rate library then needs to
+register the calculated statistics with the metrics library. This is
+done using the helper function ``rte_stats_bitrate_reg()``.
+
+.. code-block:: c
+
+struct rte_stats_bitrates *bitrate_data;
+
+bitrate_data = rte_stats_bitrate_create();
+if (bitrate_data == NULL)
+rte_exit(EXIT_FAILURE, "Could not allocate bit-rate data.\n");
+rte_stats_bitrate_reg(bitrate_data);
+
+Controlling the sampling rate
+~
+
+Since the library works by periodic sampling but does not use an
+internal thread, the application has to periodically call
+``rt

[dpdk-dev] [PATCH v11 4/7] app/test-pmd: add bitrate statistics calculation

[Remy Horton]

Calculate bitrate statistics using the bitrate stats library. The
resulting statistics can be viewed via proc_info.

Signed-off-by: Remy Horton 
---
 app/test-pmd/testpmd.c | 41 -
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bfb2f8e..b31a300 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -79,6 +79,10 @@
 #include 
 #endif
 #include 
+#include 
+#ifdef RTE_LIBRTE_BITRATE
+#include 
+#endif
 
 #include "testpmd.h"
 
@@ -323,6 +327,9 @@ uint16_t nb_rx_queue_stats_mappings = 0;
 
 unsigned max_socket = 0;
 
+/* Bitrate statistics */
+struct rte_stats_bitrates *bitrate_data;
+
 /* Forward function declarations */
 static void map_port_queue_stats_mapping_registers(uint8_t pi, struct rte_port 
*port);
 static void check_all_ports_link_status(uint32_t port_mask);
@@ -922,12 +929,32 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t 
pkt_fwd)
struct fwd_stream **fsm;
streamid_t nb_fs;
streamid_t sm_id;
+#ifdef RTE_LIBRTE_BITRATE
+   uint64_t tics_per_1sec;
+   uint64_t tics_datum;
+   uint64_t tics_current;
+   uint8_t idx_port, cnt_ports;
+#endif
 
+#ifdef RTE_LIBRTE_BITRATE
+   cnt_ports = rte_eth_dev_count();
+   tics_datum = rte_rdtsc();
+   tics_per_1sec = rte_get_timer_hz();
+#endif
fsm = &fwd_streams[fc->stream_idx];
nb_fs = fc->stream_nb;
do {
for (sm_id = 0; sm_id < nb_fs; sm_id++)
(*pkt_fwd)(fsm[sm_id]);
+#ifdef RTE_LIBRTE_BITRATE
+   tics_current = rte_rdtsc();
+   if (tics_current - tics_datum >= tics_per_1sec) {
+   /* Periodic bitrate calculation */
+   for (idx_port = 0; idx_port < cnt_ports; idx_port++)
+   rte_stats_bitrate_calc(bitrate_data, idx_port);
+   tics_datum = tics_current;
+   }
+#endif
} while (! fc->stopped);
 }
 
@@ -2139,6 +2166,18 @@ main(int argc, char** argv)
FOREACH_PORT(port_id, ports)
rte_eth_promiscuous_enable(port_id);
 
+   /* Init metrics library */
+   rte_metrics_init(rte_socket_id());
+
+   /* Setup bitrate stats */
+#ifdef RTE_LIBRTE_BITRATE
+   bitrate_data = rte_stats_bitrate_create();
+   if (bitrate_data == NULL)
+   rte_exit(EXIT_FAILURE, "Could not allocate bitrate data.\n");
+   rte_stats_bitrate_reg(bitrate_data);
+#endif
+
+
 #ifdef RTE_LIBRTE_CMDLINE
if (interactive == 1) {
if (auto_start) {
-- 
2.5.5

[dpdk-dev] [PATCH v11 5/7] mbuf: add a timestamp to the mbuf for latencystats

[Remy Horton]

From: Harry van Haaren 

This commit adds a uint64_t to the mbuf struct,
allowing collection of latency and jitter statistics
by measuring packet I/O timestamps. This change is
required by the latencystats library.

Signed-off-by: Reshma Pattan 
Signed-off-by: Harry van Haaren 
---
 lib/librte_mbuf/rte_mbuf.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index ce57d47..e0dad6e 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -514,6 +514,9 @@ struct rte_mbuf {
 
/** Timesync flags for use with IEEE1588. */
uint16_t timesync;
+
+   /** Timestamp for measuring latency. */
+   uint64_t timestamp;
 } __rte_cache_aligned;
 
 /**
-- 
2.5.5

[dpdk-dev] [PATCH v11 6/7] lib: added new library for latency stats

[Remy Horton]

From: Harry van Haaren 

Add a library designed to calculate latency statistics and report them
to the application when queried. The library measures minimum, average and
maximum latencies, and jitter in nano seconds. The current implementation
supports global latency stats, i.e. per application stats.

Signed-off-by: Reshma Pattan 
Signed-off-by: Remy Horton 
Signed-off-by: Harry van Haaren 
---
 MAINTAINERS|   4 +
 config/common_base |   5 +
 doc/api/doxy-api-index.md  |   1 +
 doc/api/doxy-api.conf  |   1 +
 doc/guides/prog_guide/metrics_lib.rst  |  58 +++-
 doc/guides/rel_notes/release_17_02.rst |   1 +
 doc/guides/rel_notes/release_17_05.rst |   5 +
 lib/Makefile   |   1 +
 lib/librte_latencystats/Makefile   |  56 
 lib/librte_latencystats/rte_latencystats.c | 366 +
 lib/librte_latencystats/rte_latencystats.h | 154 +
 .../rte_latencystats_version.map   |  11 +
 mk/rte.app.mk  |   2 +-
 13 files changed, 662 insertions(+), 3 deletions(-)
 create mode 100644 lib/librte_latencystats/Makefile
 create mode 100644 lib/librte_latencystats/rte_latencystats.c
 create mode 100644 lib/librte_latencystats/rte_latencystats.h
 create mode 100644 lib/librte_latencystats/rte_latencystats_version.map

diff --git a/MAINTAINERS b/MAINTAINERS
index 8abf4fd..0dc95f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -643,6 +643,10 @@ Bit-rate statistica
 M: Remy Horton 
 F: lib/librte_bitratestats/
 
+Latency Stats
+M: Reshma Pattan 
+F: lib/librte_latencystats/
+
 
 Test Applications
 -
diff --git a/config/common_base b/config/common_base
index d700ee0..9d7bddf 100644
--- a/config/common_base
+++ b/config/common_base
@@ -635,3 +635,8 @@ CONFIG_RTE_APP_CRYPTO_PERF=y
 # Compile the bitrate statistics library
 #
 CONFIG_RTE_LIBRTE_BITRATE=y
+
+#
+# Compile the latency statistics library
+#
+CONFIG_RTE_LIBRTE_LATENCY_STATS=y
diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index 8492bce..9d1818c 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -158,4 +158,5 @@ There are many libraries, so their headers may be grouped 
by topics:
   [keepalive]  (@ref rte_keepalive.h),
   [device metrics] (@ref rte_metrics.h),
   [bitrate statistics] (@ref rte_bitrate.h),
+  [latency statistics] (@ref rte_latencystats.h),
   [version](@ref rte_version.h)
diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf
index c4b3b68..5babafa 100644
--- a/doc/api/doxy-api.conf
+++ b/doc/api/doxy-api.conf
@@ -49,6 +49,7 @@ INPUT   = doc/api/doxy-api-index.md \
   lib/librte_jobstats \
   lib/librte_kni \
   lib/librte_kvargs \
+  lib/librte_latencystats \
   lib/librte_lpm \
   lib/librte_mbuf \
   lib/librte_mempool \
diff --git a/doc/guides/prog_guide/metrics_lib.rst 
b/doc/guides/prog_guide/metrics_lib.rst
index 1c2a28f..702c29d 100644
--- a/doc/guides/prog_guide/metrics_lib.rst
+++ b/doc/guides/prog_guide/metrics_lib.rst
@@ -201,8 +201,8 @@ statistics can be obtained by querying the metrics library.
 Initialization
 ~~
 
-Before it is used the bit-rate statistics library has to be initialised
-by calling ``rte_stats_bitrate_create()``, which will return a bit-rate
+Before the library can be used, it has to be initialised by calling
+``rte_stats_bitrate_create()``, which will return a bit-rate
 calculation object. Since the bit-rate library uses the metrics library
 to report the calculated statistics, the bit-rate library then needs to
 register the calculated statistics with the metrics library. This is
@@ -243,3 +243,57 @@ desired, this function should be called once a second.
}
 /* ... */
 }
+
+
+Latency statistics library
+--
+
+The latency statistics library calculates the latency of packet
+processing by a DPDK application, reporting the minimum, average,
+and maximum nano-seconds that packet processing takes, as well as
+the jitter in processing delay. These statistics are then reported
+via the metrics library using the following names:
+
+- ``min_latency_ns``: Minimum processing latency (nano-seconds)
+- ``avg_latency_ns``:  Average  processing latency (nano-seconds)
+- ``mac_latency_ns``:  Maximum  processing latency (nano-seconds)
+- ``jitter_ns``: Variance in processing latency (nano-seconds)
+
+Once initialised and clocked at the appropriate frequency, these
+statistics can be obtained by querying the metrics library.
+
+Initialization
+~~
+
+Before the library can be used, it has to b

[dpdk-dev] [PATCH v11 7/7] app/test-pmd: add latency statistics calculation

[Remy Horton]

From: Harry van Haaren 

This patch adds latency stats commandline argument to testpmd,
allowing to specify the lcore to use for latencystats updates.

Signed-off-by: Reshma Pattan 
Signed-off-by: Harry van Haaren 
Signed-off-by: Remy Horton 
---
 app/test-pmd/parameters.c | 20 +++-
 app/test-pmd/testpmd.c| 37 +
 app/test-pmd/testpmd.h|  6 +-
 3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 28db8cd..30b60ba 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -149,6 +149,10 @@ usage(char* progname)
   "the packet will be enqueued into the rx drop-queue. "
   "If the drop-queue doesn't exist, the packet is dropped. "
   "By default drop-queue=127.\n");
+#ifdef RTE_LIBRTE_LATENCY_STATS
+   printf("  --latencystats=N: enable latency and jitter statistcs "
+  "monitoring on lcore id N.\n");
+#endif
printf("  --crc-strip: enable CRC stripping by hardware.\n");
printf("  --enable-lro: enable large receive offload.\n");
printf("  --enable-rx-cksum: enable rx hardware checksum offload.\n");
@@ -526,6 +530,7 @@ launch_args_parse(int argc, char** argv)
{ "pkt-filter-report-hash", 1, 0, 0 },
{ "pkt-filter-size",1, 0, 0 },
{ "pkt-filter-drop-queue",  1, 0, 0 },
+   { "latencystats",   1, 0, 0 },
{ "crc-strip",  0, 0, 0 },
{ "enable-lro", 0, 0, 0 },
{ "enable-rx-cksum",0, 0, 0 },
@@ -766,6 +771,19 @@ launch_args_parse(int argc, char** argv)
 "drop queue %d invalid - must"
 "be >= 0 \n", n);
}
+#ifdef RTE_LIBRTE_LATENCY_STATS
+   if (!strcmp(lgopts[opt_idx].name,
+   "latencystats")) {
+   n = atoi(optarg);
+   if (n >= 0) {
+   latencystats_lcore_id = (lcoreid_t) n;
+   latencystats_enabled = 1;
+   } else
+   rte_exit(EXIT_FAILURE,
+"invalid lcore id %d for 
latencystats"
+" must be >= 0\n", n);
+   }
+#endif
if (!strcmp(lgopts[opt_idx].name, "crc-strip"))
rx_mode.hw_strip_crc = 1;
if (!strcmp(lgopts[opt_idx].name, "enable-lro"))
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index b31a300..524e758 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -83,6 +83,10 @@
 #ifdef RTE_LIBRTE_BITRATE
 #include 
 #endif
+#include 
+#ifdef RTE_LIBRTE_LATENCY_STATS
+#include 
+#endif
 
 #include "testpmd.h"
 
@@ -276,6 +280,20 @@ uint32_t bypass_timeout = RTE_BYPASS_TMT_OFF;
 
 #endif
 
+#ifdef RTE_LIBRTE_LATENCY_STATS
+
+/*
+ * Set when latency stats is enabled in the commandline
+ */
+uint8_t latencystats_enabled;
+
+/*
+ * Lcore ID to serive latency statistics.
+ */
+lcoreid_t latencystats_lcore_id = -1;
+
+#endif
+
 /*
  * Ethernet device configuration.
  */
@@ -955,6 +973,11 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t 
pkt_fwd)
tics_datum = tics_current;
}
 #endif
+#ifdef RTE_LIBRTE_LATENCY_STATS
+   if (latencystats_lcore_id == rte_lcore_id())
+   rte_latencystats_update();
+#endif
+
} while (! fc->stopped);
 }
 
@@ -2108,6 +2131,9 @@ signal_handler(int signum)
/* uninitialize packet capture framework */
rte_pdump_uninit();
 #endif
+#ifdef RTE_LIBRTE_LATENCY_STATS
+   rte_latencystats_uninit();
+#endif
force_quit();
/* exit with the expected status */
signal(signum, SIG_DFL);
@@ -2169,6 +2195,17 @@ main(int argc, char** argv)
/* Init metrics library */
rte_metrics_init(rte_socket_id());
 
+#ifdef RTE_LIBRTE_LATENCY_STATS
+   if (latencystats_enabled != 0) {
+   int ret = rte_latencystats_init(1, NULL);
+   if (ret)
+   printf("Warning: latencystats init()"
+   " returned error %d\n", ret);
+   printf("Latencystats running on lcore %d\n",
+   latenc

Re: [dpdk-dev] [PATCH] net/ixgbe: Put correct id values in ixgbevf_dev_xstats_get

[Ferruh Yigit]

On 3/7/2017 2:28 PM, Ido Barnea (ibarnea) wrote:
> Without setting the id, calling xstats_get twice with same array causes 
> memory corruption.
> Also, if IXGBEVF_NB_XSTATS will be different than 1 in the future, this will 
> cause issues.
> 
> Signed-off-by: Ido Barnea 

Acked-by: Ferruh Yigit 

Re: [dpdk-dev] [PATCH 0/4] net/bonding: bonding and LACP fixes

[Declan Doherty]

On 09/03/2017 1:19 PM, Thomas Monjalon wrote:

2017-02-08 18:14, Thomas Monjalon:

2016-08-01 16:42, Robert Sanford:

In this patch series, we fix two bonding driver bugs and
enhance testpmd so that bonding mode 4 (LACP) ports remain
operational even when idle.


These patches are blocked because we are waiting a v2.

Waiting so long for a fix without having any feedback makes me think
that either the bug is not so important or the PMD is not used.


Last call: anyone interested in this series?



Hey Thomas, Robert doesn't seem to be active anymore on the list, so 
I'll set aside some time over the next couple of days and put together a 
v2 of these patches.


Declan

Re: [dpdk-dev] [PATCH v5 1/3] ethdev: new API to free consumed buffers in Tx ring

[Thomas Monjalon]

2017-03-09 10:49, Billy McFall:
> On Tue, Mar 7, 2017 at 11:03 AM, Thomas Monjalon 
> wrote:
> > 2017-03-07 09:29, Billy McFall:
> > > On Mon, Feb 27, 2017 at 8:48 AM, Thomas Monjalon <
> > thomas.monja...@6wind.com>
> > > wrote:
> > > > I think you could use rte_errno (while keeping negative return codes).
> > > >
> > >
> > > I can do that if you want, but if I understand your comment, it will make
> > > the implementation of the function not as clean. I cannot use the
> > existing
> > > RTE_ETH_VALID_PORTID_OR_ERR_RET(..) and RTE_FUNC_PTR_OR_ERR_RET(..)
> > MACROs
> > > because they are handling the return on error. Or am I missing something?
> >
> > Yes. Maybe we need new macros for basic error management with rte_errno.
> >
> > Looking at the code. Do you think we need new MACROs or just set rte_errno
> in the existing? What would be the down side to setting rte_errno for all
> the existing calls?
> 
> Looks like RTE_ETH_VALID_PORTID_OR_ERR_RET(..) is being called ~135 times.
> Most calls are with retval set to either -ENODEV or -EINVAL. A few
> instances of 0 and -1, but not many.
> 
> Looks like RTE_FUNC_PTR_OR_ERR_RET(..) is being called ~100 times. Most
> calls are with retval set to -ENOTSUP. A few instances of 0, but not many.
> 
> I was thinking:
> /* Macros to check for valid port */
> #define RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, retval) do { \
> if (!rte_eth_dev_is_valid_port(port_id)) { \
> RTE_PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id); \
> + if (retval < 0) \
> + rte_errno = -retval; \
> return retval; \
> } \
> } while (0)

Yes it seems acceptable.
This rework may be done separately.

Re: [dpdk-dev] [PATCH] net/ixgbe: Put correct id values in ixgbevf_dev_xstats_get

[Ferruh Yigit]

On 3/9/2017 4:46 PM, Ferruh Yigit wrote:
> On 3/7/2017 2:28 PM, Ido Barnea (ibarnea) wrote:
>> Without setting the id, calling xstats_get twice with same array causes 
>> memory corruption.
>> Also, if IXGBEVF_NB_XSTATS will be different than 1 in the future, this will 
>> cause issues.
>>
>> Signed-off-by: Ido Barnea 
> 
> Acked-by: Ferruh Yigit 

Applied to dpdk-next-net/master, thanks.

[dpdk-dev] [PATCH v2 0/6] Support running Solarflare PMD over PCI VFs

[Andrew Rybchenko]

Running Solarflare PMD over PCI virtual functions requires few fixes:
 - periodic MAC stats DMA is not supported for VFs
 - promiscious and all-multicast modes may be denied by FW/PF
 - MCDI proxy authorization made by FW should be properly handled by
   the VF driver

v1 -> v2
 * rephrase git logs
 * add release notes
 * add SR-IOV feature to the driver list
 * reorder patches to advertise VF support when it works

Andrew Rybchenko (1):
  net/sfc/base: don't ignore requested MAC stats update period

Ivan Malov (5):
  net/sfc: add kvarg control for MAC statistics update period
  net/sfc: poll MAC stats if periodic DMA is not supported
  net/sfc: avoid failure on port start if Rx mode is rejected
  net/sfc: add support for MCDI proxy
  net/sfc: add VFs to the table of PCI IDs for supported NICs

 doc/guides/nics/features/sfc_efx.ini   |   1 +
 doc/guides/nics/sfc_efx.rst|   9 +++
 doc/guides/rel_notes/release_17_05.rst |   4 ++
 drivers/net/sfc/base/efx_mcdi.c|  24 +---
 drivers/net/sfc/base/efx_mcdi.h|   2 +-
 drivers/net/sfc/efsys.h|   2 +-
 drivers/net/sfc/sfc.h  |  13 
 drivers/net/sfc/sfc_ethdev.c   |   4 ++
 drivers/net/sfc/sfc_kvargs.c   |  20 ++
 drivers/net/sfc/sfc_kvargs.h   |   5 ++
 drivers/net/sfc/sfc_mcdi.c |  74 +--
 drivers/net/sfc/sfc_port.c | 107 +
 drivers/net/sfc/sfc_rx.c   |  54 -
 13 files changed, 285 insertions(+), 34 deletions(-)

-- 
2.9.3

[dpdk-dev] [PATCH v2 1/6] net/sfc/base: don't ignore requested MAC stats update period

[Andrew Rybchenko]

Signed-off-by: Andrew Rybchenko 
---
 drivers/net/sfc/base/efx_mcdi.c | 24 ++--
 drivers/net/sfc/base/efx_mcdi.h |  2 +-
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/net/sfc/base/efx_mcdi.c b/drivers/net/sfc/base/efx_mcdi.c
index ac432a6..c9d29a7 100644
--- a/drivers/net/sfc/base/efx_mcdi.c
+++ b/drivers/net/sfc/base/efx_mcdi.c
@@ -1722,7 +1722,8 @@ static__checkReturn   efx_rc_t
 efx_mcdi_mac_stats(
__inefx_nic_t *enp,
__in_optefsys_mem_t *esmp,
-   __inefx_stats_action_t action)
+   __inefx_stats_action_t action,
+   __inuint16_t period_ms)
 {
efx_mcdi_req_t req;
uint8_t payload[MAX(MC_CMD_MAC_STATS_IN_LEN,
@@ -1747,7 +1748,7 @@ efx_mcdi_mac_stats(
MAC_STATS_IN_PERIODIC_CHANGE, enable | events | disable,
MAC_STATS_IN_PERIODIC_ENABLE, enable | events,
MAC_STATS_IN_PERIODIC_NOEVENT, !events,
-   MAC_STATS_IN_PERIOD_MS, (enable | events) ? 1000 : 0);
+   MAC_STATS_IN_PERIOD_MS, (enable | events) ? period_ms : 0);
 
if (esmp != NULL) {
int bytes = MC_CMD_MAC_NSTATS * sizeof (uint64_t);
@@ -1797,7 +1798,7 @@ efx_mcdi_mac_stats_clear(
 {
efx_rc_t rc;
 
-   if ((rc = efx_mcdi_mac_stats(enp, NULL, EFX_STATS_CLEAR)) != 0)
+   if ((rc = efx_mcdi_mac_stats(enp, NULL, EFX_STATS_CLEAR, 0)) != 0)
goto fail1;
 
return (0);
@@ -1820,7 +1821,7 @@ efx_mcdi_mac_stats_upload(
 * avoid having to pull the statistics buffer into the cache to
 * maintain cumulative statistics.
 */
-   if ((rc = efx_mcdi_mac_stats(enp, esmp, EFX_STATS_UPLOAD)) != 0)
+   if ((rc = efx_mcdi_mac_stats(enp, esmp, EFX_STATS_UPLOAD, 0)) != 0)
goto fail1;
 
return (0);
@@ -1835,7 +1836,7 @@ efx_mcdi_mac_stats_upload(
 efx_mcdi_mac_stats_periodic(
__inefx_nic_t *enp,
__inefsys_mem_t *esmp,
-   __inuint16_t period,
+   __inuint16_t period_ms,
__inboolean_t events)
 {
efx_rc_t rc;
@@ -1844,14 +1845,17 @@ efx_mcdi_mac_stats_periodic(
 * The MC DMAs aggregate statistics for our convenience, so we can
 * avoid having to pull the statistics buffer into the cache to
 * maintain cumulative statistics.
-* Huntington uses a fixed 1sec period, so use that on Siena too.
+* Huntington uses a fixed 1sec period.
+* Medford uses a fixed 1sec period before v6.2.1.1033 firmware.
 */
-   if (period == 0)
-   rc = efx_mcdi_mac_stats(enp, NULL, EFX_STATS_DISABLE);
+   if (period_ms == 0)
+   rc = efx_mcdi_mac_stats(enp, NULL, EFX_STATS_DISABLE, 0);
else if (events)
-   rc = efx_mcdi_mac_stats(enp, esmp, EFX_STATS_ENABLE_EVENTS);
+   rc = efx_mcdi_mac_stats(enp, esmp, EFX_STATS_ENABLE_EVENTS,
+   period_ms);
else
-   rc = efx_mcdi_mac_stats(enp, esmp, EFX_STATS_ENABLE_NOEVENTS);
+   rc = efx_mcdi_mac_stats(enp, esmp, EFX_STATS_ENABLE_NOEVENTS,
+   period_ms);
 
if (rc != 0)
goto fail1;
diff --git a/drivers/net/sfc/base/efx_mcdi.h b/drivers/net/sfc/base/efx_mcdi.h
index 814f3f4..7faabbb 100644
--- a/drivers/net/sfc/base/efx_mcdi.h
+++ b/drivers/net/sfc/base/efx_mcdi.h
@@ -216,7 +216,7 @@ extern  __checkReturn   efx_rc_t
 efx_mcdi_mac_stats_periodic(
__inefx_nic_t *enp,
__inefsys_mem_t *esmp,
-   __inuint16_t period,
+   __inuint16_t period_ms,
__inboolean_t events);
 
 
-- 
2.9.3

[dpdk-dev] [PATCH v2 4/6] net/sfc: avoid failure on port start if Rx mode is rejected

[Andrew Rybchenko]

From: Ivan Malov 

If Rx mode is unacceptable, in particular, when promiscuous
or all-multicast filters are not allowed while running over
PCI function which is not a member of appropriate privilege
groups, the driver has to cope with the failures gracefully

Signed-off-by: Ivan Malov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
---
 drivers/net/sfc/sfc_rx.c | 54 +---
 1 file changed, 51 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index 906536e..0a9fa42 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -401,6 +401,56 @@ sfc_rx_qflush(struct sfc_adapter *sa, unsigned int 
sw_index)
sfc_rx_qpurge(rxq);
 }
 
+static int
+sfc_rx_default_rxq_set_filter(struct sfc_adapter *sa, struct sfc_rxq *rxq)
+{
+   boolean_t rss = (sa->rss_channels > 1) ? B_TRUE : B_FALSE;
+   struct sfc_port *port = &sa->port;
+   int rc;
+
+   /*
+* If promiscuous or all-multicast mode has been requested, setting
+* filter for the default Rx queue might fail, in particular, while
+* running over PCI function which is not a member of corresponding
+* privilege groups; if this occurs, few iterations will be made to
+* repeat this step without promiscuous and all-multicast flags set
+*/
+retry:
+   rc = efx_mac_filter_default_rxq_set(sa->nic, rxq->common, rss);
+   if (rc == 0)
+   return 0;
+   else if (rc != EOPNOTSUPP)
+   return rc;
+
+   if (port->promisc) {
+   sfc_warn(sa, "promiscuous mode has been requested, "
+"but the HW rejects it");
+   sfc_warn(sa, "promiscuous mode will be disabled");
+
+   port->promisc = B_FALSE;
+   rc = sfc_set_rx_mode(sa);
+   if (rc != 0)
+   return rc;
+
+   goto retry;
+   }
+
+   if (port->allmulti) {
+   sfc_warn(sa, "all-multicast mode has been requested, "
+"but the HW rejects it");
+   sfc_warn(sa, "all-multicast mode will be disabled");
+
+   port->allmulti = B_FALSE;
+   rc = sfc_set_rx_mode(sa);
+   if (rc != 0)
+   return rc;
+
+   goto retry;
+   }
+
+   return rc;
+}
+
 int
 sfc_rx_qstart(struct sfc_adapter *sa, unsigned int sw_index)
 {
@@ -439,9 +489,7 @@ sfc_rx_qstart(struct sfc_adapter *sa, unsigned int sw_index)
sfc_rx_qrefill(rxq);
 
if (sw_index == 0) {
-   rc = efx_mac_filter_default_rxq_set(sa->nic, rxq->common,
-   (sa->rss_channels > 1) ?
-   B_TRUE : B_FALSE);
+   rc = sfc_rx_default_rxq_set_filter(sa, rxq);
if (rc != 0)
goto fail_mac_filter_default_rxq_set;
}
-- 
2.9.3

[dpdk-dev] [PATCH v2 3/6] net/sfc: poll MAC stats if periodic DMA is not supported

[Andrew Rybchenko]

From: Ivan Malov 

If periodic DMA statistics feature is absent (particularly,
while running over VF), the PMD must provide an ability to
cope with it using explicit update requests which are kept
restrained according to 'stats_update_period_ms' parameter

Signed-off-by: Ivan Malov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andy Moreton 
---
 drivers/net/sfc/sfc.h  |  9 +
 drivers/net/sfc/sfc_port.c | 27 ---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index f634f92..96cd38c 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -156,6 +156,8 @@ struct sfc_port {
boolean_t   mac_stats_reset_pending;
uint16_tmac_stats_update_period_ms;
uint32_tmac_stats_update_generation;
+   boolean_t   mac_stats_periodic_dma_supported;
+   uint64_tmac_stats_last_request_timestamp;
 
uint32_tmac_stats_mask[EFX_MAC_STATS_MASK_NPAGES];
 };
@@ -255,6 +257,13 @@ sfc_adapter_lock_fini(__rte_unused struct sfc_adapter *sa)
/* Just for symmetry of the API */
 }
 
+/** Get the number of milliseconds since boot from the default timer */
+static inline uint64_t
+sfc_get_system_msecs(void)
+{
+   return rte_get_timer_cycles() * MS_PER_S / rte_get_timer_hz();
+}
+
 int sfc_dma_alloc(const struct sfc_adapter *sa, const char *name, uint16_t id,
  size_t len, int socket_id, efsys_mem_t *esmp);
 void sfc_dma_free(const struct sfc_adapter *sa, efsys_mem_t *esmp);
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index 84dde76..2276131 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -67,8 +67,23 @@ sfc_port_update_mac_stats(struct sfc_adapter *sa)
if (sa->state != SFC_ADAPTER_STARTED)
return EINVAL;
 
-   /* If periodic statistics DMA'ing is off, request explicitly */
-   if (port->mac_stats_update_period_ms == 0) {
+   /*
+* If periodic statistics DMA'ing is off or if not supported,
+* make a manual request and keep an eye on timer if need be
+*/
+   if (!port->mac_stats_periodic_dma_supported ||
+   (port->mac_stats_update_period_ms == 0)) {
+   if (port->mac_stats_update_period_ms != 0) {
+   uint64_t timestamp = sfc_get_system_msecs();
+
+   if ((timestamp -
+port->mac_stats_last_request_timestamp) <
+   port->mac_stats_update_period_ms)
+   return 0;
+
+   port->mac_stats_last_request_timestamp = timestamp;
+   }
+
rc = efx_mac_stats_upload(sa->nic, esmp);
if (rc != 0)
return rc;
@@ -209,8 +224,14 @@ sfc_port_start(struct sfc_adapter *sa)
rc = efx_mac_stats_periodic(sa->nic, &port->mac_stats_dma_mem,
port->mac_stats_update_period_ms,
B_FALSE);
-   if (rc != 0)
+   if (rc == 0) {
+   port->mac_stats_periodic_dma_supported = B_TRUE;
+   } else if (rc == EOPNOTSUPP) {
+   port->mac_stats_periodic_dma_supported = B_FALSE;
+   port->mac_stats_last_request_timestamp = 0;
+   } else {
goto fail_mac_stats_periodic;
+   }
}
 
sfc_log_init(sa, "disable MAC drain");
-- 
2.9.3

[dpdk-dev] [PATCH v2 2/6] net/sfc: add kvarg control for MAC statistics update period

[Andrew Rybchenko]

From: Ivan Malov 

The patch is to make MAC statistics update interval tunable
by means of 'stats_update_period_ms' kvarg parameter making
it possible to use values different from 1000 ms in case of
SFN8xxx boards provided that firmware version is 6.2.1.1033

Signed-off-by: Ivan Malov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
---
 doc/guides/nics/sfc_efx.rst  |  9 +
 drivers/net/sfc/sfc.h|  2 ++
 drivers/net/sfc/sfc_ethdev.c |  1 +
 drivers/net/sfc/sfc_kvargs.c | 20 +++
 drivers/net/sfc/sfc_kvargs.h |  5 +++
 drivers/net/sfc/sfc_port.c   | 86 +---
 6 files changed, 110 insertions(+), 13 deletions(-)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 0a05a0a..0130290 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -197,3 +197,12 @@ boolean parameters value.
   Enable extra logging of the communication with the NIC's management CPU.
   The logging is done using RTE_LOG() with INFO level and PMD type.
   The format is consumed by the Solarflare netlogdecode cross-platform tool.
+
+- ``stats_update_period_ms`` [long] (default **1000**)
+
+  Adjust period in milliseconds to update port hardware statistics.
+  The accepted range is 0 to 65535. The value of **0** may be used
+  to disable periodic statistics update. One should note that it's
+  only possible to set an arbitrary value on SFN8xxx provided that
+  firmware version is 6.2.1.1033 or higher, otherwise any positive
+  value will select a fixed update period of **1000** milliseconds
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 655328f..f634f92 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -154,6 +154,8 @@ struct sfc_port {
uint64_t*mac_stats_buf;
efsys_mem_t mac_stats_dma_mem;
boolean_t   mac_stats_reset_pending;
+   uint16_tmac_stats_update_period_ms;
+   uint32_tmac_stats_update_generation;
 
uint32_tmac_stats_mask[EFX_MAC_STATS_MASK_NPAGES];
 };
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index cac01ac..4115454 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1382,5 +1382,6 @@ RTE_PMD_REGISTER_PCI_TABLE(net_sfc_efx, 
pci_id_sfc_efx_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_sfc_efx, "* igb_uio | uio_pci_generic | vfio");
 RTE_PMD_REGISTER_PARAM_STRING(net_sfc_efx,
SFC_KVARG_PERF_PROFILE "=" SFC_KVARG_VALUES_PERF_PROFILE " "
+   SFC_KVARG_STATS_UPDATE_PERIOD_MS "= "
SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL " "
SFC_KVARG_DEBUG_INIT "=" SFC_KVARG_VALUES_BOOL);
diff --git a/drivers/net/sfc/sfc_kvargs.c b/drivers/net/sfc/sfc_kvargs.c
index 227a8db..60a7ac1 100644
--- a/drivers/net/sfc/sfc_kvargs.c
+++ b/drivers/net/sfc/sfc_kvargs.c
@@ -42,6 +42,7 @@ sfc_kvargs_parse(struct sfc_adapter *sa)
struct rte_eth_dev *eth_dev = (sa)->eth_dev;
struct rte_devargs *devargs = eth_dev->device->devargs;
const char **params = (const char *[]){
+   SFC_KVARG_STATS_UPDATE_PERIOD_MS,
SFC_KVARG_DEBUG_INIT,
SFC_KVARG_MCDI_LOGGING,
SFC_KVARG_PERF_PROFILE,
@@ -110,3 +111,22 @@ sfc_kvarg_bool_handler(__rte_unused const char *key,
 
return 0;
 }
+
+int
+sfc_kvarg_long_handler(__rte_unused const char *key,
+  const char *value_str, void *opaque)
+{
+   long value;
+   char *endptr;
+
+   if (!value_str || !opaque)
+   return -EINVAL;
+
+   value = strtol(value_str, &endptr, 0);
+   if (endptr == value_str)
+   return -EINVAL;
+
+   *(long *)opaque = value;
+
+   return 0;
+}
diff --git a/drivers/net/sfc/sfc_kvargs.h b/drivers/net/sfc/sfc_kvargs.h
index 2fea9c7..6c6003a 100644
--- a/drivers/net/sfc/sfc_kvargs.h
+++ b/drivers/net/sfc/sfc_kvargs.h
@@ -52,6 +52,8 @@ extern "C" {
SFC_KVARG_PERF_PROFILE_THROUGHPUT "|" \
SFC_KVARG_PERF_PROFILE_LOW_LATENCY "]"
 
+#define SFC_KVARG_STATS_UPDATE_PERIOD_MS   "stats_update_period_ms"
+
 struct sfc_adapter;
 
 int sfc_kvargs_parse(struct sfc_adapter *sa);
@@ -63,6 +65,9 @@ int sfc_kvargs_process(struct sfc_adapter *sa, const char 
*key_match,
 int sfc_kvarg_bool_handler(const char *key, const char *value_str,
   void *opaque);
 
+int sfc_kvarg_long_handler(const char *key, const char *value_str,
+  void *opaque);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index e2f5043..84dde76 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -31,6 +31,16 @@
 
 #include "sfc.h"
 #include "sfc_log.h"
+#include "sfc_kvargs.h"
+
+/** Default MAC statistics update period is 1 second */
+#define SFC_MAC_STATS_UPDATE_PERIO

[dpdk-dev] [PATCH v2 6/6] net/sfc: add VFs to the table of PCI IDs for supported NICs

[Andrew Rybchenko]

From: Ivan Malov 

Signed-off-by: Ivan Malov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andrew Lee 
---
 doc/guides/nics/features/sfc_efx.ini   | 1 +
 doc/guides/rel_notes/release_17_05.rst | 4 
 drivers/net/sfc/sfc_ethdev.c   | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/doc/guides/nics/features/sfc_efx.ini 
b/doc/guides/nics/features/sfc_efx.ini
index 3a15baa..e811395 100644
--- a/doc/guides/nics/features/sfc_efx.ini
+++ b/doc/guides/nics/features/sfc_efx.ini
@@ -18,6 +18,7 @@ Multicast MAC filter = Y
 RSS hash = Y
 RSS key update   = Y
 RSS reta update  = Y
+SR-IOV   = Y
 Flow control = Y
 VLAN offload = P
 L3 checksum offload  = Y
diff --git a/doc/guides/rel_notes/release_17_05.rst 
b/doc/guides/rel_notes/release_17_05.rst
index 123eeb4..5ae8d97 100644
--- a/doc/guides/rel_notes/release_17_05.rst
+++ b/doc/guides/rel_notes/release_17_05.rst
@@ -64,6 +64,10 @@ New Features
   performance enhancements viz. configurable TX data ring, Receive
   Data Ring, ability to register memory regions.
 
+* **Updated the sfc_efx driver.**
+
+  * Support virtual functions (VFs)
+
 
 Resolved Issues
 ---
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 4115454..240e87d 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1358,8 +1358,11 @@ sfc_eth_dev_uninit(struct rte_eth_dev *dev)
 
 static const struct rte_pci_id pci_id_sfc_efx_map[] = {
{ RTE_PCI_DEVICE(EFX_PCI_VENID_SFC, EFX_PCI_DEVID_FARMINGDALE) },
+   { RTE_PCI_DEVICE(EFX_PCI_VENID_SFC, EFX_PCI_DEVID_FARMINGDALE_VF) },
{ RTE_PCI_DEVICE(EFX_PCI_VENID_SFC, EFX_PCI_DEVID_GREENPORT) },
+   { RTE_PCI_DEVICE(EFX_PCI_VENID_SFC, EFX_PCI_DEVID_GREENPORT_VF) },
{ RTE_PCI_DEVICE(EFX_PCI_VENID_SFC, EFX_PCI_DEVID_MEDFORD) },
+   { RTE_PCI_DEVICE(EFX_PCI_VENID_SFC, EFX_PCI_DEVID_MEDFORD_VF) },
{ .vendor_id = 0 /* sentinel */ }
 };
 
-- 
2.9.3

[dpdk-dev] [PATCH v2 5/6] net/sfc: add support for MCDI proxy

[Andrew Rybchenko]

From: Ivan Malov 

The patch is to add support for MCDI proxy which comes in
useful, particularly, while running over VF: few commands
will normally fail with EPERM, but in some cases the host
driver (i.e. running over the corresponding PF, typically,
within a hypervisor) may set itself as a proxy to conduct
authorization for the commands coming from VFs; these are
forwarded to the corresponding access control application
which may decline or approve authorization by replying to
the requests; all in all, the guest driver has to process
the replies forwarded back by the firmware MC in order to
give up gracefully (by setting return code which could be
understood by 'libefx') or re-issue the original commands

Signed-off-by: Ivan Malov 
Signed-off-by: Andrew Rybchenko 
Reviewed-by: Andy Moreton 
---
 drivers/net/sfc/efsys.h|  2 +-
 drivers/net/sfc/sfc.h  |  2 ++
 drivers/net/sfc/sfc_mcdi.c | 74 ++
 3 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/drivers/net/sfc/efsys.h b/drivers/net/sfc/efsys.h
index 60829be..d52552b 100644
--- a/drivers/net/sfc/efsys.h
+++ b/drivers/net/sfc/efsys.h
@@ -177,7 +177,7 @@ prefetch_read_once(const volatile void *addr)
 /* MCDI is required for SFN7xxx and SFN8xx */
 #define EFSYS_OPT_MCDI 1
 #define EFSYS_OPT_MCDI_LOGGING 1
-#define EFSYS_OPT_MCDI_PROXY_AUTH 0
+#define EFSYS_OPT_MCDI_PROXY_AUTH 1
 
 #define EFSYS_OPT_MAC_STATS 1
 
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 96cd38c..114bf95 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -125,6 +125,8 @@ struct sfc_mcdi {
enum sfc_mcdi_state state;
efx_mcdi_transport_ttransport;
boollogging;
+   uint32_tproxy_handle;
+   efx_rc_tproxy_result;
 };
 
 struct sfc_intr {
diff --git a/drivers/net/sfc/sfc_mcdi.c b/drivers/net/sfc/sfc_mcdi.c
index 3bed2e0..43dbf13 100644
--- a/drivers/net/sfc/sfc_mcdi.c
+++ b/drivers/net/sfc/sfc_mcdi.c
@@ -36,6 +36,7 @@
 #include "sfc.h"
 #include "sfc_log.h"
 #include "sfc_kvargs.h"
+#include "sfc_ev.h"
 
 #define SFC_MCDI_POLL_INTERVAL_MIN_US  10  /* 10us in 1us units */
 #define SFC_MCDI_POLL_INTERVAL_MAX_US  (US_PER_S / 10) /* 100ms in 1us units */
@@ -49,8 +50,22 @@ sfc_mcdi_timeout(struct sfc_adapter *sa)
sfc_panic(sa, "MCDI timeout handling is not implemented\n");
 }
 
+static inline boolean_t
+sfc_mcdi_proxy_event_available(struct sfc_adapter *sa)
+{
+   struct sfc_mcdi *mcdi = &sa->mcdi;
+
+   mcdi->proxy_handle = 0;
+   mcdi->proxy_result = ETIMEDOUT;
+   sfc_ev_mgmt_qpoll(sa);
+   if (mcdi->proxy_result != ETIMEDOUT)
+   return B_TRUE;
+
+   return B_FALSE;
+}
+
 static void
-sfc_mcdi_poll(struct sfc_adapter *sa)
+sfc_mcdi_poll(struct sfc_adapter *sa, boolean_t proxy)
 {
efx_nic_t *enp;
unsigned int delay_total;
@@ -62,13 +77,20 @@ sfc_mcdi_poll(struct sfc_adapter *sa)
enp = sa->nic;
 
do {
-   if (efx_mcdi_request_poll(enp))
+   boolean_t poll_completed;
+
+   poll_completed = (proxy) ? sfc_mcdi_proxy_event_available(sa) :
+  efx_mcdi_request_poll(enp);
+   if (poll_completed)
return;
 
if (delay_total > SFC_MCDI_WATCHDOG_INTERVAL_US) {
-   aborted = efx_mcdi_request_abort(enp);
-   SFC_ASSERT(aborted);
-   sfc_mcdi_timeout(sa);
+   if (!proxy) {
+   aborted = efx_mcdi_request_abort(enp);
+   SFC_ASSERT(aborted);
+   sfc_mcdi_timeout(sa);
+   }
+
return;
}
 
@@ -90,13 +112,42 @@ sfc_mcdi_execute(void *arg, efx_mcdi_req_t *emrp)
 {
struct sfc_adapter *sa = (struct sfc_adapter *)arg;
struct sfc_mcdi *mcdi = &sa->mcdi;
+   uint32_t proxy_handle;
 
rte_spinlock_lock(&mcdi->lock);
 
SFC_ASSERT(mcdi->state == SFC_MCDI_INITIALIZED);
 
efx_mcdi_request_start(sa->nic, emrp, B_FALSE);
-   sfc_mcdi_poll(sa);
+   sfc_mcdi_poll(sa, B_FALSE);
+
+   if (efx_mcdi_get_proxy_handle(sa->nic, emrp, &proxy_handle) == 0) {
+   /*
+* Authorization is required for the MCDI request;
+* wait for an MCDI proxy response event to bring
+* a non-zero proxy handle (should be the same as
+* the value obtained above) and operation status
+*/
+   sfc_mcdi_poll(sa, B_TRUE);
+
+   if ((mcdi->proxy_handle != 0) &&
+   (mcdi->proxy_handle != proxy_handle)) {
+   sfc_err(sa, "Unexpected MCDI proxy event");
+   emrp->emr_rc = EFAULT;
+   

Re: [dpdk-dev] [PATCH v2 00/11] Support flow API in Solarflare PMD

[Ferruh Yigit]

On 3/9/2017 3:26 PM, Andrew Rybchenko wrote:
> Support simple queue destination flow API filters in Solarflare
> libefx-based PMD including:
>  - Ethernet source/destination, EtherType exact matching
>  - VLAN ID exact matching including double-tagging
>  - IPv4/6 source/destination and IP protocol exact matching
>  - TCP/UDP source/destination exact matching
> 
> Supported combinations of fields mentioned above depend on
> firmware (including running variant) and correctly processed by
> validate callback.
> 
> v1 -> v2:
>  - fix spelling
>  - add comments to protocol parsers
>  - add release notes
>  - add l3 and l4 layer enum items on demand
> 
> Andrew Rybchenko (2):
>   net/sfc: implement dummy filter control callback
>   net/sfc: provide a way to check if filter is supported
> 
> Mark Spender (2):
>   net/sfc/base: split local MAC I/G back into separate flags
>   net/sfc/base: improve API to get supported filter matches
> 
> Roman Zhukov (7):
>   net/sfc: add flow API filters support
>   net/sfc: add VLAN in flow API filters support
>   net/sfc: add IPV4 in flow API filters support
>   net/sfc: add IPV6 in flow API filters support
>   net/sfc: add TCP in flow API filters support
>   net/sfc: add UDP in flow API filters support
>   net/sfc: add unknown unicast/multicast match in flow API

Series applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH v3] eal: sPAPR IOMMU support in pci probing for vfio-pci in ppc64le

[Thomas Monjalon]

2017-03-09 09:38, Chao Zhu:
> > From: Gowrishankar Muthukrishnan 
> > 
> > Below changes adds pci probing support for vfio-pci devices in power8.
> > 
> > v3 - better validation for kernel not implementing few iocts called
> > v2 - kernel version checked and doc updated
> > 
> > Signed-off-by: Gowrishankar Muthukrishnan
> > 
> 
> Acked-by: Chao Zhu 

Applied, thanks

Re: [dpdk-dev] [PATCH] rte, eal: Rename pci_update_device and make it public

[Thomas Monjalon]

2017-02-13 10:53, Ziye Yang:
> The reaon is that sometimes we only like to rebound the
> kernel driver or VFIO or UIO or other drivers for
> this device after rte_eal_detach function. Function
> rte_eal_pci_probe_one not only updates the device but
> also probes the rte_eal_driver for this device, it is
> not flexible.
> 
> Signed-off-by: Ziye Yang 
> ---
>  lib/librte_eal/bsdapp/eal/eal_pci.c |  2 +-
>  lib/librte_eal/common/eal_common_pci.c  |  2 +-
>  lib/librte_eal/common/eal_private.h | 13 -
>  lib/librte_eal/common/include/rte_pci.h | 14 ++
>  lib/librte_eal/linuxapp/eal/eal_pci.c   |  2 +-
>  5 files changed, 17 insertions(+), 16 deletions(-)

The PCI bus management is going to change with the new bus framework.
Please check pending developments or future version for your needs.

This patch will be marked as rejected because of the conflicting work
in progress.

Re: [dpdk-dev] [PATCH 1/2] eal_interrupts: support external rxq interrupt handlers

[Thomas Monjalon]

2017-02-08 15:57, Shahaf Shuler:
>  /**
> + * It deletes registered eventfds.
> + *
> + * @param intr_handle
> + *   Pointer to the interrupt handle.
> + */
> +void
> +rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle);

This patch looks OK except a miss in the EAL map files for this new function.
The title should be "eal/linux: support external Rx interrupt"

I think this patch should target next-net.
Please Ferruh, take care of this patchset. Thanks

Re: [dpdk-dev] [PATCH 2/2] ethdev: move queue id check in generic layer

[Thomas Monjalon]

2017-02-17 16:25, Olivier Matz:
> The check of queue_id is done in all drivers implementing
> rte_eth_rx_queue_count(). Factorize this check in the generic function.
> 
> Note that the nfp driver was doing the check differently, which could
> induce crashes if the queue index was too big.
> 
> Signed-off-by: Olivier Matz 
> ---
> 
> This commit was part of RFC patchset that will be reworked:
> http://dpdk.org/dev/patchwork/patch/17232/
> It can be processed separately.

Series applied, thanks

Re: [dpdk-dev] [PATCH v11 5/7] mbuf: add a timestamp to the mbuf for latencystats

[Stephen Hemminger]

On Thu,  9 Mar 2017 16:25:32 +
Remy Horton  wrote:

> From: Harry van Haaren 
> 
> This commit adds a uint64_t to the mbuf struct,
> allowing collection of latency and jitter statistics
> by measuring packet I/O timestamps. This change is
> required by the latencystats library.
> 
> Signed-off-by: Reshma Pattan 
> Signed-off-by: Harry van Haaren 
> ---
>  lib/librte_mbuf/rte_mbuf.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index ce57d47..e0dad6e 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -514,6 +514,9 @@ struct rte_mbuf {
>  
>   /** Timesync flags for use with IEEE1588. */
>   uint16_t timesync;
> +
> + /** Timestamp for measuring latency. */
> + uint64_t timestamp;
>  } __rte_cache_aligned;
>  
>  /**

This creates a hole in the mbuf structure, and won't apply to current
version of mbuf that has priv_size.

[dpdk-dev] [PATCH v2 0/3] crypto/qat: add support for Intel QAT device D15xx

[Fiona Trahe]

Add support for Intel QuickAssist Technology device D15xx

v2 changes:
 - update documentation

Fiona Trahe (3):
  crypto/qat: add support for Intel QAT device D15xx
  doc: restructure QAT PMD guide
  doc: add support for Intel QAT device D15xx

 doc/guides/cryptodevs/qat.rst  | 374 ++---
 drivers/crypto/qat/rte_qat_cryptodev.c |   3 +
 2 files changed, 155 insertions(+), 222 deletions(-)

-- 
2.5.0

[dpdk-dev] [PATCH v2 1/3] crypto/qat: add support for Intel QAT device D15xx

[Fiona Trahe]

Add support for Intel QuickAssist Technology device D15xx

Signed-off-by: Fiona Trahe 
---
 drivers/crypto/qat/rte_qat_cryptodev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/crypto/qat/rte_qat_cryptodev.c 
b/drivers/crypto/qat/rte_qat_cryptodev.c
index 5b34f5e..386a449 100644
--- a/drivers/crypto/qat/rte_qat_cryptodev.c
+++ b/drivers/crypto/qat/rte_qat_cryptodev.c
@@ -77,6 +77,9 @@ static const struct rte_pci_id pci_id_qat_map[] = {
{
RTE_PCI_DEVICE(0x8086, 0x19e3),
},
+   {
+   RTE_PCI_DEVICE(0x8086, 0x6f55),
+   },
{.device_id = 0},
 };
 
-- 
2.5.0

[dpdk-dev] [PATCH v2 3/3] doc: add support for Intel QAT device D15xx

[Fiona Trahe]

Add instructions for D15xx device to the QAT PMD guide.

Signed-off-by: Fiona Trahe 
---
 doc/guides/cryptodevs/qat.rst | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/doc/guides/cryptodevs/qat.rst b/doc/guides/cryptodevs/qat.rst
index 4927a00..897919e 100644
--- a/doc/guides/cryptodevs/qat.rst
+++ b/doc/guides/cryptodevs/qat.rst
@@ -36,6 +36,7 @@ hardware accelerator devices:
 * ``Intel QuickAssist Technology DH895xCC``
 * ``Intel QuickAssist Technology C62x``
 * ``Intel QuickAssist Technology C3xxx``
+* ``Intel QuickAssist Technology D15xx``
 
 
 Features
@@ -111,12 +112,15 @@ available kernel drivers and device ids are :
DH895xCC,4.4+,qat_dh895xcc,dh895xcc,435,1,443,32
C62x,4.5+,qat_c62x,c6xx,37c8,3,37c9,16
C3xxx,4.5+,qat_c3xxx,c3xxx,19e2,1,19e3,16
+   D15xx,**,qat_d15xx,d15xx,6f54,1,6f55,16
 
 
 The ``driver`` column indicates either the linux kernel version in which 
support
 for this device was introduced or a driver available on Intel's 01.org website.
 There are both linux and 01.org kernel drivers available for some devices.
 
+** Release to be confirmed.
+
 
 If you are running on a kernel which includes a driver for your device, see
 `Installation using kernel.org driver`_ below. Otherwise see
@@ -308,7 +312,7 @@ the unbind command below::
 done; \
 done
 
-For **Intel(R) QuickAssist Technology C3xxx** device:
+For **Intel(R) QuickAssist Technology C3xxx or D15xx** device:
 
 The unbind command below assumes ``bdfs`` of ``01:01.00-01:02.07``,
 if your VFs are different adjust the unbind command below::
-- 
2.5.0

[dpdk-dev] [PATCH v2 2/3] doc: restructure QAT PMD guide

[Fiona Trahe]

Restructure QAT PMD instructions and add a device table to
minimise duplication for each device and make it easier to add devices.
Fix some device name typos.

Signed-off-by: Fiona Trahe 
---
 doc/guides/cryptodevs/qat.rst | 370 +-
 1 file changed, 148 insertions(+), 222 deletions(-)

diff --git a/doc/guides/cryptodevs/qat.rst b/doc/guides/cryptodevs/qat.rst
index 9ecd19b..8a31741 100644
--- a/doc/guides/cryptodevs/qat.rst
+++ b/doc/guides/cryptodevs/qat.rst
@@ -30,9 +30,12 @@
 Intel(R) QuickAssist (QAT) Crypto Poll Mode Driver
 ==
 
-The QAT PMD provides poll mode crypto driver support for **Intel QuickAssist
-Technology DH895xxC**, **Intel QuickAssist Technology C62x** and
-**Intel QuickAssist Technology C3xxx** hardware accelerator.
+The QAT PMD provides poll mode crypto driver support for the following
+hardware accelerator devices:
+
+* ``Intel QuickAssist Technology DH895xCC``
+* ``Intel QuickAssist Technology C62x``
+* ``Intel QuickAssist Technology C3xxx``
 
 
 Features
@@ -84,37 +87,116 @@ Limitations
 Installation
 
 
-To use the DPDK QAT PMD an SRIOV-enabled QAT kernel driver is required. The
-VF devices exposed by this driver will be used by QAT PMD.
-
-To enable QAT in DPDK, follow the instructions mentioned in
-http://dpdk.org/doc/guides/linux_gsg/build_dpdk.html
+To enable QAT in DPDK, follow the instructions for modifying the compile-time
+configuration file as described `here 
`_.
 
-Quick instructions as follows:
+Quick instructions are as follows:
 
 .. code-block:: console
 
+   cd to the top-level DPDK directory
make config T=x86_64-native-linuxapp-gcc
sed -i 's,\(CONFIG_RTE_LIBRTE_PMD_QAT\)=n,\1=y,' build/.config
make
 
-If you are running on kernel 4.4 or greater, see instructions for
-`Installation using kernel.org driver`_ below. If you are on a kernel earlier
-than 4.4, see `Installation using 01.org QAT driver`_.
+To use the DPDK QAT PMD an SRIOV-enabled QAT kernel driver is required. The
+VF devices exposed by this driver will be used by QAT PMD. The devices and
+available kernel drivers and device ids are :
 
-For **Intel QuickAssist Technology C62x** and **Intel QuickAssist Technology 
C3xxx**
-device, kernel 4.5 or greater is needed.
-See instructions for `Installation using kernel.org driver`_ below.
+.. csv-table::
+   :header: "device", "driver", "kernel module", "pci driver", "PF DID", "num 
PFs", "VF DID", "num VFs per PF"
+   :stub-columns: 1
 
+   DH895xCC,01.org,icp_qa_al, n/a,435,1,443,32
+   DH895xCC,4.4+,qat_dh895xcc,dh895xcc,435,1,443,32
+   C62x,4.5+,qat_c62x,c6xx,37c8,3,37c9,16
+   C3xxx,4.5+,qat_c3xxx,c3xxx,19e2,1,19e3,16
 
-Installation using 01.org QAT driver
+
+The ``driver`` column indicates either the linux kernel version in which 
support
+for this device was introduced or a driver available on Intel's 01.org website.
+There are both linux and 01.org kernel drivers available for some devices.
+
+
+If you are running on a kernel which includes a driver for your device, see
+`Installation using kernel.org driver`_ below. Otherwise see
+`Installation using 01.org QAT driver`_.
+
+
+Installation using kernel.org driver
 
 
-NOTE: There is no driver available for **Intel QuickAssist Technology C62x** 
and
-**Intel QuickAssist Technology C3xxx** devices on 01.org.
+The examples below are based on the C62x device, if you have a different device
+use the corresponding values in the above table.
+
+In BIOS ensure that SRIOV is enabled and either
+a) disable VT-d or
+b) enable VT-d and set ``"intel_iommu=on iommu=pt"`` in the grub file.
+
+Check that the QAT driver is loaded on your system, by executing::
+
+lsmod | grep qa
+
+You should see the kernel module for your device listed, e.g.::
+
+qat_c62x   5626  0
+intel_qat  82336  1 qat_c62x
+
+Next, you need to expose the Virtual Functions (VFs) using the sysfs file 
system.
+
+First find the bdfs of the physical functions (PFs) of your device, e.g.::
+
+lspci -d : 37c8
+
+You should see output similar to::
+
+1a:00.0 Co-processor: Intel Corporation Device 37c8
+3d:00.0 Co-processor: Intel Corporation Device 37c8
+3f:00.0 Co-processor: Intel Corporation Device 37c8
+
+
+Enable the VFs for each PF by echoing the number of VFs per PF to the pci 
driver::
+
+ echo 16 > /sys/bus/pci/drivers/c6xx/\:1a\:00.0/sriov_numvfs
+ echo 16 > /sys/bus/pci/drivers/c6xx/\:3d\:00.0/sriov_numvfs
+ echo 16 > /sys/bus/pci/drivers/c6xx/\:3f\:00.0/sriov_numvfs
+
+
+Check that the VFs are available for use - e.g.
+``lspci -d:37c9`` should list 48 VF devices available for a ``C62x`` device.
+
+To complete the installation - follow instructions in `Binding the available 
VFs to the DPDK UIO driver`_.
+
+**Notes**:
+
+If the QAT kernel modules are not lo

Re: [dpdk-dev] [PATCH] examples: ethtool: Link against librte_pmd_ixgbe if necessary

[Thomas Monjalon]

2017-02-16 16:17, Markos Chandras:
> The librte_ethtool library depends on librte_pmd_ixgbe if that
> pmd driver is enabled so we need to link against it when we compile
> the ethtool application. It fixes the following build problem:
> 
> /usr/lib64/gcc/x86_64-suse-linux/6/../../../../x86_64-suse-linux/bin/ld:
> warning: librte_pmd_ixgbe.so.1, needed by /home/abuild/rpmbuild/BUILD/
> dpdk-17.02/examples/ethtool/lib/x86_64-native-linuxapp-gcc/lib/
> librte_ethtool.so, not found (try using -rpath or -rpath-link)
> /home/abuild/rpmbuild/BUILD/dpdk-17.02/examples/ethtool/lib/
> x86_64-native-linuxapp-gcc/lib/librte_ethtool.so: undefined reference to
> `rte_pmd_ixgbe_set_vf_rxmode@DPDK_17.02'
> collect2: error: ld returned 1 exit status

We may add this line:
Fixes: 077d223e25c3 ("examples/ethtool: use ixgbe public function")

> Cc: Nirmoy Das 
> Signed-off-by: Markos Chandras 
[...]
> --- a/examples/ethtool/ethtool-app/Makefile
> +++ b/examples/ethtool/ethtool-app/Makefile
> +ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
> +LDLIBS += -lrte_pmd_ixgbe
> +endif

Please, could you confirm we can remove the same LDLIBS line from
examples/ethtool/lib/Makefile ?

Re: [dpdk-dev] [PATCH] doc: fix crypto overview matrix for missing QAT items

[Thomas Monjalon]

2017-03-03 12:51, Fiona Trahe:
> Support for all the following was added in release 16.11
> but not updated in the matrix:
> 3DES, MD5_HMAC, SHA224, SHA383, NULL, KASUMI F8/F9, GMAC.
> 
> Fixes: e1b7f509e6f2 ("crypto/qat: add 3DES cipher algorithm")
> Fixes: 61ec5181625f ("crypto/qat: add MD5 HMAC capability")
> Fixes: ebdbe12fbfc1 ("crypto/qat: add aes-sha224-hmac capability")
> Fixes: d905ee32d0dc ("crypto/qat: add aes-sha384-hmac capability")
> Fixes: db0e952a5c01 ("crypto/qat: add NULL capability")
> Fixes: d4f2745300e0 ("crypto/qat: add KASUMI")
> Fixes: 2fa64f840d65 ("crypto/qat: add GMAC capability")

There was already a miss for ZUC added in previous release.
It is hard to maintain such matrix...
>  
> -   "NULL",,x,,
> +   "NULL",x,x,,
> "AES_CBC_128",x,,x,x
> "AES_CBC_192",x,,x,
> "AES_CBC_256",x,,x,
> "AES_CTR_128",x,,x,
> "AES_CTR_192",x,,x,
> "AES_CTR_256",x,,x,
> +   "3DES_CBC",x,,,
> +   "3DES_CTR",x,,,
> "DES_CBC",x,,,
> "SNOW3G_UEA2",xx,,,
> -   "KASUMI_F8",,x,,
> +   "KASUMI_F8",x,x,,
> "ZUC_EEA3",,,x,
> "AES_DOCSISBPI",x,,,
> "DES_DOCSISBPI",x,,,

... and even harder when it so unreadable!

Please switch to the features directory system used for networking drivers.