net/pcap: support MTU set

Message ID 20220317174347.110909-1-ido@cgstowernetworks.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series net/pcap: support MTU set |

Checks

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

Commit Message

Ido Goshen March 17, 2022, 5:43 p.m. UTC
  Support rte_eth_dev_set_mtu by pcap vdevs
Enforce mtu on rx/tx
For more details see https://bugs.dpdk.org/show_bug.cgi?id=961

Signed-off-by: ido g <ido@cgstowernetworks.com>
---
 drivers/net/pcap/pcap_ethdev.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)
  

Comments

Stephen Hemminger March 17, 2022, 6:20 p.m. UTC | #1
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.
  
Ido Goshen March 17, 2022, 7:11 p.m. UTC | #2
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.
  
Ido Goshen March 22, 2022, 1:02 p.m. UTC | #3
This test https://doc.dpdk.org/dts/test_plans/jumboframes_test_plan.html#test-case-jumbo-frames-with-no-jumbo-frame-support fails for pcap pmd 
Jumbo packet is unexpectedly received and transmitted

----------------------------------------------------------------------------------------
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.
  
Ferruh Yigit April 26, 2022, 5:03 p.m. UTC | #4
On 3/22/2022 1:02 PM, Ido Goshen wrote:
> This test https://doc.dpdk.org/dts/test_plans/jumboframes_test_plan.html#test-case-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.

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

> ----------------------------------------------------------------------------------------
> 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.
  
Ido Goshen April 27, 2022, 6:21 p.m. UTC | #5
> 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.
  
Stephen Hemminger April 27, 2022, 7:14 p.m. UTC | #6
On Wed, 27 Apr 2022 18:21:37 +0000
Ido Goshen <Ido@cgstowernetworks.com> wrote:

> > 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?

Why not use existing tools to filter the pcap file before you feed it to CI tests?

Other drivers may (or may not) receive packets greater than MTU. It is really driver
and hardware dependent.
  
Ido Goshen May 23, 2022, 7:48 a.m. UTC | #7
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, 27 April 2022 22:15
> To: Ido Goshen <Ido@cgstowernetworks.com>
> Cc: Ferruh Yigit <ferruh.yigit@xilinx.com>; dev@dpdk.org; Danny Raveh
> <Danny@cgstowernetworks.com>
> Subject: Re: [PATCH] net/pcap: support MTU set
> 
> >
> > [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?
> 
> Why not use existing tools to filter the pcap file before you feed it to CI tests?
> 

[idog] how can we test if the mtu feature in our app works if the pcap is filtered externally?  :-D

> Other drivers may (or may not) receive packets greater than MTU. It is really
> driver and hardware dependent.
  
Stephen Hemminger July 10, 2023, 4:45 p.m. UTC | #8
On Thu, 17 Mar 2022 19:43:47 +0200
ido g <ido@cgstowernetworks.com> wrote:

> Support rte_eth_dev_set_mtu by pcap vdevs
> Enforce mtu on rx/tx
> For more details see https://bugs.dpdk.org/show_bug.cgi?id=961
> 
> Signed-off-by: ido g <ido@cgstowernetworks.com>

Feeling less convinced that this is needed.
The motivation appears to be to test with an underlying Linux device.
If so, then why not use af_packet or tap devices which already should handle MTU?
  
Ferruh Yigit July 10, 2023, 5:46 p.m. UTC | #9
On 7/10/2023 5:45 PM, Stephen Hemminger wrote:
> On Thu, 17 Mar 2022 19:43:47 +0200
> ido g <ido@cgstowernetworks.com> wrote:
> 
>> Support rte_eth_dev_set_mtu by pcap vdevs
>> Enforce mtu on rx/tx
>> For more details see https://bugs.dpdk.org/show_bug.cgi?id=961
>>
>> Signed-off-by: ido g <ido@cgstowernetworks.com>
> 
> Feeling less convinced that this is needed.
> The motivation appears to be to test with an underlying Linux device.
> If so, then why not use af_packet or tap devices which already should handle MTU?
>

I am feeling similar for case that requires underlying physical device.

If there is a need/use case for .pcap file, as far as I can see v4
version of this set is handling .pcap file case.
  
Ido Goshen July 11, 2023, 9:41 a.m. UTC | #10
af_packet is hard coded limited to 1518 bytes so it cannot be used for jumbo

Indifferent, why is it better that pcap would NOT support api that all other pmds do (especially if it doesn't change legacy behavior when it's not used)?

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Monday, 10 July 2023 20:47
> To: Stephen Hemminger <stephen@networkplumber.org>; Ido Goshen
> <Ido@cgstowernetworks.com>
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH] net/pcap: support MTU set
> 
> On 7/10/2023 5:45 PM, Stephen Hemminger wrote:
> > On Thu, 17 Mar 2022 19:43:47 +0200
> > ido g <ido@cgstowernetworks.com> wrote:
> >
> >> Support rte_eth_dev_set_mtu by pcap vdevs Enforce mtu on rx/tx For
> >> more details see https://bugs.dpdk.org/show_bug.cgi?id=961
> >>
> >> Signed-off-by: ido g <ido@cgstowernetworks.com>
> >
> > Feeling less convinced that this is needed.
> > The motivation appears to be to test with an underlying Linux device.
> > If so, then why not use af_packet or tap devices which already should
> handle MTU?
> >
> 
> I am feeling similar for case that requires underlying physical device.
> 
> If there is a need/use case for .pcap file, as far as I can see v4 version of this
> set is handling .pcap file case.
  

Patch

diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index ec29fd6bc5..476d03c6b1 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -278,11 +278,12 @@  eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	const u_char *packet;
 	struct rte_mbuf *mbuf;
 	struct pcap_rx_queue *pcap_q = queue;
+	struct rte_eth_dev *dev = &rte_eth_devices[pcap_q->port_id];
 	uint16_t num_rx = 0;
 	uint32_t rx_bytes = 0;
 	pcap_t *pcap;
 
-	pp = rte_eth_devices[pcap_q->port_id].process_private;
+	pp = dev->process_private;
 	pcap = pp->rx_pcap[pcap_q->queue_id];
 
 	if (unlikely(pcap == NULL || nb_pkts == 0))
@@ -303,6 +304,12 @@  eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 			break;
 		}
 
+		if (unlikely(header.caplen > dev->data->mtu)) {
+			pcap_q->rx_stat.err_pkts++;
+			rte_pktmbuf_free(mbuf);
+			break;
+		}
+
 		if (header.caplen <= rte_pktmbuf_tailroom(mbuf)) {
 			/* pcap packet will fit in the mbuf, can copy it */
 			rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), packet,
@@ -378,6 +385,7 @@  eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	struct rte_mbuf *mbuf;
 	struct pmd_process_private *pp;
 	struct pcap_tx_queue *dumper_q = queue;
+	struct rte_eth_dev *dev = &rte_eth_devices[dumper_q->port_id];
 	uint16_t num_tx = 0;
 	uint32_t tx_bytes = 0;
 	struct pcap_pkthdr header;
@@ -385,7 +393,7 @@  eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN];
 	size_t len, caplen;
 
-	pp = rte_eth_devices[dumper_q->port_id].process_private;
+	pp = dev->process_private;
 	dumper = pp->tx_dumper[dumper_q->queue_id];
 
 	if (dumper == NULL || nb_pkts == 0)
@@ -396,6 +404,12 @@  eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	for (i = 0; i < nb_pkts; i++) {
 		mbuf = bufs[i];
 		len = caplen = rte_pktmbuf_pkt_len(mbuf);
+
+		if (unlikely(len > dev->data->mtu)) {
+			rte_pktmbuf_free(mbuf);
+			continue;
+		}
+
 		if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
 				len > sizeof(temp_data))) {
 			caplen = sizeof(temp_data);
@@ -464,13 +478,14 @@  eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	struct rte_mbuf *mbuf;
 	struct pmd_process_private *pp;
 	struct pcap_tx_queue *tx_queue = queue;
+	struct rte_eth_dev *dev = &rte_eth_devices[tx_queue->port_id];
 	uint16_t num_tx = 0;
 	uint32_t tx_bytes = 0;
 	pcap_t *pcap;
 	unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN];
 	size_t len;
 
-	pp = rte_eth_devices[tx_queue->port_id].process_private;
+	pp = dev->process_private;
 	pcap = pp->tx_pcap[tx_queue->queue_id];
 
 	if (unlikely(nb_pkts == 0 || pcap == NULL))
@@ -479,6 +494,12 @@  eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	for (i = 0; i < nb_pkts; i++) {
 		mbuf = bufs[i];
 		len = rte_pktmbuf_pkt_len(mbuf);
+
+		if (unlikely(len > dev->data->mtu)) {
+			rte_pktmbuf_free(mbuf);
+			continue;
+		}
+
 		if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
 				len > sizeof(temp_data))) {
 			PMD_LOG(ERR,
@@ -807,6 +828,12 @@  eth_stats_reset(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static int eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	PMD_LOG(INFO, "mtu set %s %u\n", dev->device->name, mtu);
+	return 0;
+}
+
 static inline void
 infinite_rx_ring_free(struct rte_ring *pkts)
 {
@@ -1004,6 +1031,7 @@  static const struct eth_dev_ops ops = {
 	.link_update = eth_link_update,
 	.stats_get = eth_stats_get,
 	.stats_reset = eth_stats_reset,
+	.mtu_set = eth_mtu_set,
 };
 
 static int