diff mbox

[dpdk-dev,1/6] ethdev: add a function to look up Rx offload names

Message ID 1515658359-1041-2-git-send-email-arybchenko@solarflare.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Andrew Rybchenko Jan. 11, 2018, 8:12 a.m. UTC 
  From: Ivan Malov <ivan.malov@oktetlabs.ru>

Commonly, drivers converted to the new offload API
may need to log unsupported offloads as a response
to wrong settings. From this perspective, it would
be convenient to have generic functions to look up
offload names. The patch adds such a helper for Rx.

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

Cc: Thomas Monjalon <thomas@monjalon.net>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Shahaf Shuler <shahafs@mellanox.com>

---
 lib/librte_ether/rte_ethdev.c           | 42 +++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.h           | 15 ++++++++++++
 lib/librte_ether/rte_ethdev_version.map |  6 +++++
 3 files changed, 63 insertions(+)

Comments

Ferruh Yigit Jan. 17, 2018, 4:55 p.m. UTC | #1 
On 1/11/2018 8:12 AM, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> 
> Commonly, drivers converted to the new offload API
> may need to log unsupported offloads as a response
> to wrong settings. From this perspective, it would
> be convenient to have generic functions to look up
> offload names. The patch adds such a helper for Rx.
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: Shahaf Shuler <shahafs@mellanox.com>

Hi Thomas, Shahaf,

Any comment on ethdev patches?
Thomas Monjalon Jan. 17, 2018, 5:06 p.m. UTC | #2 
17/01/2018 17:55, Ferruh Yigit:
> On 1/11/2018 8:12 AM, Andrew Rybchenko wrote:
> > From: Ivan Malov <ivan.malov@oktetlabs.ru>
> > 
> > Commonly, drivers converted to the new offload API
> > may need to log unsupported offloads as a response
> > to wrong settings. From this perspective, it would
> > be convenient to have generic functions to look up
> > offload names. The patch adds such a helper for Rx.
> > 
> > Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > 
> > Cc: Thomas Monjalon <thomas@monjalon.net>
> > Cc: Ferruh Yigit <ferruh.yigit@intel.com>
> > Cc: Shahaf Shuler <shahafs@mellanox.com>
> 
> Hi Thomas, Shahaf,
> 
> Any comment on ethdev patches?

Yes, one comment: it should be experimental.
Thomas Monjalon Jan. 17, 2018, 5:33 p.m. UTC | #3 
Hi, 2 comments below

11/01/2018 09:12, Andrew Rybchenko:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> 
> +#define	RTE_RX_OFFLOAD_BIT2STR(_name)	\
> +	{ DEV_RX_OFFLOAD_##_name, #_name }
> +
> +static const struct {
> +	uint64_t offload;
> +	const char *name;
> +} rte_rx_offload_names[] = {
> +	RTE_RX_OFFLOAD_BIT2STR(VLAN_STRIP),
> +	RTE_RX_OFFLOAD_BIT2STR(IPV4_CKSUM),
> +	RTE_RX_OFFLOAD_BIT2STR(UDP_CKSUM),
> +	RTE_RX_OFFLOAD_BIT2STR(TCP_CKSUM),
> +	RTE_RX_OFFLOAD_BIT2STR(TCP_LRO),
> +	RTE_RX_OFFLOAD_BIT2STR(QINQ_STRIP),
> +	RTE_RX_OFFLOAD_BIT2STR(OUTER_IPV4_CKSUM),
> +	RTE_RX_OFFLOAD_BIT2STR(MACSEC_STRIP),
> +	RTE_RX_OFFLOAD_BIT2STR(HEADER_SPLIT),
> +	RTE_RX_OFFLOAD_BIT2STR(VLAN_FILTER),
> +	RTE_RX_OFFLOAD_BIT2STR(VLAN_EXTEND),
> +	RTE_RX_OFFLOAD_BIT2STR(JUMBO_FRAME),
> +	RTE_RX_OFFLOAD_BIT2STR(CRC_STRIP),
> +	RTE_RX_OFFLOAD_BIT2STR(SCATTER),
> +	RTE_RX_OFFLOAD_BIT2STR(TIMESTAMP),
> +	RTE_RX_OFFLOAD_BIT2STR(SECURITY),
> +};
> +
> +#undef RTE_RX_OFFLOAD_BIT2STR

Why this undef?

> --- a/lib/librte_ether/rte_ethdev_version.map
> +++ b/lib/librte_ether/rte_ethdev_version.map
> @@ -198,6 +198,12 @@ DPDK_17.11 {
>  
>  } DPDK_17.08;
>  
> +DPDK_18.02 {
> +	global:
> +
> +	rte_eth_dev_rx_offload_name;
> +} DPDK_17.11;

New functions should be experimental.

>  EXPERIMENTAL {
>  	global:
Shahaf Shuler Jan. 18, 2018, 6:32 a.m. UTC | #4 
Wednesday, January 17, 2018 7:33 PM, Thomas Monjalon:
09:12, Andrew Rybchenko:
> > From: Ivan Malov <ivan.malov@oktetlabs.ru>
> >
> > +#define	RTE_RX_OFFLOAD_BIT2STR(_name)	\
> > +	{ DEV_RX_OFFLOAD_##_name, #_name }
> > +
> > +static const struct {
> > +	uint64_t offload;
> > +	const char *name;
> > +} rte_rx_offload_names[] = {
> > +	RTE_RX_OFFLOAD_BIT2STR(VLAN_STRIP),
> > +	RTE_RX_OFFLOAD_BIT2STR(IPV4_CKSUM),
> > +	RTE_RX_OFFLOAD_BIT2STR(UDP_CKSUM),
> > +	RTE_RX_OFFLOAD_BIT2STR(TCP_CKSUM),
> > +	RTE_RX_OFFLOAD_BIT2STR(TCP_LRO),
> > +	RTE_RX_OFFLOAD_BIT2STR(QINQ_STRIP),
> > +	RTE_RX_OFFLOAD_BIT2STR(OUTER_IPV4_CKSUM),
> > +	RTE_RX_OFFLOAD_BIT2STR(MACSEC_STRIP),
> > +	RTE_RX_OFFLOAD_BIT2STR(HEADER_SPLIT),
> > +	RTE_RX_OFFLOAD_BIT2STR(VLAN_FILTER),
> > +	RTE_RX_OFFLOAD_BIT2STR(VLAN_EXTEND),
> > +	RTE_RX_OFFLOAD_BIT2STR(JUMBO_FRAME),
> > +	RTE_RX_OFFLOAD_BIT2STR(CRC_STRIP),
> > +	RTE_RX_OFFLOAD_BIT2STR(SCATTER),
> > +	RTE_RX_OFFLOAD_BIT2STR(TIMESTAMP),
> > +	RTE_RX_OFFLOAD_BIT2STR(SECURITY),
> > +};
> > +
> > +#undef RTE_RX_OFFLOAD_BIT2STR
> 
> Why this undef?
> 
> > --- a/lib/librte_ether/rte_ethdev_version.map
> > +++ b/lib/librte_ether/rte_ethdev_version.map
> > @@ -198,6 +198,12 @@ DPDK_17.11 {
> >
> >  } DPDK_17.08;
> >
> > +DPDK_18.02 {
> > +	global:
> > +
> > +	rte_eth_dev_rx_offload_name;
> > +} DPDK_17.11;
> 
> New functions should be experimental.
> 
> >  EXPERIMENTAL {
> >  	global:

Apart from Thomas comments looks OK to me.
Andrew Rybchenko Jan. 18, 2018, 7:16 a.m. UTC | #5 
On 01/17/2018 08:33 PM, Thomas Monjalon wrote:
> Hi, 2 comments below
>
> 11/01/2018 09:12, Andrew Rybchenko:
>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>
>> +#define	RTE_RX_OFFLOAD_BIT2STR(_name)	\
>> +	{ DEV_RX_OFFLOAD_##_name, #_name }
>> +
>> +static const struct {
>> +	uint64_t offload;
>> +	const char *name;
>> +} rte_rx_offload_names[] = {
>> +	RTE_RX_OFFLOAD_BIT2STR(VLAN_STRIP),
>> +	RTE_RX_OFFLOAD_BIT2STR(IPV4_CKSUM),
>> +	RTE_RX_OFFLOAD_BIT2STR(UDP_CKSUM),
>> +	RTE_RX_OFFLOAD_BIT2STR(TCP_CKSUM),
>> +	RTE_RX_OFFLOAD_BIT2STR(TCP_LRO),
>> +	RTE_RX_OFFLOAD_BIT2STR(QINQ_STRIP),
>> +	RTE_RX_OFFLOAD_BIT2STR(OUTER_IPV4_CKSUM),
>> +	RTE_RX_OFFLOAD_BIT2STR(MACSEC_STRIP),
>> +	RTE_RX_OFFLOAD_BIT2STR(HEADER_SPLIT),
>> +	RTE_RX_OFFLOAD_BIT2STR(VLAN_FILTER),
>> +	RTE_RX_OFFLOAD_BIT2STR(VLAN_EXTEND),
>> +	RTE_RX_OFFLOAD_BIT2STR(JUMBO_FRAME),
>> +	RTE_RX_OFFLOAD_BIT2STR(CRC_STRIP),
>> +	RTE_RX_OFFLOAD_BIT2STR(SCATTER),
>> +	RTE_RX_OFFLOAD_BIT2STR(TIMESTAMP),
>> +	RTE_RX_OFFLOAD_BIT2STR(SECURITY),
>> +};
>> +
>> +#undef RTE_RX_OFFLOAD_BIT2STR
> Why this undef?

It is a constant-local macro with assumption to be used as
initializer of the corresponding structure. So, under is just to
limit its context and be sure that it is will not clash or be reused
most likely in a wrong way.

>> --- a/lib/librte_ether/rte_ethdev_version.map
>> +++ b/lib/librte_ether/rte_ethdev_version.map
>> @@ -198,6 +198,12 @@ DPDK_17.11 {
>>   
>>   } DPDK_17.08;
>>   
>> +DPDK_18.02 {
>> +	global:
>> +
>> +	rte_eth_dev_rx_offload_name;
>> +} DPDK_17.11;
> New functions should be experimental.

Thanks, I've moved it to EXPERIMENTAL and added a comment
as highlighted by Ferruh in another review.

I've not removed DPDK_17.11 base for EXPERIMENTAL in map file.
If it should be done, it should be done separately.

Thanks.

>>   EXPERIMENTAL {
>>   	global:
Andrew Rybchenko Jan. 18, 2018, 7:52 a.m. UTC | #6 
On 01/18/2018 10:16 AM, Andrew Rybchenko wrote:
> On 01/17/2018 08:33 PM, Thomas Monjalon wrote:
>> Hi, 2 comments below
>>
>> 11/01/2018 09:12, Andrew Rybchenko:
>>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>>
>>> +#define    RTE_RX_OFFLOAD_BIT2STR(_name)    \
>>> +    { DEV_RX_OFFLOAD_##_name, #_name }
>>> +
>>> +static const struct {
>>> +    uint64_t offload;
>>> +    const char *name;
>>> +} rte_rx_offload_names[] = {
>>> +    RTE_RX_OFFLOAD_BIT2STR(VLAN_STRIP),
>>> +    RTE_RX_OFFLOAD_BIT2STR(IPV4_CKSUM),
>>> +    RTE_RX_OFFLOAD_BIT2STR(UDP_CKSUM),
>>> +    RTE_RX_OFFLOAD_BIT2STR(TCP_CKSUM),
>>> +    RTE_RX_OFFLOAD_BIT2STR(TCP_LRO),
>>> +    RTE_RX_OFFLOAD_BIT2STR(QINQ_STRIP),
>>> +    RTE_RX_OFFLOAD_BIT2STR(OUTER_IPV4_CKSUM),
>>> +    RTE_RX_OFFLOAD_BIT2STR(MACSEC_STRIP),
>>> +    RTE_RX_OFFLOAD_BIT2STR(HEADER_SPLIT),
>>> +    RTE_RX_OFFLOAD_BIT2STR(VLAN_FILTER),
>>> +    RTE_RX_OFFLOAD_BIT2STR(VLAN_EXTEND),
>>> +    RTE_RX_OFFLOAD_BIT2STR(JUMBO_FRAME),
>>> +    RTE_RX_OFFLOAD_BIT2STR(CRC_STRIP),
>>> +    RTE_RX_OFFLOAD_BIT2STR(SCATTER),
>>> +    RTE_RX_OFFLOAD_BIT2STR(TIMESTAMP),
>>> +    RTE_RX_OFFLOAD_BIT2STR(SECURITY),
>>> +};
>>> +
>>> +#undef RTE_RX_OFFLOAD_BIT2STR
>> Why this undef?
>
> It is a constant-local macro with assumption to be used as

I mean context-local, sorry.

> initializer of the corresponding structure. So, under is just to
> limit its context and be sure that it is will not clash or be reused
> most likely in a wrong way.
Thomas Monjalon Jan. 18, 2018, 9:24 a.m. UTC | #7 
18/01/2018 08:52, Andrew Rybchenko:
> On 01/18/2018 10:16 AM, Andrew Rybchenko wrote:
> > On 01/17/2018 08:33 PM, Thomas Monjalon wrote:
> >> Hi, 2 comments below
> >>
> >> 11/01/2018 09:12, Andrew Rybchenko:
> >>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> >>>
> >>> +#define    RTE_RX_OFFLOAD_BIT2STR(_name)    \
> >>> +    { DEV_RX_OFFLOAD_##_name, #_name }
> >>> +
> >>> +static const struct {
> >>> +    uint64_t offload;
> >>> +    const char *name;
> >>> +} rte_rx_offload_names[] = {
> >>> +    RTE_RX_OFFLOAD_BIT2STR(VLAN_STRIP),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(IPV4_CKSUM),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(UDP_CKSUM),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(TCP_CKSUM),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(TCP_LRO),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(QINQ_STRIP),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(OUTER_IPV4_CKSUM),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(MACSEC_STRIP),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(HEADER_SPLIT),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(VLAN_FILTER),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(VLAN_EXTEND),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(JUMBO_FRAME),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(CRC_STRIP),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(SCATTER),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(TIMESTAMP),
> >>> +    RTE_RX_OFFLOAD_BIT2STR(SECURITY),
> >>> +};
> >>> +
> >>> +#undef RTE_RX_OFFLOAD_BIT2STR
> >> Why this undef?
> >
> > It is a constant-local macro with assumption to be used as
> 
> I mean context-local, sorry.
> 
> > initializer of the corresponding structure. So, under is just to
> > limit its context and be sure that it is will not clash or be reused
> > most likely in a wrong way.

We do not take this precaution usually in DPDK.
No strong opinion however.
diff mbox

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index b349599..3d09950 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -89,6 +89,32 @@  static const struct rte_eth_xstats_name_off rte_txq_stats_strings[] = {
 #define RTE_NB_TXQ_STATS (sizeof(rte_txq_stats_strings) /	\
 		sizeof(rte_txq_stats_strings[0]))
 
+#define	RTE_RX_OFFLOAD_BIT2STR(_name)	\
+	{ DEV_RX_OFFLOAD_##_name, #_name }
+
+static const struct {
+	uint64_t offload;
+	const char *name;
+} rte_rx_offload_names[] = {
+	RTE_RX_OFFLOAD_BIT2STR(VLAN_STRIP),
+	RTE_RX_OFFLOAD_BIT2STR(IPV4_CKSUM),
+	RTE_RX_OFFLOAD_BIT2STR(UDP_CKSUM),
+	RTE_RX_OFFLOAD_BIT2STR(TCP_CKSUM),
+	RTE_RX_OFFLOAD_BIT2STR(TCP_LRO),
+	RTE_RX_OFFLOAD_BIT2STR(QINQ_STRIP),
+	RTE_RX_OFFLOAD_BIT2STR(OUTER_IPV4_CKSUM),
+	RTE_RX_OFFLOAD_BIT2STR(MACSEC_STRIP),
+	RTE_RX_OFFLOAD_BIT2STR(HEADER_SPLIT),
+	RTE_RX_OFFLOAD_BIT2STR(VLAN_FILTER),
+	RTE_RX_OFFLOAD_BIT2STR(VLAN_EXTEND),
+	RTE_RX_OFFLOAD_BIT2STR(JUMBO_FRAME),
+	RTE_RX_OFFLOAD_BIT2STR(CRC_STRIP),
+	RTE_RX_OFFLOAD_BIT2STR(SCATTER),
+	RTE_RX_OFFLOAD_BIT2STR(TIMESTAMP),
+	RTE_RX_OFFLOAD_BIT2STR(SECURITY),
+};
+
+#undef RTE_RX_OFFLOAD_BIT2STR
 
 /**
  * The user application callback description.
@@ -746,6 +772,22 @@  rte_eth_convert_rx_offloads(const uint64_t rx_offloads,
 		rxmode->security = 0;
 }
 
+const char *
+rte_eth_dev_rx_offload_name(uint64_t offload)
+{
+	const char *name = "UNKNOWN";
+	unsigned int i;
+
+	for (i = 0; i < RTE_DIM(rte_rx_offload_names); ++i) {
+		if (offload == rte_rx_offload_names[i].offload) {
+			name = rte_rx_offload_names[i].name;
+			break;
+		}
+	}
+
+	return name;
+}
+
 int
 rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 		      const struct rte_eth_conf *dev_conf)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index f0eeefe..88ceffa 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -945,6 +945,11 @@  struct rte_eth_conf {
 			     DEV_RX_OFFLOAD_VLAN_FILTER | \
 			     DEV_RX_OFFLOAD_VLAN_EXTEND)
 
+/*
+ * If new Rx offload capabilities are defined, they also must be
+ * mentioned in rte_rx_offload_names in rte_ethdev.c file.
+ */
+
 /**
  * TX offload capabilities of a device.
  */
@@ -1922,6 +1927,16 @@  int rte_eth_dev_detach(uint16_t port_id, char *devname);
 uint32_t rte_eth_speed_bitflag(uint32_t speed, int duplex);
 
 /**
+ * Get DEV_RX_OFFLOAD_* flag name
+ *
+ * @param offload
+ *   Offload flag
+ * @return
+ *   Offload name or 'UNKNOWN' if the flag cannot be recognised
+ */
+const char *rte_eth_dev_rx_offload_name(uint64_t offload);
+
+/**
  * Configure an Ethernet device.
  * This function must be invoked first before any other function in the
  * Ethernet API. This function can also be re-invoked when a device is in the
diff --git a/lib/librte_ether/rte_ethdev_version.map b/lib/librte_ether/rte_ethdev_version.map
index e9681ac..03455cf 100644
--- a/lib/librte_ether/rte_ethdev_version.map
+++ b/lib/librte_ether/rte_ethdev_version.map
@@ -198,6 +198,12 @@  DPDK_17.11 {
 
 } DPDK_17.08;
 
+DPDK_18.02 {
+	global:
+
+	rte_eth_dev_rx_offload_name;
+} DPDK_17.11;
+
 EXPERIMENTAL {
 	global: