All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Assorted fixes for the flow API
@ 2017-01-10 13:08 Adrien Mazarguil
  2017-01-10 13:08 ` [PATCH 1/5] app/testpmd: fix array bounds checks Adrien Mazarguil
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Adrien Mazarguil @ 2017-01-10 13:08 UTC (permalink / raw)
  To: dev

This series makes a few adjustments to the flow API (rte_flow) based on
issues encountered by preliminary PMD implementations and addresses bugs
reported by Coverity in the testpmd flow command.

Adrien Mazarguil (5):
  app/testpmd: fix array bounds checks
  ethdev: modify flow API's error setting function
  ethdev: clarify MARK and FLAG actions in flow API
  ethdev: clarify RSS action in flow API
  ethdev: define default item masks in flow API

 app/test-pmd/config.c              |   8 +-
 app/test-pmd/rxonly.c              |   3 +-
 doc/guides/prog_guide/rte_flow.rst |  57 +++++++------
 lib/librte_ether/rte_flow.c        |  24 +++---
 lib/librte_ether/rte_flow.h        | 137 +++++++++++++++++++++++++++-----
 lib/librte_ether/rte_flow_driver.h |   6 +-
 6 files changed, 170 insertions(+), 65 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] app/testpmd: fix array bounds checks
  2017-01-10 13:08 [PATCH 0/5] Assorted fixes for the flow API Adrien Mazarguil
@ 2017-01-10 13:08 ` Adrien Mazarguil
  2017-01-10 13:08 ` [PATCH 2/5] ethdev: modify flow API's error setting function Adrien Mazarguil
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Adrien Mazarguil @ 2017-01-10 13:08 UTC (permalink / raw)
  To: dev

This commit addresses several obvious issues reported by Coverity (139596,
139597, 139598 and 139599) with array bounds checks in functions related to
the flow API.

Fixes: 938a184a1870 ("app/testpmd: implement basic support for flow API")

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
 app/test-pmd/config.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 9716ce7..e1af064 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -884,7 +884,7 @@ port_flow_new(const struct rte_flow_attr *attr,
 	do {
 		struct rte_flow_item *dst = NULL;
 
-		if ((unsigned int)item->type > RTE_DIM(flow_item) ||
+		if ((unsigned int)item->type >= RTE_DIM(flow_item) ||
 		    !flow_item[item->type].name)
 			goto notsup;
 		if (pf)
@@ -918,7 +918,7 @@ port_flow_new(const struct rte_flow_attr *attr,
 	do {
 		struct rte_flow_action *dst = NULL;
 
-		if ((unsigned int)action->type > RTE_DIM(flow_action) ||
+		if ((unsigned int)action->type >= RTE_DIM(flow_action) ||
 		    !flow_action[action->type].name)
 			goto notsup;
 		if (pf)
@@ -977,7 +977,7 @@ port_flow_complain(struct rte_flow_error *error)
 	char buf[32];
 	int err = rte_errno;
 
-	if ((unsigned int)error->type > RTE_DIM(errstrlist) ||
+	if ((unsigned int)error->type >= RTE_DIM(errstrlist) ||
 	    !errstrlist[error->type])
 		errstr = "unknown type";
 	else
@@ -1146,7 +1146,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
 		printf("Flow rule #%u not found\n", rule);
 		return -ENOENT;
 	}
-	if ((unsigned int)action > RTE_DIM(flow_action) ||
+	if ((unsigned int)action >= RTE_DIM(flow_action) ||
 	    !flow_action[action].name)
 		name = "unknown";
 	else
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] ethdev: modify flow API's error setting function
  2017-01-10 13:08 [PATCH 0/5] Assorted fixes for the flow API Adrien Mazarguil
  2017-01-10 13:08 ` [PATCH 1/5] app/testpmd: fix array bounds checks Adrien Mazarguil
@ 2017-01-10 13:08 ` Adrien Mazarguil
  2017-01-10 13:08 ` [PATCH 3/5] ethdev: clarify MARK and FLAG actions in flow API Adrien Mazarguil
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Adrien Mazarguil @ 2017-01-10 13:08 UTC (permalink / raw)
  To: dev; +Cc: Nelio Laranjeiro, Wei Zhao, Beilei Xing

Based on initial PMD implementations of the flow API, returning the error
structure which may be NULL is useless and always discarded.

Returning the error code instead appears to be much more convenient.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Cc: Wei Zhao <wei.zhao1@intel.com>
Cc: Beilei Xing <beilei.xing@intel.com>
---
 lib/librte_ether/rte_flow.c        | 24 ++++++++++++------------
 lib/librte_ether/rte_flow_driver.h |  6 +++---
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c
index d98fb1b..aaa70d6 100644
--- a/lib/librte_ether/rte_flow.c
+++ b/lib/librte_ether/rte_flow.c
@@ -78,9 +78,9 @@ rte_flow_validate(uint8_t port_id,
 		return -rte_errno;
 	if (likely(!!ops->validate))
 		return ops->validate(dev, attr, pattern, actions, error);
-	rte_flow_error_set(error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-			   NULL, rte_strerror(ENOSYS));
-	return -rte_errno;
+	return -rte_flow_error_set(error, ENOSYS,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL, rte_strerror(ENOSYS));
 }
 
 /* Create a flow rule on a given port. */
@@ -116,9 +116,9 @@ rte_flow_destroy(uint8_t port_id,
 		return -rte_errno;
 	if (likely(!!ops->destroy))
 		return ops->destroy(dev, flow, error);
-	rte_flow_error_set(error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-			   NULL, rte_strerror(ENOSYS));
-	return -rte_errno;
+	return -rte_flow_error_set(error, ENOSYS,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL, rte_strerror(ENOSYS));
 }
 
 /* Destroy all flow rules associated with a port. */
@@ -133,9 +133,9 @@ rte_flow_flush(uint8_t port_id,
 		return -rte_errno;
 	if (likely(!!ops->flush))
 		return ops->flush(dev, error);
-	rte_flow_error_set(error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-			   NULL, rte_strerror(ENOSYS));
-	return -rte_errno;
+	return -rte_flow_error_set(error, ENOSYS,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL, rte_strerror(ENOSYS));
 }
 
 /* Query an existing flow rule. */
@@ -153,7 +153,7 @@ rte_flow_query(uint8_t port_id,
 		return -rte_errno;
 	if (likely(!!ops->query))
 		return ops->query(dev, flow, action, data, error);
-	rte_flow_error_set(error, ENOSYS, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-			   NULL, rte_strerror(ENOSYS));
-	return -rte_errno;
+	return -rte_flow_error_set(error, ENOSYS,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL, rte_strerror(ENOSYS));
 }
diff --git a/lib/librte_ether/rte_flow_driver.h b/lib/librte_ether/rte_flow_driver.h
index cc97785..d020536 100644
--- a/lib/librte_ether/rte_flow_driver.h
+++ b/lib/librte_ether/rte_flow_driver.h
@@ -139,9 +139,9 @@ struct rte_flow_ops {
  *   Human-readable error message.
  *
  * @return
- *   Pointer to flow error structure.
+ *   Error code (\code).
  */
-static inline struct rte_flow_error *
+static inline int
 rte_flow_error_set(struct rte_flow_error *error,
 		   int code,
 		   enum rte_flow_error_type type,
@@ -156,7 +156,7 @@ rte_flow_error_set(struct rte_flow_error *error,
 		};
 	}
 	rte_errno = code;
-	return error;
+	return code;
 }
 
 /**
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/5] ethdev: clarify MARK and FLAG actions in flow API
  2017-01-10 13:08 [PATCH 0/5] Assorted fixes for the flow API Adrien Mazarguil
  2017-01-10 13:08 ` [PATCH 1/5] app/testpmd: fix array bounds checks Adrien Mazarguil
  2017-01-10 13:08 ` [PATCH 2/5] ethdev: modify flow API's error setting function Adrien Mazarguil
@ 2017-01-10 13:08 ` Adrien Mazarguil
  2017-01-10 13:08 ` [PATCH 4/5] ethdev: clarify RSS action " Adrien Mazarguil
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Adrien Mazarguil @ 2017-01-10 13:08 UTC (permalink / raw)
  To: dev; +Cc: John McNamara

Both actions share the PKT_RX_FDIR mbuf flag, as a result there is no way
to tell them apart. Moreover, the maximum allowed value for the MARK action
may not necessarily cover the entire 32-bit space.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: John McNamara <john.mcnamara@intel.com>
---
 doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++------------
 lib/librte_ether/rte_flow.h        | 19 ++++++++++---------
 2 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index f415a73..a6acbbf 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1049,31 +1049,31 @@ flow rules:
 Action: ``MARK``
 ^^^^^^^^^^^^^^^^
 
-Attaches a 32 bit value to packets.
+Attaches an integer value to packets and sets ``PKT_RX_FDIR`` and
+``PKT_RX_FDIR_ID`` mbuf flags.
 
-This value is arbitrary and application-defined. For compatibility with FDIR
-it is returned in the ``hash.fdir.hi`` mbuf field. ``PKT_RX_FDIR_ID`` is
-also set in ``ol_flags``.
+This value is arbitrary and application-defined. Maximum allowed value
+depends on the underlying implementation. It is returned in the
+``hash.fdir.hi`` mbuf field.
 
 .. _table_rte_flow_action_mark:
 
 .. table:: MARK
 
-   +--------+-------------------------------------+
-   | Field  | Value                               |
-   +========+=====================================+
-   | ``id`` | 32 bit value to return with packets |
-   +--------+-------------------------------------+
+   +--------+--------------------------------------+
+   | Field  | Value                                |
+   +========+======================================+
+   | ``id`` | integer value to return with packets |
+   +--------+--------------------------------------+
 
 Action: ``FLAG``
 ^^^^^^^^^^^^^^^^
 
-Flag packets. Similar to `Action: MARK`_ but only affects ``ol_flags``.
+Flags packets. Similar to `Action: MARK`_ without a specific value; only
+sets the ``PKT_RX_FDIR`` mbuf flag.
 
 - No configurable properties.
 
-Note: a distinctive flag must be defined for it.
-
 .. _table_rte_flow_action_flag:
 
 .. table:: FLAG
diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index 98084ac..c2b9fc5 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -556,7 +556,8 @@ enum rte_flow_action_type {
 	/**
 	 * [META]
 	 *
-	 * Attaches a 32 bit value to packets.
+	 * Attaches an integer value to packets and sets PKT_RX_FDIR and
+	 * PKT_RX_FDIR_ID mbuf flags.
 	 *
 	 * See struct rte_flow_action_mark.
 	 */
@@ -565,9 +566,8 @@ enum rte_flow_action_type {
 	/**
 	 * [META]
 	 *
-	 * Flag packets. Similar to MARK but only affects ol_flags.
-	 *
-	 * Note: a distinctive flag must be defined for it.
+	 * Flags packets. Similar to MARK without a specific value; only
+	 * sets the PKT_RX_FDIR mbuf flag.
 	 *
 	 * No associated configuration structure.
 	 */
@@ -640,14 +640,15 @@ enum rte_flow_action_type {
 /**
  * RTE_FLOW_ACTION_TYPE_MARK
  *
- * Attaches a 32 bit value to packets.
+ * Attaches an integer value to packets and sets PKT_RX_FDIR and
+ * PKT_RX_FDIR_ID mbuf flags.
  *
- * This value is arbitrary and application-defined. For compatibility with
- * FDIR it is returned in the hash.fdir.hi mbuf field. PKT_RX_FDIR_ID is
- * also set in ol_flags.
+ * This value is arbitrary and application-defined. Maximum allowed value
+ * depends on the underlying implementation. It is returned in the
+ * hash.fdir.hi mbuf field.
  */
 struct rte_flow_action_mark {
-	uint32_t id; /**< 32 bit value to return with packets. */
+	uint32_t id; /**< Integer value to return with packets. */
 };
 
 /**
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] ethdev: clarify RSS action in flow API
  2017-01-10 13:08 [PATCH 0/5] Assorted fixes for the flow API Adrien Mazarguil
                   ` (2 preceding siblings ...)
  2017-01-10 13:08 ` [PATCH 3/5] ethdev: clarify MARK and FLAG actions in flow API Adrien Mazarguil
@ 2017-01-10 13:08 ` Adrien Mazarguil
  2017-01-10 13:08 ` [PATCH 5/5] ethdev: define default item masks " Adrien Mazarguil
  2017-01-11 15:57 ` [PATCH 0/5] Assorted fixes for the " Thomas Monjalon
  5 siblings, 0 replies; 7+ messages in thread
From: Adrien Mazarguil @ 2017-01-10 13:08 UTC (permalink / raw)
  To: dev; +Cc: Jingjing Wu, John McNamara

Contrary to the current description, mbuf RSS hash result storage does not
overlap with the returned MARK value (hash.fdir.lo vs. hash.fdir.hi), and
both may be combined.

Reflect this change by allowing testpmd to display both values
simultaneously.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Jingjing Wu <jingjing.wu@intel.com>
Cc: John McNamara <john.mcnamara@intel.com>
---
 app/test-pmd/rxonly.c              | 3 ++-
 doc/guides/prog_guide/rte_flow.rst | 8 +++-----
 lib/librte_ether/rte_flow.h        | 8 +++-----
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
index cf00576..dcc8346 100644
--- a/app/test-pmd/rxonly.c
+++ b/app/test-pmd/rxonly.c
@@ -148,7 +148,8 @@ pkt_burst_receive(struct fwd_stream *fs)
 		if (ol_flags & PKT_RX_RSS_HASH) {
 			printf(" - RSS hash=0x%x", (unsigned) mb->hash.rss);
 			printf(" - RSS queue=0x%x",(unsigned) fs->rx_queue);
-		} else if (ol_flags & PKT_RX_FDIR) {
+		}
+		if (ol_flags & PKT_RX_FDIR) {
 			printf(" - FDIR matched ");
 			if (ol_flags & PKT_RX_FDIR_ID)
 				printf("ID=0x%x",
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index a6acbbf..93d0dc2 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1187,11 +1187,9 @@ Action: ``RSS``
 Similar to QUEUE, except RSS is additionally performed on packets to spread
 them among several queues according to the provided parameters.
 
-Note: RSS hash result is normally stored in the ``hash.rss`` mbuf field,
-however it conflicts with `Action: MARK`_ as they share the same space. When
-both actions are specified, the RSS hash is discarded and
-``PKT_RX_RSS_HASH`` is not set in ``ol_flags``. MARK has priority. The mbuf
-structure should eventually evolve to store both.
+Note: RSS hash result is stored in the ``hash.rss`` mbuf field which
+overlaps ``hash.fdir.lo``. Since `Action: MARK`_ sets the ``hash.fdir.hi``
+field only, both can be requested simultaneously.
 
 - Terminating by default.
 
diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index c2b9fc5..59dc1ef 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -696,11 +696,9 @@ struct rte_flow_action_dup {
  * Similar to QUEUE, except RSS is additionally performed on packets to
  * spread them among several queues according to the provided parameters.
  *
- * Note: RSS hash result is normally stored in the hash.rss mbuf field,
- * however it conflicts with the MARK action as they share the same
- * space. When both actions are specified, the RSS hash is discarded and
- * PKT_RX_RSS_HASH is not set in ol_flags. MARK has priority. The mbuf
- * structure should eventually evolve to store both.
+ * Note: RSS hash result is stored in the hash.rss mbuf field which overlaps
+ * hash.fdir.lo. Since the MARK action sets the hash.fdir.hi field only,
+ * both can be requested simultaneously.
  *
  * Terminating by default.
  */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] ethdev: define default item masks in flow API
  2017-01-10 13:08 [PATCH 0/5] Assorted fixes for the flow API Adrien Mazarguil
                   ` (3 preceding siblings ...)
  2017-01-10 13:08 ` [PATCH 4/5] ethdev: clarify RSS action " Adrien Mazarguil
@ 2017-01-10 13:08 ` Adrien Mazarguil
  2017-01-11 15:57 ` [PATCH 0/5] Assorted fixes for the " Thomas Monjalon
  5 siblings, 0 replies; 7+ messages in thread
From: Adrien Mazarguil @ 2017-01-10 13:08 UTC (permalink / raw)
  To: dev; +Cc: John McNamara, Nelio Laranjeiro, Wei Zhao, Beilei Xing

Leaving default pattern item mask values up for interpretation by PMDs is
an undefined behavior that applications might find difficult to use in the
wild. It also needlessly complicates PMD implementation.

This commit addresses this by defining consistent default masks for each
item type.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: John McNamara <john.mcnamara@intel.com>
Cc: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Cc: Wei Zhao <wei.zhao1@intel.com>
Cc: Beilei Xing <beilei.xing@intel.com>
---
 doc/guides/prog_guide/rte_flow.rst |  25 ++++++--
 lib/librte_ether/rte_flow.h        | 110 +++++++++++++++++++++++++++++---
 2 files changed, 121 insertions(+), 14 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 93d0dc2..3bf8871 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -217,13 +217,11 @@ Usage restrictions and expected behavior:
   values lower than those in ``spec`` are not supported.
 
 - Setting ``spec`` and optionally ``last`` without ``mask`` causes the PMD
-  to only take the fields it can recognize into account. There is no error
-  checking for unsupported fields.
+  to use the default mask defined for that item (defined as
+  ``rte_flow_item_{name}_mask`` constants).
 
-- Not setting any of them (assuming item type allows it) uses default
-  parameters that depend on the item type. Most of the time, particularly
-  for protocol header items, it is equivalent to providing an empty (zeroed)
-  ``mask``.
+- Not setting any of them (assuming item type allows it) is equivalent to
+  providing an empty (zeroed) ``mask`` for broad (nonspecific) matching.
 
 - ``mask`` is a simple bit-mask applied before interpreting the contents of
   ``spec`` and ``last``, which may yield unexpected results if not used
@@ -550,6 +548,7 @@ duplicated between device instances by default.
 - Can be specified multiple times to match traffic addressed to several VF
   IDs.
 - Can be combined with a PF item to match both PF and VF traffic.
+- Default ``mask`` matches any VF ID.
 
 .. _table_rte_flow_item_vf:
 
@@ -583,6 +582,8 @@ not be contiguous.
 As a device property, the list of allowed values as well as the value
 associated with a port_id should be retrieved by other means.
 
+- Default ``mask`` matches any port index.
+
 .. _table_rte_flow_item_port:
 
 .. table:: PORT
@@ -616,6 +617,8 @@ stand for several protocol layers.
 This is usually specified as the first pattern item when looking for a
 protocol anywhere in a packet.
 
+- Default ``mask`` stands for any number of layers.
+
 .. _table_rte_flow_item_any:
 
 .. table:: ANY
@@ -673,6 +676,7 @@ Matching a zero-length pattern is allowed, doing so resets the relative
 offset for subsequent items.
 
 - This type does not support ranges (``last`` field).
+- Default ``mask`` matches all fields exactly.
 
 .. _table_rte_flow_item_raw:
 
@@ -785,6 +789,7 @@ Matches an Ethernet header.
 - ``dst``: destination MAC.
 - ``src``: source MAC.
 - ``type``: EtherType.
+- Default ``mask`` matches destination and source addresses only.
 
 Item: ``VLAN``
 ^^^^^^^^^^^^^^
@@ -793,6 +798,7 @@ Matches an 802.1Q/ad VLAN tag.
 
 - ``tpid``: tag protocol identifier.
 - ``tci``: tag control information.
+- Default ``mask`` matches TCI only.
 
 Item: ``IPV4``
 ^^^^^^^^^^^^^^
@@ -802,6 +808,7 @@ Matches an IPv4 header.
 Note: IPv4 options are handled by dedicated pattern items.
 
 - ``hdr``: IPv4 header definition (``rte_ip.h``).
+- Default ``mask`` matches source and destination addresses only.
 
 Item: ``IPV6``
 ^^^^^^^^^^^^^^
@@ -811,6 +818,7 @@ Matches an IPv6 header.
 Note: IPv6 options are handled by dedicated pattern items.
 
 - ``hdr``: IPv6 header definition (``rte_ip.h``).
+- Default ``mask`` matches source and destination addresses only.
 
 Item: ``ICMP``
 ^^^^^^^^^^^^^^
@@ -818,6 +826,7 @@ Item: ``ICMP``
 Matches an ICMP header.
 
 - ``hdr``: ICMP header definition (``rte_icmp.h``).
+- Default ``mask`` matches ICMP type and code only.
 
 Item: ``UDP``
 ^^^^^^^^^^^^^
@@ -825,6 +834,7 @@ Item: ``UDP``
 Matches a UDP header.
 
 - ``hdr``: UDP header definition (``rte_udp.h``).
+- Default ``mask`` matches source and destination ports only.
 
 Item: ``TCP``
 ^^^^^^^^^^^^^
@@ -832,6 +842,7 @@ Item: ``TCP``
 Matches a TCP header.
 
 - ``hdr``: TCP header definition (``rte_tcp.h``).
+- Default ``mask`` matches source and destination ports only.
 
 Item: ``SCTP``
 ^^^^^^^^^^^^^^
@@ -839,6 +850,7 @@ Item: ``SCTP``
 Matches a SCTP header.
 
 - ``hdr``: SCTP header definition (``rte_sctp.h``).
+- Default ``mask`` matches source and destination ports only.
 
 Item: ``VXLAN``
 ^^^^^^^^^^^^^^^
@@ -849,6 +861,7 @@ Matches a VXLAN header (RFC 7348).
 - ``rsvd0``: reserved, normally 0x000000.
 - ``vni``: VXLAN network identifier.
 - ``rsvd1``: reserved, normally 0x00.
+- Default ``mask`` matches VNI only.
 
 Actions
 ~~~~~~~
diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
index 59dc1ef..06da341 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -282,7 +282,12 @@ enum rte_flow_item_type {
  * A zeroed mask stands for any number of layers.
  */
 struct rte_flow_item_any {
-	uint32_t num; /* Number of layers covered. */
+	uint32_t num; /**< Number of layers covered. */
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_ANY. */
+static const struct rte_flow_item_any rte_flow_item_any_mask = {
+	.num = 0x00000000,
 };
 
 /**
@@ -307,6 +312,11 @@ struct rte_flow_item_vf {
 	uint32_t id; /**< Destination VF ID. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_VF. */
+static const struct rte_flow_item_vf rte_flow_item_vf_mask = {
+	.id = 0x00000000,
+};
+
 /**
  * RTE_FLOW_ITEM_TYPE_PORT
  *
@@ -331,6 +341,11 @@ struct rte_flow_item_port {
 	uint32_t index; /**< Physical port index. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_PORT. */
+static const struct rte_flow_item_port rte_flow_item_port_mask = {
+	.index = 0x00000000,
+};
+
 /**
  * RTE_FLOW_ITEM_TYPE_RAW
  *
@@ -359,6 +374,16 @@ struct rte_flow_item_raw {
 	uint8_t pattern[]; /**< Byte string to look for. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_RAW. */
+static const struct rte_flow_item_raw rte_flow_item_raw_mask = {
+	.relative = 1,
+	.search = 1,
+	.reserved = 0x3fffffff,
+	.offset = 0xffffffff,
+	.limit = 0xffff,
+	.length = 0xffff,
+};
+
 /**
  * RTE_FLOW_ITEM_TYPE_ETH
  *
@@ -370,6 +395,13 @@ struct rte_flow_item_eth {
 	uint16_t type; /**< EtherType. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */
+static const struct rte_flow_item_eth rte_flow_item_eth_mask = {
+	.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
+	.src.addr_bytes = "\xff\xff\xff\xff\xff\xff",
+	.type = 0x0000,
+};
+
 /**
  * RTE_FLOW_ITEM_TYPE_VLAN
  *
@@ -383,6 +415,12 @@ struct rte_flow_item_vlan {
 	uint16_t tci; /**< Tag control information. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_VLAN. */
+static const struct rte_flow_item_vlan rte_flow_item_vlan_mask = {
+	.tpid = 0x0000,
+	.tci = 0xffff,
+};
+
 /**
  * RTE_FLOW_ITEM_TYPE_IPV4
  *
@@ -394,6 +432,14 @@ struct rte_flow_item_ipv4 {
 	struct ipv4_hdr hdr; /**< IPv4 header definition. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_IPV4. */
+static const struct rte_flow_item_ipv4 rte_flow_item_ipv4_mask = {
+	.hdr = {
+		.src_addr = 0xffffffff,
+		.dst_addr = 0xffffffff,
+	},
+};
+
 /**
  * RTE_FLOW_ITEM_TYPE_IPV6.
  *
@@ -405,6 +451,18 @@ struct rte_flow_item_ipv6 {
 	struct ipv6_hdr hdr; /**< IPv6 header definition. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_IPV6. */
+static const struct rte_flow_item_ipv6 rte_flow_item_ipv6_mask = {
+	.hdr = {
+		.src_addr =
+			"\xff\xff\xff\xff\xff\xff\xff\xff"
+			"\xff\xff\xff\xff\xff\xff\xff\xff",
+		.dst_addr =
+			"\xff\xff\xff\xff\xff\xff\xff\xff"
+			"\xff\xff\xff\xff\xff\xff\xff\xff",
+	},
+};
+
 /**
  * RTE_FLOW_ITEM_TYPE_ICMP.
  *
@@ -414,6 +472,14 @@ struct rte_flow_item_icmp {
 	struct icmp_hdr hdr; /**< ICMP header definition. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_ICMP. */
+static const struct rte_flow_item_icmp rte_flow_item_icmp_mask = {
+	.hdr = {
+		.icmp_type = 0xff,
+		.icmp_code = 0xff,
+	},
+};
+
 /**
  * RTE_FLOW_ITEM_TYPE_UDP.
  *
@@ -423,6 +489,14 @@ struct rte_flow_item_udp {
 	struct udp_hdr hdr; /**< UDP header definition. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_UDP. */
+static const struct rte_flow_item_udp rte_flow_item_udp_mask = {
+	.hdr = {
+		.src_port = 0xffff,
+		.dst_port = 0xffff,
+	},
+};
+
 /**
  * RTE_FLOW_ITEM_TYPE_TCP.
  *
@@ -432,6 +506,14 @@ struct rte_flow_item_tcp {
 	struct tcp_hdr hdr; /**< TCP header definition. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_TCP. */
+static const struct rte_flow_item_tcp rte_flow_item_tcp_mask = {
+	.hdr = {
+		.src_port = 0xffff,
+		.dst_port = 0xffff,
+	},
+};
+
 /**
  * RTE_FLOW_ITEM_TYPE_SCTP.
  *
@@ -441,6 +523,14 @@ struct rte_flow_item_sctp {
 	struct sctp_hdr hdr; /**< SCTP header definition. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_SCTP. */
+static const struct rte_flow_item_sctp rte_flow_item_sctp_mask = {
+	.hdr = {
+		.src_port = 0xffff,
+		.dst_port = 0xffff,
+	},
+};
+
 /**
  * RTE_FLOW_ITEM_TYPE_VXLAN.
  *
@@ -453,6 +543,11 @@ struct rte_flow_item_vxlan {
 	uint8_t rsvd1; /**< Reserved, normally 0x00. */
 };
 
+/** Default mask for RTE_FLOW_ITEM_TYPE_VXLAN. */
+static const struct rte_flow_item_vxlan rte_flow_item_vxlan_mask = {
+	.vni = "\xff\xff\xff",
+};
+
 /**
  * Matching pattern item definition.
  *
@@ -464,15 +559,18 @@ struct rte_flow_item_vxlan {
  * Patterns are terminated by END items.
  *
  * The spec field should be a valid pointer to a structure of the related
- * item type. It may be set to NULL in many cases to use default values.
+ * item type. It may remain unspecified (NULL) in many cases to request
+ * broad (nonspecific) matching. In such cases, last and mask must also be
+ * set to NULL.
  *
  * Optionally, last can point to a structure of the same type to define an
  * inclusive range. This is mostly supported by integer and address fields,
  * may cause errors otherwise. Fields that do not support ranges must be set
  * to 0 or to the same value as the corresponding fields in spec.
  *
- * By default all fields present in spec are considered relevant (see note
- * below). This behavior can be altered by providing a mask structure of the
+ * Only the fields defined to nonzero values in the default masks (see
+ * rte_flow_item_{name}_mask constants) are considered relevant by
+ * default. This can be overridden by providing a mask structure of the
  * same type with applicable bits set to one. It can also be used to
  * partially filter out specific fields (e.g. as an alternate mean to match
  * ranges of IP addresses).
@@ -482,10 +580,6 @@ struct rte_flow_item_vxlan {
  * carefully. For example, if for an IPv4 address field, spec provides
  * 10.1.2.3, last provides 10.3.4.5 and mask provides 255.255.0.0, the
  * effective range becomes 10.1.0.0 to 10.3.255.255.
- *
- * Note: the defaults for data-matching items such as IPv4 when mask is not
- * specified actually depend on the underlying implementation since only
- * recognized fields can be taken into account.
  */
 struct rte_flow_item {
 	enum rte_flow_item_type type; /**< Item type. */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/5] Assorted fixes for the flow API
  2017-01-10 13:08 [PATCH 0/5] Assorted fixes for the flow API Adrien Mazarguil
                   ` (4 preceding siblings ...)
  2017-01-10 13:08 ` [PATCH 5/5] ethdev: define default item masks " Adrien Mazarguil
@ 2017-01-11 15:57 ` Thomas Monjalon
  5 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2017-01-11 15:57 UTC (permalink / raw)
  To: Adrien Mazarguil; +Cc: dev

2017-01-10 14:08, Adrien Mazarguil:
> This series makes a few adjustments to the flow API (rte_flow) based on
> issues encountered by preliminary PMD implementations and addresses bugs
> reported by Coverity in the testpmd flow command.
> 
> Adrien Mazarguil (5):
>   app/testpmd: fix array bounds checks
>   ethdev: modify flow API's error setting function
>   ethdev: clarify MARK and FLAG actions in flow API
>   ethdev: clarify RSS action in flow API
>   ethdev: define default item masks in flow API

Applied, thanks

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-01-11 15:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-10 13:08 [PATCH 0/5] Assorted fixes for the flow API Adrien Mazarguil
2017-01-10 13:08 ` [PATCH 1/5] app/testpmd: fix array bounds checks Adrien Mazarguil
2017-01-10 13:08 ` [PATCH 2/5] ethdev: modify flow API's error setting function Adrien Mazarguil
2017-01-10 13:08 ` [PATCH 3/5] ethdev: clarify MARK and FLAG actions in flow API Adrien Mazarguil
2017-01-10 13:08 ` [PATCH 4/5] ethdev: clarify RSS action " Adrien Mazarguil
2017-01-10 13:08 ` [PATCH 5/5] ethdev: define default item masks " Adrien Mazarguil
2017-01-11 15:57 ` [PATCH 0/5] Assorted fixes for the " Thomas Monjalon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.