[dpdk-dev,2/2] net/i40e: configurable PTYPE mapping
Checks
Commit Message
The patch adds 4 APIs to support configurable
PTYPE mapping for i40e device.
rte_pmd_i40e_update_ptype_mapping.
rte_pmd_i40e_reset_ptype_mapping.
rte_pmd_i40e_get_ptype_mapping.
rte_pmd_i40e_replace_ptype_mapping.
The mapping from hardware defined packet type to software defined packet
type can be updated/reset/read out with these APIs.
Also a software ptype with the most significent bit set will be regarded
as a custom defined ptype (RTE_PMD_I40E_PTYPE_USER_DEFINE_MASK) so
application can use it to defined its own PTYPE naming system.
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 190 ++++++++++++++++++++++++++++++++++++++++
drivers/net/i40e/i40e_rxtx.c | 1 -
drivers/net/i40e/i40e_rxtx.h | 2 +
drivers/net/i40e/rte_pmd_i40e.h | 81 +++++++++++++++++
4 files changed, 273 insertions(+), 1 deletion(-)
Comments
On 2/27/2017 4:56 AM, Qi Zhang wrote:
> The patch adds 4 APIs to support configurable
> PTYPE mapping for i40e device.
> rte_pmd_i40e_update_ptype_mapping.
> rte_pmd_i40e_reset_ptype_mapping.
> rte_pmd_i40e_get_ptype_mapping.
> rte_pmd_i40e_replace_ptype_mapping.
Hi Qi,
These are added as PMD specific APIs, but not used anywhere, how did you
test them? Or how others can test it?
Does it make sense to add them into testpmd?
And related to API naming, what do you think about following syntax:
<name_space>_<object>_<action> ?
This helps finding APIs for same object (ptype_mapping for this case):
rte_pmd_i40e_ptype_mapping_update()
rte_pmd_i40e_ptype_mapping_reset()
rte_pmd_i40e_ptype_mapping_get()
rte_pmd_i40e_ptype_mapping_replace()
And not directly related to this patch, but it is good idea to extract
PMD specific APIs into separate file, would you mind taking that task
before this patch? And add these new APIs to that new file?
> The mapping from hardware defined packet type to software defined packet
> type can be updated/reset/read out with these APIs.
> Also a software ptype with the most significent bit set will be regarded
> as a custom defined ptype (RTE_PMD_I40E_PTYPE_USER_DEFINE_MASK) so
> application can use it to defined its own PTYPE naming system.
>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 190 ++++++++++++++++++++++++++++++++++++++++
> drivers/net/i40e/i40e_rxtx.c | 1 -
> drivers/net/i40e/i40e_rxtx.h | 2 +
> drivers/net/i40e/rte_pmd_i40e.h | 81 +++++++++++++++++
Need to update *version.map file too, for shared library build. It is
hard to catch these issues since APIs are not used.
<...>
> +
> +int rte_pmd_i40e_update_ptype_mapping(
DPDK coding convention suggest having return type in separate line:
int
rte_pmd_i40e_update_ptype_mapping(...
> + uint8_t port,
> + struct rte_pmd_i40e_ptype_mapping *mapping_items,
> + uint16_t count,
> + uint8_t exclusive)
> +{
> + struct rte_eth_dev *dev;
> + struct i40e_adapter *ad;
> + int i;
For PMD specific APIs, port_id needs to be verified if it is i40e port
or not. There is already is_device_supported() function in i40e for this.
CC'ed Wenzhuo, he already did this a few times, and may help.
<...>
> +/**
> + * Update hardware defined ptype to software defined packet type
> + * mapping table.
> + *
> + * @param port
> + * pointer to port identifier of the device.
> + * @param mapping_items
> + * the base address of the mapping items array.
> + * @param count
> + * number of mapping items.
> + * @param exclusive
> + * the flag indicate different ptype mapping update method.
> + * -(0) only overwrite refferred PTYPE mapping,
referred
> + * keep other PTYPEs mapping unchanged.
> + * -(!0) overwrite referred PTYPE mapping,
> + * set other PTYPEs maps to PTYPE_UNKNOWN.
> + */
> +int rte_pmd_i40e_update_ptype_mapping(
> + uint8_t port,
> + struct rte_pmd_i40e_ptype_mapping *mapping_items,
> + uint16_t count,
> + uint8_t exclusive);
> +
> +/**
> + * Reset hardware defined ptype to software defined ptype
> + * mapping table to default.
> + *
> + * @param port
> + * pointer to port identifiier of the device
s/identifiier/identifier
<...>
> +/**
> + * Replace a specific or a group of software defined ptypes
> + * with a new one
> + *
> + * @param port
> + * pointer to port identifier of the device
> + * @param target
> + * the packet type to be replaced
> + * @param mask
> + * -(0) target represent a specific software defined ptype.
> + * -(!0) target is a mask to represent a group of software defined ptypes.
> + * @param pkt_type
> + * the new packet type to overwrite
> + */
> +int rte_pmd_i40e_replace_pkt_type(uint8_t port,
> + uint32_t target,
> + uint8_t mask,
> + uint32_t pkt_type);
API names does not match with one in commit log, and "pkt_type" usage is
not consistent with other APIs.
> +
> #endif /* _PMD_I40E_H_ */
>
Hi Ferruh:
Thanks all the good suggestion and help.
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, March 6, 2017 11:32 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 2/2] net/i40e: configurable PTYPE mapping
>
> On 2/27/2017 4:56 AM, Qi Zhang wrote:
> > The patch adds 4 APIs to support configurable PTYPE mapping for i40e
> > device.
> > rte_pmd_i40e_update_ptype_mapping.
> > rte_pmd_i40e_reset_ptype_mapping.
> > rte_pmd_i40e_get_ptype_mapping.
> > rte_pmd_i40e_replace_ptype_mapping.
>
> Hi Qi,
>
> These are added as PMD specific APIs, but not used anywhere, how did you
> test them? Or how others can test it?
>
> Does it make sense to add them into testpmd?
>
Yes, I plan to add
>
>
> And related to API naming, what do you think about following syntax:
> <name_space>_<object>_<action> ?
>
> This helps finding APIs for same object (ptype_mapping for this case):
>
> rte_pmd_i40e_ptype_mapping_update()
> rte_pmd_i40e_ptype_mapping_reset()
> rte_pmd_i40e_ptype_mapping_get()
> rte_pmd_i40e_ptype_mapping_replace()
Agree, that's good.
>
>
> And not directly related to this patch, but it is good idea to extract PMD
> specific APIs into separate file, would you mind taking that task before this
> patch? And add these new APIs to that new file?
>
I like this idea, since we have many changes on this recently, I'd better discuss with
other team member to decide if do the separation before merge or after the merge.
>
> > The mapping from hardware defined packet type to software defined
> > packet type can be updated/reset/read out with these APIs.
> > Also a software ptype with the most significent bit set will be
> > regarded as a custom defined ptype
> > (RTE_PMD_I40E_PTYPE_USER_DEFINE_MASK) so application can use it to
> defined its own PTYPE naming system.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 190
> ++++++++++++++++++++++++++++++++++++++++
> > drivers/net/i40e/i40e_rxtx.c | 1 -
> > drivers/net/i40e/i40e_rxtx.h | 2 +
> > drivers/net/i40e/rte_pmd_i40e.h | 81 +++++++++++++++++
>
> Need to update *version.map file too, for shared library build. It is hard to
> catch these issues since APIs are not used.
>
Will fix.
> <...>
>
> > +
> > +int rte_pmd_i40e_update_ptype_mapping(
>
> DPDK coding convention suggest having return type in separate line:
> int
> rte_pmd_i40e_update_ptype_mapping(...
>
> > + uint8_t port,
> > + struct rte_pmd_i40e_ptype_mapping *mapping_items,
> > + uint16_t count,
> > + uint8_t exclusive)
> > +{
> > + struct rte_eth_dev *dev;
> > + struct i40e_adapter *ad;
> > + int i;
>
> For PMD specific APIs, port_id needs to be verified if it is i40e port or not.
> There is already is_device_supported() function in i40e for this.
Thanks
>
> CC'ed Wenzhuo, he already did this a few times, and may help.
>
> <...>
>
> > +/**
> > + * Update hardware defined ptype to software defined packet type
> > + * mapping table.
> > + *
> > + * @param port
> > + * pointer to port identifier of the device.
> > + * @param mapping_items
> > + * the base address of the mapping items array.
> > + * @param count
> > + * number of mapping items.
> > + * @param exclusive
> > + * the flag indicate different ptype mapping update method.
> > + * -(0) only overwrite refferred PTYPE mapping,
>
> referred
>
> > + * keep other PTYPEs mapping unchanged.
> > + * -(!0) overwrite referred PTYPE mapping,
> > + * set other PTYPEs maps to PTYPE_UNKNOWN.
> > + */
> > +int rte_pmd_i40e_update_ptype_mapping(
> > + uint8_t port,
> > + struct rte_pmd_i40e_ptype_mapping *mapping_items,
> > + uint16_t count,
> > + uint8_t exclusive);
> > +
> > +/**
> > + * Reset hardware defined ptype to software defined ptype
> > + * mapping table to default.
> > + *
> > + * @param port
> > + * pointer to port identifiier of the device
>
> s/identifiier/identifier
>
> <...>
>
> > +/**
> > + * Replace a specific or a group of software defined ptypes
> > + * with a new one
> > + *
> > + * @param port
> > + * pointer to port identifier of the device
> > + * @param target
> > + * the packet type to be replaced
> > + * @param mask
> > + * -(0) target represent a specific software defined ptype.
> > + * -(!0) target is a mask to represent a group of software defined
> ptypes.
> > + * @param pkt_type
> > + * the new packet type to overwrite
> > + */
> > +int rte_pmd_i40e_replace_pkt_type(uint8_t port,
> > + uint32_t target,
> > + uint8_t mask,
> > + uint32_t pkt_type);
>
> API names does not match with one in commit log, and "pkt_type" usage is
> not consistent with other APIs.
Will fix.
>
> > +
> > #endif /* _PMD_I40E_H_ */
> >
@@ -11213,3 +11213,193 @@ rte_pmd_i40e_reset_vf_stats(uint8_t port,
return 0;
}
+
+static int check_invalid_pkt_type(uint32_t pkt_type)
+{
+
+ uint32_t l2, l3, l4, tnl, il2, il3, il4;
+
+ l2 = pkt_type & RTE_PTYPE_L2_MASK;
+ l3 = pkt_type & RTE_PTYPE_L3_MASK;
+ l4 = pkt_type & RTE_PTYPE_L4_MASK;
+ tnl = pkt_type & RTE_PTYPE_TUNNEL_MASK;
+ il2 = pkt_type & RTE_PTYPE_INNER_L2_MASK;
+ il3 = pkt_type & RTE_PTYPE_INNER_L3_MASK;
+ il4 = pkt_type & RTE_PTYPE_INNER_L4_MASK;
+
+ if (l2 != RTE_PTYPE_L2_ETHER &&
+ l2 != RTE_PTYPE_L2_ETHER_TIMESYNC &&
+ l2 != RTE_PTYPE_L2_ETHER_LLDP &&
+ l2 != RTE_PTYPE_L2_ETHER_ARP)
+ return -1;
+
+ if (l3 != RTE_PTYPE_L3_IPV4_EXT_UNKNOWN &&
+ l3 != RTE_PTYPE_L3_IPV6_EXT_UNKNOWN)
+ return -1;
+
+ if (l4 != RTE_PTYPE_L4_FRAG &&
+ l4 != RTE_PTYPE_L4_ICMP &&
+ l4 != RTE_PTYPE_L4_NONFRAG &&
+ l4 != RTE_PTYPE_L4_SCTP &&
+ l4 != RTE_PTYPE_L4_TCP &&
+ l4 != RTE_PTYPE_L4_UDP)
+ return -1;
+
+ if (tnl != RTE_PTYPE_TUNNEL_GRENAT &&
+ tnl != RTE_PTYPE_TUNNEL_IP)
+ return -1;
+
+ if (il2 != RTE_PTYPE_INNER_L2_ETHER &&
+ il2 != RTE_PTYPE_INNER_L2_ETHER_VLAN)
+ return -1;
+
+ if (il3 != RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN &&
+ il3 != RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN)
+ return -1;
+
+ if (il4 != RTE_PTYPE_INNER_L4_FRAG &&
+ il4 != RTE_PTYPE_INNER_L4_ICMP &&
+ il4 != RTE_PTYPE_INNER_L4_NONFRAG &&
+ il4 != RTE_PTYPE_INNER_L4_SCTP &&
+ il4 != RTE_PTYPE_INNER_L4_TCP &&
+ il4 != RTE_PTYPE_INNER_L4_UDP)
+ return -1;
+
+ return 0;
+}
+
+static int check_invalid_ptype_mapping(
+ struct rte_pmd_i40e_ptype_mapping *mapping_table,
+ uint16_t count)
+{
+ int i;
+
+ for (i = 0; i < count; i++) {
+ uint16_t ptype = mapping_table[i].hw_ptype;
+ uint32_t pkt_type = mapping_table[i].sw_ptype;
+
+ if (ptype >= I40E_MAX_PKT_TYPE)
+ return -1;
+
+ if (pkt_type == RTE_PTYPE_UNKNOWN)
+ continue;
+
+ if (pkt_type & RTE_PMD_I40E_PTYPE_USER_DEFINE_MASK)
+ continue;
+
+ if (check_invalid_pkt_type(pkt_type))
+ return -1;
+ }
+
+ return 0;
+}
+
+int rte_pmd_i40e_update_ptype_mapping(
+ uint8_t port,
+ struct rte_pmd_i40e_ptype_mapping *mapping_items,
+ uint16_t count,
+ uint8_t exclusive)
+{
+ struct rte_eth_dev *dev;
+ struct i40e_adapter *ad;
+ int i;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+ if (count > I40E_MAX_PKT_TYPE)
+ return -EINVAL;
+
+ if (check_invalid_ptype_mapping(mapping_items, count))
+ return -EINVAL;
+
+ dev = &rte_eth_devices[port];
+ ad = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+ if (exclusive) {
+ for (i = 0; i < I40E_MAX_PKT_TYPE; i++)
+ ad->ptype_tbl[i] = RTE_PTYPE_UNKNOWN;
+ }
+
+ for (i = 0; i < count; i++)
+ ad->ptype_tbl[mapping_items[i].hw_ptype]
+ = mapping_items[i].sw_ptype;
+
+ return 0;
+}
+
+int rte_pmd_i40e_reset_ptype_mapping(uint8_t port)
+{
+ struct rte_eth_dev *dev;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+ dev = &rte_eth_devices[port];
+ i40e_set_default_ptype_table(dev);
+
+ return 0;
+}
+
+int rte_pmd_i40e_get_ptype_mapping(
+ uint8_t port,
+ struct rte_pmd_i40e_ptype_mapping *mapping_items,
+ uint16_t size,
+ uint16_t *count,
+ uint8_t valid_only)
+{
+ struct rte_eth_dev *dev;
+ struct i40e_adapter *ad;
+ int n = 0;
+ uint16_t i;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+ dev = &rte_eth_devices[port];
+ ad = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+ for (i = 0; i < I40E_MAX_PKT_TYPE; i++) {
+ if (n >= size)
+ break;
+ if (valid_only && ad->ptype_tbl[i] == RTE_PTYPE_UNKNOWN)
+ continue;
+ mapping_items[n].hw_ptype = i;
+ mapping_items[n].sw_ptype = ad->ptype_tbl[i];
+ n++;
+ }
+
+ *count = n;
+ return 0;
+}
+
+int rte_pmd_i40e_replace_pkt_type(uint8_t port,
+ uint32_t target,
+ uint8_t mask,
+ uint32_t pkt_type)
+{
+ struct rte_eth_dev *dev;
+ struct i40e_adapter *ad;
+ uint16_t i;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+ if (!mask && check_invalid_pkt_type(target))
+ return -EINVAL;
+
+ if (check_invalid_pkt_type(pkt_type))
+ return -EINVAL;
+
+ dev = &rte_eth_devices[port];
+ ad = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+ for (i = 0; i < I40E_MAX_PKT_TYPE; i++) {
+ if (mask) {
+ if ((target | ad->ptype_tbl[i]) == target &&
+ (target & ad->ptype_tbl[i]))
+ ad->ptype_tbl[i] = pkt_type;
+ } else {
+ if (ad->ptype_tbl[i] == target)
+ ad->ptype_tbl[i] = pkt_type;
+ }
+ }
+
+ return 0;
+}
@@ -61,7 +61,6 @@
#define DEFAULT_TX_RS_THRESH 32
#define DEFAULT_TX_FREE_THRESH 32
-#define I40E_MAX_PKT_TYPE 256
#define I40E_TX_MAX_BURST 32
@@ -101,6 +101,8 @@ enum i40e_header_split_mode {
#define i40e_rx_desc i40e_32byte_rx_desc
#endif
+#define I40E_MAX_PKT_TYPE 256
+
struct i40e_rx_entry {
struct rte_mbuf *mbuf;
};
@@ -65,6 +65,13 @@ struct rte_pmd_i40e_mb_event_param {
uint16_t msglen; /**< length of the message */
};
+#define RTE_PMD_I40E_PTYPE_USER_DEFINE_MASK 0x80000000
+
+struct rte_pmd_i40e_ptype_mapping {
+ uint16_t hw_ptype; /**< hardware defined packet type*/
+ uint32_t sw_ptype; /**< software defined packet type */
+};
+
/**
* Notify VF when PF link status changes.
*
@@ -332,4 +339,78 @@ int rte_pmd_i40e_get_vf_stats(uint8_t port,
int rte_pmd_i40e_reset_vf_stats(uint8_t port,
uint16_t vf_id);
+/**
+ * Update hardware defined ptype to software defined packet type
+ * mapping table.
+ *
+ * @param port
+ * pointer to port identifier of the device.
+ * @param mapping_items
+ * the base address of the mapping items array.
+ * @param count
+ * number of mapping items.
+ * @param exclusive
+ * the flag indicate different ptype mapping update method.
+ * -(0) only overwrite refferred PTYPE mapping,
+ * keep other PTYPEs mapping unchanged.
+ * -(!0) overwrite referred PTYPE mapping,
+ * set other PTYPEs maps to PTYPE_UNKNOWN.
+ */
+int rte_pmd_i40e_update_ptype_mapping(
+ uint8_t port,
+ struct rte_pmd_i40e_ptype_mapping *mapping_items,
+ uint16_t count,
+ uint8_t exclusive);
+
+/**
+ * Reset hardware defined ptype to software defined ptype
+ * mapping table to default.
+ *
+ * @param port
+ * pointer to port identifiier of the device
+ */
+int rte_pmd_i40e_reset_ptype_mapping(uint8_t port);
+
+/**
+ * Get hardware defined ptype to software defined ptype
+ * mapping items.
+ *
+ * @param port
+ * pointer to port identifier of the device.
+ * @param mapping_items
+ * the base address of the array to store returned items.
+ * @param size
+ * the size of the input array.
+ * @param count
+ * the place to store the number of returned items.
+ * @param valid_only
+ * -(0) return full mapping table.
+ * -(!0) only return mapping items which packet_type != RTE_PTYPE_UNKNOWN.
+ */
+int rte_pmd_i40e_get_ptype_mapping(
+ uint8_t port,
+ struct rte_pmd_i40e_ptype_mapping *mapping_items,
+ uint16_t size,
+ uint16_t *count,
+ uint8_t valid_only);
+
+/**
+ * Replace a specific or a group of software defined ptypes
+ * with a new one
+ *
+ * @param port
+ * pointer to port identifier of the device
+ * @param target
+ * the packet type to be replaced
+ * @param mask
+ * -(0) target represent a specific software defined ptype.
+ * -(!0) target is a mask to represent a group of software defined ptypes.
+ * @param pkt_type
+ * the new packet type to overwrite
+ */
+int rte_pmd_i40e_replace_pkt_type(uint8_t port,
+ uint32_t target,
+ uint8_t mask,
+ uint32_t pkt_type);
+
#endif /* _PMD_I40E_H_ */