* Re: [PATCH] net/pcap: support MTU set
2022-03-17 17:43 [PATCH] net/pcap: support MTU set ido g
@ 2022-03-17 18:20 ` Stephen Hemminger
2022-03-17 19:11 ` Ido Goshen
2022-05-30 10:36 ` [PATCH v3] pcap: " Ido Goshen
` (7 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: Stephen Hemminger @ 2022-03-17 18:20 UTC (permalink / raw)
To: ido g; +Cc: Ferruh Yigit, dev
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH] net/pcap: support MTU set
2022-03-17 18:20 ` Stephen Hemminger
@ 2022-03-17 19:11 ` Ido Goshen
2022-03-22 13:02 ` Ido Goshen
0 siblings, 1 reply; 36+ messages in thread
From: Ido Goshen @ 2022-03-17 19:11 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Ferruh Yigit, dev
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH] net/pcap: support MTU set
2022-03-17 19:11 ` Ido Goshen
@ 2022-03-22 13:02 ` Ido Goshen
2022-04-26 17:03 ` Ferruh Yigit
0 siblings, 1 reply; 36+ messages in thread
From: Ido Goshen @ 2022-03-22 13:02 UTC (permalink / raw)
To: Stephen Hemminger, Ferruh Yigit; +Cc: dev
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] net/pcap: support MTU set
2022-03-22 13:02 ` Ido Goshen
@ 2022-04-26 17:03 ` Ferruh Yigit
2022-04-27 18:21 ` Ido Goshen
0 siblings, 1 reply; 36+ messages in thread
From: Ferruh Yigit @ 2022-04-26 17:03 UTC (permalink / raw)
To: Ido Goshen, Stephen Hemminger; +Cc: dev
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH] net/pcap: support MTU set
2022-04-26 17:03 ` Ferruh Yigit
@ 2022-04-27 18:21 ` Ido Goshen
2022-04-27 19:14 ` Stephen Hemminger
0 siblings, 1 reply; 36+ messages in thread
From: Ido Goshen @ 2022-04-27 18:21 UTC (permalink / raw)
To: Ferruh Yigit, Stephen Hemminger; +Cc: dev, Danny Raveh
> 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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] net/pcap: support MTU set
2022-04-27 18:21 ` Ido Goshen
@ 2022-04-27 19:14 ` Stephen Hemminger
2022-05-23 7:48 ` Ido Goshen
0 siblings, 1 reply; 36+ messages in thread
From: Stephen Hemminger @ 2022-04-27 19:14 UTC (permalink / raw)
To: Ido Goshen; +Cc: Ferruh Yigit, dev, Danny Raveh
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH] net/pcap: support MTU set
2022-04-27 19:14 ` Stephen Hemminger
@ 2022-05-23 7:48 ` Ido Goshen
0 siblings, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2022-05-23 7:48 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Ferruh Yigit, dev, Danny Raveh
> -----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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3] pcap: support MTU set
2022-03-17 17:43 [PATCH] net/pcap: support MTU set ido g
2022-03-17 18:20 ` Stephen Hemminger
@ 2022-05-30 10:36 ` Ido Goshen
2022-05-30 18:05 ` Ferruh Yigit
2022-06-06 16:21 ` [PATCH v4] " Ido Goshen
` (6 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: Ido Goshen @ 2022-05-30 10:36 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev, Ido Goshen
Support rte_eth_dev_set_mtu by pcap vdevs
Enforce mtu on rx/tx
Bugzilla ID: 961
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
---
v3:
Preserve pcap behavior to support max size packets by default
alternative to v2 in order to limit the code change to pcap only and
avoid abi change.
Enforce mtu only in case rte_eth_dev_set_mtu was explicitly called.
v2:
Preserve pcap behavior to support max size packets by default.
---
drivers/net/pcap/pcap_ethdev.c | 44 +++++++++++++++++++++++++++++++---
1 file changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index ec29fd6bc5..2e7fff9579 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -93,6 +93,7 @@ struct pmd_internals {
int single_iface;
int phy_mac;
unsigned int infinite_rx;
+ int is_mtu_set;
};
struct pmd_process_private {
@@ -278,11 +279,13 @@ 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];
+ struct pmd_internals *internals = dev->data->dev_private;
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 +306,13 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
break;
}
+ if (unlikely(header.caplen > dev->data->mtu) &&
+ internals->is_mtu_set) {
+ 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 +388,8 @@ 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];
+ struct pmd_internals *internals = dev->data->dev_private;
uint16_t num_tx = 0;
uint32_t tx_bytes = 0;
struct pcap_pkthdr header;
@@ -385,7 +397,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 +408,13 @@ 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) &&
+ internals->is_mtu_set) {
+ rte_pktmbuf_free(mbuf);
+ continue;
+ }
+
if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
len > sizeof(temp_data))) {
caplen = sizeof(temp_data);
@@ -464,13 +483,15 @@ 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];
+ struct pmd_internals *internals = dev->data->dev_private;
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 +500,13 @@ 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) &&
+ internals->is_mtu_set) {
+ rte_pktmbuf_free(mbuf);
+ continue;
+ }
+
if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
len > sizeof(temp_data))) {
PMD_LOG(ERR,
@@ -807,6 +835,14 @@ 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);
+ struct pmd_internals *internals = dev->data->dev_private;
+ internals->is_mtu_set = 1;
+ return 0;
+}
+
static inline void
infinite_rx_ring_free(struct rte_ring *pkts)
{
@@ -1004,6 +1040,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
@@ -1233,6 +1270,7 @@ pmd_init_internals(struct rte_vdev_device *vdev,
.addr_bytes = { 0x02, 0x70, 0x63, 0x61, 0x70, iface_idx++ }
};
(*internals)->phy_mac = 0;
+ (*internals)->is_mtu_set = 0;
data = (*eth_dev)->data;
data->nb_rx_queues = (uint16_t)nb_rx_queues;
data->nb_tx_queues = (uint16_t)nb_tx_queues;
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3] pcap: support MTU set
2022-05-30 10:36 ` [PATCH v3] pcap: " Ido Goshen
@ 2022-05-30 18:05 ` Ferruh Yigit
2022-05-31 13:12 ` Ido Goshen
2022-06-06 9:40 ` Ido Goshen
0 siblings, 2 replies; 36+ messages in thread
From: Ferruh Yigit @ 2022-05-30 18:05 UTC (permalink / raw)
To: Ido Goshen, ferruh.yigit, stephen; +Cc: dev, Tianli Lai
On 5/30/2022 11:36 AM, Ido Goshen wrote:
> Support rte_eth_dev_set_mtu by pcap vdevs
> Enforce mtu on rx/tx
>
Still not sure about enforcing MTU on pcap, but please find comments on
mechanical issues
> Bugzilla ID: 961
> Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
>
> ---
> v3:
> Preserve pcap behavior to support max size packets by default
> alternative to v2 in order to limit the code change to pcap only and
> avoid abi change.
> Enforce mtu only in case rte_eth_dev_set_mtu was explicitly called.
>
> v2:
> Preserve pcap behavior to support max size packets by default.
> ---
> drivers/net/pcap/pcap_ethdev.c | 44 +++++++++++++++++++++++++++++++---
> 1 file changed, 41 insertions(+), 3 deletions(-)
>
Is documentation needs to be updated as well?
And what do you think to update release notes for this update?
> diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
> index ec29fd6bc5..2e7fff9579 100644
> --- a/drivers/net/pcap/pcap_ethdev.c
> +++ b/drivers/net/pcap/pcap_ethdev.c
> @@ -93,6 +93,7 @@ struct pmd_internals {
> int single_iface;
> int phy_mac;
> unsigned int infinite_rx;
> + int is_mtu_set;
> };
>
> struct pmd_process_private {
> @@ -278,11 +279,13 @@ 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];
> + struct pmd_internals *internals = dev->data->dev_private;
'rte_eth_devices[]' needs to be used for 'process_private' but lets not
tie it to access 'dev_private'.
You can add "struct pmd_internals *" to the "struct pcap_rx_queue" &
"struct pcap_tx_queue" structs for the access. Please check null PMD for
sample.
> 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 +306,13 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> break;
> }
>
> + if (unlikely(header.caplen > dev->data->mtu) &&
> + internals->is_mtu_set) {
> + 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 +388,8 @@ 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];
> + struct pmd_internals *internals = dev->data->dev_private;
> uint16_t num_tx = 0;
> uint32_t tx_bytes = 0;
> struct pcap_pkthdr header;
> @@ -385,7 +397,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 +408,13 @@ 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) &&
> + internals->is_mtu_set) {
It is possible to save only some part of the packet to the pcap file,
please check snaplen patch [1], how MTU config should work with this
feature?
[1]
https://patchwork.dpdk.org/project/dpdk/patch/20220313112638.3945-1-laitianli@tom.com/
> + rte_pktmbuf_free(mbuf);
> + continue;
Normally a PMD should not silently free a packet itself, it should
return error and application will decide to free the packet or not.
> + }
> +
> if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
> len > sizeof(temp_data))) {
> caplen = sizeof(temp_data);
> @@ -464,13 +483,15 @@ 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];
> + struct pmd_internals *internals = dev->data->dev_private;
> 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 +500,13 @@ 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) &&
> + internals->is_mtu_set) {
> + rte_pktmbuf_free(mbuf);
> + continue;
ditto
> + }
> +
> if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
> len > sizeof(temp_data))) {
> PMD_LOG(ERR,
> @@ -807,6 +835,14 @@ eth_stats_reset(struct rte_eth_dev *dev)
> return 0;
> }
>
> +static int eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
Please follow coding convention to have return type in a separate line:
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);
Can you please move the log after variable declerations. And can be good
to capitilise MTU in the log.
> + struct pmd_internals *internals = dev->data->dev_private;
> + internals->is_mtu_set = 1;
> + return 0;
> +}
> +
> static inline void
> infinite_rx_ring_free(struct rte_ring *pkts)
> {
> @@ -1004,6 +1040,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
> @@ -1233,6 +1270,7 @@ pmd_init_internals(struct rte_vdev_device *vdev,
> .addr_bytes = { 0x02, 0x70, 0x63, 0x61, 0x70, iface_idx++ }
> };
> (*internals)->phy_mac = 0;
> + (*internals)->is_mtu_set = 0;
> data = (*eth_dev)->data;
> data->nb_rx_queues = (uint16_t)nb_rx_queues;
> data->nb_tx_queues = (uint16_t)nb_tx_queues;
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v3] pcap: support MTU set
2022-05-30 18:05 ` Ferruh Yigit
@ 2022-05-31 13:12 ` Ido Goshen
2022-06-06 9:40 ` Ido Goshen
1 sibling, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2022-05-31 13:12 UTC (permalink / raw)
To: Ferruh Yigit, stephen; +Cc: dev, Tianli Lai
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Sent: Monday, 30 May 2022 21:06
> To: Ido Goshen <Ido@cgstowernetworks.com>; ferruh.yigit@xilinx.com;
> stephen@networkplumber.org
> Cc: dev@dpdk.org; Tianli Lai <laitianli@tom.com>
> Subject: Re: [PATCH v3] pcap: support MTU set
>
> On 5/30/2022 11:36 AM, Ido Goshen wrote:
> > Support rte_eth_dev_set_mtu by pcap vdevs Enforce mtu on rx/tx
> >
>
> Still not sure about enforcing MTU on pcap, but please find comments on
> mechanical issues
[idog] Trying to detail more about use cases:
1. CI tests which are HW independent and works with --vdevs=net_pcap
For testing that mtu feature(s) work correctly in our app a pcap that contains
mix sized packets should be used. Using pcap that has only small size packets
will miss the all point.
2. Customer support - it's much simpler to debug on a workstation and not
real HW setup. We often get the customer's configuration and a pcap in order
to reproduce an issue. It will be a pain and error-prone to manipulate the pcap
before using it.
I will address the mechanical issues and post v4 patch
> > Bugzilla ID: 961
> > Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
> >
> > ---
> > v3:
> > Preserve pcap behavior to support max size packets by default
> > alternative to v2 in order to limit the code change to pcap only and
> > avoid abi change.
> > Enforce mtu only in case rte_eth_dev_set_mtu was explicitly called.
> >
> > v2:
> > Preserve pcap behavior to support max size packets by default.
> > ---
> > drivers/net/pcap/pcap_ethdev.c | 44
> +++++++++++++++++++++++++++++++---
> > 1 file changed, 41 insertions(+), 3 deletions(-)
> >
>
> Is documentation needs to be updated as well?
[idog] I don't think so
It's using the standard rte_eth_dev_set_mtu() which is already documented in
features.rst https://doc.dpdk.org/guides/nics/features.html#mtu-update
I don't see other PMDs mention explicitly they support it (it's the normal behaviour)
> And what do you think to update release notes for this update?
[idog] ok
> > +388,8 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs,
> uint16_t nb_pkts)
> > + if (unlikely(len > dev->data->mtu) &&
> > + internals->is_mtu_set) {
>
> It is possible to save only some part of the packet to the pcap file, please
> check snaplen patch [1], how MTU config should work with this feature?
>
> [1]
> https://patchwork.dpdk.org/project/dpdk/patch/20220313112638.3945-1-
> laitianli@tom.com/
>
[idog] interesting to know this is being work on.
(To your method, why is it needed? there are tools like editpcap that can be
applied on the dpdk output file and snap the packets ;-)
I think integration of the 2 features is trivial, mtu controls if the packet is
written to file or not, and snaplan controls what part of it needs to be written.
i.e. mtu is checked before snaplen.
Using snaplen > mtu will become meaningless (maybe block/warn on such configuration)
Alternative is to apply mtu only on pcap iface (assuming snaplen is applied only
on tx_pcap file)
If supporting mtu only for pcap live ifaces and not for pcap files then
the all thing can be implemented differently by setting the OS netdevice
mtu (e.g. in linux SIOCSIFMTU) instead of enforcing it by the pcap pmd
but this will require osdep use and I admit I have no idea how to do it for windows
Would this be a better approach?
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v3] pcap: support MTU set
2022-05-30 18:05 ` Ferruh Yigit
2022-05-31 13:12 ` Ido Goshen
@ 2022-06-06 9:40 ` Ido Goshen
1 sibling, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2022-06-06 9:40 UTC (permalink / raw)
To: Ferruh Yigit, stephen; +Cc: dev, Tianli Lai
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Sent: Monday, 30 May 2022 21:06
> To: Ido Goshen <Ido@cgstowernetworks.com>; ferruh.yigit@xilinx.com;
> stephen@networkplumber.org
> Cc: dev@dpdk.org; Tianli Lai <laitianli@tom.com>
> Subject: Re: [PATCH v3] pcap: support MTU set
>
> On 5/30/2022 11:36 AM, Ido Goshen wrote:
> > Support rte_eth_dev_set_mtu by pcap vdevs Enforce mtu on rx/tx
> >
>
> > + rte_pktmbuf_free(mbuf);
> > + continue;
>
> Normally a PMD should not silently free a packet itself, it should return error and
> application will decide to free the packet or not.
>
[idog]
The doc say:
' The return value can be less than the value of the *tx_pkts* parameter when
the transmit ring is full or has been filled up.'
Which is not the case
It will force failing all the burst's following packets too even if under MTU
I think in HW case oversized TX is dropped by the HW and not left to the app.
Freeing might mimic it better, simpler and safer
I do miss incrementing the oerrors for that case
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v4] pcap: support MTU set
2022-03-17 17:43 [PATCH] net/pcap: support MTU set ido g
2022-03-17 18:20 ` Stephen Hemminger
2022-05-30 10:36 ` [PATCH v3] pcap: " Ido Goshen
@ 2022-06-06 16:21 ` Ido Goshen
2022-06-06 17:10 ` Stephen Hemminger
2022-06-07 6:27 ` [PATCH v5] pcap: support MTU set for linux interafces Ido Goshen
` (5 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: Ido Goshen @ 2022-06-06 16:21 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev, Ido Goshen
Support rte_eth_dev_set_mtu by pcap vdevs
Enforce mtu on rx/tx
Bugzilla ID: 961
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
---
v4:
1. Add release notes comment
2. Access pmd internals via queue struct
3. eth_mtu_set code convention fixes
v3:
Preserve pcap behavior to support max size packets by default
alternative to v2 in order to limit the code change to pcap only and
avoid abi change.
Enforce mtu only in case rte_eth_dev_set_mtu was explicitly called.
v2:
Preserve pcap behavior to support max size packets by default.
---
doc/guides/rel_notes/release_22_07.rst | 3 ++
drivers/net/pcap/pcap_ethdev.c | 38 ++++++++++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index 0ed4f92820..717191d498 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -95,6 +95,9 @@ New Features
* Added AH mode support in lookaside protocol (IPsec) for CN9K & CN10K.
* Added AES-GMAC support in lookaside protocol (IPsec) for CN9K & CN10K.
+* **Updated pcap driver.**
+
+ * Added support for MTU via ``rte_eth_dev_set_mtu``
Removed Items
-------------
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index ec29fd6bc5..db1958f20f 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -74,6 +74,7 @@ struct pcap_rx_queue {
/* Contains pre-generated packets to be looped through */
struct rte_ring *pkts;
+ struct pmd_internals *internals;
};
struct pcap_tx_queue {
@@ -82,6 +83,7 @@ struct pcap_tx_queue {
struct queue_stat tx_stat;
char name[PATH_MAX];
char type[ETH_PCAP_ARG_MAXLEN];
+ struct pmd_internals *internals;
};
struct pmd_internals {
@@ -93,6 +95,7 @@ struct pmd_internals {
int single_iface;
int phy_mac;
unsigned int infinite_rx;
+ uint16_t mtu;
};
struct pmd_process_private {
@@ -278,6 +281,7 @@ 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 pmd_internals *internals = pcap_q->internals;
uint16_t num_rx = 0;
uint32_t rx_bytes = 0;
pcap_t *pcap;
@@ -303,6 +307,12 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
break;
}
+ if (unlikely(header.caplen > internals->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 +388,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 pmd_internals *internals = dumper_q->internals;
uint16_t num_tx = 0;
uint32_t tx_bytes = 0;
struct pcap_pkthdr header;
@@ -396,6 +407,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 > internals->mtu)) {
+ rte_pktmbuf_free(mbuf);
+ continue;
+ }
+
if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
len > sizeof(temp_data))) {
caplen = sizeof(temp_data);
@@ -464,6 +481,7 @@ 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 pmd_internals *internals = tx_queue->internals;
uint16_t num_tx = 0;
uint32_t tx_bytes = 0;
pcap_t *pcap;
@@ -479,6 +497,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 > internals->mtu)) {
+ rte_pktmbuf_free(mbuf);
+ continue;
+ }
+
if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
len > sizeof(temp_data))) {
PMD_LOG(ERR,
@@ -807,6 +831,16 @@ eth_stats_reset(struct rte_eth_dev *dev)
return 0;
}
+static int
+eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ struct pmd_internals *internals = dev->data->dev_private;
+
+ PMD_LOG(INFO, "MTU set %s %u\n", dev->device->name, mtu);
+ internals->mtu = mtu;
+ return 0;
+}
+
static inline void
infinite_rx_ring_free(struct rte_ring *pkts)
{
@@ -878,6 +912,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
pcap_q->mb_pool = mb_pool;
pcap_q->port_id = dev->data->port_id;
pcap_q->queue_id = rx_queue_id;
+ pcap_q->internals = internals;
dev->data->rx_queues[rx_queue_id] = pcap_q;
if (internals->infinite_rx) {
@@ -952,6 +987,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev,
pcap_q->port_id = dev->data->port_id;
pcap_q->queue_id = tx_queue_id;
+ pcap_q->internals = internals;
dev->data->tx_queues[tx_queue_id] = pcap_q;
return 0;
@@ -1004,6 +1040,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
@@ -1233,6 +1270,7 @@ pmd_init_internals(struct rte_vdev_device *vdev,
.addr_bytes = { 0x02, 0x70, 0x63, 0x61, 0x70, iface_idx++ }
};
(*internals)->phy_mac = 0;
+ (*internals)->mtu = RTE_ETH_PCAP_SNAPLEN;
data = (*eth_dev)->data;
data->nb_rx_queues = (uint16_t)nb_rx_queues;
data->nb_tx_queues = (uint16_t)nb_tx_queues;
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4] pcap: support MTU set
2022-06-06 16:21 ` [PATCH v4] " Ido Goshen
@ 2022-06-06 17:10 ` Stephen Hemminger
2022-06-06 19:07 ` Ido Goshen
0 siblings, 1 reply; 36+ messages in thread
From: Stephen Hemminger @ 2022-06-06 17:10 UTC (permalink / raw)
To: Ido Goshen; +Cc: ferruh.yigit, dev
On Mon, 6 Jun 2022 19:21:47 +0300
Ido Goshen <ido@cgstowernetworks.com> wrote:
> Support rte_eth_dev_set_mtu by pcap vdevs
> Enforce mtu on rx/tx
>
> Bugzilla ID: 961
This is not really a bug, it is an enhancement specific to your
test setup. It should not be backported to stable.
Since it is change in behavior it might be better to add a vdev argument
for this rather than overloading meaning of MTU. Also, this does not behave
the same as virtio or hardware drivers.
> Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
>
> ---
> v4:
> 1. Add release notes comment
> 2. Access pmd internals via queue struct
> 3. eth_mtu_set code convention fixes
>
> v3:
> Preserve pcap behavior to support max size packets by default
> alternative to v2 in order to limit the code change to pcap only and
> avoid abi change.
> Enforce mtu only in case rte_eth_dev_set_mtu was explicitly called.
>
> v2:
> Preserve pcap behavior to support max size packets by default.
> ---
> doc/guides/rel_notes/release_22_07.rst | 3 ++
> drivers/net/pcap/pcap_ethdev.c | 38 ++++++++++++++++++++++++++
> 2 files changed, 41 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> index 0ed4f92820..717191d498 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -95,6 +95,9 @@ New Features
> * Added AH mode support in lookaside protocol (IPsec) for CN9K & CN10K.
> * Added AES-GMAC support in lookaside protocol (IPsec) for CN9K & CN10K.
>
> +* **Updated pcap driver.**
> +
> + * Added support for MTU via ``rte_eth_dev_set_mtu``
>
> Removed Items
> -------------
> diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
> index ec29fd6bc5..db1958f20f 100644
> --- a/drivers/net/pcap/pcap_ethdev.c
> +++ b/drivers/net/pcap/pcap_ethdev.c
> @@ -74,6 +74,7 @@ struct pcap_rx_queue {
>
> /* Contains pre-generated packets to be looped through */
> struct rte_ring *pkts;
> + struct pmd_internals *internals;
> };
>
> struct pcap_tx_queue {
> @@ -82,6 +83,7 @@ struct pcap_tx_queue {
> struct queue_stat tx_stat;
> char name[PATH_MAX];
> char type[ETH_PCAP_ARG_MAXLEN];
> + struct pmd_internals *internals;
> };
>
> struct pmd_internals {
> @@ -93,6 +95,7 @@ struct pmd_internals {
> int single_iface;
> int phy_mac;
> unsigned int infinite_rx;
> + uint16_t mtu;
> };
The mtu is already in dev->data->mtu, why copy it?
> struct pmd_process_private {
> @@ -278,6 +281,7 @@ 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 pmd_internals *internals = pcap_q->internals;
> uint16_t num_rx = 0;
> uint32_t rx_bytes = 0;
> pcap_t *pcap;
> @@ -303,6 +307,12 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> break;
> }
>
> + if (unlikely(header.caplen > internals->mtu)) {
> + pcap_q->rx_stat.err_pkts++;
> + rte_pktmbuf_free(mbuf);
> + break;
> + }
This doesn't account for VLAN header.
> 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 +388,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 pmd_internals *internals = dumper_q->internals;
> uint16_t num_tx = 0;
> uint32_t tx_bytes = 0;
> struct pcap_pkthdr header;
> @@ -396,6 +407,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 > internals->mtu)) {
> + rte_pktmbuf_free(mbuf);
> + continue;
> + }
There needs to be a per queue counter any and all drops.
<
> +
> if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
> len > sizeof(temp_data))) {
> caplen = sizeof(temp_data);
> @@ -464,6 +481,7 @@ 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 pmd_internals *internals = tx_queue->internals;
> uint16_t num_tx = 0;
> uint32_t tx_bytes = 0;
> pcap_t *pcap;
> @@ -479,6 +497,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 > internals->mtu)) {
> + rte_pktmbuf_free(mbuf);
> + continue;
> + }
> +
> if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
> len > sizeof(temp_data))) {
> PMD_LOG(ERR,
> @@ -807,6 +831,16 @@ eth_stats_reset(struct rte_eth_dev *dev)
> return 0;
> }
>
> +static int
> +eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
> + struct pmd_internals *internals = dev->data->dev_private;
> +
> + PMD_LOG(INFO, "MTU set %s %u\n", dev->device->name, mtu);
> + internals->mtu = mtu;
> + return 0;
> +}
If you drop internals->mtu (redundant) then this just becomes stub (ie return 0)
> +
> static inline void
> infinite_rx_ring_free(struct rte_ring *pkts)
> {
> @@ -878,6 +912,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
> pcap_q->mb_pool = mb_pool;
> pcap_q->port_id = dev->data->port_id;
> pcap_q->queue_id = rx_queue_id;
> + pcap_q->internals = internals;
> dev->data->rx_queues[rx_queue_id] = pcap_q;
>
> if (internals->infinite_rx) {
> @@ -952,6 +987,7 @@ eth_tx_queue_setup(struct rte_eth_dev *dev,
>
> pcap_q->port_id = dev->data->port_id;
> pcap_q->queue_id = tx_queue_id;
> + pcap_q->internals = internals;
> dev->data->tx_queues[tx_queue_id] = pcap_q;
>
> return 0;
> @@ -1004,6 +1040,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
> @@ -1233,6 +1270,7 @@ pmd_init_internals(struct rte_vdev_device *vdev,
> .addr_bytes = { 0x02, 0x70, 0x63, 0x61, 0x70, iface_idx++ }
> };
> (*internals)->phy_mac = 0;
> + (*internals)->mtu = RTE_ETH_PCAP_SNAPLEN;
Use dev->data->mtu not internal value.
> data = (*eth_dev)->data;
> data->nb_rx_queues = (uint16_t)nb_rx_queues;
> data->nb_tx_queues = (uint16_t)nb_tx_queues;
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v4] pcap: support MTU set
2022-06-06 17:10 ` Stephen Hemminger
@ 2022-06-06 19:07 ` Ido Goshen
0 siblings, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2022-06-06 19:07 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: ferruh.yigit, dev
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, 6 June 2022 20:10
> To: Ido Goshen <Ido@cgstowernetworks.com>
> Cc: ferruh.yigit@xilinx.com; dev@dpdk.org
> Subject: Re: [PATCH v4] pcap: support MTU set
>
> On Mon, 6 Jun 2022 19:21:47 +0300
> Ido Goshen <ido@cgstowernetworks.com> wrote:
>
> > Support rte_eth_dev_set_mtu by pcap vdevs Enforce mtu on rx/tx
> >
> > Bugzilla ID: 961
>
> This is not really a bug, it is an enhancement specific to your test setup. It should
> not be backported to stable.
>
> Since it is change in behavior it might be better to add a vdev argument for this
> rather than overloading meaning of MTU.
[idog] The default behavior stays the same and long packets will continue to pass as used to,
Only if 'rte_eth_dev_set_mtu' is explicitly used it will take effect.
I doubt it'll break anything cause no one could use it so far as it returns -ENOTSUP,
and I assume that would be the expected behavior for anyone who will set it.
Adding it as an argument to vdev (e.g. vdev='net_pcap0,iface=eth0,mtu=9400') seems to me
like a duplication to an existing API.
> Also, this does not behave the same[idog] as virtio or hardware drivers.
[idog] The idea of this patch is to make pcap behave more like HW NICs.
Couple of HW NICs (ixgbe, i40e) I've checked do respect MTU
Please see test outputs in https://bugs.dpdk.org/show_bug.cgi?id=961
Though probably it's done by the HW and not by the driver
Alternative might be to set the network interfaces MTU and not do it in pmd, so
It'll be like the "HW" is doing it, but this will work only for ifaces and not for pcap files.
>
> The mtu is already in dev->data->mtu, why copy it?
>
[idog] That's what I was using so far, but I got a request from ferruh.yigit@xilinx.com
not to use 'dev' but access 'internals' via the 'pcap_rx/tx_queue' struct.
> > + if (unlikely(header.caplen > internals->mtu)) {
> > + pcap_q->rx_stat.err_pkts++;
> > + rte_pktmbuf_free(mbuf);
> > + break;
> > + }
>
> This doesn't account for VLAN header.
[idog] Good point, I'm never sure what overhead should be considered?
Please advice what should I add
e.g. '(RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN + RTE_VLAN_HLEN * 2)'
however caller can always set it a bit higher if needed
> > +
> > + if (unlikely(len > internals->mtu)) {
> > + rte_pktmbuf_free(mbuf);
> > + continue;
> > + }
>
> There needs to be a per queue counter any and all drops.
[idog] It will be counted few lines below by
'dumper_q->tx_stat.err_pkts += nb_pkts - num_tx;'
as this case doesn't increment the 'num_tx'
> >
> > +static int
> > +eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
> > + struct pmd_internals *internals = dev->data->dev_private;
> > +
> > + PMD_LOG(INFO, "MTU set %s %u\n", dev->device->name, mtu);
> > + internals->mtu = mtu;
> > + return 0;
> > +}
>
> If you drop internals->mtu (redundant) then this just becomes stub (ie return 0)
>
[idog] Again I'm not sure if it's right to use 'dev->data->mtu' directly where later needed.
ferruh.yigit@xilinx.com ?
Anyway this function is needed even if it does nothing (or just logs) in order for the
eth_dev_ops.mtu_set to be supported
> >
> > static int
> > @@ -1233,6 +1270,7 @@ pmd_init_internals(struct rte_vdev_device *vdev,
> > .addr_bytes = { 0x02, 0x70, 0x63, 0x61, 0x70, iface_idx++ }
> > };
> > (*internals)->phy_mac = 0;
> > + (*internals)->mtu = RTE_ETH_PCAP_SNAPLEN;
>
>
> Use dev->data->mtu not internal value.
>
[idog] This runs early when the probe creates the device
Later 'dev->data->mtu' will be overwritten later in 'rte_eth_dev_configure'
To hard-coded 1500
if (dev_conf->rxmode.mtu == 0)
dev->data->dev_conf.rxmode.mtu = RTE_ETHER_MTU;
ret = eth_dev_validate_mtu(port_id, &dev_info,
dev->data->dev_conf.rxmode.mtu);
if (ret != 0)
goto rollback;
dev->data->mtu = dev->data->dev_conf.rxmode.mtu;
I tried to overcome it by [PATCH v2] http://mails.dpdk.org/archives/dev/2022-May/241974.html
But this code change spills out of the pcap pmd and changes rte_ethdev abi which I rather avoid
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v5] pcap: support MTU set for linux interafces
2022-03-17 17:43 [PATCH] net/pcap: support MTU set ido g
` (2 preceding siblings ...)
2022-06-06 16:21 ` [PATCH v4] " Ido Goshen
@ 2022-06-07 6:27 ` Ido Goshen
2022-06-08 16:04 ` [PATCH v6] " Ido Goshen
2022-06-19 9:30 ` [PATCH v7 0/3] pcap: support MTU set for linux interfaces Ido Goshen
` (4 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: Ido Goshen @ 2022-06-07 6:27 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev, Ido Goshen
Support rte_eth_dev_set_mtu for pcap ifaces vdevs by
setting the underlying OS network interface's MTU.
Support is for pcap ifaces only and not for pcap files.
Support is for Linux only.
Bugzilla ID: 961
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
---
v5:
Alternative approach
Instead of checking MTU in the pmd set it on the OS interface and
let it do the enforcment.
v4:
1. Add release notes comment
2. Access pmd internals via queue struct
3. eth_mtu_set code convention fixes
v3:
Preserve pcap behavior to support max size packets by default
alternative to v2 in order to limit the code change to pcap only and
avoid abi change.
Enforce mtu only in case rte_eth_dev_set_mtu was explicitly called.
v2:
Preserve pcap behavior to support max size packets by default.
---
doc/guides/rel_notes/release_22_07.rst | 3 ++
drivers/net/pcap/pcap_ethdev.c | 43 ++++++++++++++++++++++++++
drivers/net/pcap/pcap_osdep.h | 1 +
drivers/net/pcap/pcap_osdep_freebsd.c | 7 +++++
drivers/net/pcap/pcap_osdep_linux.c | 21 +++++++++++++
drivers/net/pcap/pcap_osdep_windows.c | 7 +++++
6 files changed, 82 insertions(+)
diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index 0ed4f92820..b90cfefc55 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -95,6 +95,9 @@ New Features
* Added AH mode support in lookaside protocol (IPsec) for CN9K & CN10K.
* Added AES-GMAC support in lookaside protocol (IPsec) for CN9K & CN10K.
+* **Updated pcap driver.**
+
+ * Added support for MTU on Linux network interfaces
Removed Items
-------------
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index ec29fd6bc5..3d76d50ef1 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -807,6 +807,48 @@ eth_stats_reset(struct rte_eth_dev *dev)
return 0;
}
+static int
+eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ unsigned int i;
+ struct pmd_internals *internals = dev->data->dev_private;
+ int is_supported = 0;
+ int is_err = 0;
+
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct pcap_rx_queue* queue = &internals->rx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_IN_ARG) == 0)) {
+ is_supported = 1;
+ if (osdep_iface_mtu_set(queue->name, mtu) < 0)
+ is_err = 1;
+ }
+ }
+
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ struct pcap_tx_queue* queue = &internals->tx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_TX_IFACE_ARG) == 0)) {
+ is_supported = 1;
+ if (osdep_iface_mtu_set(queue->name, mtu) < 0)
+ is_err = 1;
+ }
+ }
+
+ if (!is_supported)
+ return -ENOTSUP;
+
+ if (is_err)
+ return -1;
+
+ 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 +1046,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
diff --git a/drivers/net/pcap/pcap_osdep.h b/drivers/net/pcap/pcap_osdep.h
index bf41cba982..ef8be7543c 100644
--- a/drivers/net/pcap/pcap_osdep.h
+++ b/drivers/net/pcap/pcap_osdep.h
@@ -14,5 +14,6 @@ extern int eth_pcap_logtype;
int osdep_iface_index_get(const char *name);
int osdep_iface_mac_get(const char *name, struct rte_ether_addr *mac);
+int osdep_iface_mtu_set(const char *if_name, uint16_t mtu);
#endif
diff --git a/drivers/net/pcap/pcap_osdep_freebsd.c b/drivers/net/pcap/pcap_osdep_freebsd.c
index 20556b3e92..52d4a0e6db 100644
--- a/drivers/net/pcap/pcap_osdep_freebsd.c
+++ b/drivers/net/pcap/pcap_osdep_freebsd.c
@@ -57,3 +57,10 @@ osdep_iface_mac_get(const char *if_name, struct rte_ether_addr *mac)
rte_free(buf);
return 0;
}
+
+int
+osdep_iface_mtu_set(const char *if_name, uint16_t mtu)
+{
+ PMD_LOG(WARN, "mtu set not supported on freebsd\n");
+ return -ENOTSUP;
+}
diff --git a/drivers/net/pcap/pcap_osdep_linux.c b/drivers/net/pcap/pcap_osdep_linux.c
index 97033f57c5..b0b4a716fe 100644
--- a/drivers/net/pcap/pcap_osdep_linux.c
+++ b/drivers/net/pcap/pcap_osdep_linux.c
@@ -40,3 +40,24 @@ osdep_iface_mac_get(const char *if_name, struct rte_ether_addr *mac)
close(if_fd);
return 0;
}
+
+int
+osdep_iface_mtu_set(const char *if_name, uint16_t mtu)
+{
+ struct ifreq ifr;
+ int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+ if (if_fd == -1)
+ return -1;
+
+ rte_strscpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+ ifr.ifr_mtu = mtu;
+ if (ioctl(if_fd, SIOCSIFMTU, &ifr)) {
+ PMD_LOG(ERR, "%s mtu set to %d failed\n", if_name, mtu);
+ close(if_fd);
+ return -1;
+ }
+
+ close(if_fd);
+ return 0;
+}
diff --git a/drivers/net/pcap/pcap_osdep_windows.c b/drivers/net/pcap/pcap_osdep_windows.c
index 1d398dc7ed..15205a521e 100644
--- a/drivers/net/pcap/pcap_osdep_windows.c
+++ b/drivers/net/pcap/pcap_osdep_windows.c
@@ -116,3 +116,10 @@ osdep_iface_mac_get(const char *device_name, struct rte_ether_addr *mac)
free(info);
return ret;
}
+
+int
+osdep_iface_mtu_set(const char *if_name, uint16_t mtu)
+{
+ PMD_LOG(WARN, "mtu set not supported on Windows\n", sys_ret);
+ return -ENOTSUP;
+}
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v6] pcap: support MTU set for linux interafces
2022-06-07 6:27 ` [PATCH v5] pcap: support MTU set for linux interafces Ido Goshen
@ 2022-06-08 16:04 ` Ido Goshen
2022-06-08 16:23 ` Stephen Hemminger
0 siblings, 1 reply; 36+ messages in thread
From: Ido Goshen @ 2022-06-08 16:04 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev, Ido Goshen
Support rte_eth_dev_set_mtu for pcap ifaces vdevs by
setting the underlying OS network interface's MTU.
Support is for pcap ifaces only and not for pcap files.
Support is for Linux only.
Bugzilla ID: 961
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
---
v6:
Fixes for v5 approach
1. freebsd compilation fix
2. checkpatch warning fix
v5:
Alternative approach
Instead of checking MTU in the pmd set it on the OS interface and
let it do the enforcment.
v4:
1. Add release notes comment
2. Access pmd internals via queue struct
3. eth_mtu_set code convention fixes
v3:
Preserve pcap behavior to support max size packets by default
alternative to v2 in order to limit the code change to pcap only and
avoid abi change.
Enforce mtu only in case rte_eth_dev_set_mtu was explicitly called.
v2:
Preserve pcap behavior to support max size packets by default.
---
doc/guides/rel_notes/release_22_07.rst | 3 ++
drivers/net/pcap/pcap_ethdev.c | 42 ++++++++++++++++++++++++++
drivers/net/pcap/pcap_osdep.h | 1 +
drivers/net/pcap/pcap_osdep_freebsd.c | 7 +++++
drivers/net/pcap/pcap_osdep_linux.c | 21 +++++++++++++
drivers/net/pcap/pcap_osdep_windows.c | 7 +++++
6 files changed, 81 insertions(+)
diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index 0ed4f92820..b90cfefc55 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -95,6 +95,9 @@ New Features
* Added AH mode support in lookaside protocol (IPsec) for CN9K & CN10K.
* Added AES-GMAC support in lookaside protocol (IPsec) for CN9K & CN10K.
+* **Updated pcap driver.**
+
+ * Added support for MTU on Linux network interfaces
Removed Items
-------------
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index ec29fd6bc5..2221c53051 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -807,6 +807,47 @@ eth_stats_reset(struct rte_eth_dev *dev)
return 0;
}
+static int
+eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ unsigned int i;
+ struct pmd_internals *internals = dev->data->dev_private;
+ int is_supported = 0;
+ int is_err = 0;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct pcap_rx_queue *queue = &internals->rx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_IN_ARG) == 0)) {
+ is_supported = 1;
+ if (osdep_iface_mtu_set(queue->name, mtu) < 0)
+ is_err = 1;
+ }
+ }
+
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ struct pcap_tx_queue *queue = &internals->tx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_TX_IFACE_ARG) == 0)) {
+ is_supported = 1;
+ if (osdep_iface_mtu_set(queue->name, mtu) < 0)
+ is_err = 1;
+ }
+ }
+
+ if (!is_supported)
+ return -ENOTSUP;
+
+ if (is_err)
+ return -1;
+
+ 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 +1045,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
diff --git a/drivers/net/pcap/pcap_osdep.h b/drivers/net/pcap/pcap_osdep.h
index bf41cba982..ef8be7543c 100644
--- a/drivers/net/pcap/pcap_osdep.h
+++ b/drivers/net/pcap/pcap_osdep.h
@@ -14,5 +14,6 @@ extern int eth_pcap_logtype;
int osdep_iface_index_get(const char *name);
int osdep_iface_mac_get(const char *name, struct rte_ether_addr *mac);
+int osdep_iface_mtu_set(const char *if_name, uint16_t mtu);
#endif
diff --git a/drivers/net/pcap/pcap_osdep_freebsd.c b/drivers/net/pcap/pcap_osdep_freebsd.c
index 20556b3e92..a9961ba3e3 100644
--- a/drivers/net/pcap/pcap_osdep_freebsd.c
+++ b/drivers/net/pcap/pcap_osdep_freebsd.c
@@ -57,3 +57,10 @@ osdep_iface_mac_get(const char *if_name, struct rte_ether_addr *mac)
rte_free(buf);
return 0;
}
+
+int
+osdep_iface_mtu_set(__rte_unused const char *if_name, __rte_unused uint16_t mtu)
+{
+ PMD_LOG(ERR, "mtu set not supported on freebsd\n");
+ return -ENOTSUP;
+}
diff --git a/drivers/net/pcap/pcap_osdep_linux.c b/drivers/net/pcap/pcap_osdep_linux.c
index 97033f57c5..b0b4a716fe 100644
--- a/drivers/net/pcap/pcap_osdep_linux.c
+++ b/drivers/net/pcap/pcap_osdep_linux.c
@@ -40,3 +40,24 @@ osdep_iface_mac_get(const char *if_name, struct rte_ether_addr *mac)
close(if_fd);
return 0;
}
+
+int
+osdep_iface_mtu_set(const char *if_name, uint16_t mtu)
+{
+ struct ifreq ifr;
+ int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+ if (if_fd == -1)
+ return -1;
+
+ rte_strscpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+ ifr.ifr_mtu = mtu;
+ if (ioctl(if_fd, SIOCSIFMTU, &ifr)) {
+ PMD_LOG(ERR, "%s mtu set to %d failed\n", if_name, mtu);
+ close(if_fd);
+ return -1;
+ }
+
+ close(if_fd);
+ return 0;
+}
diff --git a/drivers/net/pcap/pcap_osdep_windows.c b/drivers/net/pcap/pcap_osdep_windows.c
index 1d398dc7ed..69cf052e6d 100644
--- a/drivers/net/pcap/pcap_osdep_windows.c
+++ b/drivers/net/pcap/pcap_osdep_windows.c
@@ -116,3 +116,10 @@ osdep_iface_mac_get(const char *device_name, struct rte_ether_addr *mac)
free(info);
return ret;
}
+
+int
+osdep_iface_mtu_set(__rte_unused const char *if_name, __rte_unused uint16_t mtu)
+{
+ PMD_LOG(ERR, "mtu set not supported on Windows\n");
+ return -ENOTSUP;
+}
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v6] pcap: support MTU set for linux interafces
2022-06-08 16:04 ` [PATCH v6] " Ido Goshen
@ 2022-06-08 16:23 ` Stephen Hemminger
0 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2022-06-08 16:23 UTC (permalink / raw)
To: Ido Goshen; +Cc: ferruh.yigit, dev
On Wed, 8 Jun 2022 19:04:19 +0300
Ido Goshen <ido@cgstowernetworks.com> wrote:
> +int
> +osdep_iface_mtu_set(const char *if_name, uint16_t mtu)
> +{
> + struct ifreq ifr;
> + int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
> +
> + if (if_fd == -1)
> + return -1;
> +
> + rte_strscpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
> + ifr.ifr_mtu = mtu;
> + if (ioctl(if_fd, SIOCSIFMTU, &ifr)) {
> + PMD_LOG(ERR, "%s mtu set to %d failed\n", if_name, mtu);
> + close(if_fd);
> + return -1;
> + }
> +
> + close(if_fd);
> + return 0;
> +}
This should work on FreeBSD as well.
FreeBSD has the same ioctl()
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v7 0/3] pcap: support MTU set for linux interfaces
2022-03-17 17:43 [PATCH] net/pcap: support MTU set ido g
` (3 preceding siblings ...)
2022-06-07 6:27 ` [PATCH v5] pcap: support MTU set for linux interafces Ido Goshen
@ 2022-06-19 9:30 ` Ido Goshen
2022-06-19 9:30 ` [PATCH v7 1/3] " Ido Goshen
` (2 more replies)
2022-06-20 8:39 ` [PATCH v8 0/3] pcap: support MTU set for linux interfaces Ido Goshen
` (3 subsequent siblings)
8 siblings, 3 replies; 36+ messages in thread
From: Ido Goshen @ 2022-06-19 9:30 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev
Support rte_eth_dev_set_mtu for pcap ifaces vdevs by
setting the underlying OS network interface's MTU.
Support is for pcap ifaces only and not for pcap files.
Support is for Linux only.
patch series:
[PATCH v7 1/3] pcap: support MTU set for linux interfaces
[PATCH v7 2/3] pcap: support MTU set for linux interfaces TX
[PATCH v7 3/3] pcap: support MTU set for linux interfaces count
doc/guides/rel_notes/release_22_07.rst | 3 ++
drivers/net/pcap/pcap_ethdev.c | 126 +++++++++++++++++++++++++++++++++-----------
drivers/net/pcap/pcap_osdep.h | 1 +
drivers/net/pcap/pcap_osdep_freebsd.c | 7 +++
drivers/net/pcap/pcap_osdep_linux.c | 21 ++++++++
drivers/net/pcap/pcap_osdep_windows.c | 7 +++
6 files changed, 134 insertions(+), 31 deletions(-)
---
v7:
1. TX drop only individual oversized packets and not the entire burst
2. Count oversize drops by i/oerrors
v6:
Fixes for v5 approach
1. freebsd compilation fix
2. checkpatch warning fix
v5:
Alternative approach
Instead of checking MTU in the pmd set it on the OS interface and
let it do the enforcment.
v4:
1. Add release notes comment
2. Access pmd internals via queue struct
3. eth_mtu_set code convention fixes
v3:
Preserve pcap behavior to support max size packets by default
alternative to v2 in order to limit the code change to pcap only and
avoid abi change.
Enforce mtu only in case rte_eth_dev_set_mtu was explicitly called.
v2:
Preserve pcap behavior to support max size packets by default.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v7 1/3] pcap: support MTU set for linux interfaces
2022-06-19 9:30 ` [PATCH v7 0/3] pcap: support MTU set for linux interfaces Ido Goshen
@ 2022-06-19 9:30 ` Ido Goshen
2022-06-19 9:30 ` [PATCH v7 2/3] pcap: support MTU set for linux interfaces TX enhancment Ido Goshen
2022-06-19 9:30 ` [PATCH v7 3/3] pcap: support MTU set for linux interfaces count ierrors Ido Goshen
2 siblings, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2022-06-19 9:30 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev, Ido Goshen
Support rte_eth_dev_set_mtu for pcap ifaces vdevs by
setting the underlying OS network interface's MTU.
Support is for pcap ifaces only and not for pcap files.
Support is for Linux only.
Bugzilla ID: 961
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
---
doc/guides/rel_notes/release_22_07.rst | 3 ++
drivers/net/pcap/pcap_ethdev.c | 42 ++++++++++++++++++++++++++
drivers/net/pcap/pcap_osdep.h | 1 +
drivers/net/pcap/pcap_osdep_freebsd.c | 7 +++++
drivers/net/pcap/pcap_osdep_linux.c | 21 +++++++++++++
drivers/net/pcap/pcap_osdep_windows.c | 7 +++++
6 files changed, 81 insertions(+)
diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index 0ed4f92820..a354144044 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -95,6 +95,9 @@ New Features
* Added AH mode support in lookaside protocol (IPsec) for CN9K & CN10K.
* Added AES-GMAC support in lookaside protocol (IPsec) for CN9K & CN10K.
+* **Updated pcap driver.**
+
+ * Added support for MTU on Linux network interfaces.
Removed Items
-------------
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index ec29fd6bc5..2221c53051 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -807,6 +807,47 @@ eth_stats_reset(struct rte_eth_dev *dev)
return 0;
}
+static int
+eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ unsigned int i;
+ struct pmd_internals *internals = dev->data->dev_private;
+ int is_supported = 0;
+ int is_err = 0;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct pcap_rx_queue *queue = &internals->rx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_IN_ARG) == 0)) {
+ is_supported = 1;
+ if (osdep_iface_mtu_set(queue->name, mtu) < 0)
+ is_err = 1;
+ }
+ }
+
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ struct pcap_tx_queue *queue = &internals->tx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_TX_IFACE_ARG) == 0)) {
+ is_supported = 1;
+ if (osdep_iface_mtu_set(queue->name, mtu) < 0)
+ is_err = 1;
+ }
+ }
+
+ if (!is_supported)
+ return -ENOTSUP;
+
+ if (is_err)
+ return -1;
+
+ 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 +1045,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
diff --git a/drivers/net/pcap/pcap_osdep.h b/drivers/net/pcap/pcap_osdep.h
index bf41cba982..043677ec77 100644
--- a/drivers/net/pcap/pcap_osdep.h
+++ b/drivers/net/pcap/pcap_osdep.h
@@ -14,5 +14,6 @@ extern int eth_pcap_logtype;
int osdep_iface_index_get(const char *name);
int osdep_iface_mac_get(const char *name, struct rte_ether_addr *mac);
+int osdep_iface_mtu_set(const char *name, uint16_t mtu);
#endif
diff --git a/drivers/net/pcap/pcap_osdep_freebsd.c b/drivers/net/pcap/pcap_osdep_freebsd.c
index 20556b3e92..a9961ba3e3 100644
--- a/drivers/net/pcap/pcap_osdep_freebsd.c
+++ b/drivers/net/pcap/pcap_osdep_freebsd.c
@@ -57,3 +57,10 @@ osdep_iface_mac_get(const char *if_name, struct rte_ether_addr *mac)
rte_free(buf);
return 0;
}
+
+int
+osdep_iface_mtu_set(__rte_unused const char *if_name, __rte_unused uint16_t mtu)
+{
+ PMD_LOG(ERR, "mtu set not supported on freebsd\n");
+ return -ENOTSUP;
+}
diff --git a/drivers/net/pcap/pcap_osdep_linux.c b/drivers/net/pcap/pcap_osdep_linux.c
index 97033f57c5..b0b4a716fe 100644
--- a/drivers/net/pcap/pcap_osdep_linux.c
+++ b/drivers/net/pcap/pcap_osdep_linux.c
@@ -40,3 +40,24 @@ osdep_iface_mac_get(const char *if_name, struct rte_ether_addr *mac)
close(if_fd);
return 0;
}
+
+int
+osdep_iface_mtu_set(const char *if_name, uint16_t mtu)
+{
+ struct ifreq ifr;
+ int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+ if (if_fd == -1)
+ return -1;
+
+ rte_strscpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+ ifr.ifr_mtu = mtu;
+ if (ioctl(if_fd, SIOCSIFMTU, &ifr)) {
+ PMD_LOG(ERR, "%s mtu set to %d failed\n", if_name, mtu);
+ close(if_fd);
+ return -1;
+ }
+
+ close(if_fd);
+ return 0;
+}
diff --git a/drivers/net/pcap/pcap_osdep_windows.c b/drivers/net/pcap/pcap_osdep_windows.c
index 1d398dc7ed..db34fc49e2 100644
--- a/drivers/net/pcap/pcap_osdep_windows.c
+++ b/drivers/net/pcap/pcap_osdep_windows.c
@@ -116,3 +116,10 @@ osdep_iface_mac_get(const char *device_name, struct rte_ether_addr *mac)
free(info);
return ret;
}
+
+int
+osdep_iface_mtu_set(__rte_unused const char *device_name, __rte_unused uint16_t mtu)
+{
+ PMD_LOG(ERR, "mtu set not supported on Windows\n");
+ return -ENOTSUP;
+}
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v7 2/3] pcap: support MTU set for linux interfaces TX enhancment
2022-06-19 9:30 ` [PATCH v7 0/3] pcap: support MTU set for linux interfaces Ido Goshen
2022-06-19 9:30 ` [PATCH v7 1/3] " Ido Goshen
@ 2022-06-19 9:30 ` Ido Goshen
2022-06-20 22:52 ` Stephen Hemminger
2022-06-19 9:30 ` [PATCH v7 3/3] pcap: support MTU set for linux interfaces count ierrors Ido Goshen
2 siblings, 1 reply; 36+ messages in thread
From: Ido Goshen @ 2022-06-19 9:30 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev, Ido Goshen
Drop only the oversized packets and not its entrie burst
mbuf will be freed and will be counted as oerror
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
---
drivers/net/pcap/pcap_ethdev.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index 2221c53051..ff98762058 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -494,8 +494,14 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
*/
ret = pcap_sendpacket(pcap,
rte_pktmbuf_read(mbuf, 0, len, temp_data), len);
- if (unlikely(ret != 0))
- break;
+ if (unlikely(ret != 0)) {
+ if (errno == EMSGSIZE) {
+ rte_pktmbuf_free(mbuf);
+ continue;
+ } else {
+ break;
+ }
+ }
num_tx++;
tx_bytes += len;
rte_pktmbuf_free(mbuf);
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 2/3] pcap: support MTU set for linux interfaces TX enhancment
2022-06-19 9:30 ` [PATCH v7 2/3] pcap: support MTU set for linux interfaces TX enhancment Ido Goshen
@ 2022-06-20 22:52 ` Stephen Hemminger
2022-06-21 9:07 ` Ido Goshen
0 siblings, 1 reply; 36+ messages in thread
From: Stephen Hemminger @ 2022-06-20 22:52 UTC (permalink / raw)
To: Ido Goshen; +Cc: ferruh.yigit, dev
On Sun, 19 Jun 2022 12:30:33 +0300
Ido Goshen <ido@cgstowernetworks.com> wrote:
> Drop only the oversized packets and not its entrie burst
> mbuf will be freed and will be counted as oerror
>
> Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
> ---
> drivers/net/pcap/pcap_ethdev.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
> index 2221c53051..ff98762058 100644
> --- a/drivers/net/pcap/pcap_ethdev.c
> +++ b/drivers/net/pcap/pcap_ethdev.c
> @@ -494,8 +494,14 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> */
> ret = pcap_sendpacket(pcap,
> rte_pktmbuf_read(mbuf, 0, len, temp_data), len);
> - if (unlikely(ret != 0))
> - break;
> + if (unlikely(ret != 0)) {
> + if (errno == EMSGSIZE) {
Will this show up in tx_errors?
> + rte_pktmbuf_free(mbuf);
> + continue;
> + } else {
> + break;
> + }
else is not needed here.
> + }
> num_tx++;
> tx_bytes += len;
> rte_pktmbuf_free(mbuf);
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v7 2/3] pcap: support MTU set for linux interfaces TX enhancment
2022-06-20 22:52 ` Stephen Hemminger
@ 2022-06-21 9:07 ` Ido Goshen
0 siblings, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2022-06-21 9:07 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: ferruh.yigit, dev
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, 21 June 2022 1:53
> To: Ido Goshen <Ido@cgstowernetworks.com>
> Cc: ferruh.yigit@xilinx.com; dev@dpdk.org
> Subject: Re: [PATCH v7 2/3] pcap: support MTU set for linux interfaces TX
> enhancment
>
> On Sun, 19 Jun 2022 12:30:33 +0300
> Ido Goshen <ido@cgstowernetworks.com> wrote:
>
> > Drop only the oversized packets and not its entrie burst mbuf will be
> > freed and will be counted as oerror
> >
> > Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
> > ---
> > drivers/net/pcap/pcap_ethdev.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/pcap/pcap_ethdev.c
> > b/drivers/net/pcap/pcap_ethdev.c index 2221c53051..ff98762058 100644
> > --- a/drivers/net/pcap/pcap_ethdev.c
> > +++ b/drivers/net/pcap/pcap_ethdev.c
> > @@ -494,8 +494,14 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs,
> uint16_t nb_pkts)
> > */
> > ret = pcap_sendpacket(pcap,
> > rte_pktmbuf_read(mbuf, 0, len, temp_data), len);
> > - if (unlikely(ret != 0))
> > - break;
> > + if (unlikely(ret != 0)) {
> > + if (errno == EMSGSIZE) {
>
> Will this show up in tx_errors?
>
[idog] yes
It will be counted few lines below by
'dumper_q->tx_stat.err_pkts += nb_pkts - num_tx;'
as this case doesn't increment the 'num_tx'
test example:
build/app/dpdk-testpmd --no-huge -m1024 -l 0-2 --vdev='net_pcap0,iface=veth0' --vdev='net_pcap1,iface=veth1' -- -i
...
testpmd> port config mtu 0 9400
testpmd> port config mtu 1 1500
testpmd> start
...
testpmd> stop
Telling cores to stop...
Waiting for lcores to finish...
---------------------- Forward statistics for port 0 ----------------------
RX-packets: 1 RX-dropped: 0 RX-total: 1
TX-packets: 0 TX-dropped: 0 TX-total: 0
----------------------------------------------------------------------------
---------------------- Forward statistics for port 1 ----------------------
RX-packets: 0 RX-dropped: 0 RX-total: 0
TX-packets: 0 TX-dropped: 0 TX-total: 0
----------------------------------------------------------------------------
+++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
RX-packets: 1 RX-dropped: 0 RX-total: 1
TX-packets: 0 TX-dropped: 1 TX-total: 1
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Done.
testpmd> show port stats all
######################## NIC statistics for port 0 ########################
RX-packets: 1 RX-missed: 0 RX-bytes: 8996
RX-errors: 0
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
############################################################################
######################## NIC statistics for port 1 ########################
RX-packets: 0 RX-missed: 0 RX-bytes: 0
RX-errors: 0
RX-nombuf: 0
TX-packets: 0 TX-errors: 1 TX-bytes: 0
Throughput (since last show)
Rx-pps: 0 Rx-bps: 0
Tx-pps: 0 Tx-bps: 0
############################################################################
> > + rte_pktmbuf_free(mbuf);
> > + continue;
> > + } else {
> > + break;
> > + }
> else is not needed here.
[idog] ok
> > + }
> > num_tx++;
> > tx_bytes += len;
> > rte_pktmbuf_free(mbuf);
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v7 3/3] pcap: support MTU set for linux interfaces count ierrors
2022-06-19 9:30 ` [PATCH v7 0/3] pcap: support MTU set for linux interfaces Ido Goshen
2022-06-19 9:30 ` [PATCH v7 1/3] " Ido Goshen
2022-06-19 9:30 ` [PATCH v7 2/3] pcap: support MTU set for linux interfaces TX enhancment Ido Goshen
@ 2022-06-19 9:30 ` Ido Goshen
2 siblings, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2022-06-19 9:30 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev, Ido Goshen
Count oversized packets that are dropped by the interface
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
---
drivers/net/pcap/pcap_ethdev.c | 74 +++++++++++++++++++++-------------
1 file changed, 45 insertions(+), 29 deletions(-)
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index ff98762058..46f18e9b10 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -54,7 +54,7 @@ struct queue_stat {
volatile unsigned long rx_nombuf;
};
-struct queue_missed_stat {
+struct queue_pcap_stat {
/* last value retrieved from pcap */
unsigned int pcap;
/* stores values lost by pcap stop or rollover */
@@ -63,12 +63,19 @@ struct queue_missed_stat {
unsigned long reset;
};
+enum {
+ QUEUE_PCAP_STAT_FIRST = 0,
+ QUEUE_PCAP_STAT_MISSED = QUEUE_PCAP_STAT_FIRST,
+ QUEUE_PCAP_STAT_ERROR,
+ QUEUE_PCAP_STAT_NUM
+};
+
struct pcap_rx_queue {
uint16_t port_id;
uint16_t queue_id;
struct rte_mempool *mb_pool;
struct queue_stat rx_stat;
- struct queue_missed_stat missed_stat;
+ struct queue_pcap_stat queue_pcap_stat[QUEUE_PCAP_STAT_NUM];
char name[PATH_MAX];
char type[ETH_PCAP_ARG_MAXLEN];
@@ -144,54 +151,62 @@ static struct rte_eth_link pmd_link = {
RTE_LOG_REGISTER_DEFAULT(eth_pcap_logtype, NOTICE);
-static struct queue_missed_stat*
-queue_missed_stat_update(struct rte_eth_dev *dev, unsigned int qid)
+static struct queue_pcap_stat*
+queue_pcap_stat_update(struct rte_eth_dev *dev, unsigned int qid, int type)
{
struct pmd_internals *internals = dev->data->dev_private;
- struct queue_missed_stat *missed_stat =
- &internals->rx_queue[qid].missed_stat;
+ struct queue_pcap_stat *queue_pcap_stat =
+ &internals->rx_queue[qid].queue_pcap_stat[type];
const struct pmd_process_private *pp = dev->process_private;
pcap_t *pcap = pp->rx_pcap[qid];
struct pcap_stat stat;
+ u_int value;
if (!pcap || (pcap_stats(pcap, &stat) != 0))
- return missed_stat;
+ return queue_pcap_stat;
+ value = (type == QUEUE_PCAP_STAT_ERROR) ? stat.ps_ifdrop : stat.ps_drop;
/* rollover check - best effort fixup assuming single rollover */
- if (stat.ps_drop < missed_stat->pcap)
- missed_stat->mnemonic += UINT_MAX;
- missed_stat->pcap = stat.ps_drop;
+ if (value < queue_pcap_stat->pcap)
+ queue_pcap_stat->mnemonic += UINT_MAX;
+ queue_pcap_stat->pcap = value;
- return missed_stat;
+ return queue_pcap_stat;
}
static void
-queue_missed_stat_on_stop_update(struct rte_eth_dev *dev, unsigned int qid)
+queue_pcap_stat_on_stop_update(struct rte_eth_dev *dev, unsigned int qid)
{
- struct queue_missed_stat *missed_stat =
- queue_missed_stat_update(dev, qid);
+ int type;
+ struct queue_pcap_stat *queue_pcap_stat;
- missed_stat->mnemonic += missed_stat->pcap;
- missed_stat->pcap = 0;
+ for (type = QUEUE_PCAP_STAT_FIRST; type < QUEUE_PCAP_STAT_NUM; type++) {
+ queue_pcap_stat = queue_pcap_stat_update(dev, qid, type);
+ queue_pcap_stat->mnemonic += queue_pcap_stat->pcap;
+ queue_pcap_stat->pcap = 0;
+ }
}
static void
-queue_missed_stat_reset(struct rte_eth_dev *dev, unsigned int qid)
+queue_pcap_stat_reset(struct rte_eth_dev *dev, unsigned int qid)
{
- struct queue_missed_stat *missed_stat =
- queue_missed_stat_update(dev, qid);
+ int type;
+ struct queue_pcap_stat *queue_pcap_stat;
- missed_stat->reset = missed_stat->pcap;
- missed_stat->mnemonic = 0;
+ for (type = QUEUE_PCAP_STAT_FIRST; type < QUEUE_PCAP_STAT_NUM; type++) {
+ queue_pcap_stat = queue_pcap_stat_update(dev, qid, type);
+ queue_pcap_stat->reset = queue_pcap_stat->pcap;
+ queue_pcap_stat->mnemonic = 0;
+ }
}
static unsigned long
-queue_missed_stat_get(struct rte_eth_dev *dev, unsigned int qid)
+queue_pcap_stat_get(struct rte_eth_dev *dev, unsigned int qid, int type)
{
- const struct queue_missed_stat *missed_stat =
- queue_missed_stat_update(dev, qid);
+ const struct queue_pcap_stat *queue_pcap_stat =
+ queue_pcap_stat_update(dev, qid, type);
- return missed_stat->pcap + missed_stat->mnemonic - missed_stat->reset;
+ return queue_pcap_stat->pcap + queue_pcap_stat->mnemonic - queue_pcap_stat->reset;
}
static int
@@ -684,7 +699,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
/* Special iface case. Single pcap is open and shared between tx/rx. */
if (internals->single_iface) {
- queue_missed_stat_on_stop_update(dev, 0);
+ queue_pcap_stat_on_stop_update(dev, 0);
if (pp->tx_pcap[0] != NULL) {
pcap_close(pp->tx_pcap[0]);
pp->tx_pcap[0] = NULL;
@@ -707,7 +722,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
for (i = 0; i < dev->data->nb_rx_queues; i++) {
if (pp->rx_pcap[i] != NULL) {
- queue_missed_stat_on_stop_update(dev, i);
+ queue_pcap_stat_on_stop_update(dev, i);
pcap_close(pp->rx_pcap[i]);
pp->rx_pcap[i] = NULL;
}
@@ -766,7 +781,8 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
rx_err_total += internal->rx_queue[i].rx_stat.err_pkts;
rx_packets_total += stats->q_ipackets[i];
rx_bytes_total += stats->q_ibytes[i];
- rx_missed_total += queue_missed_stat_get(dev, i);
+ rx_missed_total += queue_pcap_stat_get(dev, i, QUEUE_PCAP_STAT_MISSED);
+ rx_err_total += queue_pcap_stat_get(dev, i, QUEUE_PCAP_STAT_ERROR);
}
for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
@@ -801,7 +817,7 @@ eth_stats_reset(struct rte_eth_dev *dev)
internal->rx_queue[i].rx_stat.bytes = 0;
internal->rx_queue[i].rx_stat.err_pkts = 0;
internal->rx_queue[i].rx_stat.rx_nombuf = 0;
- queue_missed_stat_reset(dev, i);
+ queue_pcap_stat_reset(dev, i);
}
for (i = 0; i < dev->data->nb_tx_queues; i++) {
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v8 0/3] pcap: support MTU set for linux interfaces
2022-03-17 17:43 [PATCH] net/pcap: support MTU set ido g
` (4 preceding siblings ...)
2022-06-19 9:30 ` [PATCH v7 0/3] pcap: support MTU set for linux interfaces Ido Goshen
@ 2022-06-20 8:39 ` Ido Goshen
2022-06-20 8:39 ` [PATCH v8 1/3] " Ido Goshen
` (2 more replies)
2023-07-04 17:43 ` [PATCH] pcap: support MTU set Stephen Hemminger
` (2 subsequent siblings)
8 siblings, 3 replies; 36+ messages in thread
From: Ido Goshen @ 2022-06-20 8:39 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev
Support rte_eth_dev_set_mtu for pcap ifaces vdevs by
setting the underlying OS network interface's MTU.
Support is for pcap ifaces only and not for pcap files.
Support is for Linux only.
patch series:
[PATCH v8 1/3] pcap: support MTU set for linux interfaces
[PATCH v8 2/3] pcap: support MTU set for linux interfaces TX
[PATCH v8 3/3] pcap: support MTU set for linux interfaces count
doc/guides/rel_notes/release_22_07.rst | 3 +
drivers/net/pcap/pcap_ethdev.c | 126 +++++++++++++++++++++++++--------
drivers/net/pcap/pcap_osdep.h | 1 +
drivers/net/pcap/pcap_osdep_freebsd.c | 7 ++
drivers/net/pcap/pcap_osdep_linux.c | 21 ++++++
drivers/net/pcap/pcap_osdep_windows.c | 7 ++
6 files changed, 134 insertions(+), 31 deletions(-)
---
v8:
cosmetics only
1. checkpatch typo fix
2. rel_notes apply patch fix by rebasing on latest main
v7:
1. TX drop only individual oversized packets and not the entire burst
2. Count oversize drops by i/oerrors
v6:
Fixes for v5 approach
1. freebsd compilation fix
2. checkpatch warning fix
v5:
Alternative approach
Instead of checking MTU in the pmd set it on the OS interface and
let it do the enforcment.
v4:
1. Add release notes comment
2. Access pmd internals via queue struct
3. eth_mtu_set code convention fixes
v3:
Preserve pcap behavior to support max size packets by default
alternative to v2 in order to limit the code change to pcap only and
avoid abi change.
Enforce mtu only in case rte_eth_dev_set_mtu was explicitly called.
v2:
Preserve pcap behavior to support max size packets by default.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v8 1/3] pcap: support MTU set for linux interfaces
2022-06-20 8:39 ` [PATCH v8 0/3] pcap: support MTU set for linux interfaces Ido Goshen
@ 2022-06-20 8:39 ` Ido Goshen
2022-06-20 8:39 ` [PATCH v8 2/3] pcap: support MTU set for linux interfaces TX enhancment Ido Goshen
2022-06-20 8:39 ` [PATCH v8 3/3] pcap: support MTU set for linux interfaces count ierrors Ido Goshen
2 siblings, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2022-06-20 8:39 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev, Ido Goshen
Support rte_eth_dev_set_mtu for pcap ifaces vdevs by
setting the underlying OS network interface's MTU.
Support is for pcap ifaces only and not for pcap files.
Support is for Linux only.
Bugzilla ID: 961
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
---
doc/guides/rel_notes/release_22_07.rst | 3 ++
drivers/net/pcap/pcap_ethdev.c | 42 ++++++++++++++++++++++++++
drivers/net/pcap/pcap_osdep.h | 1 +
drivers/net/pcap/pcap_osdep_freebsd.c | 7 +++++
drivers/net/pcap/pcap_osdep_linux.c | 21 +++++++++++++
drivers/net/pcap/pcap_osdep_windows.c | 7 +++++
6 files changed, 81 insertions(+)
diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index dd371952c3..a966973302 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -220,6 +220,9 @@ New Features
Merged l3fwd-acl code into l3fwd as l3fwd-acl contains duplicate
and common functions to l3fwd.
+* **Updated pcap driver.**
+
+ * Added support for MTU on Linux network interfaces.
Removed Items
-------------
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index ec29fd6bc5..2221c53051 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -807,6 +807,47 @@ eth_stats_reset(struct rte_eth_dev *dev)
return 0;
}
+static int
+eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ unsigned int i;
+ struct pmd_internals *internals = dev->data->dev_private;
+ int is_supported = 0;
+ int is_err = 0;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct pcap_rx_queue *queue = &internals->rx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_IN_ARG) == 0)) {
+ is_supported = 1;
+ if (osdep_iface_mtu_set(queue->name, mtu) < 0)
+ is_err = 1;
+ }
+ }
+
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ struct pcap_tx_queue *queue = &internals->tx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_TX_IFACE_ARG) == 0)) {
+ is_supported = 1;
+ if (osdep_iface_mtu_set(queue->name, mtu) < 0)
+ is_err = 1;
+ }
+ }
+
+ if (!is_supported)
+ return -ENOTSUP;
+
+ if (is_err)
+ return -1;
+
+ 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 +1045,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
diff --git a/drivers/net/pcap/pcap_osdep.h b/drivers/net/pcap/pcap_osdep.h
index bf41cba982..043677ec77 100644
--- a/drivers/net/pcap/pcap_osdep.h
+++ b/drivers/net/pcap/pcap_osdep.h
@@ -14,5 +14,6 @@ extern int eth_pcap_logtype;
int osdep_iface_index_get(const char *name);
int osdep_iface_mac_get(const char *name, struct rte_ether_addr *mac);
+int osdep_iface_mtu_set(const char *name, uint16_t mtu);
#endif
diff --git a/drivers/net/pcap/pcap_osdep_freebsd.c b/drivers/net/pcap/pcap_osdep_freebsd.c
index 20556b3e92..a9961ba3e3 100644
--- a/drivers/net/pcap/pcap_osdep_freebsd.c
+++ b/drivers/net/pcap/pcap_osdep_freebsd.c
@@ -57,3 +57,10 @@ osdep_iface_mac_get(const char *if_name, struct rte_ether_addr *mac)
rte_free(buf);
return 0;
}
+
+int
+osdep_iface_mtu_set(__rte_unused const char *if_name, __rte_unused uint16_t mtu)
+{
+ PMD_LOG(ERR, "mtu set not supported on freebsd\n");
+ return -ENOTSUP;
+}
diff --git a/drivers/net/pcap/pcap_osdep_linux.c b/drivers/net/pcap/pcap_osdep_linux.c
index 97033f57c5..b0b4a716fe 100644
--- a/drivers/net/pcap/pcap_osdep_linux.c
+++ b/drivers/net/pcap/pcap_osdep_linux.c
@@ -40,3 +40,24 @@ osdep_iface_mac_get(const char *if_name, struct rte_ether_addr *mac)
close(if_fd);
return 0;
}
+
+int
+osdep_iface_mtu_set(const char *if_name, uint16_t mtu)
+{
+ struct ifreq ifr;
+ int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+ if (if_fd == -1)
+ return -1;
+
+ rte_strscpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+ ifr.ifr_mtu = mtu;
+ if (ioctl(if_fd, SIOCSIFMTU, &ifr)) {
+ PMD_LOG(ERR, "%s mtu set to %d failed\n", if_name, mtu);
+ close(if_fd);
+ return -1;
+ }
+
+ close(if_fd);
+ return 0;
+}
diff --git a/drivers/net/pcap/pcap_osdep_windows.c b/drivers/net/pcap/pcap_osdep_windows.c
index 1d398dc7ed..db34fc49e2 100644
--- a/drivers/net/pcap/pcap_osdep_windows.c
+++ b/drivers/net/pcap/pcap_osdep_windows.c
@@ -116,3 +116,10 @@ osdep_iface_mac_get(const char *device_name, struct rte_ether_addr *mac)
free(info);
return ret;
}
+
+int
+osdep_iface_mtu_set(__rte_unused const char *device_name, __rte_unused uint16_t mtu)
+{
+ PMD_LOG(ERR, "mtu set not supported on Windows\n");
+ return -ENOTSUP;
+}
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v8 2/3] pcap: support MTU set for linux interfaces TX enhancment
2022-06-20 8:39 ` [PATCH v8 0/3] pcap: support MTU set for linux interfaces Ido Goshen
2022-06-20 8:39 ` [PATCH v8 1/3] " Ido Goshen
@ 2022-06-20 8:39 ` Ido Goshen
2022-06-20 8:39 ` [PATCH v8 3/3] pcap: support MTU set for linux interfaces count ierrors Ido Goshen
2 siblings, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2022-06-20 8:39 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev, Ido Goshen
Drop only the oversized packets and not its entire burst
mbuf will be freed and will be counted as oerror
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
---
drivers/net/pcap/pcap_ethdev.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index 2221c53051..ff98762058 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -494,8 +494,14 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
*/
ret = pcap_sendpacket(pcap,
rte_pktmbuf_read(mbuf, 0, len, temp_data), len);
- if (unlikely(ret != 0))
- break;
+ if (unlikely(ret != 0)) {
+ if (errno == EMSGSIZE) {
+ rte_pktmbuf_free(mbuf);
+ continue;
+ } else {
+ break;
+ }
+ }
num_tx++;
tx_bytes += len;
rte_pktmbuf_free(mbuf);
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v8 3/3] pcap: support MTU set for linux interfaces count ierrors
2022-06-20 8:39 ` [PATCH v8 0/3] pcap: support MTU set for linux interfaces Ido Goshen
2022-06-20 8:39 ` [PATCH v8 1/3] " Ido Goshen
2022-06-20 8:39 ` [PATCH v8 2/3] pcap: support MTU set for linux interfaces TX enhancment Ido Goshen
@ 2022-06-20 8:39 ` Ido Goshen
2 siblings, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2022-06-20 8:39 UTC (permalink / raw)
To: ferruh.yigit, stephen; +Cc: dev, Ido Goshen
Count oversized packets that are dropped by the interface
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
---
drivers/net/pcap/pcap_ethdev.c | 74 +++++++++++++++++++++-------------
1 file changed, 45 insertions(+), 29 deletions(-)
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index ff98762058..46f18e9b10 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -54,7 +54,7 @@ struct queue_stat {
volatile unsigned long rx_nombuf;
};
-struct queue_missed_stat {
+struct queue_pcap_stat {
/* last value retrieved from pcap */
unsigned int pcap;
/* stores values lost by pcap stop or rollover */
@@ -63,12 +63,19 @@ struct queue_missed_stat {
unsigned long reset;
};
+enum {
+ QUEUE_PCAP_STAT_FIRST = 0,
+ QUEUE_PCAP_STAT_MISSED = QUEUE_PCAP_STAT_FIRST,
+ QUEUE_PCAP_STAT_ERROR,
+ QUEUE_PCAP_STAT_NUM
+};
+
struct pcap_rx_queue {
uint16_t port_id;
uint16_t queue_id;
struct rte_mempool *mb_pool;
struct queue_stat rx_stat;
- struct queue_missed_stat missed_stat;
+ struct queue_pcap_stat queue_pcap_stat[QUEUE_PCAP_STAT_NUM];
char name[PATH_MAX];
char type[ETH_PCAP_ARG_MAXLEN];
@@ -144,54 +151,62 @@ static struct rte_eth_link pmd_link = {
RTE_LOG_REGISTER_DEFAULT(eth_pcap_logtype, NOTICE);
-static struct queue_missed_stat*
-queue_missed_stat_update(struct rte_eth_dev *dev, unsigned int qid)
+static struct queue_pcap_stat*
+queue_pcap_stat_update(struct rte_eth_dev *dev, unsigned int qid, int type)
{
struct pmd_internals *internals = dev->data->dev_private;
- struct queue_missed_stat *missed_stat =
- &internals->rx_queue[qid].missed_stat;
+ struct queue_pcap_stat *queue_pcap_stat =
+ &internals->rx_queue[qid].queue_pcap_stat[type];
const struct pmd_process_private *pp = dev->process_private;
pcap_t *pcap = pp->rx_pcap[qid];
struct pcap_stat stat;
+ u_int value;
if (!pcap || (pcap_stats(pcap, &stat) != 0))
- return missed_stat;
+ return queue_pcap_stat;
+ value = (type == QUEUE_PCAP_STAT_ERROR) ? stat.ps_ifdrop : stat.ps_drop;
/* rollover check - best effort fixup assuming single rollover */
- if (stat.ps_drop < missed_stat->pcap)
- missed_stat->mnemonic += UINT_MAX;
- missed_stat->pcap = stat.ps_drop;
+ if (value < queue_pcap_stat->pcap)
+ queue_pcap_stat->mnemonic += UINT_MAX;
+ queue_pcap_stat->pcap = value;
- return missed_stat;
+ return queue_pcap_stat;
}
static void
-queue_missed_stat_on_stop_update(struct rte_eth_dev *dev, unsigned int qid)
+queue_pcap_stat_on_stop_update(struct rte_eth_dev *dev, unsigned int qid)
{
- struct queue_missed_stat *missed_stat =
- queue_missed_stat_update(dev, qid);
+ int type;
+ struct queue_pcap_stat *queue_pcap_stat;
- missed_stat->mnemonic += missed_stat->pcap;
- missed_stat->pcap = 0;
+ for (type = QUEUE_PCAP_STAT_FIRST; type < QUEUE_PCAP_STAT_NUM; type++) {
+ queue_pcap_stat = queue_pcap_stat_update(dev, qid, type);
+ queue_pcap_stat->mnemonic += queue_pcap_stat->pcap;
+ queue_pcap_stat->pcap = 0;
+ }
}
static void
-queue_missed_stat_reset(struct rte_eth_dev *dev, unsigned int qid)
+queue_pcap_stat_reset(struct rte_eth_dev *dev, unsigned int qid)
{
- struct queue_missed_stat *missed_stat =
- queue_missed_stat_update(dev, qid);
+ int type;
+ struct queue_pcap_stat *queue_pcap_stat;
- missed_stat->reset = missed_stat->pcap;
- missed_stat->mnemonic = 0;
+ for (type = QUEUE_PCAP_STAT_FIRST; type < QUEUE_PCAP_STAT_NUM; type++) {
+ queue_pcap_stat = queue_pcap_stat_update(dev, qid, type);
+ queue_pcap_stat->reset = queue_pcap_stat->pcap;
+ queue_pcap_stat->mnemonic = 0;
+ }
}
static unsigned long
-queue_missed_stat_get(struct rte_eth_dev *dev, unsigned int qid)
+queue_pcap_stat_get(struct rte_eth_dev *dev, unsigned int qid, int type)
{
- const struct queue_missed_stat *missed_stat =
- queue_missed_stat_update(dev, qid);
+ const struct queue_pcap_stat *queue_pcap_stat =
+ queue_pcap_stat_update(dev, qid, type);
- return missed_stat->pcap + missed_stat->mnemonic - missed_stat->reset;
+ return queue_pcap_stat->pcap + queue_pcap_stat->mnemonic - queue_pcap_stat->reset;
}
static int
@@ -684,7 +699,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
/* Special iface case. Single pcap is open and shared between tx/rx. */
if (internals->single_iface) {
- queue_missed_stat_on_stop_update(dev, 0);
+ queue_pcap_stat_on_stop_update(dev, 0);
if (pp->tx_pcap[0] != NULL) {
pcap_close(pp->tx_pcap[0]);
pp->tx_pcap[0] = NULL;
@@ -707,7 +722,7 @@ eth_dev_stop(struct rte_eth_dev *dev)
for (i = 0; i < dev->data->nb_rx_queues; i++) {
if (pp->rx_pcap[i] != NULL) {
- queue_missed_stat_on_stop_update(dev, i);
+ queue_pcap_stat_on_stop_update(dev, i);
pcap_close(pp->rx_pcap[i]);
pp->rx_pcap[i] = NULL;
}
@@ -766,7 +781,8 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
rx_err_total += internal->rx_queue[i].rx_stat.err_pkts;
rx_packets_total += stats->q_ipackets[i];
rx_bytes_total += stats->q_ibytes[i];
- rx_missed_total += queue_missed_stat_get(dev, i);
+ rx_missed_total += queue_pcap_stat_get(dev, i, QUEUE_PCAP_STAT_MISSED);
+ rx_err_total += queue_pcap_stat_get(dev, i, QUEUE_PCAP_STAT_ERROR);
}
for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
@@ -801,7 +817,7 @@ eth_stats_reset(struct rte_eth_dev *dev)
internal->rx_queue[i].rx_stat.bytes = 0;
internal->rx_queue[i].rx_stat.err_pkts = 0;
internal->rx_queue[i].rx_stat.rx_nombuf = 0;
- queue_missed_stat_reset(dev, i);
+ queue_pcap_stat_reset(dev, i);
}
for (i = 0; i < dev->data->nb_tx_queues; i++) {
--
2.17.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] pcap: support MTU set
2022-03-17 17:43 [PATCH] net/pcap: support MTU set ido g
` (5 preceding siblings ...)
2022-06-20 8:39 ` [PATCH v8 0/3] pcap: support MTU set for linux interfaces Ido Goshen
@ 2023-07-04 17:43 ` Stephen Hemminger
2023-07-04 21:02 ` [PATCH v2] " Stephen Hemminger
2023-07-10 16:45 ` [PATCH] net/pcap: " Stephen Hemminger
8 siblings, 0 replies; 36+ messages in thread
From: Stephen Hemminger @ 2023-07-04 17:43 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Ido Goshen
Support rte_eth_dev_set_mtu for pcap driver when the
pcap device is convigured to point to a network interface.
This is rebased an consolidated from earlier version.
Added support for FreeBSD.
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/pcap/pcap_ethdev.c | 49 ++++++++++++++++++++++++++-
drivers/net/pcap/pcap_osdep.h | 1 +
drivers/net/pcap/pcap_osdep_linux.c | 21 ++++++++++++
drivers/net/pcap/pcap_osdep_windows.c | 7 ++++
4 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index bfec0850456f..672f42d30d8e 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -5,6 +5,7 @@
*/
#include <stdlib.h>
+#include <stdbool.h>
#include <time.h>
#include <pcap.h>
@@ -495,8 +496,13 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
*/
ret = pcap_sendpacket(pcap,
rte_pktmbuf_read(mbuf, 0, len, temp_data), len);
- if (unlikely(ret != 0))
+ if (unlikely(ret != 0)) {
+ /* Oversize packet dropped due to MTU */
+ if (errno == EMSGSIZE)
+ continue;
break;
+ }
+
num_tx++;
tx_bytes += len;
rte_pktmbuf_free(mbuf);
@@ -808,6 +814,46 @@ eth_stats_reset(struct rte_eth_dev *dev)
return 0;
}
+static int
+eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ struct pmd_internals *internals = dev->data->dev_private;
+ unsigned int i;
+ bool is_supported = false;
+ int ret;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct pcap_rx_queue *queue = &internals->rx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_IN_ARG) == 0)) {
+ ret = osdep_iface_mtu_set(queue->name, mtu);
+ if (ret < 0)
+ return ret;
+ is_supported = true;
+ }
+ }
+
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ struct pcap_tx_queue *queue = &internals->tx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_TX_IFACE_ARG) == 0)) {
+ ret = osdep_iface_mtu_set(queue->name, mtu);
+ if (ret < 0)
+ return ret;
+ is_supported = true;
+ }
+ }
+
+ if (!is_supported)
+ return -ENOTSUP;
+
+ 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)
{
@@ -1005,6 +1051,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
diff --git a/drivers/net/pcap/pcap_osdep.h b/drivers/net/pcap/pcap_osdep.h
index bf41cba982ec..043677ec777d 100644
--- a/drivers/net/pcap/pcap_osdep.h
+++ b/drivers/net/pcap/pcap_osdep.h
@@ -14,5 +14,6 @@ extern int eth_pcap_logtype;
int osdep_iface_index_get(const char *name);
int osdep_iface_mac_get(const char *name, struct rte_ether_addr *mac);
+int osdep_iface_mtu_set(const char *name, uint16_t mtu);
#endif
diff --git a/drivers/net/pcap/pcap_osdep_linux.c b/drivers/net/pcap/pcap_osdep_linux.c
index 97033f57c5d9..845296595f86 100644
--- a/drivers/net/pcap/pcap_osdep_linux.c
+++ b/drivers/net/pcap/pcap_osdep_linux.c
@@ -40,3 +40,24 @@ osdep_iface_mac_get(const char *if_name, struct rte_ether_addr *mac)
close(if_fd);
return 0;
}
+
+int
+osdep_iface_mtu_set(const char *if_name, uint16_t mtu)
+{
+ struct ifreq ifr;
+ int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+ if (if_fd == -1)
+ return -errno;
+
+ rte_strscpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+ ifr.ifr_mtu = mtu;
+ if (ioctl(if_fd, SIOCSIFMTU, &ifr)) {
+ PMD_LOG(ERR, "%s mtu set to %d failed\n", if_name, mtu);
+ close(if_fd);
+ return -errno;
+ }
+
+ close(if_fd);
+ return 0;
+}
diff --git a/drivers/net/pcap/pcap_osdep_windows.c b/drivers/net/pcap/pcap_osdep_windows.c
index 1d398dc7ed9f..db34fc49e22e 100644
--- a/drivers/net/pcap/pcap_osdep_windows.c
+++ b/drivers/net/pcap/pcap_osdep_windows.c
@@ -116,3 +116,10 @@ osdep_iface_mac_get(const char *device_name, struct rte_ether_addr *mac)
free(info);
return ret;
}
+
+int
+osdep_iface_mtu_set(__rte_unused const char *device_name, __rte_unused uint16_t mtu)
+{
+ PMD_LOG(ERR, "mtu set not supported on Windows\n");
+ return -ENOTSUP;
+}
--
2.39.2
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] pcap: support MTU set
2022-03-17 17:43 [PATCH] net/pcap: support MTU set ido g
` (6 preceding siblings ...)
2023-07-04 17:43 ` [PATCH] pcap: support MTU set Stephen Hemminger
@ 2023-07-04 21:02 ` Stephen Hemminger
2023-07-05 11:37 ` Ferruh Yigit
2023-07-10 16:45 ` [PATCH] net/pcap: " Stephen Hemminger
8 siblings, 1 reply; 36+ messages in thread
From: Stephen Hemminger @ 2023-07-04 21:02 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Ido Goshen
Support rte_eth_dev_set_mtu for pcap driver when the
pcap device is convigured to point to a network interface.
This is rebased an consolidated from earlier version.
Added support for FreeBSD.
Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v2 - fix FreeBSD part
drivers/net/pcap/pcap_ethdev.c | 49 ++++++++++++++++++++++++++-
drivers/net/pcap/pcap_osdep.h | 1 +
drivers/net/pcap/pcap_osdep_freebsd.c | 23 +++++++++++++
drivers/net/pcap/pcap_osdep_linux.c | 21 ++++++++++++
drivers/net/pcap/pcap_osdep_windows.c | 7 ++++
5 files changed, 100 insertions(+), 1 deletion(-)
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index bfec0850456f..672f42d30d8e 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -5,6 +5,7 @@
*/
#include <stdlib.h>
+#include <stdbool.h>
#include <time.h>
#include <pcap.h>
@@ -495,8 +496,13 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
*/
ret = pcap_sendpacket(pcap,
rte_pktmbuf_read(mbuf, 0, len, temp_data), len);
- if (unlikely(ret != 0))
+ if (unlikely(ret != 0)) {
+ /* Oversize packet dropped due to MTU */
+ if (errno == EMSGSIZE)
+ continue;
break;
+ }
+
num_tx++;
tx_bytes += len;
rte_pktmbuf_free(mbuf);
@@ -808,6 +814,46 @@ eth_stats_reset(struct rte_eth_dev *dev)
return 0;
}
+static int
+eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ struct pmd_internals *internals = dev->data->dev_private;
+ unsigned int i;
+ bool is_supported = false;
+ int ret;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct pcap_rx_queue *queue = &internals->rx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_RX_IFACE_IN_ARG) == 0)) {
+ ret = osdep_iface_mtu_set(queue->name, mtu);
+ if (ret < 0)
+ return ret;
+ is_supported = true;
+ }
+ }
+
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ struct pcap_tx_queue *queue = &internals->tx_queue[i];
+
+ if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
+ (strcmp(queue->type, ETH_PCAP_TX_IFACE_ARG) == 0)) {
+ ret = osdep_iface_mtu_set(queue->name, mtu);
+ if (ret < 0)
+ return ret;
+ is_supported = true;
+ }
+ }
+
+ if (!is_supported)
+ return -ENOTSUP;
+
+ 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)
{
@@ -1005,6 +1051,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
diff --git a/drivers/net/pcap/pcap_osdep.h b/drivers/net/pcap/pcap_osdep.h
index bf41cba982ec..043677ec777d 100644
--- a/drivers/net/pcap/pcap_osdep.h
+++ b/drivers/net/pcap/pcap_osdep.h
@@ -14,5 +14,6 @@ extern int eth_pcap_logtype;
int osdep_iface_index_get(const char *name);
int osdep_iface_mac_get(const char *name, struct rte_ether_addr *mac);
+int osdep_iface_mtu_set(const char *name, uint16_t mtu);
#endif
diff --git a/drivers/net/pcap/pcap_osdep_freebsd.c b/drivers/net/pcap/pcap_osdep_freebsd.c
index 20556b3e9215..de6358197391 100644
--- a/drivers/net/pcap/pcap_osdep_freebsd.c
+++ b/drivers/net/pcap/pcap_osdep_freebsd.c
@@ -6,7 +6,9 @@
#include <net/if.h>
#include <net/if_dl.h>
+#include <sys/ioctl.h>
#include <sys/sysctl.h>
+#include <unistd.h>
#include <rte_malloc.h>
#include <rte_memcpy.h>
@@ -57,3 +59,24 @@ osdep_iface_mac_get(const char *if_name, struct rte_ether_addr *mac)
rte_free(buf);
return 0;
}
+
+int
+osdep_iface_mtu_set(const char *if_name, uint16_t mtu)
+{
+ struct ifreq ifr = { };
+ int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+ if (if_fd == -1)
+ return -errno;
+
+ strlcpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+ ifr.ifr_mtu = mtu;
+ if (ioctl(if_fd, SIOCSIFMTU, &ifr)) {
+ PMD_LOG(ERR, "%s mtu set to %d failed\n", if_name, mtu);
+ close(if_fd);
+ return -errno;
+ }
+
+ close(if_fd);
+ return 0;
+}
diff --git a/drivers/net/pcap/pcap_osdep_linux.c b/drivers/net/pcap/pcap_osdep_linux.c
index 97033f57c5d9..845296595f86 100644
--- a/drivers/net/pcap/pcap_osdep_linux.c
+++ b/drivers/net/pcap/pcap_osdep_linux.c
@@ -40,3 +40,24 @@ osdep_iface_mac_get(const char *if_name, struct rte_ether_addr *mac)
close(if_fd);
return 0;
}
+
+int
+osdep_iface_mtu_set(const char *if_name, uint16_t mtu)
+{
+ struct ifreq ifr;
+ int if_fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+ if (if_fd == -1)
+ return -errno;
+
+ rte_strscpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+ ifr.ifr_mtu = mtu;
+ if (ioctl(if_fd, SIOCSIFMTU, &ifr)) {
+ PMD_LOG(ERR, "%s mtu set to %d failed\n", if_name, mtu);
+ close(if_fd);
+ return -errno;
+ }
+
+ close(if_fd);
+ return 0;
+}
diff --git a/drivers/net/pcap/pcap_osdep_windows.c b/drivers/net/pcap/pcap_osdep_windows.c
index 1d398dc7ed9f..db34fc49e22e 100644
--- a/drivers/net/pcap/pcap_osdep_windows.c
+++ b/drivers/net/pcap/pcap_osdep_windows.c
@@ -116,3 +116,10 @@ osdep_iface_mac_get(const char *device_name, struct rte_ether_addr *mac)
free(info);
return ret;
}
+
+int
+osdep_iface_mtu_set(__rte_unused const char *device_name, __rte_unused uint16_t mtu)
+{
+ PMD_LOG(ERR, "mtu set not supported on Windows\n");
+ return -ENOTSUP;
+}
--
2.39.2
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] pcap: support MTU set
2023-07-04 21:02 ` [PATCH v2] " Stephen Hemminger
@ 2023-07-05 11:37 ` Ferruh Yigit
2023-07-05 15:18 ` Stephen Hemminger
0 siblings, 1 reply; 36+ messages in thread
From: Ferruh Yigit @ 2023-07-05 11:37 UTC (permalink / raw)
To: Stephen Hemminger, dev; +Cc: Ido Goshen
On 7/4/2023 10:02 PM, Stephen Hemminger wrote:
> Support rte_eth_dev_set_mtu for pcap driver when the
> pcap device is convigured to point to a network interface.
>
> This is rebased an consolidated from earlier version.
> Added support for FreeBSD.
>
As far as I understand motivation is to make pcap PMD behave close the
physical NIC and able to test the application MTU feature.
If so, Ido's v4 was simpler, which doesn't distinguish if pcap backed by
physical interface or .pcap file.
What was wrong with that approach?
> Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v2 - fix FreeBSD part
>
> drivers/net/pcap/pcap_ethdev.c | 49 ++++++++++++++++++++++++++-
> drivers/net/pcap/pcap_osdep.h | 1 +
> drivers/net/pcap/pcap_osdep_freebsd.c | 23 +++++++++++++
> drivers/net/pcap/pcap_osdep_linux.c | 21 ++++++++++++
> drivers/net/pcap/pcap_osdep_windows.c | 7 ++++
> 5 files changed, 100 insertions(+), 1 deletion(-)
>
Better to update driver documentation and release notes too.
> diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
> index bfec0850456f..672f42d30d8e 100644
> --- a/drivers/net/pcap/pcap_ethdev.c
> +++ b/drivers/net/pcap/pcap_ethdev.c
> @@ -5,6 +5,7 @@
> */
>
> #include <stdlib.h>
> +#include <stdbool.h>
> #include <time.h>
>
> #include <pcap.h>
> @@ -495,8 +496,13 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> */
> ret = pcap_sendpacket(pcap,
> rte_pktmbuf_read(mbuf, 0, len, temp_data), len);
> - if (unlikely(ret != 0))
> + if (unlikely(ret != 0)) {
> + /* Oversize packet dropped due to MTU */
> + if (errno == EMSGSIZE)
> + continue;
This will leak mbuf, since loop continues application will assume driver
sent the packet and freed mbuf. Ido's version does it right.
> break;
> + }
> +
> num_tx++;
> tx_bytes += len;
> rte_pktmbuf_free(mbuf);
> @@ -808,6 +814,46 @@ eth_stats_reset(struct rte_eth_dev *dev)
> return 0;
> }
>
> +static int
> +eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
> + struct pmd_internals *internals = dev->data->dev_private;
> + unsigned int i;
> + bool is_supported = false;
> + int ret;
> +
> + for (i = 0; i < dev->data->nb_rx_queues; i++) {
> + struct pcap_rx_queue *queue = &internals->rx_queue[i];
> +
> + if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
> + (strcmp(queue->type, ETH_PCAP_RX_IFACE_ARG) == 0) ||
> + (strcmp(queue->type, ETH_PCAP_RX_IFACE_IN_ARG) == 0)) {
> + ret = osdep_iface_mtu_set(queue->name, mtu);
> + if (ret < 0)
> + return ret;
> + is_supported = true;
> + }
If there are multiple devices (each pcap queue represents a device), and
one in the middle one fails to set MTU, there will be mixed MTU values
in devices.
> + }
> +
> + for (i = 0; i < dev->data->nb_tx_queues; i++) {
> + struct pcap_tx_queue *queue = &internals->tx_queue[i];
> +
> + if ((strcmp(queue->type, ETH_PCAP_IFACE_ARG) == 0) ||
Rx devices and Tx devices can be same device, causing duplicated MTU set
for same device, but it is hard to detect if they are same devices.
At least it is easier to detect when queue type is 'ETH_PCAP_IFACE_ARG',
what about set MTU once for this queue type?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2] pcap: support MTU set
2023-07-05 11:37 ` Ferruh Yigit
@ 2023-07-05 15:18 ` Stephen Hemminger
2023-07-06 10:45 ` Ido Goshen
0 siblings, 1 reply; 36+ messages in thread
From: Stephen Hemminger @ 2023-07-05 15:18 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, Ido Goshen
On Wed, 5 Jul 2023 12:37:41 +0100
Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> On 7/4/2023 10:02 PM, Stephen Hemminger wrote:
> > Support rte_eth_dev_set_mtu for pcap driver when the
> > pcap device is convigured to point to a network interface.
> >
> > This is rebased an consolidated from earlier version.
> > Added support for FreeBSD.
> >
>
> As far as I understand motivation is to make pcap PMD behave close the
> physical NIC and able to test the application MTU feature.
> If so, Ido's v4 was simpler, which doesn't distinguish if pcap backed by
> physical interface or .pcap file.
> What was wrong with that approach?
I started with Ido's patch, then:
- combined the two patches into one.
- fixed the error handling (propogate errno correctly)
- add missing freebsd support
Normally would just give feedback but patch was so old not sure if it was
stuck and unlikely to get merged.
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v2] pcap: support MTU set
2023-07-05 15:18 ` Stephen Hemminger
@ 2023-07-06 10:45 ` Ido Goshen
0 siblings, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2023-07-06 10:45 UTC (permalink / raw)
To: Stephen Hemminger, Ferruh Yigit; +Cc: dev
I've suggested 2 ways to do it
1. Data path enforcement by pcap pmd [PATCH v4]
http://patches.dpdk.org/project/dpdk/patch/20220606162147.57218-1-ido@cgstowernetworks.com/
2. Control path only sets the underlying OS network interface MTU [PATCH v8]
http://patches.dpdk.org/project/dpdk/cover/20220620083944.51517-1-ido@cgstowernetworks.com/
It was pretty long ago - not sure which is in favor
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, 5 July 2023 18:19
> To: Ferruh Yigit <ferruh.yigit@amd.com>
> Cc: dev@dpdk.org; Ido Goshen <Ido@cgstowernetworks.com>
> Subject: Re: [PATCH v2] pcap: support MTU set
>
> On Wed, 5 Jul 2023 12:37:41 +0100
> Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> > On 7/4/2023 10:02 PM, Stephen Hemminger wrote:
> > > Support rte_eth_dev_set_mtu for pcap driver when the pcap device is
> > > convigured to point to a network interface.
> > >
> > > This is rebased an consolidated from earlier version.
> > > Added support for FreeBSD.
> > >
> >
> > As far as I understand motivation is to make pcap PMD behave close the
> > physical NIC and able to test the application MTU feature.
> > If so, Ido's v4 was simpler, which doesn't distinguish if pcap backed
> > by physical interface or .pcap file.
> > What was wrong with that approach?
>
> I started with Ido's patch, then:
> - combined the two patches into one.
> - fixed the error handling (propogate errno correctly)
> - add missing freebsd support
>
> Normally would just give feedback but patch was so old not sure if it was
> stuck and unlikely to get merged.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] net/pcap: support MTU set
2022-03-17 17:43 [PATCH] net/pcap: support MTU set ido g
` (7 preceding siblings ...)
2023-07-04 21:02 ` [PATCH v2] " Stephen Hemminger
@ 2023-07-10 16:45 ` Stephen Hemminger
2023-07-10 17:46 ` Ferruh Yigit
8 siblings, 1 reply; 36+ messages in thread
From: Stephen Hemminger @ 2023-07-10 16:45 UTC (permalink / raw)
To: ido g; +Cc: Ferruh Yigit, dev
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?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] net/pcap: support MTU set
2023-07-10 16:45 ` [PATCH] net/pcap: " Stephen Hemminger
@ 2023-07-10 17:46 ` Ferruh Yigit
2023-07-11 9:41 ` Ido Goshen
0 siblings, 1 reply; 36+ messages in thread
From: Ferruh Yigit @ 2023-07-10 17:46 UTC (permalink / raw)
To: Stephen Hemminger, ido g; +Cc: Ferruh Yigit, dev
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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH] net/pcap: support MTU set
2023-07-10 17:46 ` Ferruh Yigit
@ 2023-07-11 9:41 ` Ido Goshen
0 siblings, 0 replies; 36+ messages in thread
From: Ido Goshen @ 2023-07-11 9:41 UTC (permalink / raw)
To: Ferruh Yigit, Stephen Hemminger; +Cc: Ferruh Yigit, dev
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.
^ permalink raw reply [flat|nested] 36+ messages in thread