RE: [PATCH] net/pcap: support MTU set

[Ido Goshen <Ido@cgstowernetworks.com>]

> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Sent: Tuesday, 26 April 2022 20:04
> 
> On 3/22/2022 1:02 PM, Ido Goshen wrote:
> > This test
> > https://doc.dpdk.org/dts/test_plans/jumboframes_test_plan.html#test-ca
> > se-jumbo-frames-with-no-jumbo-frame-support fails for pcap pmd Jumbo
> > packet is unexpectedly received and transmitted
> >
> 
> Hi Ido,
> 
> Yes, pcap ignores MTU, but I don't see why it should use MTU (except from
> making above DTS test pass).
> 
> For the cases packets written to .pcap file or read from a .pcap file, most
> probably user is interested in all packets, I don't think using MTU to filter the
> packets is a good idea, missing packets (because of MTU) can confuse users.

[idog] 
receiving/sending unexpected packets may be confusing too (it is subjective)
The rx-err counter should clarify it. More advanced PMDs also have a rx_oversize counter 
but this will require xstats which seems an overkill.
It is also out of bound memory access prone if app is not expecting segmented mbufs and 
refers to data within mbuf->pkt_len which is beyond the 1st segment mbuf->data_len

As is there is no control, packets will pass whether one likes it or not
This patch provides the control.
If the current behaviour (not to drop) seems better then it can become the default
i.e. auto set the initial mtu to 9K or 16K

> 
> Unless there is a good use case, I am for rejecting this feature.

[idog] 
The main use case is for testing, which is probably the main reason for pcap pmd. 
We support jumbo and mtu in our products but our pcap based CI tests cannot cover it.
We also have a SW pcap based simulator which we’d like to behave the same as possible
as our HW products.
Is there a good reason that pcap pmd will behave different then other pmds?

> > ----------------------------------------------------------------------
> > ------------------
> > without patch:
> >
> > root@u18c_3nbp:/home/cgs/workspace/master/jumbo# ./dpdk-testpmd
> > --no-huge -m1024 -l 0-2  --
> vdev='net_pcap0,rx_pcap=rx_pcap=jumbo_9000.pcap,tx_pcap=file_tx.pcap' -- --
> no-flush-rx --total-num-mbufs=2048 -i ...
> > testpmd> start
> > ...
> > testpmd> show port stats 0
> >
> >    ######################## NIC statistics for port 0
> ########################
> >    RX-packets: 1          RX-missed: 0          RX-bytes:  8996
> >    RX-errors: 0
> >    RX-nombuf:  0
> >    TX-packets: 1          TX-errors: 0          TX-bytes:  8996
> >
> >    Throughput (since last show)
> >    Rx-pps:            0          Rx-bps:            0
> >    Tx-pps:            0          Tx-bps:            0
> >
> >
> #################################################################
> #####
> > ######
> >
> > ----------------------------------------------------------------------
> > ------------------ While with the patch it will fail unless
> > --max-pkt-len is used to support jumbo
> >
> > root@u18c_3nbp:/home/cgs/workspace/master/jumbo# ./dpdk-testpmd-
> patch
> > --no-huge -m1024 -l 0-2  --
> vdev='net_pcap0,rx_pcap=rx_pcap=jumbo_9000.pcap,tx_pcap=file_tx.pcap' -- --
> no-flush-rx --total-num-mbufs=2048 -i ...
> > testpmd> start
> > ...
> > testpmd> show port stats 0
> >
> >    ######################## NIC statistics for port 0
> ########################
> >    RX-packets: 0          RX-missed: 0          RX-bytes:  0
> >    RX-errors: 1
> >    RX-nombuf:  0
> >    TX-packets: 0          TX-errors: 0          TX-bytes:  0
> >
> >    Throughput (since last show)
> >    Rx-pps:            0          Rx-bps:            0
> >    Tx-pps:            0          Tx-bps:            0
> >
> >
> #################################################################
> #####
> > ######
> >
> > root@u18c_3nbp:/home/cgs/workspace/master/jumbo# ./dpdk-testpmd-
> patch
> > --no-huge -m1024 -l 0-2  --
> vdev='net_pcap0,rx_pcap=rx_pcap=jumbo_9000.pcap,tx_pcap=file_tx.pcap' -- --
> no-flush-rx --total-num-mbufs=2048 -i --max-pkt-len 9400 ...
> > testpmd> start
> > ...
> > testpmd> show port stats 0
> >
> >    ######################## NIC statistics for port 0
> ########################
> >    RX-packets: 1          RX-missed: 0          RX-bytes:  8996
> >    RX-errors: 0
> >    RX-nombuf:  0
> >    TX-packets: 1          TX-errors: 0          TX-bytes:  8996
> >
> >    Throughput (since last show)
> >    Rx-pps:            0          Rx-bps:            0
> >    Tx-pps:            0          Tx-bps:            0
> >
> >
> #################################################################
> #####
> > ######
> >
> >> -----Original Message-----
> >> From: Ido Goshen
> >> Sent: Thursday, 17 March 2022 21:12
> >> To: Stephen Hemminger <stephen@networkplumber.org>
> >> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; dev@dpdk.org
> >> Subject: RE: [PATCH] net/pcap: support MTU set
> >>
> >> As far as I can see the initial device MTU is derived from port *RX*
> >> configuration in struct rte_eth_rxmode https://doc.dpdk.org/api-
> >> 21.11/structrte__eth__rxmode.html Couple of real NICs I've tested
> >> (ixgbe, i40e based) don't allow oversized, tests details can be seen
> >> in https://bugs.dpdk.org/show_bug.cgi?id=961
> >>
> >>> -----Original Message-----
> >>> From: Stephen Hemminger <stephen@networkplumber.org>
> >>> Sent: Thursday, 17 March 2022 20:21
> >>> To: Ido Goshen <Ido@cgstowernetworks.com>
> >>> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; dev@dpdk.org
> >>> Subject: Re: [PATCH] net/pcap: support MTU set
> >>>
> >>> On Thu, 17 Mar 2022 19:43:47 +0200
> >>> ido g <ido@cgstowernetworks.com> wrote:
> >>>
> >>>> +		if (unlikely(header.caplen > dev->data->mtu)) {
> >>>> +			pcap_q->rx_stat.err_pkts++;
> >>>> +			rte_pktmbuf_free(mbuf);
> >>>> +			break;
> >>>> +		}
> >>>
> >>> MTU should only be enforced on transmit.
> >>> Other real network devices allow oversized packets.
> >>>
> >>> Since the pcap file is something user provides, if you don't want
> >>> that then use something to filter the file.