[dpdk-dev] [PATCH v8 1/2] app/testpmd: fix max rx packet length for VLAN packets

[Thomas Monjalon]

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 at intel.com>
> >>>>>
> >>>>> Reviewed-by: Ferruh Yigit <ferruh.yigit at 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.