diff mbox series

[2/2] net/memif: fix driver init with default MTU

Message ID 20211026153802.1820636-2-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [1/2] net/af_packet: fix driver init with default MTU |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-mellanox-Performance fail Performance Testing issues
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS

Commit Message

Ferruh Yigit Oct. 26, 2021, 3:38 p.m. UTC 
  Driver is using 'ETH_FRAME_LEN' Linux defined value as max frame length,
which doesn't include FCS (4 bytes CRC). But ethdev by default uses
frame size with FCS when application doesn't define any explicit value.

As a result device configuration fails because device is tried to be
configured with a frame size length that is bigger than what device
reported as supported. Device reports as max supported frame size is
1514 but configured value is 1518.

Instead use DPDK macro, 'RTE_ETHER_MAX_LEN', that includes FCS in the
driver to report the max supported frame size, this matches to the
initial intention.

Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Reported-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/net/memif/rte_eth_memif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

David Christensen Oct. 26, 2021, 3:45 p.m. UTC | #1 
On 10/26/21 8:38 AM, Ferruh Yigit wrote:
> Driver is using 'ETH_FRAME_LEN' Linux defined value as max frame length,
> which doesn't include FCS (4 bytes CRC). But ethdev by default uses
> frame size with FCS when application doesn't define any explicit value.
> 
> As a result device configuration fails because device is tried to be
> configured with a frame size length that is bigger than what device
> reported as supported. Device reports as max supported frame size is
> 1514 but configured value is 1518.
> 
> Instead use DPDK macro, 'RTE_ETHER_MAX_LEN', that includes FCS in the
> driver to report the max supported frame size, this matches to the
> initial intention.
> 
> Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Reported-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
>   drivers/net/memif/rte_eth_memif.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
> index 8cec493ffdb9..cbc99067c1c8 100644
> --- a/drivers/net/memif/rte_eth_memif.c
> +++ b/drivers/net/memif/rte_eth_memif.c
> @@ -195,7 +195,7 @@ static int
>   memif_dev_info(struct rte_eth_dev *dev __rte_unused, struct rte_eth_dev_info *dev_info)
>   {
>   	dev_info->max_mac_addrs = 1;
> -	dev_info->max_rx_pktlen = (uint32_t)ETH_FRAME_LEN;
> +	dev_info->max_rx_pktlen = (uint32_t)RTE_ETHER_MAX_LEN;
>   	dev_info->max_rx_queues = ETH_MEMIF_MAX_NUM_Q_PAIRS;
>   	dev_info->max_tx_queues = ETH_MEMIF_MAX_NUM_Q_PAIRS;
>   	dev_info->min_rx_bufsize = 0;
> 

Tested-by: David Christensen <drc@linux.vnet.ibm.com>
Stephen Hemminger Oct. 26, 2021, 8:47 p.m. UTC | #2 
On Tue, 26 Oct 2021 16:38:01 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
> index 8cec493ffdb9..cbc99067c1c8 100644
> --- a/drivers/net/memif/rte_eth_memif.c
> +++ b/drivers/net/memif/rte_eth_memif.c
> @@ -195,7 +195,7 @@ static int
>  memif_dev_info(struct rte_eth_dev *dev __rte_unused, struct rte_eth_dev_info *dev_info)
>  {
>  	dev_info->max_mac_addrs = 1;
> -	dev_info->max_rx_pktlen = (uint32_t)ETH_FRAME_LEN;
> +	dev_info->max_rx_pktlen = (uint32_t)RTE_ETHER_MAX_LEN;

The cast is not necessary here. Not sure why the original code had it?
Ferruh Yigit Oct. 27, 2021, 8:27 a.m. UTC | #3 
On 10/26/2021 9:47 PM, Stephen Hemminger wrote:
> On Tue, 26 Oct 2021 16:38:01 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
>> index 8cec493ffdb9..cbc99067c1c8 100644
>> --- a/drivers/net/memif/rte_eth_memif.c
>> +++ b/drivers/net/memif/rte_eth_memif.c
>> @@ -195,7 +195,7 @@ static int
>>   memif_dev_info(struct rte_eth_dev *dev __rte_unused, struct rte_eth_dev_info *dev_info)
>>   {
>>   	dev_info->max_mac_addrs = 1;
>> -	dev_info->max_rx_pktlen = (uint32_t)ETH_FRAME_LEN;
>> +	dev_info->max_rx_pktlen = (uint32_t)RTE_ETHER_MAX_LEN;
> 
> The cast is not necessary here. Not sure why the original code had it?
> 

ack.

I will send a new version to remove the cast, since touching those lines.
diff mbox series

Patch

diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 8cec493ffdb9..cbc99067c1c8 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -195,7 +195,7 @@  static int
 memif_dev_info(struct rte_eth_dev *dev __rte_unused, struct rte_eth_dev_info *dev_info)
 {
 	dev_info->max_mac_addrs = 1;
-	dev_info->max_rx_pktlen = (uint32_t)ETH_FRAME_LEN;
+	dev_info->max_rx_pktlen = (uint32_t)RTE_ETHER_MAX_LEN;
 	dev_info->max_rx_queues = ETH_MEMIF_MAX_NUM_Q_PAIRS;
 	dev_info->max_tx_queues = ETH_MEMIF_MAX_NUM_Q_PAIRS;
 	dev_info->min_rx_bufsize = 0;