DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>,
	"Yang, SteveX" <stevex.yang@intel.com>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	"Xing, Beilei" <beilei.xing@intel.com>,
	"Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
	"Iremonger, Bernard" <bernard.iremonger@intel.com>,
	"Yang, Qiming" <qiming.yang@intel.com>,
	"mdr@ashroe.eu" <mdr@ashroe.eu>,
	"david.marchand@redhat.com" <david.marchand@redhat.com>,
	jerinj@marvell.com, ajit.khaparde@broadcom.com,
	maxime.coquelin@redhat.com, matan@nvidia.com,
	viacheslavo@nvidia.com, hemant.agrawal@nxp.com,
	bruce.richardson@intel.com, stephen@networkplumber.org
Subject: Re: [dpdk-dev] [PATCH v8 1/2] app/testpmd: fix max rx packet length for VLAN packets
Date: Thu, 5 Nov 2020 10:50:45 +0000
Message-ID: <2aa37ca9-720d-c611-0867-44518a4f20ca@intel.com> (raw)
In-Reply-To: <2109640.M41klPuLie@thomas>

On 11/5/2020 10:48 AM, Thomas Monjalon wrote:
> + more maintainers Cc'ed
> 
> We have a critical issue with testpmd in -rc2.
> It is blocking a lot of testing.
> Would be good to do a -rc3 today.
> Please see below.
> 
> 05/11/2020 11:44, Thomas Monjalon:
>> 05/11/2020 11:37, Ferruh Yigit:
>>> On 11/5/2020 9:33 AM, Yang, SteveX wrote:
>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>>> Sent: Thursday, November 5, 2020 4:54 PM
>>>>> To: Thomas Monjalon <thomas@monjalon.net>; Yang, SteveX
>>>>> <stevex.yang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
>>>>> Cc: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
>>>>> Xing, Beilei <beilei.xing@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
>>>>> Iremonger, Bernard <bernard.iremonger@intel.com>; Yang, Qiming
>>>>> <qiming.yang@intel.com>; mdr@ashroe.eu; nhorman@tuxdriver.com;
>>>>> david.marchand@redhat.com
>>>>> Subject: Re: [dpdk-dev] [PATCH v8 1/2] app/testpmd: fix max rx packet
>>>>> length for VLAN packets
>>>>>
>>>>> On 11/4/20 11:39 PM, Thomas Monjalon wrote:
>>>>>> 04/11/2020 21:19, Ferruh Yigit:
>>>>>>> On 11/4/2020 5:55 PM, Thomas Monjalon wrote:
>>>>>>>> 04/11/2020 18:07, Ferruh Yigit:
>>>>>>>>> On 11/4/2020 4:51 PM, Thomas Monjalon wrote:
>>>>>>>>>> 03/11/2020 14:29, Ferruh Yigit:
>>>>>>>>>>> On 11/2/2020 11:48 AM, Ferruh Yigit wrote:
>>>>>>>>>>>> On 11/2/2020 8:52 AM, SteveX Yang wrote:
>>>>>>>>>>>>> When the max rx packet length is smaller than the sum of mtu
>>>>>>>>>>>>> size and ether overhead size, it should be enlarged, otherwise
>>>>>>>>>>>>> the VLAN packets will be dropped.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Fixes: 35b2d13fd6fd ("net: add rte prefix to ether defines")
>>>>>>>>>>>>>
>>>>>>>>>>>>> Signed-off-by: SteveX Yang <stevex.yang@intel.com>
>>>>>>>>>>>>
>>>>>>>>>>>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>>>>>>>
>>>>>>>>>>> Applied to dpdk-next-net/main, thanks.
>>>>>>>>>>>
>>>>>>>>>>> only 1/2 applied since discussion is going on for 2/2.
>>>>>>>>>>
>>>>>>>>>> I'm not sure this testpmd change is good.
>>>>>>>>>>
>>>>>>>>>> Reminder: testpmd is for testing the PMDs.
>>>>>>>>>> Don't we want to see VLAN packets dropped in the case described
>>>>> above?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The patch set 'max_rx_pkt_len' in a way to make MTU 1500 for all
>>>>>>>>> PMDs, otherwise testpmd set hard-coded 'RTE_ETHER_MAX_LEN'
>>>>> value,
>>>>>>>>> which makes MTU between 1492-1500 depending on PMD.
>>>>>>>>>
>>>>>>>>> It is application responsibility to provide correct 'max_rx_pkt_len'.
>>>>>>>>> I guess the original intention was to set MTU as 1500 but was not
>>>>>>>>> correct for all PMDs and this patch is fixing it.
>>>>>>>>>
>>>>>>>>> The same problem in the ethdev, (assuming 'RTE_ETHER_MAX_LEN'
>>>>> will
>>>>>>>>> give MTU 1500), the other patch in the set is to fix it later.
>>>>>>>>
>>>>>>>> OK but the testpmd patch is just hiding the issue, isn't it?
>>>>>>>>
>>>>>>>
>>>>>>> I don't think so, issue was application (testpmd) setting the
>>>>> 'max_rx_pkt_len'
>>>>>>> wrong.
>>>>>>>
>>>>>>> What is hidden?
>>>>>>
>>>>>> I was looking for adding a helper in ethdev API.
>>>>>> But I think I can agree with your way of thinking.
>>>>>>
>>>>>
>>>>> The patch breaks running testpmd on Virtio-Net because the driver
>>>>> populates dev_info.max_rx_pktlen but keeps dev_info.max_mtu equal to
>>>>> UINT16_MAX as it was filled in by ethdev. As the result:
>>>>>
>>>>> Ethdev port_id=0 max_rx_pkt_len 11229 > max valid value 9728 Fail to
>>>>> configure port 0
>>>>
>>>> Similar issue occurred for other net PMD drivers which use default max_mtu (UINT16_MAX).
>>>> More strict checking condition will be added within new patch sooner.
>>>>
>>>
>>> :(
>>>
>>> For drivers not providing 'max_mtu' information explicitly, the default
>>> 'UINT16_MAX' is set in ethdev layer.
>>> This prevents calculating PMD specific 'overhead' and the logic in the patch is
>>> broken.
>>>
>>> Indeed this makes inconsistency in the driver too, for example for virtio, it
>>> claims 'max_rx_pktlen' as "VIRTIO_MAX_RX_PKTLEN (9728)" and 'max_mtu' as
>>> UINT16_MAX. From 'virtio_mtu_set()' we can see the real limit is
>>> 'VIRTIO_MAX_RX_PKTLEN'.
>>>
>>> When PMDs fixed, the logic in this patch can work but not sure if post -rc2 is
>>> good time to start fixing the PMDs.
>>
>> Do you suggest revert is the best choice here?
> 
> 

(copy/pasting previous reply to this eamil)

One option is revert, but than the issue this patch is trying to fix still remain.

Other option is the extend the patch as Steve sent [1], the check there is more 
like workaround in application, so not nice to have them, but with extending the 
deprecation notice (other patch in this patchset) to fix PMDs too in next 
release, I would be OK to have these checks. What do you think?

[1]
https://patches.dpdk.org/patch/83717/


  reply	other threads:[~2020-11-05 10:50 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-16  5:52 [dpdk-dev] [PATCH v1 0/5] fix default max mtu size when device configured SteveX Yang
2020-09-16  5:52 ` [dpdk-dev] [PATCH v1 1/5] net/e1000: fix max mtu size packets with vlan tag cannot be received by default SteveX Yang
2020-09-16  5:52 ` [dpdk-dev] [PATCH v1 2/5] net/igc: " SteveX Yang
2020-09-16  5:52 ` [dpdk-dev] [PATCH v1 3/5] net/ice: " SteveX Yang
2020-09-16  5:52 ` [dpdk-dev] [PATCH v1 4/5] net/iavf: " SteveX Yang
2020-09-16  5:52 ` [dpdk-dev] [PATCH v1 5/5] net/i40e: " SteveX Yang
2020-09-16 14:41   ` Ananyev, Konstantin
     [not found]     ` <DM6PR11MB4362E5FF332551D12AA20017F93E0@DM6PR11MB4362.namprd11.prod.outlook.com>
2020-09-17 12:18       ` Ananyev, Konstantin
2020-09-22  1:23 ` [dpdk-dev] [PATCH v2 0/5] fix default max mtu size when device configured SteveX Yang
2020-09-22  1:23   ` [dpdk-dev] [PATCH v2 1/5] net/e1000: fix max mtu size packets with vlan tag cannot be received by default SteveX Yang
2020-09-22  1:23   ` [dpdk-dev] [PATCH v2 2/5] net/igc: " SteveX Yang
2020-09-22  1:23   ` [dpdk-dev] [PATCH v2 3/5] net/ice: " SteveX Yang
2020-09-22  1:23   ` [dpdk-dev] [PATCH v2 4/5] net/i40e: " SteveX Yang
2020-09-22 10:47     ` Ananyev, Konstantin
2020-09-22  1:23   ` [dpdk-dev] [PATCH v2 5/5] net/iavf: " SteveX Yang
2020-09-23  4:09   ` [dpdk-dev] [PATCH v3 0/5] fix default max mtu size when device configured SteveX Yang
2020-09-23  4:09     ` [dpdk-dev] [PATCH v3 1/5] net/e1000: fix max mtu size packets with vlan tag cannot be received by default SteveX Yang
2020-09-23  4:09     ` [dpdk-dev] [PATCH v3 2/5] net/igc: " SteveX Yang
2020-09-23  4:09     ` [dpdk-dev] [PATCH v3 3/5] net/ice: " SteveX Yang
2020-09-23  4:09     ` [dpdk-dev] [PATCH v3 4/5] net/i40e: " SteveX Yang
2020-09-23  4:09     ` [dpdk-dev] [PATCH v3 5/5] net/iavf: " SteveX Yang
2020-09-28  6:55     ` [dpdk-dev] [PATCH v4 0/5] fix default max mtu size when device configured SteveX Yang
2020-09-28  6:55       ` [dpdk-dev] [PATCH v4 1/5] net/e1000: fix max mtu size packets with vlan tag cannot be received by default SteveX Yang
2020-09-28  6:55       ` [dpdk-dev] [PATCH v4 2/5] net/igc: " SteveX Yang
2020-09-28  6:55       ` [dpdk-dev] [PATCH v4 3/5] net/ice: " SteveX Yang
2020-09-29 11:59         ` Zhang, Qi Z
2020-09-29 23:01           ` Ananyev, Konstantin
2020-09-30  0:34             ` Zhang, Qi Z
     [not found]               ` <DM6PR11MB4362515283D00E27A793E6B0F9330@DM6PR11MB4362.namprd11.prod.outlook.com>
2020-09-30  2:32                 ` Zhang, Qi Z
2020-10-14 15:38                   ` Ferruh Yigit
     [not found]                     ` <DM6PR11MB43628BBF9DCE7CC4D7C05AD8F91E0@DM6PR11MB4362.namprd11.prod.outlook.com>
2020-10-19 10:49                       ` Ananyev, Konstantin
2020-10-19 13:07                         ` Ferruh Yigit
2020-10-19 14:07                           ` Ananyev, Konstantin
2020-10-19 14:28                             ` Ananyev, Konstantin
2020-10-19 18:01                               ` Ferruh Yigit
2020-10-20  9:07                                 ` Ananyev, Konstantin
2020-10-20 12:29                                   ` Ferruh Yigit
2020-10-21  9:47                                     ` Ananyev, Konstantin
2020-10-21 10:36                                       ` Ferruh Yigit
2020-10-21 10:44                                         ` Ananyev, Konstantin
2020-10-21 10:53                                           ` Ferruh Yigit
2020-10-19 18:05                       ` Ferruh Yigit
     [not found]                         ` <DM6PR11MB4362F936BFC715BF6BABBAD0F91F0@DM6PR11MB4362.namprd11.prod.outlook.com>
2020-10-20  8:13                           ` Ferruh Yigit
2020-09-28  6:55       ` [dpdk-dev] [PATCH v4 4/5] net/i40e: " SteveX Yang
2020-09-28  6:55       ` [dpdk-dev] [PATCH v4 5/5] net/iavf: " SteveX Yang
2020-10-14  9:19       ` [dpdk-dev] [PATCH v5 0/5] fix default max mtu size when device configured SteveX Yang
2020-10-14  9:19         ` [dpdk-dev] [PATCH v5 1/5] net/e1000: fix max mtu size packets with vlan tag cannot be received by default SteveX Yang
2020-10-14  9:19         ` [dpdk-dev] [PATCH v5 2/5] net/igc: " SteveX Yang
2020-10-14  9:19         ` [dpdk-dev] [PATCH v5 3/5] net/ice: " SteveX Yang
2020-10-14 11:35           ` Zhang, Qi Z
2020-10-14  9:19         ` [dpdk-dev] [PATCH v5 4/5] net/i40e: " SteveX Yang
2020-10-14 10:30           ` Ananyev, Konstantin
2020-10-14  9:19         ` [dpdk-dev] [PATCH v5 5/5] net/iavf: " SteveX Yang
2020-10-14 11:43         ` [dpdk-dev] [PATCH v5 0/5] fix default max mtu size when device configured Zhang, Qi Z
2020-10-22  8:48         ` [dpdk-dev] [PATCH v6 0/2] " SteveX Yang
2020-10-22  8:48           ` [dpdk-dev] [PATCH v6 1/2] app/testpmd: fix max rx packet length for VLAN packets SteveX Yang
2020-10-22 16:22             ` Ferruh Yigit
2020-10-22  8:48           ` [dpdk-dev] [PATCH v6 2/2] librte_ethdev: fix MTU size exceeds max rx packet length SteveX Yang
2020-10-22 16:31             ` Ferruh Yigit
2020-10-22 16:52             ` Ananyev, Konstantin
2020-10-28  3:03           ` [dpdk-dev] [PATCH v7 0/1] fix default max mtu size when device configured SteveX Yang
2020-10-28  3:03             ` [dpdk-dev] [PATCH v7 1/1] app/testpmd: fix max rx packet length for VLAN packets SteveX Yang
2020-10-29  8:41               ` Ferruh Yigit
2020-11-02  8:52             ` [dpdk-dev] [PATCH v8 0/2] fix default max mtu size when device configured SteveX Yang
2020-11-02  8:52               ` [dpdk-dev] [PATCH v8 1/2] app/testpmd: fix max rx packet length for VLAN packets SteveX Yang
2020-11-02 11:48                 ` Ferruh Yigit
2020-11-03 13:29                   ` Ferruh Yigit
2020-11-04 16:51                     ` Thomas Monjalon
2020-11-04 17:07                       ` Ferruh Yigit
2020-11-04 17:55                         ` Thomas Monjalon
2020-11-04 20:19                           ` Ferruh Yigit
2020-11-04 20:39                             ` Thomas Monjalon
2020-11-05  8:54                               ` Andrew Rybchenko
     [not found]                                 ` <DM6PR11MB43622CC5DF485DD034037CD3F9EE0@DM6PR11MB4362.namprd11.prod.outlook.com>
2020-11-05 10:37                                   ` Ferruh Yigit
2020-11-05 10:44                                     ` Thomas Monjalon
2020-11-05 10:48                                       ` Thomas Monjalon
2020-11-05 10:50                                         ` Ferruh Yigit [this message]
2020-11-05 13:52                                           ` Olivier Matz
2020-11-05 15:11                                             ` Lance Richardson
2020-11-05 15:56                                               ` Ferruh Yigit
2020-11-05 16:23                                                 ` Lance Richardson
2020-11-05 17:44                                                 ` [dpdk-dev] [PATCH 1/1] app/testpmd: revert max Rx packet length adjustment Thomas Monjalon
2020-11-05 18:02                                                   ` Lance Richardson
2020-11-05 18:11                                                   ` Ferruh Yigit
2020-11-05 18:18                                                     ` Thomas Monjalon
2020-11-05 10:49                                       ` [dpdk-dev] [PATCH v8 1/2] app/testpmd: fix max rx packet length for VLAN packets Ferruh Yigit
2020-11-02  8:52               ` [dpdk-dev] [PATCH v8 2/2] doc: annouce deprecation of jumbo frame flag condition SteveX Yang
2020-11-02 11:50                 ` Ferruh Yigit
2020-11-02 13:18                 ` Andrew Rybchenko
2020-11-02 13:58                   ` Ferruh Yigit
2020-11-02 16:05                     ` Ananyev, Konstantin
     [not found]                       ` <DM6PR11MB43625C5CF594BEDC9CE479F7F9110@DM6PR11MB4362.namprd11.prod.outlook.com>
2020-11-24 17:46                         ` Ferruh Yigit
2020-11-27 12:19                           ` Andrew Rybchenko
2020-11-27 17:08                             ` Bruce Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2aa37ca9-720d-c611-0867-44518a4f20ca@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=beilei.xing@intel.com \
    --cc=bernard.iremonger@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerinj@marvell.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=matan@nvidia.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=mdr@ashroe.eu \
    --cc=qiming.yang@intel.com \
    --cc=stephen@networkplumber.org \
    --cc=stevex.yang@intel.com \
    --cc=thomas@monjalon.net \
    --cc=viacheslavo@nvidia.com \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git