[dpdk-dev,08/12] pmd/mlx4: add dev_ptype_info_get implementation
Commit Message
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
drivers/net/mlx4/mlx4.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
Comments
Hi Jianfeng,
I'm only commenting the mlx4/mlx5 bits in this message, see below.
On Thu, Dec 31, 2015 at 02:53:15PM +0800, Jianfeng Tan wrote:
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
> drivers/net/mlx4/mlx4.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index 207bfe2..85afa32 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -2836,6 +2836,8 @@ rxq_cleanup(struct rxq *rxq)
> * @param flags
> * RX completion flags returned by poll_length_flags().
> *
> + * @note: fix mlx4_dev_ptype_info_get() if any change here.
> + *
> * @return
> * Packet type for struct rte_mbuf.
> */
> @@ -4268,6 +4270,30 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
> priv_unlock(priv);
> }
>
> +static int
> +mlx4_dev_ptype_info_get(struct rte_eth_dev *dev, uint32_t ptype_mask,
> + uint32_t ptypes[])
> +{
> + int num = 0;
> +
> + if ((dev->rx_pkt_burst == mlx4_rx_burst)
> + || (dev->rx_pkt_burst == mlx4_rx_burst_sp)) {
> + /* refers to rxq_cq_to_pkt_type() */
> + if ((ptype_mask & RTE_PTYPE_L3_MASK) == RTE_PTYPE_L3_MASK) {
> + ptypes[num++] = RTE_PTYPE_L3_IPV4;
> + ptypes[num++] = RTE_PTYPE_L3_IPV6;
> + }
> +
> + if ((ptype_mask & RTE_PTYPE_INNER_L3_MASK) == RTE_PTYPE_INNER_L3_MASK) {
> + ptypes[num++] = RTE_PTYPE_INNER_L3_IPV4;
> + ptypes[num++] = RTE_PTYPE_INNER_L3_IPV6;
> + }
> + } else
> + num = -ENOTSUP;
> +
> + return num;
> +}
I think checking for mlx4_rx_burst and mlx4_rx_burst_sp is unnecessary at
the moment, all RX burst functions do update the packet_type field, no need
for extra complexity.
Same comment for mlx5.
> +
> /**
> * DPDK callback to get device statistics.
> *
> @@ -4989,6 +5015,7 @@ static const struct eth_dev_ops mlx4_dev_ops = {
> .stats_reset = mlx4_stats_reset,
> .queue_stats_mapping_set = NULL,
> .dev_infos_get = mlx4_dev_infos_get,
> + .dev_ptypes_info_get = mlx4_dev_ptype_info_get,
> .vlan_filter_set = mlx4_vlan_filter_set,
> .vlan_tpid_set = NULL,
> .vlan_strip_queue_set = NULL,
> --
> 2.1.4
>
On 1/4/2016 7:11 PM, Adrien Mazarguil wrote:
> Hi Jianfeng,
>
> I'm only commenting the mlx4/mlx5 bits in this message, see below.
>
> On Thu, Dec 31, 2015 at 02:53:15PM +0800, Jianfeng Tan wrote:
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>> ---
>> drivers/net/mlx4/mlx4.c | 27 +++++++++++++++++++++++++++
>> 1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
>> index 207bfe2..85afa32 100644
>> --- a/drivers/net/mlx4/mlx4.c
>> +++ b/drivers/net/mlx4/mlx4.c
>> @@ -2836,6 +2836,8 @@ rxq_cleanup(struct rxq *rxq)
>> * @param flags
>> * RX completion flags returned by poll_length_flags().
>> *
>> + * @note: fix mlx4_dev_ptype_info_get() if any change here.
>> + *
>> * @return
>> * Packet type for struct rte_mbuf.
>> */
>> @@ -4268,6 +4270,30 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
>> priv_unlock(priv);
>> }
>>
>> +static int
>> +mlx4_dev_ptype_info_get(struct rte_eth_dev *dev, uint32_t ptype_mask,
>> + uint32_t ptypes[])
>> +{
>> + int num = 0;
>> +
>> + if ((dev->rx_pkt_burst == mlx4_rx_burst)
>> + || (dev->rx_pkt_burst == mlx4_rx_burst_sp)) {
>> + /* refers to rxq_cq_to_pkt_type() */
>> + if ((ptype_mask & RTE_PTYPE_L3_MASK) == RTE_PTYPE_L3_MASK) {
>> + ptypes[num++] = RTE_PTYPE_L3_IPV4;
>> + ptypes[num++] = RTE_PTYPE_L3_IPV6;
>> + }
>> +
>> + if ((ptype_mask & RTE_PTYPE_INNER_L3_MASK) == RTE_PTYPE_INNER_L3_MASK) {
>> + ptypes[num++] = RTE_PTYPE_INNER_L3_IPV4;
>> + ptypes[num++] = RTE_PTYPE_INNER_L3_IPV6;
>> + }
>> + } else
>> + num = -ENOTSUP;
>> +
>> + return num;
>> +}
> I think checking for mlx4_rx_burst and mlx4_rx_burst_sp is unnecessary at
> the moment, all RX burst functions do update the packet_type field, no need
> for extra complexity.
>
> Same comment for mlx5.
Hi Mazarguil,
My original thought is that rx_pkt_burst could be also set as
removed_rx_burst, which does not make sense indeed
because it's only possible when the device is closed.
Another consideration is to keep same style with other devices. Each
kind of device could have several rx burst functions.
So current implementation can keep extensibility to add new rx burst
functions. How do you think of it?
Thanks,
Jianfeng
>
>> +
>> /**
>> * DPDK callback to get device statistics.
>> *
>> @@ -4989,6 +5015,7 @@ static const struct eth_dev_ops mlx4_dev_ops = {
>> .stats_reset = mlx4_stats_reset,
>> .queue_stats_mapping_set = NULL,
>> .dev_infos_get = mlx4_dev_infos_get,
>> + .dev_ptypes_info_get = mlx4_dev_ptype_info_get,
>> .vlan_filter_set = mlx4_vlan_filter_set,
>> .vlan_tpid_set = NULL,
>> .vlan_strip_queue_set = NULL,
>> --
>> 2.1.4
>>
On Tue, Jan 05, 2016 at 11:08:04AM +0800, Tan, Jianfeng wrote:
>
>
> On 1/4/2016 7:11 PM, Adrien Mazarguil wrote:
> >Hi Jianfeng,
> >
> >I'm only commenting the mlx4/mlx5 bits in this message, see below.
> >
> >On Thu, Dec 31, 2015 at 02:53:15PM +0800, Jianfeng Tan wrote:
> >>Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> >>---
> >> drivers/net/mlx4/mlx4.c | 27 +++++++++++++++++++++++++++
> >> 1 file changed, 27 insertions(+)
> >>
> >>diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> >>index 207bfe2..85afa32 100644
> >>--- a/drivers/net/mlx4/mlx4.c
> >>+++ b/drivers/net/mlx4/mlx4.c
> >>@@ -2836,6 +2836,8 @@ rxq_cleanup(struct rxq *rxq)
> >> * @param flags
> >> * RX completion flags returned by poll_length_flags().
> >> *
> >>+ * @note: fix mlx4_dev_ptype_info_get() if any change here.
> >>+ *
> >> * @return
> >> * Packet type for struct rte_mbuf.
> >> */
> >>@@ -4268,6 +4270,30 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
> >> priv_unlock(priv);
> >> }
> >>+static int
> >>+mlx4_dev_ptype_info_get(struct rte_eth_dev *dev, uint32_t ptype_mask,
> >>+ uint32_t ptypes[])
Note this line is not properly indented (uint32_t should be aligned like the
rest of the file).
> >>+{
> >>+ int num = 0;
> >>+
> >>+ if ((dev->rx_pkt_burst == mlx4_rx_burst)
> >>+ || (dev->rx_pkt_burst == mlx4_rx_burst_sp)) {
I prefer operators/separators at the end of the previous line, indentation
should be fixed as well.
> >>+ /* refers to rxq_cq_to_pkt_type() */
> >>+ if ((ptype_mask & RTE_PTYPE_L3_MASK) == RTE_PTYPE_L3_MASK) {
> >>+ ptypes[num++] = RTE_PTYPE_L3_IPV4;
> >>+ ptypes[num++] = RTE_PTYPE_L3_IPV6;
> >>+ }
> >>+
> >>+ if ((ptype_mask & RTE_PTYPE_INNER_L3_MASK) == RTE_PTYPE_INNER_L3_MASK) {
> >>+ ptypes[num++] = RTE_PTYPE_INNER_L3_IPV4;
> >>+ ptypes[num++] = RTE_PTYPE_INNER_L3_IPV6;
> >>+ }
> >>+ } else
> >>+ num = -ENOTSUP;
> >>+
> >>+ return num;
> >>+}
> >I think checking for mlx4_rx_burst and mlx4_rx_burst_sp is unnecessary at
> >the moment, all RX burst functions do update the packet_type field, no need
> >for extra complexity.
> >
> >Same comment for mlx5.
>
> Hi Mazarguil,
>
> My original thought is that rx_pkt_burst could be also set as
> removed_rx_burst, which does not make sense indeed
> because it's only possible when the device is closed.
Yes, indeed.
> Another consideration is to keep same style with other devices. Each
> kind of device could have several rx burst functions.
> So current implementation can keep extensibility to add new rx burst
> functions. How do you think of it?
OK, that makes sense. Please check my above comments about coding
style/indents (I know I'm annoying).
> >>+
> >> /**
> >> * DPDK callback to get device statistics.
> >> *
> >>@@ -4989,6 +5015,7 @@ static const struct eth_dev_ops mlx4_dev_ops = {
> >> .stats_reset = mlx4_stats_reset,
> >> .queue_stats_mapping_set = NULL,
> >> .dev_infos_get = mlx4_dev_infos_get,
> >>+ .dev_ptypes_info_get = mlx4_dev_ptype_info_get,
> >> .vlan_filter_set = mlx4_vlan_filter_set,
> >> .vlan_tpid_set = NULL,
> >> .vlan_strip_queue_set = NULL,
> >>--
> >>2.1.4
> >>
>
> OK, that makes sense. Please check my above comments about coding
> style/indents (I know I'm annoying).
Thank you, Mazarguil. I'll fix it when sending out v2 patch.
Jianfeng
@@ -2836,6 +2836,8 @@ rxq_cleanup(struct rxq *rxq)
* @param flags
* RX completion flags returned by poll_length_flags().
*
+ * @note: fix mlx4_dev_ptype_info_get() if any change here.
+ *
* @return
* Packet type for struct rte_mbuf.
*/
@@ -4268,6 +4270,30 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
priv_unlock(priv);
}
+static int
+mlx4_dev_ptype_info_get(struct rte_eth_dev *dev, uint32_t ptype_mask,
+ uint32_t ptypes[])
+{
+ int num = 0;
+
+ if ((dev->rx_pkt_burst == mlx4_rx_burst)
+ || (dev->rx_pkt_burst == mlx4_rx_burst_sp)) {
+ /* refers to rxq_cq_to_pkt_type() */
+ if ((ptype_mask & RTE_PTYPE_L3_MASK) == RTE_PTYPE_L3_MASK) {
+ ptypes[num++] = RTE_PTYPE_L3_IPV4;
+ ptypes[num++] = RTE_PTYPE_L3_IPV6;
+ }
+
+ if ((ptype_mask & RTE_PTYPE_INNER_L3_MASK) == RTE_PTYPE_INNER_L3_MASK) {
+ ptypes[num++] = RTE_PTYPE_INNER_L3_IPV4;
+ ptypes[num++] = RTE_PTYPE_INNER_L3_IPV6;
+ }
+ } else
+ num = -ENOTSUP;
+
+ return num;
+}
+
/**
* DPDK callback to get device statistics.
*
@@ -4989,6 +5015,7 @@ static const struct eth_dev_ops mlx4_dev_ops = {
.stats_reset = mlx4_stats_reset,
.queue_stats_mapping_set = NULL,
.dev_infos_get = mlx4_dev_infos_get,
+ .dev_ptypes_info_get = mlx4_dev_ptype_info_get,
.vlan_filter_set = mlx4_vlan_filter_set,
.vlan_tpid_set = NULL,
.vlan_strip_queue_set = NULL,