diff mbox series

[v3,2/4] net/af_xdp: specify minimal and maximal MTU

Message ID 20190417085653.110559-3-xiaolong.ye@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series some fixes for AF_XDP pmd |

Checks

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

Commit Message

Xiaolong Ye April 17, 2019, 8:56 a.m. UTC 
  Since AF_XDP pmd doesn't support multi segment, we need to add a valid
check in eth_dev_mtu_set.

Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 drivers/net/af_xdp/rte_eth_af_xdp.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

David Marchand April 17, 2019, 9:38 a.m. UTC | #1 
On Wed, Apr 17, 2019 at 11:02 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:

> Since AF_XDP pmd doesn't support multi segment, we need to add a valid
> check in eth_dev_mtu_set.
>

How about:
Properly report mtu capability in port device info.


Reported-by: David Marchand <david.marchand@redhat.com>

Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> ---
>  drivers/net/af_xdp/rte_eth_af_xdp.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
> b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index 5cc643ce2..8430921af 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -351,6 +351,9 @@ eth_dev_info(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *dev_info)
>         dev_info->max_rx_queues = 1;
>         dev_info->max_tx_queues = 1;
>
> +       dev_info->min_mtu = ETHER_MIN_MTU;
> +       dev_info->max_mtu = ETH_AF_XDP_FRAME_SIZE -
> ETH_AF_XDP_DATA_HEADROOM;
> +
>
        dev_info->default_rxportconf.nb_queues = 1;
>         dev_info->default_txportconf.nb_queues = 1;
>         dev_info->default_rxportconf.ring_size = ETH_AF_XDP_DFLT_NUM_DESCS;
> @@ -654,6 +657,15 @@ eth_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>         int ret;
>         int s;
>
> +       if (mtu > ETH_AF_XDP_FRAME_SIZE - ETH_AF_XDP_DATA_HEADROOM ||
> +                       mtu < ETHER_MIN_MTU) {
> +               AF_XDP_LOG(ERR, "Unsupported MTU of %d. "
> +                       "max mtu: %d, min mtu: %d", mtu,
> +                       ETH_AF_XDP_FRAME_SIZE - ETH_AF_XDP_DATA_HEADROOM,
> +                       ETHER_MIN_MTU);
> +               return -EINVAL;
> +       }
> +
>

Sorry, I suppose my previous mail was confusing.
If you provide min/max_mtu, ethdev will enforce those checks for you and
you don't need to care about it.

See:
https://git.dpdk.org/dpdk/tree/lib/librte_ethdev/rte_ethdev.c?h=v19.05-rc1#n2630

With this block removed, you can add my review tag.

Thanks.
Xiaolong Ye April 17, 2019, 1:25 p.m. UTC | #2 
On 04/17, David Marchand wrote:
>On Wed, Apr 17, 2019 at 11:02 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
>
>> Since AF_XDP pmd doesn't support multi segment, we need to add a valid
>> check in eth_dev_mtu_set.
>>
>
>How about:
>Properly report mtu capability in port device info.

Sounds better.

>
>
>Reported-by: David Marchand <david.marchand@redhat.com>
>
>Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> ---
>>  drivers/net/af_xdp/rte_eth_af_xdp.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
>> b/drivers/net/af_xdp/rte_eth_af_xdp.c
>> index 5cc643ce2..8430921af 100644
>> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
>> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
>> @@ -351,6 +351,9 @@ eth_dev_info(struct rte_eth_dev *dev, struct
>> rte_eth_dev_info *dev_info)
>>         dev_info->max_rx_queues = 1;
>>         dev_info->max_tx_queues = 1;
>>
>> +       dev_info->min_mtu = ETHER_MIN_MTU;
>> +       dev_info->max_mtu = ETH_AF_XDP_FRAME_SIZE -
>> ETH_AF_XDP_DATA_HEADROOM;
>> +
>>
>        dev_info->default_rxportconf.nb_queues = 1;
>>         dev_info->default_txportconf.nb_queues = 1;
>>         dev_info->default_rxportconf.ring_size = ETH_AF_XDP_DFLT_NUM_DESCS;
>> @@ -654,6 +657,15 @@ eth_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>>         int ret;
>>         int s;
>>
>> +       if (mtu > ETH_AF_XDP_FRAME_SIZE - ETH_AF_XDP_DATA_HEADROOM ||
>> +                       mtu < ETHER_MIN_MTU) {
>> +               AF_XDP_LOG(ERR, "Unsupported MTU of %d. "
>> +                       "max mtu: %d, min mtu: %d", mtu,
>> +                       ETH_AF_XDP_FRAME_SIZE - ETH_AF_XDP_DATA_HEADROOM,
>> +                       ETHER_MIN_MTU);
>> +               return -EINVAL;
>> +       }
>> +
>>
>
>Sorry, I suppose my previous mail was confusing.
>If you provide min/max_mtu, ethdev will enforce those checks for you and
>you don't need to care about it.
>
>See:
>https://git.dpdk.org/dpdk/tree/lib/librte_ethdev/rte_ethdev.c?h=v19.05-rc1#n2630
>
>With this block removed, you can add my review tag.

Got it, will remove it in next version.

Thanks,
Xiaolong
>
>Thanks.
>
>-- 
>David Marchand
diff mbox series

Patch

diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 5cc643ce2..8430921af 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -351,6 +351,9 @@  eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_rx_queues = 1;
 	dev_info->max_tx_queues = 1;
 
+	dev_info->min_mtu = ETHER_MIN_MTU;
+	dev_info->max_mtu = ETH_AF_XDP_FRAME_SIZE - ETH_AF_XDP_DATA_HEADROOM;
+
 	dev_info->default_rxportconf.nb_queues = 1;
 	dev_info->default_txportconf.nb_queues = 1;
 	dev_info->default_rxportconf.ring_size = ETH_AF_XDP_DFLT_NUM_DESCS;
@@ -654,6 +657,15 @@  eth_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	int ret;
 	int s;
 
+	if (mtu > ETH_AF_XDP_FRAME_SIZE - ETH_AF_XDP_DATA_HEADROOM ||
+			mtu < ETHER_MIN_MTU) {
+		AF_XDP_LOG(ERR, "Unsupported MTU of %d. "
+			"max mtu: %d, min mtu: %d", mtu,
+			ETH_AF_XDP_FRAME_SIZE - ETH_AF_XDP_DATA_HEADROOM,
+			ETHER_MIN_MTU);
+		return -EINVAL;
+	}
+
 	s = socket(PF_INET, SOCK_DGRAM, 0);
 	if (s < 0)
 		return -EINVAL;