* [PATCH v3 net-next] pktgen: introduce 'rx' mode @ 2015-05-01 5:12 Alexei Starovoitov 2015-05-01 16:54 ` Eric Dumazet ` (2 more replies) 0 siblings, 3 replies; 38+ messages in thread From: Alexei Starovoitov @ 2015-05-01 5:12 UTC (permalink / raw) To: David S. Miller; +Cc: Eric Dumazet, netdev Introduce 'RX' mode for pktgen which generates the packets using familiar pktgen commands, but feeds them into netif_receive_skb() instead of ndo_start_xmit(). It is designed to test netif_receive_skb and ingress qdisc performace only. Make sure to understand how it works before using it for other rx benchmarking. Sample script 'pktgen.sh': \#!/bin/bash function pgset() { local result echo $1 > $PGDEV result=`cat $PGDEV | fgrep "Result: OK:"` if [ "$result" = "" ]; then cat $PGDEV | fgrep Result: fi } ETH=$1 PGDEV=/proc/net/pktgen/kpktgend_0 pgset "rem_device_all" pgset "add_device $ETH" PGDEV=/proc/net/pktgen/$ETH pgset "rx" pgset "pkt_size 60" pgset "dst 99.1.1.2" pgset "dst_mac 90:e2:ba:6e:a8:e5" pgset "count 10000000" pgset "burst 32" PGDEV=/proc/net/pktgen/pgctrl echo "Running... ctrl^C to stop" pgset "start" echo "Done" cat /proc/net/pktgen/$ETH Usage: $ sudo ./pktgen.sh eth2 ... Result: OK: 232376(c232372+d3) usec, 10000000 (60byte,0frags) 43033682pps 20656Mb/sec (20656167360bps) errors: 10000000 Raw netif_receive_skb speed should be ~43 million packet per second on 3.7Ghz x86 and 'perf report' should look like: 37.69% kpktgend_0 [kernel.vmlinux] [k] __netif_receive_skb_core 25.81% kpktgend_0 [kernel.vmlinux] [k] kfree_skb 7.22% kpktgend_0 [kernel.vmlinux] [k] ip_rcv 5.68% kpktgend_0 [pktgen] [k] pktgen_thread_worker if fib_table_lookup is seen on top, it means skb was processed by the stack. To benchmark netif_receive_skb only make sure that 'dst_mac' of your pktgen script is different from receiving device mac and it will be dropped by ip_rcv Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> --- v2->v3: addressed more Eric comments. Thanks! v1->v2: as suggested by Eric: - dropped 'clone_skb' flag, now it will return enotsupp - fix rps/rfs bug by checking skb->users after every netif_receive_skb - tested with RPS/RFS, taps, veth, physical devs, various tc cls/act net/core/pktgen.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 508155b283dd..34fd5ece2681 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -203,6 +203,7 @@ #define F_NODE (1<<15) /* Node memory alloc*/ #define F_UDPCSUM (1<<16) /* Include UDP checksum */ #define F_NO_TIMESTAMP (1<<17) /* Don't timestamp packets (default TS) */ +#define F_DO_RX (1<<18) /* generate packets for RX */ /* Thread control flag bits */ #define T_STOP (1<<0) /* Stop run */ @@ -1081,7 +1082,8 @@ static ssize_t pktgen_if_write(struct file *file, if (len < 0) return len; if ((value > 0) && - (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) + ((pkt_dev->flags & F_DO_RX) || + !(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) return -ENOTSUPP; i += len; pkt_dev->clone_skb = value; @@ -1089,6 +1091,12 @@ static ssize_t pktgen_if_write(struct file *file, sprintf(pg_result, "OK: clone_skb=%d", pkt_dev->clone_skb); return count; } + if (!strcmp(name, "rx")) { + pkt_dev->flags |= F_DO_RX; + + sprintf(pg_result, "OK: RX is ON"); + return count; + } if (!strcmp(name, "count")) { len = num_arg(&user_buffer[i], 10, &value); if (len < 0) @@ -1134,7 +1142,7 @@ static ssize_t pktgen_if_write(struct file *file, return len; i += len; - if ((value > 1) && + if ((value > 1) && !(pkt_dev->flags & F_DO_RX) && (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) return -ENOTSUPP; pkt_dev->burst = value < 1 ? 1 : value; @@ -3317,6 +3325,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) unsigned int burst = ACCESS_ONCE(pkt_dev->burst); struct net_device *odev = pkt_dev->odev; struct netdev_queue *txq; + struct sk_buff *skb; int ret; /* If device is offline, then don't send */ @@ -3349,11 +3358,46 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) pkt_dev->last_pkt_size = pkt_dev->skb->len; pkt_dev->allocated_skbs++; pkt_dev->clone_count = 0; /* reset counter */ + if (pkt_dev->flags & F_DO_RX) + pkt_dev->skb->protocol = eth_type_trans(pkt_dev->skb, + pkt_dev->skb->dev); } if (pkt_dev->delay && pkt_dev->last_ok) spin(pkt_dev, pkt_dev->next_tx); + if (pkt_dev->flags & F_DO_RX) { + skb = pkt_dev->skb; + atomic_add(burst, &skb->users); + local_bh_disable(); + do { + ret = netif_receive_skb(skb); + if (ret == NET_RX_DROP) + pkt_dev->errors++; + pkt_dev->sofar++; + pkt_dev->seq_num++; + if (atomic_read(&skb->users) != burst) { + /* skb was queued by rps/rfs or taps, + * so cannot reuse this skb + */ + atomic_sub(burst - 1, &skb->users); + /* get out of the loop and wait + * until skb is consumed + */ + pkt_dev->last_ok = 1; + pkt_dev->clone_skb = 0; + break; + } + /* skb was 'freed' by stack, so clean few + * bits and reuse it + */ +#ifdef CONFIG_NET_CLS_ACT + skb->tc_verd = 0; /* reset reclass/redir ttl */ +#endif + } while (--burst > 0); + goto out; + } + txq = skb_get_tx_queue(odev, pkt_dev->skb); local_bh_disable(); @@ -3401,6 +3445,7 @@ xmit_more: unlock: HARD_TX_UNLOCK(odev, txq); +out: local_bh_enable(); /* If pkt_dev->count is zero, then run forever */ -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v3 net-next] pktgen: introduce 'rx' mode 2015-05-01 5:12 [PATCH v3 net-next] pktgen: introduce 'rx' mode Alexei Starovoitov @ 2015-05-01 16:54 ` Eric Dumazet 2015-05-02 8:46 ` Jesper Dangaard Brouer 2015-05-05 20:29 ` [PATCH 0/2] pktgen changes Jesper Dangaard Brouer 2 siblings, 0 replies; 38+ messages in thread From: Eric Dumazet @ 2015-05-01 16:54 UTC (permalink / raw) To: Alexei Starovoitov; +Cc: David S. Miller, Eric Dumazet, netdev On Thu, 2015-04-30 at 22:12 -0700, Alexei Starovoitov wrote: > Introduce 'RX' mode for pktgen which generates the packets > using familiar pktgen commands, but feeds them into > netif_receive_skb() instead of ndo_start_xmit(). > Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> > --- > v2->v3: addressed more Eric comments. Thanks! > > v1->v2: as suggested by Eric: > - dropped 'clone_skb' flag, now it will return enotsupp > - fix rps/rfs bug by checking skb->users after every netif_receive_skb > - tested with RPS/RFS, taps, veth, physical devs, various tc cls/act Acked-by: Eric Dumazet <edumazet@google.com> Thanks ! ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 net-next] pktgen: introduce 'rx' mode 2015-05-01 5:12 [PATCH v3 net-next] pktgen: introduce 'rx' mode Alexei Starovoitov 2015-05-01 16:54 ` Eric Dumazet @ 2015-05-02 8:46 ` Jesper Dangaard Brouer 2015-05-02 9:44 ` Daniel Borkmann 2015-05-02 16:01 ` Alexei Starovoitov 2015-05-05 20:29 ` [PATCH 0/2] pktgen changes Jesper Dangaard Brouer 2 siblings, 2 replies; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-02 8:46 UTC (permalink / raw) To: Alexei Starovoitov Cc: brouer, David S. Miller, Eric Dumazet, netdev, Robert Olsson, Ben Greear On Thu, 30 Apr 2015 22:12:10 -0700 Alexei Starovoitov <ast@plumgrid.com> wrote: > Introduce 'RX' mode for pktgen which generates the packets > using familiar pktgen commands, but feeds them into > netif_receive_skb() instead of ndo_start_xmit(). > > It is designed to test netif_receive_skb and ingress qdisc > performace only. Make sure to understand how it works before > using it for other rx benchmarking. Hi Alexei First of all I love the idea of modifying pktgen to performance test the RX path. I'm not sure the simple "rx" flag is a good "name". It likely conflicts with other work where pktgen can receive it own packets, e.g. https://people.kth.se/~danieltt/pktgen/ or Ben Greer's solution. In your patch several things are not pktgen "compliant": 1. Flags in pktgen are normally in upper-case "RX" 2. Flags also require a disable "!RX" option 3. You didn't add the flag to list of supported flags 4. You don't output if the flag is enabled 5. You didn't update the documentation (Documentation/networking/pktgen.txt) Cc.ed Robert and Ben, and left the patch below for their review. > Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> > --- > v2->v3: addressed more Eric comments. Thanks! > > v1->v2: as suggested by Eric: > - dropped 'clone_skb' flag, now it will return enotsupp > - fix rps/rfs bug by checking skb->users after every netif_receive_skb > - tested with RPS/RFS, taps, veth, physical devs, various tc cls/act > > net/core/pktgen.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 47 insertions(+), 2 deletions(-) > > diff --git a/net/core/pktgen.c b/net/core/pktgen.c > index 508155b283dd..34fd5ece2681 100644 > --- a/net/core/pktgen.c > +++ b/net/core/pktgen.c > @@ -203,6 +203,7 @@ > #define F_NODE (1<<15) /* Node memory alloc*/ > #define F_UDPCSUM (1<<16) /* Include UDP checksum */ > #define F_NO_TIMESTAMP (1<<17) /* Don't timestamp packets (default TS) */ > +#define F_DO_RX (1<<18) /* generate packets for RX */ > > /* Thread control flag bits */ > #define T_STOP (1<<0) /* Stop run */ > @@ -1081,7 +1082,8 @@ static ssize_t pktgen_if_write(struct file *file, > if (len < 0) > return len; > if ((value > 0) && > - (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) > + ((pkt_dev->flags & F_DO_RX) || > + !(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) > return -ENOTSUPP; > i += len; > pkt_dev->clone_skb = value; > @@ -1089,6 +1091,12 @@ static ssize_t pktgen_if_write(struct file *file, > sprintf(pg_result, "OK: clone_skb=%d", pkt_dev->clone_skb); > return count; > } > + if (!strcmp(name, "rx")) { > + pkt_dev->flags |= F_DO_RX; > + > + sprintf(pg_result, "OK: RX is ON"); > + return count; > + } > if (!strcmp(name, "count")) { > len = num_arg(&user_buffer[i], 10, &value); > if (len < 0) > @@ -1134,7 +1142,7 @@ static ssize_t pktgen_if_write(struct file *file, > return len; > > i += len; > - if ((value > 1) && > + if ((value > 1) && !(pkt_dev->flags & F_DO_RX) && > (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) > return -ENOTSUPP; > pkt_dev->burst = value < 1 ? 1 : value; > @@ -3317,6 +3325,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) > unsigned int burst = ACCESS_ONCE(pkt_dev->burst); > struct net_device *odev = pkt_dev->odev; > struct netdev_queue *txq; > + struct sk_buff *skb; > int ret; > > /* If device is offline, then don't send */ > @@ -3349,11 +3358,46 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) > pkt_dev->last_pkt_size = pkt_dev->skb->len; > pkt_dev->allocated_skbs++; > pkt_dev->clone_count = 0; /* reset counter */ > + if (pkt_dev->flags & F_DO_RX) > + pkt_dev->skb->protocol = eth_type_trans(pkt_dev->skb, > + pkt_dev->skb->dev); > } > > if (pkt_dev->delay && pkt_dev->last_ok) > spin(pkt_dev, pkt_dev->next_tx); > > + if (pkt_dev->flags & F_DO_RX) { > + skb = pkt_dev->skb; > + atomic_add(burst, &skb->users); > + local_bh_disable(); > + do { > + ret = netif_receive_skb(skb); > + if (ret == NET_RX_DROP) > + pkt_dev->errors++; > + pkt_dev->sofar++; > + pkt_dev->seq_num++; > + if (atomic_read(&skb->users) != burst) { > + /* skb was queued by rps/rfs or taps, > + * so cannot reuse this skb > + */ > + atomic_sub(burst - 1, &skb->users); > + /* get out of the loop and wait > + * until skb is consumed > + */ > + pkt_dev->last_ok = 1; > + pkt_dev->clone_skb = 0; > + break; > + } > + /* skb was 'freed' by stack, so clean few > + * bits and reuse it > + */ > +#ifdef CONFIG_NET_CLS_ACT > + skb->tc_verd = 0; /* reset reclass/redir ttl */ > +#endif > + } while (--burst > 0); > + goto out; > + } > + > txq = skb_get_tx_queue(odev, pkt_dev->skb); > > local_bh_disable(); > @@ -3401,6 +3445,7 @@ xmit_more: > unlock: > HARD_TX_UNLOCK(odev, txq); > > +out: > local_bh_enable(); > > /* If pkt_dev->count is zero, then run forever */ -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 net-next] pktgen: introduce 'rx' mode 2015-05-02 8:46 ` Jesper Dangaard Brouer @ 2015-05-02 9:44 ` Daniel Borkmann 2015-05-02 9:54 ` Jesper Dangaard Brouer 2015-05-02 16:01 ` Alexei Starovoitov 1 sibling, 1 reply; 38+ messages in thread From: Daniel Borkmann @ 2015-05-02 9:44 UTC (permalink / raw) To: Jesper Dangaard Brouer, Alexei Starovoitov Cc: David S. Miller, Eric Dumazet, netdev, Robert Olsson, Ben Greear Hi Jesper, On 05/02/2015 10:46 AM, Jesper Dangaard Brouer wrote: ... > First of all I love the idea of modifying pktgen to performance test > the RX path. > > I'm not sure the simple "rx" flag is a good "name". It likely > conflicts with other work where pktgen can receive it own packets, e.g. > https://people.kth.se/~danieltt/pktgen/ or Ben Greer's solution. Why do we start caring about out of tree code now? We never have, really. If there is no interest in merging this stuff upstream, then it's always the case that _their code_ needs to adapt iff you want to run on a more recent kernel; the kernel dictates the uapi, not some out of tree module. ;) Cheers, Daniel ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 net-next] pktgen: introduce 'rx' mode 2015-05-02 9:44 ` Daniel Borkmann @ 2015-05-02 9:54 ` Jesper Dangaard Brouer 2015-05-02 10:30 ` Daniel Borkmann 0 siblings, 1 reply; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-02 9:54 UTC (permalink / raw) To: Daniel Borkmann Cc: brouer, Alexei Starovoitov, David S. Miller, Eric Dumazet, netdev, Robert Olsson, Ben Greear On Sat, 02 May 2015 11:44:45 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote: > Hi Jesper, > > On 05/02/2015 10:46 AM, Jesper Dangaard Brouer wrote: > ... > > First of all I love the idea of modifying pktgen to performance test > > the RX path. > > > > I'm not sure the simple "rx" flag is a good "name". It likely > > conflicts with other work where pktgen can receive it own packets, e.g. > > https://people.kth.se/~danieltt/pktgen/ or Ben Greer's solution. > > Why do we start caring about out of tree code now? We never have, > really. If there is no interest in merging this stuff upstream, > then it's always the case that _their code_ needs to adapt iff you > want to run on a more recent kernel; the kernel dictates the uapi, > not some out of tree module. ;) Sure, out-of-tree code should not control our choices. I personally just don't like the "RX" flag name. What about "STACK_RX" or "RX_INJECT"? -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 net-next] pktgen: introduce 'rx' mode 2015-05-02 9:54 ` Jesper Dangaard Brouer @ 2015-05-02 10:30 ` Daniel Borkmann 0 siblings, 0 replies; 38+ messages in thread From: Daniel Borkmann @ 2015-05-02 10:30 UTC (permalink / raw) To: Jesper Dangaard Brouer Cc: Alexei Starovoitov, David S. Miller, Eric Dumazet, netdev, Robert Olsson, Ben Greear On 05/02/2015 11:54 AM, Jesper Dangaard Brouer wrote: ... > I personally just don't like the "RX" flag name. > What about "STACK_RX" or "RX_INJECT"? I'm not sure if it actually qualifies as a flag from user space point of view, besides that it's used as an implementation detail. You could have an option "xmit_mode [rx|tx]" (where obviously tx is the default), perhaps that makes it clearer that you xmit into the kernel's rx path. Cheers, Daniel ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 net-next] pktgen: introduce 'rx' mode 2015-05-02 8:46 ` Jesper Dangaard Brouer 2015-05-02 9:44 ` Daniel Borkmann @ 2015-05-02 16:01 ` Alexei Starovoitov 2015-05-02 16:46 ` Jesper Dangaard Brouer 1 sibling, 1 reply; 38+ messages in thread From: Alexei Starovoitov @ 2015-05-02 16:01 UTC (permalink / raw) To: Jesper Dangaard Brouer Cc: David S. Miller, Eric Dumazet, netdev, Robert Olsson, Ben Greear On 5/2/15 1:46 AM, Jesper Dangaard Brouer wrote: > On Thu, 30 Apr 2015 22:12:10 -0700 > Alexei Starovoitov <ast@plumgrid.com> wrote: > >> Introduce 'RX' mode for pktgen which generates the packets >> using familiar pktgen commands, but feeds them into >> netif_receive_skb() instead of ndo_start_xmit(). >> >> It is designed to test netif_receive_skb and ingress qdisc >> performace only. Make sure to understand how it works before >> using it for other rx benchmarking. > > Hi Alexei > > First of all I love the idea of modifying pktgen to performance test > the RX path. > > I'm not sure the simple "rx" flag is a good "name". It likely > conflicts with other work where pktgen can receive it own packets, e.g. > https://people.kth.se/~danieltt/pktgen/ or Ben Greer's solution. > > In your patch several things are not pktgen "compliant": > 1. Flags in pktgen are normally in upper-case "RX" > 2. Flags also require a disable "!RX" option > 3. You didn't add the flag to list of supported flags > 4. You don't output if the flag is enabled > 5. You didn't update the documentation (Documentation/networking/pktgen.txt) It's actually not a flag, because it cannot be disabled by design. It cannot be flipped back and forth, because it affects what other real flags can be applied. It's a _mode_. I don't see yet how I can safely switch this mode back into tx while things are running. It would need a whole new mechanism of stopping things and so on. I wanted to start simple. For 5, yeah, agree, need to update the doc. As far as name, I don't have preferences. Will 'stack_inject' sound better? I can respin with that name if you like, but disabling it on the fly is a major change. I'd rather do it in small steps. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 net-next] pktgen: introduce 'rx' mode 2015-05-02 16:01 ` Alexei Starovoitov @ 2015-05-02 16:46 ` Jesper Dangaard Brouer 2015-05-02 17:07 ` Alexei Starovoitov 0 siblings, 1 reply; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-02 16:46 UTC (permalink / raw) To: Alexei Starovoitov Cc: brouer, David S. Miller, Eric Dumazet, netdev, Robert Olsson, Ben Greear On Sat, 02 May 2015 09:01:27 -0700 Alexei Starovoitov <ast@plumgrid.com> wrote: > On 5/2/15 1:46 AM, Jesper Dangaard Brouer wrote: > > On Thu, 30 Apr 2015 22:12:10 -0700 > > Alexei Starovoitov <ast@plumgrid.com> wrote: > > > >> Introduce 'RX' mode for pktgen which generates the packets > >> using familiar pktgen commands, but feeds them into > >> netif_receive_skb() instead of ndo_start_xmit(). > >> > >> It is designed to test netif_receive_skb and ingress qdisc > >> performace only. Make sure to understand how it works before > >> using it for other rx benchmarking. > > > > Hi Alexei > > > > First of all I love the idea of modifying pktgen to performance test > > the RX path. > > > > I'm not sure the simple "rx" flag is a good "name". It likely > > conflicts with other work where pktgen can receive it own packets, e.g. > > https://people.kth.se/~danieltt/pktgen/ or Ben Greer's solution. > > > > In your patch several things are not pktgen "compliant": > > 1. Flags in pktgen are normally in upper-case "RX" > > 2. Flags also require a disable "!RX" option > > 3. You didn't add the flag to list of supported flags > > 4. You don't output if the flag is enabled > > 5. You didn't update the documentation (Documentation/networking/pktgen.txt) > > It's actually not a flag, because it cannot be disabled by design. > It cannot be flipped back and forth, because it affects what other > real flags can be applied. It's a _mode_. > I don't see yet how I can safely switch this mode back into tx while > things are running. It would need a whole new mechanism of stopping > things and so on. I wanted to start simple. > For 5, yeah, agree, need to update the doc. > As far as name, I don't have preferences. Will 'stack_inject' sound > better? I can respin with that name if you like, but disabling it on > the fly is a major change. I'd rather do it in small steps. Okay, I guess it makes sense as a "mode". Calling it "stack_inject" is fairly descriptive, but I also like Daniel's idea of option "xmit_mode [rx|tx]". I would not allow it to be reconfigured on the fly, but only when the test/thread is stopped (!pkt_dev->running), allow re-configuring back to "TX" mode. Although, removing (and free) the entire pkt_dev, would implicitly reset it back. -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 net-next] pktgen: introduce 'rx' mode 2015-05-02 16:46 ` Jesper Dangaard Brouer @ 2015-05-02 17:07 ` Alexei Starovoitov 2015-05-05 18:15 ` Jesper Dangaard Brouer 0 siblings, 1 reply; 38+ messages in thread From: Alexei Starovoitov @ 2015-05-02 17:07 UTC (permalink / raw) To: Jesper Dangaard Brouer Cc: David S. Miller, Eric Dumazet, netdev, Robert Olsson, Ben Greear On 5/2/15 9:46 AM, Jesper Dangaard Brouer wrote: > Okay, I guess it makes sense as a "mode". > > Calling it "stack_inject" is fairly descriptive, but I also like > Daniel's idea of option "xmit_mode [rx|tx]". sure. that's fine too. > I would not allow it to be reconfigured on the fly, but only when the > test/thread is stopped (!pkt_dev->running), allow re-configuring back > to "TX" mode. Although, removing (and free) the entire pkt_dev, would > implicitly reset it back. all makes sense. btw, I'm off the grid till Monday and it sounds like you see concrete spots in the code to hack this stuff in, so if you want to take over, please go ahead :) ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 net-next] pktgen: introduce 'rx' mode 2015-05-02 17:07 ` Alexei Starovoitov @ 2015-05-05 18:15 ` Jesper Dangaard Brouer 2015-05-05 18:30 ` Alexei Starovoitov 0 siblings, 1 reply; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-05 18:15 UTC (permalink / raw) To: Alexei Starovoitov Cc: brouer, David S. Miller, Eric Dumazet, netdev, Robert Olsson, Ben Greear On Sat, 02 May 2015 10:07:48 -0700 Alexei Starovoitov <ast@plumgrid.com> wrote: > On 5/2/15 9:46 AM, Jesper Dangaard Brouer wrote: [...] > > all makes sense. > btw, I'm off the grid till Monday and it sounds like you see concrete > spots in the code to hack this stuff in, so if you want to take over, > please go ahead :) Okay, I'll address my own concerns and post a patch in your name("From:") As I would like to see this feature in pktgen, as it provides a really easy and quick way to measure/profile changes in the ingress path... -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 net-next] pktgen: introduce 'rx' mode 2015-05-05 18:15 ` Jesper Dangaard Brouer @ 2015-05-05 18:30 ` Alexei Starovoitov 0 siblings, 0 replies; 38+ messages in thread From: Alexei Starovoitov @ 2015-05-05 18:30 UTC (permalink / raw) To: Jesper Dangaard Brouer Cc: David S. Miller, Eric Dumazet, netdev, Robert Olsson, Ben Greear On 5/5/15 11:15 AM, Jesper Dangaard Brouer wrote: > > On Sat, 02 May 2015 10:07:48 -0700 Alexei Starovoitov <ast@plumgrid.com> wrote: >> On 5/2/15 9:46 AM, Jesper Dangaard Brouer wrote: > [...] >> >> all makes sense. >> btw, I'm off the grid till Monday and it sounds like you see concrete >> spots in the code to hack this stuff in, so if you want to take over, >> please go ahead :) > > Okay, I'll address my own concerns and post a patch in your name("From:") lol :) I seriously don't care about commit count. Thank you for taking over. I'm swamped with tracing stuff at the moment. > As I would like to see this feature in pktgen, as it provides a really > easy and quick way to measure/profile changes in the ingress path... same here. my motivation for this patch is to have a common tool to measure ingress speed, so we can have concrete numbers to talk about. Also, as you mentioned, there is always a danger of 'zooming in'. Any type of benchmarking shouldn't be blindly followed. At the same time all numbers are useful. They are different datapoints that make a complete picture. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 0/2] pktgen changes 2015-05-01 5:12 [PATCH v3 net-next] pktgen: introduce 'rx' mode Alexei Starovoitov 2015-05-01 16:54 ` Eric Dumazet 2015-05-02 8:46 ` Jesper Dangaard Brouer @ 2015-05-05 20:29 ` Jesper Dangaard Brouer 2015-05-05 20:29 ` [PATCH 1/2] pktgen: adjust flag NO_TIMESTAMP to be more pktgen compliant Jesper Dangaard Brouer 2015-05-05 20:30 ` [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' Jesper Dangaard Brouer 2 siblings, 2 replies; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-05 20:29 UTC (permalink / raw) To: Jesper Dangaard Brouer, netdev; +Cc: Eric Dumazet, Alexei Starovoitov The following series introduce some pktgen changes Patch01: Cleanup my own work when I introduced NO_TIMESTAMP. Patch02: Took over patch from Alexei, and addressed my own concerns, as Alexie is too busy with other work, and this will provide an easy tool for measuring ingress path performance, which is a hot topic ATM. Changes were primarily user interface related. Introduced a separate "xmit_mode" setting, instead of stealing one of the dev flags like Alexei did. --- Alexei Starovoitov (1): pktgen: introduce xmit_mode 'rx_inject' Jesper Dangaard Brouer (1): pktgen: adjust flag NO_TIMESTAMP to be more pktgen compliant Documentation/networking/pktgen.txt | 9 ++++ net/core/pktgen.c | 87 +++++++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 5 deletions(-) -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 1/2] pktgen: adjust flag NO_TIMESTAMP to be more pktgen compliant 2015-05-05 20:29 ` [PATCH 0/2] pktgen changes Jesper Dangaard Brouer @ 2015-05-05 20:29 ` Jesper Dangaard Brouer 2015-05-05 20:30 ` [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' Jesper Dangaard Brouer 1 sibling, 0 replies; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-05 20:29 UTC (permalink / raw) To: Jesper Dangaard Brouer, netdev; +Cc: Eric Dumazet, Alexei Starovoitov Allow flag NO_TIMESTAMP to turn timestamping on again, like other flags, with a negation of the flag like !NO_TIMESTAMP. Also document the option flag NO_TIMESTAMP. Fixes: afb84b626184 ("pktgen: add flag NO_TIMESTAMP to disable timestamping") Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- Documentation/networking/pktgen.txt | 2 ++ net/core/pktgen.c | 3 +++ 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/Documentation/networking/pktgen.txt b/Documentation/networking/pktgen.txt index 0344f1d..6199ee6 100644 --- a/Documentation/networking/pktgen.txt +++ b/Documentation/networking/pktgen.txt @@ -145,6 +145,7 @@ Examples: UDPCSUM, IPSEC # IPsec encapsulation (needs CONFIG_XFRM) NODE_ALLOC # node specific memory allocation + NO_TIMESTAMP # disable timestamping pgset spi SPI_VALUE Set specific SA used to transform packet. @@ -287,6 +288,7 @@ flag UDPCSUM IPSEC NODE_ALLOC + NO_TIMESTAMP dst_min dst_max diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 508155b..43bb215 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -1267,6 +1267,9 @@ static ssize_t pktgen_if_write(struct file *file, else if (strcmp(f, "NO_TIMESTAMP") == 0) pkt_dev->flags |= F_NO_TIMESTAMP; + else if (strcmp(f, "!NO_TIMESTAMP") == 0) + pkt_dev->flags &= ~F_NO_TIMESTAMP; + else { sprintf(pg_result, "Flag -:%s:- unknown\nAvailable flags, (prepend ! to un-set flag):\n%s", ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' 2015-05-05 20:29 ` [PATCH 0/2] pktgen changes Jesper Dangaard Brouer 2015-05-05 20:29 ` [PATCH 1/2] pktgen: adjust flag NO_TIMESTAMP to be more pktgen compliant Jesper Dangaard Brouer @ 2015-05-05 20:30 ` Jesper Dangaard Brouer 2015-05-06 4:33 ` Alexei Starovoitov 2015-05-06 5:32 ` Alexander Duyck 1 sibling, 2 replies; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-05 20:30 UTC (permalink / raw) To: Jesper Dangaard Brouer, netdev; +Cc: Eric Dumazet, Alexei Starovoitov From: Alexei Starovoitov <ast@plumgrid.com> Introduce xmit_mode 'rx_inject' for pktgen which generates the packets using familiar pktgen commands, but feeds them into netif_receive_skb() instead of ndo_start_xmit(). It is designed to test netif_receive_skb and ingress qdisc performace only. Make sure to understand how it works before using it for other rx benchmarking. Sample script 'pktgen.sh': \#!/bin/bash function pgset() { local result echo $1 > $PGDEV result=`cat $PGDEV | fgrep "Result: OK:"` if [ "$result" = "" ]; then cat $PGDEV | fgrep Result: fi } ETH=$1 PGDEV=/proc/net/pktgen/kpktgend_0 pgset "rem_device_all" pgset "add_device $ETH" PGDEV=/proc/net/pktgen/$ETH pgset "xmit_mode rx_inject" pgset "pkt_size 60" pgset "dst 99.1.1.2" pgset "dst_mac 90:e2:ba:6e:a8:e5" pgset "count 10000000" pgset "burst 32" PGDEV=/proc/net/pktgen/pgctrl echo "Running... ctrl^C to stop" pgset "start" echo "Done" cat /proc/net/pktgen/$ETH Usage: $ sudo ./pktgen.sh eth2 ... Result: OK: 232376(c232372+d3) usec, 10000000 (60byte,0frags) 43033682pps 20656Mb/sec (20656167360bps) errors: 10000000 Raw netif_receive_skb speed should be ~43 million packet per second on 3.7Ghz x86 and 'perf report' should look like: 37.69% kpktgend_0 [kernel.vmlinux] [k] __netif_receive_skb_core 25.81% kpktgend_0 [kernel.vmlinux] [k] kfree_skb 7.22% kpktgend_0 [kernel.vmlinux] [k] ip_rcv 5.68% kpktgend_0 [pktgen] [k] pktgen_thread_worker If fib_table_lookup is seen on top, it means skb was processed by the stack. To benchmark netif_receive_skb only make sure that 'dst_mac' of your pktgen script is different from receiving device mac and it will be dropped by ip_rcv Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- v3->v4: Jesper address his own concerns - User interface intro xmit_mode - Mention xmit_mode in doc - Make sure skb_clone cannot be used together with this mode v2->v3: addressed more Eric comments. Thanks! v1->v2: as suggested by Eric: - dropped 'clone_skb' flag, now it will return enotsupp - fix rps/rfs bug by checking skb->users after every netif_receive_skb - tested with RPS/RFS, taps, veth, physical devs, various tc cls/act --- Documentation/networking/pktgen.txt | 7 +++ net/core/pktgen.c | 84 +++++++++++++++++++++++++++++++++-- 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/Documentation/networking/pktgen.txt b/Documentation/networking/pktgen.txt index 6199ee6..bfb5f2c 100644 --- a/Documentation/networking/pktgen.txt +++ b/Documentation/networking/pktgen.txt @@ -193,6 +193,10 @@ Examples: pgset "rate 300M" set rate to 300 Mb/s pgset "ratep 1000000" set rate to 1Mpps + pgset "xmit_mode rx_inject" RX inject packets into stack netif_receive_skb() + Works with "burst" but not with "clone_skb". + Default xmit_mode is "tx". + Sample scripts ============== @@ -310,6 +314,9 @@ flowlen rate ratep +xmit_mode + + References: ftp://robur.slu.se/pub/Linux/net-development/pktgen-testing/ ftp://robur.slu.se/pub/Linux/net-development/pktgen-testing/examples/ diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 43bb215..85195b2 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -210,6 +210,10 @@ #define T_REMDEVALL (1<<2) /* Remove all devs */ #define T_REMDEV (1<<3) /* Remove one dev */ +/* Xmit modes */ +#define M_TX 0 /* Default normal TX */ +#define M_RX_INJECT 1 /* RX inject packet into stack */ + /* If lock -- protects updating of if_list */ #define if_lock(t) spin_lock(&(t->if_lock)); #define if_unlock(t) spin_unlock(&(t->if_lock)); @@ -251,13 +255,14 @@ struct pktgen_dev { * we will do a random selection from within the range. */ __u32 flags; - int removal_mark; /* non-zero => the device is marked for - * removal by worker thread */ - + int xmit_mode; int min_pkt_size; int max_pkt_size; int pkt_overhead; /* overhead for MPLS, VLANs, IPSEC etc */ int nfrags; + int removal_mark; /* non-zero => the device is marked for + * removal by worker thread */ + struct page *page; u64 delay; /* nano-seconds */ @@ -620,6 +625,9 @@ static int pktgen_if_show(struct seq_file *seq, void *v) if (pkt_dev->node >= 0) seq_printf(seq, " node: %d\n", pkt_dev->node); + if (pkt_dev->xmit_mode == M_RX_INJECT) + seq_puts(seq, " xmit_mode: rx_inject\n"); + seq_puts(seq, " Flags: "); if (pkt_dev->flags & F_IPV6) @@ -1081,7 +1089,8 @@ static ssize_t pktgen_if_write(struct file *file, if (len < 0) return len; if ((value > 0) && - (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) + ((pkt_dev->xmit_mode == M_RX_INJECT) || + !(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) return -ENOTSUPP; i += len; pkt_dev->clone_skb = value; @@ -1134,7 +1143,7 @@ static ssize_t pktgen_if_write(struct file *file, return len; i += len; - if ((value > 1) && + if ((value > 1) && (pkt_dev->xmit_mode == M_TX) && (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) return -ENOTSUPP; pkt_dev->burst = value < 1 ? 1 : value; @@ -1160,6 +1169,35 @@ static ssize_t pktgen_if_write(struct file *file, sprintf(pg_result, "ERROR: node not possible"); return count; } + if (!strcmp(name, "xmit_mode")) { + char f[32]; + + memset(f, 0, 32); + len = strn_len(&user_buffer[i], sizeof(f) - 1); + if (len < 0) + return len; + + if (copy_from_user(f, &user_buffer[i], len)) + return -EFAULT; + i += len; + + if (strcmp(f, "tx") == 0) { + pkt_dev->xmit_mode = M_TX; + } else if (strcmp(f, "rx_inject") == 0) { + /* clone_skb set earlier, not supported in this mode */ + if (pkt_dev->clone_skb > 0) + return -ENOTSUPP; + + pkt_dev->xmit_mode = M_RX_INJECT; + } else { + sprintf(pg_result, + "xmit_mode -:%s:- unknown\nAvailable modes: %s", + f, "tx, rx_inject\n"); + return count; + } + sprintf(pg_result, "OK: xmit_mode=%s", f); + return count; + } if (!strcmp(name, "flag")) { char f[32]; memset(f, 0, 32); @@ -3320,6 +3358,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) unsigned int burst = ACCESS_ONCE(pkt_dev->burst); struct net_device *odev = pkt_dev->odev; struct netdev_queue *txq; + struct sk_buff *skb; int ret; /* If device is offline, then don't send */ @@ -3352,11 +3391,45 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) pkt_dev->last_pkt_size = pkt_dev->skb->len; pkt_dev->allocated_skbs++; pkt_dev->clone_count = 0; /* reset counter */ + if (pkt_dev->xmit_mode == M_RX_INJECT) + pkt_dev->skb->protocol = eth_type_trans(pkt_dev->skb, + pkt_dev->skb->dev); } if (pkt_dev->delay && pkt_dev->last_ok) spin(pkt_dev, pkt_dev->next_tx); + if (pkt_dev->xmit_mode == M_RX_INJECT) { + skb = pkt_dev->skb; + atomic_add(burst, &skb->users); + local_bh_disable(); + do { + ret = netif_receive_skb(skb); + if (ret == NET_RX_DROP) + pkt_dev->errors++; + pkt_dev->sofar++; + pkt_dev->seq_num++; + if (atomic_read(&skb->users) != burst) { + /* skb was queued by rps/rfs or taps, + * so cannot reuse this skb + */ + atomic_sub(burst - 1, &skb->users); + /* get out of the loop and wait + * until skb is consumed + */ + pkt_dev->last_ok = 1; + break; + } + /* skb was 'freed' by stack, so clean few + * bits and reuse it + */ +#ifdef CONFIG_NET_CLS_ACT + skb->tc_verd = 0; /* reset reclass/redir ttl */ +#endif + } while (--burst > 0); + goto out; /* Skips xmit_mode M_TX */ + } + txq = skb_get_tx_queue(odev, pkt_dev->skb); local_bh_disable(); @@ -3404,6 +3477,7 @@ xmit_more: unlock: HARD_TX_UNLOCK(odev, txq); +out: local_bh_enable(); /* If pkt_dev->count is zero, then run forever */ ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' 2015-05-05 20:30 ` [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' Jesper Dangaard Brouer @ 2015-05-06 4:33 ` Alexei Starovoitov 2015-05-06 5:24 ` Jesper Dangaard Brouer 2015-05-06 5:32 ` Alexander Duyck 1 sibling, 1 reply; 38+ messages in thread From: Alexei Starovoitov @ 2015-05-06 4:33 UTC (permalink / raw) To: Jesper Dangaard Brouer, netdev; +Cc: Eric Dumazet On 5/5/15 1:30 PM, Jesper Dangaard Brouer wrote: > > Introduce xmit_mode 'rx_inject' for pktgen which generates the packets > using familiar pktgen commands, but feeds them into > netif_receive_skb() instead of ndo_start_xmit(). ... > pgset "xmit_mode rx_inject" I think 'xmit_mode rx_inject' would make native english speaker cringe, since it's saying 'transmit mode is receive' ... but I don't mind :) > > Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> ... > @@ -251,13 +255,14 @@ struct pktgen_dev { > * we will do a random selection from within the range. > */ > __u32 flags; > - int removal_mark; /* non-zero => the device is marked for > - * removal by worker thread */ > - > + int xmit_mode; > int min_pkt_size; > int max_pkt_size; > int pkt_overhead; /* overhead for MPLS, VLANs, IPSEC etc */ > int nfrags; > + int removal_mark; /* non-zero => the device is marked for > + * removal by worker thread */ I'm not sure why you're moving removal_mark field. Looks good. Thank you for doing this. Ack. My SOB is already there :) btw, these patches didn't reach my subscribed to netdev email yet... something is stalling vger. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' 2015-05-06 4:33 ` Alexei Starovoitov @ 2015-05-06 5:24 ` Jesper Dangaard Brouer 2015-05-06 10:17 ` Daniel Borkmann 0 siblings, 1 reply; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-06 5:24 UTC (permalink / raw) To: Alexei Starovoitov; +Cc: netdev, Eric Dumazet, brouer On Tue, 05 May 2015 21:33:26 -0700 Alexei Starovoitov <ast@plumgrid.com> wrote: > On 5/5/15 1:30 PM, Jesper Dangaard Brouer wrote: > > > > Introduce xmit_mode 'rx_inject' for pktgen which generates the packets > > using familiar pktgen commands, but feeds them into > > netif_receive_skb() instead of ndo_start_xmit(). > ... > > pgset "xmit_mode rx_inject" > > I think 'xmit_mode rx_inject' would make native english speaker cringe, > since it's saying 'transmit mode is receive' ... but I don't mind :) Yes, I know. Like Daniel suggested, I considered only calling it "rx" but it made me cringe for this exact reason, thus I extended it with "inject". I'm flexible with the name of this... > > Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > ... > > @@ -251,13 +255,14 @@ struct pktgen_dev { > > * we will do a random selection from within the range. > > */ > > __u32 flags; > > - int removal_mark; /* non-zero => the device is marked for > > - * removal by worker thread */ > > - > > + int xmit_mode; > > int min_pkt_size; > > int max_pkt_size; > > int pkt_overhead; /* overhead for MPLS, VLANs, IPSEC etc */ > > int nfrags; > > + int removal_mark; /* non-zero => the device is marked for > > + * removal by worker thread */ > > I'm not sure why you're moving removal_mark field. Because I wanted to place 'xmit_more' on a read-only/mostly cache-line, although it likely does not matter too much, I just wanted to avoid any funny cache coherency protocol interactions. > Looks good. Thank you for doing this. > Ack. My SOB is already there :) > > btw, these patches didn't reach my subscribed to netdev email yet... > something is stalling vger. Hmm, that is strange. I think I see them. And they are on patchwork too. https://patchwork.ozlabs.org/patch/468378/ https://patchwork.ozlabs.org/patch/468390/ -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' 2015-05-06 5:24 ` Jesper Dangaard Brouer @ 2015-05-06 10:17 ` Daniel Borkmann 2015-05-06 11:22 ` Jesper Dangaard Brouer 0 siblings, 1 reply; 38+ messages in thread From: Daniel Borkmann @ 2015-05-06 10:17 UTC (permalink / raw) To: Jesper Dangaard Brouer, Alexei Starovoitov; +Cc: netdev, Eric Dumazet On 05/06/2015 07:24 AM, Jesper Dangaard Brouer wrote: > On Tue, 05 May 2015 21:33:26 -0700 > Alexei Starovoitov <ast@plumgrid.com> wrote: >> On 5/5/15 1:30 PM, Jesper Dangaard Brouer wrote: >>> >>> Introduce xmit_mode 'rx_inject' for pktgen which generates the packets >>> using familiar pktgen commands, but feeds them into >>> netif_receive_skb() instead of ndo_start_xmit(). >> ... >>> pgset "xmit_mode rx_inject" >> >> I think 'xmit_mode rx_inject' would make native english speaker cringe, >> since it's saying 'transmit mode is receive' ... but I don't mind :) > > Yes, I know. Like Daniel suggested, I considered only calling it "rx" > but it made me cringe for this exact reason, thus I extended it with > "inject". I'm flexible with the name of this... I don't mind how you name it eventually. ;) 'xmit_mode' I think is good, and rx|tx would be symmetric. I believe you don't like "rx" due to these two out-of-tree pktgen projects you mentioned having rx capabilities. Is that correct? From my perspective, it would be more worth however to improve packet sockets and eBPF that could already do the same thing instead of a dedicated possible pktgen receive/ capturing device for such analysis. Anyway, I can also live with a rx_inject. Cheers, Daniel ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' 2015-05-06 10:17 ` Daniel Borkmann @ 2015-05-06 11:22 ` Jesper Dangaard Brouer 0 siblings, 0 replies; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-06 11:22 UTC (permalink / raw) To: Daniel Borkmann; +Cc: Alexei Starovoitov, netdev, Eric Dumazet, brouer On Wed, 06 May 2015 12:17:02 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote: > On 05/06/2015 07:24 AM, Jesper Dangaard Brouer wrote: > > On Tue, 05 May 2015 21:33:26 -0700 > > Alexei Starovoitov <ast@plumgrid.com> wrote: > >> On 5/5/15 1:30 PM, Jesper Dangaard Brouer wrote: > >>> > >>> Introduce xmit_mode 'rx_inject' for pktgen which generates the packets > >>> using familiar pktgen commands, but feeds them into > >>> netif_receive_skb() instead of ndo_start_xmit(). > >> ... > >>> pgset "xmit_mode rx_inject" > >> > >> I think 'xmit_mode rx_inject' would make native english speaker cringe, > >> since it's saying 'transmit mode is receive' ... but I don't mind :) > > > > Yes, I know. Like Daniel suggested, I considered only calling it "rx" > > but it made me cringe for this exact reason, thus I extended it with > > "inject". I'm flexible with the name of this... > > I don't mind how you name it eventually. ;) 'xmit_mode' I think is > good, and rx|tx would be symmetric. I believe you don't like "rx" due > to these two out-of-tree pktgen projects you mentioned having rx > capabilities. Is that correct? Not correct, I really don't care about the two out-of-tree pktgen projects. My main concern is to avoid polluting the pktgen "user-interface" (more than it already is), with a bare option like "rx" which is in no-way descriptive of its functionality. > From my perspective, it would be more > worth however to improve packet sockets and eBPF that could already > do the same thing instead of a dedicated possible pktgen receive/ > capturing device for such analysis. Anyway, I can also live with a > rx_inject. We could call it "stack_inject" instead? ... to take out the confusing "rx" part of an "transmit/xmit" mode that "receives". If someone have time, I would like to see better tools that allow us to measure different parts of the stack. For now, this is what we got. And this feature provide an easy and quick way to measure the ingress code-path, which have gotten a lot of discussion lately... which after this patch can easily be resolved by measuring instead of hand-waving ;-) -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' 2015-05-05 20:30 ` [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' Jesper Dangaard Brouer 2015-05-06 4:33 ` Alexei Starovoitov @ 2015-05-06 5:32 ` Alexander Duyck 2015-05-06 8:44 ` Jesper Dangaard Brouer 1 sibling, 1 reply; 38+ messages in thread From: Alexander Duyck @ 2015-05-06 5:32 UTC (permalink / raw) To: Jesper Dangaard Brouer, netdev; +Cc: Eric Dumazet, Alexei Starovoitov On 05/05/2015 01:30 PM, Jesper Dangaard Brouer wrote: <snip> > diff --git a/net/core/pktgen.c b/net/core/pktgen.c > index 43bb215..85195b2 100644 > --- a/net/core/pktgen.c > +++ b/net/core/pktgen.c > @@ -210,6 +210,10 @@ <snip> > @@ -3320,6 +3358,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) > unsigned int burst = ACCESS_ONCE(pkt_dev->burst); > struct net_device *odev = pkt_dev->odev; > struct netdev_queue *txq; > + struct sk_buff *skb; > int ret; > > /* If device is offline, then don't send */ > @@ -3352,11 +3391,45 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) > pkt_dev->last_pkt_size = pkt_dev->skb->len; > pkt_dev->allocated_skbs++; > pkt_dev->clone_count = 0; /* reset counter */ > + if (pkt_dev->xmit_mode == M_RX_INJECT) > + pkt_dev->skb->protocol = eth_type_trans(pkt_dev->skb, > + pkt_dev->skb->dev); > } > I was just wondering. Since M_RX_INJECT is not compatible with clone_skb couldn't the lines added above be moved down into the block below so that you could avoid the additional conditional jump? > if (pkt_dev->delay && pkt_dev->last_ok) > spin(pkt_dev, pkt_dev->next_tx); > > + if (pkt_dev->xmit_mode == M_RX_INJECT) { > + skb = pkt_dev->skb; > + atomic_add(burst, &skb->users); > + local_bh_disable(); > + do { > + ret = netif_receive_skb(skb); > + if (ret == NET_RX_DROP) > + pkt_dev->errors++; > + pkt_dev->sofar++; > + pkt_dev->seq_num++; > + if (atomic_read(&skb->users) != burst) { > + /* skb was queued by rps/rfs or taps, > + * so cannot reuse this skb > + */ > + atomic_sub(burst - 1, &skb->users); > + /* get out of the loop and wait > + * until skb is consumed > + */ > + pkt_dev->last_ok = 1; > + break; > + } > + /* skb was 'freed' by stack, so clean few > + * bits and reuse it > + */ > +#ifdef CONFIG_NET_CLS_ACT > + skb->tc_verd = 0; /* reset reclass/redir ttl */ > +#endif > + } while (--burst > 0); > + goto out; /* Skips xmit_mode M_TX */ > + } > + > txq = skb_get_tx_queue(odev, pkt_dev->skb); > > local_bh_disable(); > @@ -3404,6 +3477,7 @@ xmit_more: > unlock: > HARD_TX_UNLOCK(odev, txq); > > +out: > local_bh_enable(); > > /* If pkt_dev->count is zero, then run forever */ > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' 2015-05-06 5:32 ` Alexander Duyck @ 2015-05-06 8:44 ` Jesper Dangaard Brouer 2015-05-06 14:35 ` Alexei Starovoitov 0 siblings, 1 reply; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-06 8:44 UTC (permalink / raw) To: Alexander Duyck; +Cc: netdev, Eric Dumazet, Alexei Starovoitov, brouer On Tue, 05 May 2015 22:32:34 -0700 Alexander Duyck <alexander.duyck@gmail.com> wrote: > On 05/05/2015 01:30 PM, Jesper Dangaard Brouer wrote: > > <snip> > > > diff --git a/net/core/pktgen.c b/net/core/pktgen.c > > index 43bb215..85195b2 100644 > > --- a/net/core/pktgen.c > > +++ b/net/core/pktgen.c > > @@ -210,6 +210,10 @@ > > <snip> > > > @@ -3320,6 +3358,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) > > unsigned int burst = ACCESS_ONCE(pkt_dev->burst); > > struct net_device *odev = pkt_dev->odev; > > struct netdev_queue *txq; > > + struct sk_buff *skb; > > int ret; > > > > /* If device is offline, then don't send */ > > @@ -3352,11 +3391,45 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) > > pkt_dev->last_pkt_size = pkt_dev->skb->len; > > pkt_dev->allocated_skbs++; > > pkt_dev->clone_count = 0; /* reset counter */ > > + if (pkt_dev->xmit_mode == M_RX_INJECT) > > + pkt_dev->skb->protocol = eth_type_trans(pkt_dev->skb, > > + pkt_dev->skb->dev); > > } > > > > I was just wondering. Since M_RX_INJECT is not compatible with > clone_skb couldn't the lines added above be moved down into the block > below so that you could avoid the additional conditional jump? Sure, that is possible. I'll let Alexei answer, as it is his code. (And repost if he also thinks so...) > > if (pkt_dev->delay && pkt_dev->last_ok) > > spin(pkt_dev, pkt_dev->next_tx); > > > > + if (pkt_dev->xmit_mode == M_RX_INJECT) { > > + skb = pkt_dev->skb; > > + atomic_add(burst, &skb->users); > > + local_bh_disable(); > > + do { > > + ret = netif_receive_skb(skb); > > + if (ret == NET_RX_DROP) > > + pkt_dev->errors++; > > + pkt_dev->sofar++; > > + pkt_dev->seq_num++; > > + if (atomic_read(&skb->users) != burst) { > > + /* skb was queued by rps/rfs or taps, > > + * so cannot reuse this skb > > + */ > > + atomic_sub(burst - 1, &skb->users); > > + /* get out of the loop and wait > > + * until skb is consumed > > + */ > > + pkt_dev->last_ok = 1; > > + break; > > + } > > + /* skb was 'freed' by stack, so clean few > > + * bits and reuse it > > + */ > > +#ifdef CONFIG_NET_CLS_ACT > > + skb->tc_verd = 0; /* reset reclass/redir ttl */ > > +#endif > > + } while (--burst > 0); > > + goto out; /* Skips xmit_mode M_TX */ > > + } > > + > > txq = skb_get_tx_queue(odev, pkt_dev->skb); > > > > local_bh_disable(); > > @@ -3404,6 +3477,7 @@ xmit_more: > > unlock: > > HARD_TX_UNLOCK(odev, txq); > > > > +out: > > local_bh_enable(); > > > > /* If pkt_dev->count is zero, then run forever */ > > > > -- > > To unsubscribe from this list: send the line "unsubscribe netdev" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' 2015-05-06 8:44 ` Jesper Dangaard Brouer @ 2015-05-06 14:35 ` Alexei Starovoitov 2015-05-07 14:34 ` [PATCH v5 0/2] pktgen changes Jesper Dangaard Brouer 0 siblings, 1 reply; 38+ messages in thread From: Alexei Starovoitov @ 2015-05-06 14:35 UTC (permalink / raw) To: Jesper Dangaard Brouer, Alexander Duyck; +Cc: netdev, Eric Dumazet On 5/6/15 1:44 AM, Jesper Dangaard Brouer wrote: >> >> I was just wondering. Since M_RX_INJECT is not compatible with >> clone_skb couldn't the lines added above be moved down into the block >> below so that you could avoid the additional conditional jump? > > Sure, that is possible. I'll let Alexei answer, as it is his code. > (And repost if he also thinks so...) I think this optimization makes sense. Cleans up the code a little too. > We could call it "stack_inject" instead? ... to take out the confusing > "rx" part of an "transmit/xmit" mode that "receives" I would vote for: 'mode netif_receive' + 'mode start_xmit' then if out-of-tree guys would want to upstream their stuff something like 'mode parse_and_consume' can fit. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v5 0/2] pktgen changes 2015-05-06 14:35 ` Alexei Starovoitov @ 2015-05-07 14:34 ` Jesper Dangaard Brouer 2015-05-07 14:34 ` [PATCH v5 1/2] pktgen: adjust flag NO_TIMESTAMP to be more pktgen compliant Jesper Dangaard Brouer ` (2 more replies) 0 siblings, 3 replies; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-07 14:34 UTC (permalink / raw) To: Jesper Dangaard Brouer, netdev; +Cc: Eric Dumazet, Alexei Starovoitov The following series introduce some pktgen changes Patch01: Cleanup my own work when I introduced NO_TIMESTAMP. Patch02: Took over patch from Alexei, and addressed my own concerns, as Alexie is too busy with other work, and this will provide an easy tool for measuring ingress path performance, which is a hot topic ATM. Changes were primarily user interface related. Introduced a separate "xmit_mode" setting, instead of stealing one of the dev flags like Alexei did. --- Alexei Starovoitov (1): pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Jesper Dangaard Brouer (1): pktgen: adjust flag NO_TIMESTAMP to be more pktgen compliant Documentation/networking/pktgen.txt | 9 ++++ net/core/pktgen.c | 85 +++++++++++++++++++++++++++++++++-- 2 files changed, 89 insertions(+), 5 deletions(-) -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v5 1/2] pktgen: adjust flag NO_TIMESTAMP to be more pktgen compliant 2015-05-07 14:34 ` [PATCH v5 0/2] pktgen changes Jesper Dangaard Brouer @ 2015-05-07 14:34 ` Jesper Dangaard Brouer 2015-05-07 14:35 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Jesper Dangaard Brouer 2015-05-10 2:26 ` [PATCH v5 0/2] pktgen changes David Miller 2 siblings, 0 replies; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-07 14:34 UTC (permalink / raw) To: Jesper Dangaard Brouer, netdev; +Cc: Eric Dumazet, Alexei Starovoitov Allow flag NO_TIMESTAMP to turn timestamping on again, like other flags, with a negation of the flag like !NO_TIMESTAMP. Also document the option flag NO_TIMESTAMP. Fixes: afb84b626184 ("pktgen: add flag NO_TIMESTAMP to disable timestamping") Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- Documentation/networking/pktgen.txt | 2 ++ net/core/pktgen.c | 3 +++ 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/Documentation/networking/pktgen.txt b/Documentation/networking/pktgen.txt index 0344f1d..6199ee6 100644 --- a/Documentation/networking/pktgen.txt +++ b/Documentation/networking/pktgen.txt @@ -145,6 +145,7 @@ Examples: UDPCSUM, IPSEC # IPsec encapsulation (needs CONFIG_XFRM) NODE_ALLOC # node specific memory allocation + NO_TIMESTAMP # disable timestamping pgset spi SPI_VALUE Set specific SA used to transform packet. @@ -287,6 +288,7 @@ flag UDPCSUM IPSEC NODE_ALLOC + NO_TIMESTAMP dst_min dst_max diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 508155b..43bb215 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -1267,6 +1267,9 @@ static ssize_t pktgen_if_write(struct file *file, else if (strcmp(f, "NO_TIMESTAMP") == 0) pkt_dev->flags |= F_NO_TIMESTAMP; + else if (strcmp(f, "!NO_TIMESTAMP") == 0) + pkt_dev->flags &= ~F_NO_TIMESTAMP; + else { sprintf(pg_result, "Flag -:%s:- unknown\nAvailable flags, (prepend ! to un-set flag):\n%s", ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' 2015-05-07 14:34 ` [PATCH v5 0/2] pktgen changes Jesper Dangaard Brouer 2015-05-07 14:34 ` [PATCH v5 1/2] pktgen: adjust flag NO_TIMESTAMP to be more pktgen compliant Jesper Dangaard Brouer @ 2015-05-07 14:35 ` Jesper Dangaard Brouer 2015-05-07 16:28 ` Alexei Starovoitov 2015-05-10 2:26 ` [PATCH v5 0/2] pktgen changes David Miller 2 siblings, 1 reply; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-07 14:35 UTC (permalink / raw) To: Jesper Dangaard Brouer, netdev; +Cc: Eric Dumazet, Alexei Starovoitov From: Alexei Starovoitov <ast@plumgrid.com> Introduce xmit_mode 'netif_receive' for pktgen which generates the packets using familiar pktgen commands, but feeds them into netif_receive_skb() instead of ndo_start_xmit(). Default mode is called 'start_xmit'. It is designed to test netif_receive_skb and ingress qdisc performace only. Make sure to understand how it works before using it for other rx benchmarking. Sample script 'pktgen.sh': \#!/bin/bash function pgset() { local result echo $1 > $PGDEV result=`cat $PGDEV | fgrep "Result: OK:"` if [ "$result" = "" ]; then cat $PGDEV | fgrep Result: fi } [ -z "$1" ] && echo "Usage: $0 DEV" && exit 1 ETH=$1 PGDEV=/proc/net/pktgen/kpktgend_0 pgset "rem_device_all" pgset "add_device $ETH" PGDEV=/proc/net/pktgen/$ETH pgset "xmit_mode netif_receive" pgset "pkt_size 60" pgset "dst 198.18.0.1" pgset "dst_mac 90:e2:ba:ff:ff:ff" pgset "count 10000000" pgset "burst 32" PGDEV=/proc/net/pktgen/pgctrl echo "Running... ctrl^C to stop" pgset "start" echo "Done" cat /proc/net/pktgen/$ETH Usage: $ sudo ./pktgen.sh eth2 ... Result: OK: 232376(c232372+d3) usec, 10000000 (60byte,0frags) 43033682pps 20656Mb/sec (20656167360bps) errors: 10000000 Raw netif_receive_skb speed should be ~43 million packet per second on 3.7Ghz x86 and 'perf report' should look like: 37.69% kpktgend_0 [kernel.vmlinux] [k] __netif_receive_skb_core 25.81% kpktgend_0 [kernel.vmlinux] [k] kfree_skb 7.22% kpktgend_0 [kernel.vmlinux] [k] ip_rcv 5.68% kpktgend_0 [pktgen] [k] pktgen_thread_worker If fib_table_lookup is seen on top, it means skb was processed by the stack. To benchmark netif_receive_skb only make sure that 'dst_mac' of your pktgen script is different from receiving device mac and it will be dropped by ip_rcv Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- v4->v5: - Rename xmit_mode's <start_xmit|netif_receive> - Save one branch when calling eth_type_trans(), noticed by Alex Duyck v3->v4: Jesper address his own concerns - User interface intro xmit_mode - Mention xmit_mode in doc - Make sure skb_clone cannot be used together with this mode v2->v3: addressed more Eric comments. Thanks! v1->v2: as suggested by Eric: - dropped 'clone_skb' flag, now it will return enotsupp - fix rps/rfs bug by checking skb->users after every netif_receive_skb - tested with RPS/RFS, taps, veth, physical devs, various tc cls/act --- Documentation/networking/pktgen.txt | 7 +++ net/core/pktgen.c | 82 +++++++++++++++++++++++++++++++++-- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/Documentation/networking/pktgen.txt b/Documentation/networking/pktgen.txt index 6199ee6..747facc 100644 --- a/Documentation/networking/pktgen.txt +++ b/Documentation/networking/pktgen.txt @@ -193,6 +193,10 @@ Examples: pgset "rate 300M" set rate to 300 Mb/s pgset "ratep 1000000" set rate to 1Mpps + pgset "xmit_mode netif_receive" RX inject into stack netif_receive_skb() + Works with "burst" but not with "clone_skb". + Default xmit_mode is "start_xmit". + Sample scripts ============== @@ -310,6 +314,9 @@ flowlen rate ratep +xmit_mode <start_xmit|netif_receive> + + References: ftp://robur.slu.se/pub/Linux/net-development/pktgen-testing/ ftp://robur.slu.se/pub/Linux/net-development/pktgen-testing/examples/ diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 43bb215..8f2687d 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -210,6 +210,10 @@ #define T_REMDEVALL (1<<2) /* Remove all devs */ #define T_REMDEV (1<<3) /* Remove one dev */ +/* Xmit modes */ +#define M_START_XMIT 0 /* Default normal TX */ +#define M_NETIF_RECEIVE 1 /* Inject packets into stack */ + /* If lock -- protects updating of if_list */ #define if_lock(t) spin_lock(&(t->if_lock)); #define if_unlock(t) spin_unlock(&(t->if_lock)); @@ -251,13 +255,14 @@ struct pktgen_dev { * we will do a random selection from within the range. */ __u32 flags; - int removal_mark; /* non-zero => the device is marked for - * removal by worker thread */ - + int xmit_mode; int min_pkt_size; int max_pkt_size; int pkt_overhead; /* overhead for MPLS, VLANs, IPSEC etc */ int nfrags; + int removal_mark; /* non-zero => the device is marked for + * removal by worker thread */ + struct page *page; u64 delay; /* nano-seconds */ @@ -620,6 +625,9 @@ static int pktgen_if_show(struct seq_file *seq, void *v) if (pkt_dev->node >= 0) seq_printf(seq, " node: %d\n", pkt_dev->node); + if (pkt_dev->xmit_mode == M_NETIF_RECEIVE) + seq_puts(seq, " xmit_mode: netif_receive\n"); + seq_puts(seq, " Flags: "); if (pkt_dev->flags & F_IPV6) @@ -1081,7 +1089,8 @@ static ssize_t pktgen_if_write(struct file *file, if (len < 0) return len; if ((value > 0) && - (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) + ((pkt_dev->xmit_mode == M_NETIF_RECEIVE) || + !(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) return -ENOTSUPP; i += len; pkt_dev->clone_skb = value; @@ -1134,7 +1143,7 @@ static ssize_t pktgen_if_write(struct file *file, return len; i += len; - if ((value > 1) && + if ((value > 1) && (pkt_dev->xmit_mode == M_START_XMIT) && (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) return -ENOTSUPP; pkt_dev->burst = value < 1 ? 1 : value; @@ -1160,6 +1169,35 @@ static ssize_t pktgen_if_write(struct file *file, sprintf(pg_result, "ERROR: node not possible"); return count; } + if (!strcmp(name, "xmit_mode")) { + char f[32]; + + memset(f, 0, 32); + len = strn_len(&user_buffer[i], sizeof(f) - 1); + if (len < 0) + return len; + + if (copy_from_user(f, &user_buffer[i], len)) + return -EFAULT; + i += len; + + if (strcmp(f, "start_xmit") == 0) { + pkt_dev->xmit_mode = M_START_XMIT; + } else if (strcmp(f, "netif_receive") == 0) { + /* clone_skb set earlier, not supported in this mode */ + if (pkt_dev->clone_skb > 0) + return -ENOTSUPP; + + pkt_dev->xmit_mode = M_NETIF_RECEIVE; + } else { + sprintf(pg_result, + "xmit_mode -:%s:- unknown\nAvailable modes: %s", + f, "start_xmit, netif_receive\n"); + return count; + } + sprintf(pg_result, "OK: xmit_mode=%s", f); + return count; + } if (!strcmp(name, "flag")) { char f[32]; memset(f, 0, 32); @@ -3320,6 +3358,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) unsigned int burst = ACCESS_ONCE(pkt_dev->burst); struct net_device *odev = pkt_dev->odev; struct netdev_queue *txq; + struct sk_buff *skb; int ret; /* If device is offline, then don't send */ @@ -3357,6 +3396,38 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) if (pkt_dev->delay && pkt_dev->last_ok) spin(pkt_dev, pkt_dev->next_tx); + if (pkt_dev->xmit_mode == M_NETIF_RECEIVE) { + skb = pkt_dev->skb; + skb->protocol = eth_type_trans(skb, skb->dev); + atomic_add(burst, &skb->users); + local_bh_disable(); + do { + ret = netif_receive_skb(skb); + if (ret == NET_RX_DROP) + pkt_dev->errors++; + pkt_dev->sofar++; + pkt_dev->seq_num++; + if (atomic_read(&skb->users) != burst) { + /* skb was queued by rps/rfs or taps, + * so cannot reuse this skb + */ + atomic_sub(burst - 1, &skb->users); + /* get out of the loop and wait + * until skb is consumed + */ + pkt_dev->last_ok = 1; + break; + } + /* skb was 'freed' by stack, so clean few + * bits and reuse it + */ +#ifdef CONFIG_NET_CLS_ACT + skb->tc_verd = 0; /* reset reclass/redir ttl */ +#endif + } while (--burst > 0); + goto out; /* Skips xmit_mode M_START_XMIT */ + } + txq = skb_get_tx_queue(odev, pkt_dev->skb); local_bh_disable(); @@ -3404,6 +3475,7 @@ xmit_more: unlock: HARD_TX_UNLOCK(odev, txq); +out: local_bh_enable(); /* If pkt_dev->count is zero, then run forever */ ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' 2015-05-07 14:35 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Jesper Dangaard Brouer @ 2015-05-07 16:28 ` Alexei Starovoitov 2015-05-07 17:11 ` Daniel Borkmann 0 siblings, 1 reply; 38+ messages in thread From: Alexei Starovoitov @ 2015-05-07 16:28 UTC (permalink / raw) To: Jesper Dangaard Brouer, netdev; +Cc: Eric Dumazet On 5/7/15 7:35 AM, Jesper Dangaard Brouer wrote: > From: Alexei Starovoitov <ast@plumgrid.com> > > Introduce xmit_mode 'netif_receive' for pktgen which generates the > packets using familiar pktgen commands, but feeds them into > netif_receive_skb() instead of ndo_start_xmit(). > > Default mode is called 'start_xmit'. ... > Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > > --- > v4->v5: > - Rename xmit_mode's <start_xmit|netif_receive> > - Save one branch when calling eth_type_trans(), noticed by Alex Duyck looks good to me. Thanks a lot. btw, I've started to work on a patch on top of this one that allows multiple pktgen threads to submit into the same netdev. I've used it to stress test removal of spin_lock in ingress qdisc. The idea is to add another 'name' parameter to command: 'add_device name dev' 'name' will be used to identify this pktgen thread in /proc and 'dev' used as target net_device. I think it will be useful for start_xmit testing as well. I wonder why it wasn't done earlier? The queue configuration is already supported. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' 2015-05-07 16:28 ` Alexei Starovoitov @ 2015-05-07 17:11 ` Daniel Borkmann 2015-05-07 17:16 ` Alexei Starovoitov ` (2 more replies) 0 siblings, 3 replies; 38+ messages in thread From: Daniel Borkmann @ 2015-05-07 17:11 UTC (permalink / raw) To: Alexei Starovoitov; +Cc: Jesper Dangaard Brouer, netdev, Eric Dumazet On 05/07/2015 06:28 PM, Alexei Starovoitov wrote: > On 5/7/15 7:35 AM, Jesper Dangaard Brouer wrote: >> From: Alexei Starovoitov <ast@plumgrid.com> >> >> Introduce xmit_mode 'netif_receive' for pktgen which generates the >> packets using familiar pktgen commands, but feeds them into >> netif_receive_skb() instead of ndo_start_xmit(). >> >> Default mode is called 'start_xmit'. > ... >> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> >> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> >> >> --- >> v4->v5: >> - Rename xmit_mode's <start_xmit|netif_receive> >> - Save one branch when calling eth_type_trans(), noticed by Alex Duyck > > looks good to me. Thanks a lot. > > btw, I've started to work on a patch on top of this one that allows > multiple pktgen threads to submit into the same netdev. > I've used it to stress test removal of spin_lock in ingress qdisc. > The idea is to add another 'name' parameter to command: > 'add_device name dev' > 'name' will be used to identify this pktgen thread in /proc > and 'dev' used as target net_device. > I think it will be useful for start_xmit testing as well. > I wonder why it wasn't done earlier? The queue configuration is > already supported. You mean other than below commit (iow independant of queue mapping)? commit e6fce5b916cd7f7f79b2b3e53ba74bbfc1d7cf8b Author: Robert Olsson <robert.olsson@its.uu.se> Date: Thu Aug 7 02:23:01 2008 -0700 pktgen: multiqueue etc. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' 2015-05-07 17:11 ` Daniel Borkmann @ 2015-05-07 17:16 ` Alexei Starovoitov 2015-05-07 17:20 ` Daniel Borkmann 2015-05-08 13:40 ` Multiqueue pktgen (was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') Jesper Dangaard Brouer 2015-05-08 15:39 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Jesper Dangaard Brouer 2 siblings, 1 reply; 38+ messages in thread From: Alexei Starovoitov @ 2015-05-07 17:16 UTC (permalink / raw) To: Daniel Borkmann; +Cc: Jesper Dangaard Brouer, netdev, Eric Dumazet On 5/7/15 10:11 AM, Daniel Borkmann wrote: > > You mean other than below commit (iow independant of queue mapping)? > > commit e6fce5b916cd7f7f79b2b3e53ba74bbfc1d7cf8b > Author: Robert Olsson <robert.olsson@its.uu.se> > Date: Thu Aug 7 02:23:01 2008 -0700 > > pktgen: multiqueue etc. ahh, I felt that I'm missing something ;) Too bad it's not documented. I couldn't figure that out from sources. Thanks! will give it a shot. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' 2015-05-07 17:16 ` Alexei Starovoitov @ 2015-05-07 17:20 ` Daniel Borkmann 0 siblings, 0 replies; 38+ messages in thread From: Daniel Borkmann @ 2015-05-07 17:20 UTC (permalink / raw) To: Alexei Starovoitov; +Cc: Jesper Dangaard Brouer, netdev, Eric Dumazet On 05/07/2015 07:16 PM, Alexei Starovoitov wrote: > On 5/7/15 10:11 AM, Daniel Borkmann wrote: >> >> You mean other than below commit (iow independant of queue mapping)? >> >> commit e6fce5b916cd7f7f79b2b3e53ba74bbfc1d7cf8b >> Author: Robert Olsson <robert.olsson@its.uu.se> >> Date: Thu Aug 7 02:23:01 2008 -0700 >> >> pktgen: multiqueue etc. > > ahh, I felt that I'm missing something ;) > Too bad it's not documented. I couldn't figure that out from sources. > Thanks! will give it a shot. Ok, in any case, I think it would be good if there's some documentation for that as well. :) ^ permalink raw reply [flat|nested] 38+ messages in thread
* Multiqueue pktgen (was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') 2015-05-07 17:11 ` Daniel Borkmann 2015-05-07 17:16 ` Alexei Starovoitov @ 2015-05-08 13:40 ` Jesper Dangaard Brouer 2015-05-08 15:39 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Jesper Dangaard Brouer 2 siblings, 0 replies; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-08 13:40 UTC (permalink / raw) To: Daniel Borkmann; +Cc: Alexei Starovoitov, Eric Dumazet, brouer On Thu, 07 May 2015 19:11:58 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote: > On 05/07/2015 06:28 PM, Alexei Starovoitov wrote: > > On 5/7/15 7:35 AM, Jesper Dangaard Brouer wrote: > >> From: Alexei Starovoitov <ast@plumgrid.com> > >> [...snip...] > > btw, I've started to work on a patch on top of this one that allows > > multiple pktgen threads to submit into the same netdev. > > I've used it to stress test removal of spin_lock in ingress qdisc. > > The idea is to add another 'name' parameter to command: > > 'add_device name dev' > > 'name' will be used to identify this pktgen thread in /proc > > and 'dev' used as target net_device. > > I think it will be useful for start_xmit testing as well. > > I wonder why it wasn't done earlier? The queue configuration is > > already supported. > > You mean other than below commit (iow independant of queue mapping)? > > commit e6fce5b916cd7f7f79b2b3e53ba74bbfc1d7cf8b > Author: Robert Olsson <robert.olsson@its.uu.se> > Date: Thu Aug 7 02:23:01 2008 -0700 > > pktgen: multiqueue etc. For completeness and others reading this threads... Pktgen multiqueue is already supported via mentioned commit, which adds the device naming scheme: "add_device dev@number" And yes, the documentation does not seem to mention this. I've been using it for years now... My scripts[1] take param "-t" for "threads". I've added a more plain version of a script, based on yours, below my signature. The funny thing now is that scaling does not "happen" as we stall on: atomic_long_inc(&skb->dev->rx_dropped); [1] https://github.com/netoptimizer/network-testing/tree/master/pktgen - - Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer multiqueue pktgen script: #!/bin/bash function pgset() { local result echo $1 > $PGDEV result=`cat $PGDEV | fgrep "Result: OK:"` if [ "$result" = "" ]; then cat $PGDEV | fgrep Result: fi } [ -z "$2" ] && echo "Usage: $0 DEV num_threads" && exit 1 ETH=$1 NUM_THREADS=$2 let "NUM_THREADS -= 1" echo "Number of threads to start: $2 (0 to $NUM_THREADS)" # General cleanup everything since last run PGDEV=/proc/net/pktgen/pgctrl pgset "reset" # Add devices to threads # Notice the naming scheme ETH@NUM for NUM in `seq 0 $NUM_THREADS`; do PGDEV=/proc/net/pktgen/kpktgend_${NUM} pgset "rem_device_all" pgset "add_device ${ETH}@${NUM}" done # Config each device for NUM in `seq 0 $NUM_THREADS`; do PGDEV=/proc/net/pktgen/${ETH}@${NUM} pgset "flag QUEUE_MAP_CPU" pgset "xmit_mode netif_receive" pgset "pkt_size 60" pgset "dst 198.18.0.42" pgset "dst_mac 90:e2:ba:ff:ff:ff" pgset "count 10000000" pgset "burst 32" done PGDEV=/proc/net/pktgen/pgctrl echo "Running... ctrl^C to stop" pgset "start" echo "Done" for NUM in `seq 0 $NUM_THREADS`; do echo "Device: ${ETH}@${NUM}" cat /proc/net/pktgen/${ETH}@${NUM} | grep -A2 "Result:" done ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' 2015-05-07 17:11 ` Daniel Borkmann 2015-05-07 17:16 ` Alexei Starovoitov 2015-05-08 13:40 ` Multiqueue pktgen (was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') Jesper Dangaard Brouer @ 2015-05-08 15:39 ` Jesper Dangaard Brouer 2015-05-08 15:49 ` Multiqueue pktgen and ingress path (Was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') Jesper Dangaard Brouer 2015-05-08 15:57 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Eric Dumazet 2 siblings, 2 replies; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-08 15:39 UTC (permalink / raw) To: Daniel Borkmann; +Cc: Alexei Starovoitov, netdev, Eric Dumazet, brouer On Thu, 07 May 2015 19:11:58 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote: > On 05/07/2015 06:28 PM, Alexei Starovoitov wrote: > > On 5/7/15 7:35 AM, Jesper Dangaard Brouer wrote: > >> From: Alexei Starovoitov <ast@plumgrid.com> > >> [...snip...] > > btw, I've started to work on a patch on top of this one that allows > > multiple pktgen threads to submit into the same netdev. > > I've used it to stress test removal of spin_lock in ingress qdisc. > > The idea is to add another 'name' parameter to command: > > 'add_device name dev' > > 'name' will be used to identify this pktgen thread in /proc > > and 'dev' used as target net_device. > > I think it will be useful for start_xmit testing as well. > > I wonder why it wasn't done earlier? The queue configuration is > > already supported. > > You mean other than below commit (iow independant of queue mapping)? > > commit e6fce5b916cd7f7f79b2b3e53ba74bbfc1d7cf8b > Author: Robert Olsson <robert.olsson@its.uu.se> > Date: Thu Aug 7 02:23:01 2008 -0700 > > pktgen: multiqueue etc. For completeness and others reading this threads... Pktgen multiqueue is already supported via mentioned commit, which adds the device naming scheme: "add_device dev@number" And yes, the documentation does not seem to mention this. I've been using it for years now... My scripts[1] take param "-t" for "threads". I've added a more plain version of a script, based on yours, below my signature. The funny thing now is that scaling does not "happen" as we stall on: atomic_long_inc(&skb->dev->rx_dropped); [1] https://github.com/netoptimizer/network-testing/tree/master/pktgen - - Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer multiqueue pktgen script: #!/bin/bash function pgset() { local result echo $1 > $PGDEV result=`cat $PGDEV | fgrep "Result: OK:"` if [ "$result" = "" ]; then cat $PGDEV | fgrep Result: fi } [ -z "$2" ] && echo "Usage: $0 DEV num_threads" && exit 1 ETH=$1 NUM_THREADS=$2 let "NUM_THREADS -= 1" echo "Number of threads to start: $2 (0 to $NUM_THREADS)" # General cleanup everything since last run PGDEV=/proc/net/pktgen/pgctrl pgset "reset" # Add devices to threads # Notice the naming scheme ETH@NUM for NUM in `seq 0 $NUM_THREADS`; do PGDEV=/proc/net/pktgen/kpktgend_${NUM} pgset "rem_device_all" pgset "add_device ${ETH}@${NUM}" done # Config each device for NUM in `seq 0 $NUM_THREADS`; do PGDEV=/proc/net/pktgen/${ETH}@${NUM} pgset "flag QUEUE_MAP_CPU" pgset "xmit_mode netif_receive" pgset "pkt_size 60" pgset "dst 198.18.0.42" pgset "dst_mac 90:e2:ba:ff:ff:ff" pgset "count 10000000" pgset "burst 32" done PGDEV=/proc/net/pktgen/pgctrl echo "Running... ctrl^C to stop" pgset "start" echo "Done" for NUM in `seq 0 $NUM_THREADS`; do echo "Device: ${ETH}@${NUM}" cat /proc/net/pktgen/${ETH}@${NUM} | grep -A2 "Result:" done ^ permalink raw reply [flat|nested] 38+ messages in thread
* Multiqueue pktgen and ingress path (Was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') 2015-05-08 15:39 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Jesper Dangaard Brouer @ 2015-05-08 15:49 ` Jesper Dangaard Brouer 2015-05-08 15:56 ` Eric Dumazet ` (2 more replies) 2015-05-08 15:57 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Eric Dumazet 1 sibling, 3 replies; 38+ messages in thread From: Jesper Dangaard Brouer @ 2015-05-08 15:49 UTC (permalink / raw) To: Daniel Borkmann; +Cc: Alexei Starovoitov, netdev, Eric Dumazet, brouer [-- Attachment #1: Type: text/plain, Size: 2772 bytes --] On Fri, 8 May 2015 17:39:00 +0200 Jesper Dangaard Brouer <brouer@redhat.com> wrote: > On Thu, 07 May 2015 19:11:58 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote: > > > On 05/07/2015 06:28 PM, Alexei Starovoitov wrote: > > > On 5/7/15 7:35 AM, Jesper Dangaard Brouer wrote: > > >> From: Alexei Starovoitov <ast@plumgrid.com> > > >> > [...snip...] > > > > btw, I've started to work on a patch on top of this one that allows > > > multiple pktgen threads to submit into the same netdev. > > > I've used it to stress test removal of spin_lock in ingress qdisc. > > > The idea is to add another 'name' parameter to command: > > > 'add_device name dev' > > > 'name' will be used to identify this pktgen thread in /proc > > > and 'dev' used as target net_device. > > > I think it will be useful for start_xmit testing as well. > > > I wonder why it wasn't done earlier? The queue configuration is > > > already supported. > > > > You mean other than below commit (iow independant of queue mapping)? > > > > commit e6fce5b916cd7f7f79b2b3e53ba74bbfc1d7cf8b > > Author: Robert Olsson <robert.olsson@its.uu.se> > > Date: Thu Aug 7 02:23:01 2008 -0700 > > > > pktgen: multiqueue etc. > > For completeness and others reading this threads... > > Pktgen multiqueue is already supported via mentioned commit, which adds > the device naming scheme: "add_device dev@number" > > And yes, the documentation does not seem to mention this. I've been > using it for years now... My scripts[1] take param "-t" for "threads". > [1] https://github.com/netoptimizer/network-testing/tree/master/pktgen > > I've added a more plain version of a script, based on yours, below my > signature. Now attached. > The funny thing now is that scaling does not "happen" as we stall on: > atomic_long_inc(&skb->dev->rx_dropped); More interesting observations with the mentioned script (now attached). On my system the scaling stopped a 24Mpps, when I increased the number of threads the collective scaling was stuck at 24Mpps. Then I simply removed/compiled-out the: atomic_long_inc(&skb->dev->rx_dropped); And after that change, the scaling is basically infinite/perfect. Single thread performance increased from 24.7Mpps to 31.1Mpps, which corresponds perfectly with the cost of an atomic operation on this HW (8.25ns). Diff to before: * (1/24700988*10^9)-(1/31170819*10^9) = 8.40292328196 ns When increasing the threads now, they all basically run at 31Mpps. Tried it upto 12 threads. I'm quite puzzled why a single atomic op could "freeze" my system from scaling beyond 24Mpps. -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer [-- Attachment #2: pktgen_multiqueue01_xmit_mode.sh --] [-- Type: application/x-shellscript, Size: 1217 bytes --] ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: Multiqueue pktgen and ingress path (Was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') 2015-05-08 15:49 ` Multiqueue pktgen and ingress path (Was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') Jesper Dangaard Brouer @ 2015-05-08 15:56 ` Eric Dumazet 2015-05-08 16:53 ` Alexander Duyck 2015-05-08 17:00 ` Alexei Starovoitov 2 siblings, 0 replies; 38+ messages in thread From: Eric Dumazet @ 2015-05-08 15:56 UTC (permalink / raw) To: Jesper Dangaard Brouer; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev On Fri, 2015-05-08 at 17:49 +0200, Jesper Dangaard Brouer wrote: > I'm quite puzzled why a single atomic op could "freeze" my system from > scaling beyond 24Mpps. Law of physics, since this cache line has to constantly fly among your cpus. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: Multiqueue pktgen and ingress path (Was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') 2015-05-08 15:49 ` Multiqueue pktgen and ingress path (Was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') Jesper Dangaard Brouer 2015-05-08 15:56 ` Eric Dumazet @ 2015-05-08 16:53 ` Alexander Duyck 2015-05-08 17:00 ` Alexei Starovoitov 2 siblings, 0 replies; 38+ messages in thread From: Alexander Duyck @ 2015-05-08 16:53 UTC (permalink / raw) To: Jesper Dangaard Brouer, Daniel Borkmann Cc: Alexei Starovoitov, netdev, Eric Dumazet On 05/08/2015 08:49 AM, Jesper Dangaard Brouer wrote: > More interesting observations with the mentioned script (now attached). > > On my system the scaling stopped a 24Mpps, when I increased the number > of threads the collective scaling was stuck at 24Mpps. > > Then I simply removed/compiled-out the: > atomic_long_inc(&skb->dev->rx_dropped); > > And after that change, the scaling is basically infinite/perfect. > > Single thread performance increased from 24.7Mpps to 31.1Mpps, which > corresponds perfectly with the cost of an atomic operation on this HW > (8.25ns). > > Diff to before: > * (1/24700988*10^9)-(1/31170819*10^9) = 8.40292328196 ns > > When increasing the threads now, they all basically run at 31Mpps. > Tried it upto 12 threads. > > > I'm quite puzzled why a single atomic op could "freeze" my system from > scaling beyond 24Mpps. The atomic access likely acts as a serializing event, and on top of that it would increase in time needed to be completed as you add more threads. I am guessing the 8ns is probably the cost for a single threaded setup where the memory location is available in L2 or L1 cache. If it is in L3 cache that would make it more expensive. If it is currently in use by another CPU then that would make it even more expensive. If it is in use on another socket then we are probably looking at something in the high 10s if not 100s of nanoseconds. Once you hit the point where the time for the atomic transaction multiplied by the number of threads is equal to the time it takes for any one thread to complete the operation you have hit the upper limit and everything after that is just wasted cycles spinning while waiting for cache line access. So for example if you had 2 threads on the same socket you are looking at an L3 cache access which takes about 30 cycles. That 30 cycles would likely be in addition to the 8ns you were already seeing for single thread performance, and I don't know if it includes the cache flush needed by the remote L1/L2 where the cache line currently resides. I'd be interested in seeing what the 2 socket data looked like as I suspect you would take an even heavier hit for that. - Alex ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: Multiqueue pktgen and ingress path (Was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') 2015-05-08 15:49 ` Multiqueue pktgen and ingress path (Was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') Jesper Dangaard Brouer 2015-05-08 15:56 ` Eric Dumazet 2015-05-08 16:53 ` Alexander Duyck @ 2015-05-08 17:00 ` Alexei Starovoitov 2015-05-08 18:21 ` Alexander Duyck 2 siblings, 1 reply; 38+ messages in thread From: Alexei Starovoitov @ 2015-05-08 17:00 UTC (permalink / raw) To: Jesper Dangaard Brouer, Daniel Borkmann; +Cc: netdev, Eric Dumazet On 5/8/15 8:49 AM, Jesper Dangaard Brouer wrote: >> >> I've added a more plain version of a script, based on yours, below my >> signature. > > Now attached. thanks for the script! Really useful. Could you add it to samples/pktgen/ and remove useless and confusing pktgen.conf-2-1 ? >> The funny thing now is that scaling does not "happen" as we stall on: >> atomic_long_inc(&skb->dev->rx_dropped); > > More interesting observations with the mentioned script (now attached). > > On my system the scaling stopped a 24Mpps, when I increased the number > of threads the collective scaling was stuck at 24Mpps. what was your config to start hitting that drop counter? We can convert it to per_cpu, but I'm not sure it's worth doing. If I send normal ip packets they don't go this path. Only unknown protocol packets suppose to hit it ? ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: Multiqueue pktgen and ingress path (Was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') 2015-05-08 17:00 ` Alexei Starovoitov @ 2015-05-08 18:21 ` Alexander Duyck 0 siblings, 0 replies; 38+ messages in thread From: Alexander Duyck @ 2015-05-08 18:21 UTC (permalink / raw) To: Alexei Starovoitov, Jesper Dangaard Brouer, Daniel Borkmann Cc: netdev, Eric Dumazet On 05/08/2015 10:00 AM, Alexei Starovoitov wrote: > On 5/8/15 8:49 AM, Jesper Dangaard Brouer wrote: >>> >>> I've added a more plain version of a script, based on yours, below my >>> signature. >> >> Now attached. > > thanks for the script! Really useful. > Could you add it to samples/pktgen/ and remove useless and confusing > pktgen.conf-2-1 ? > >>> The funny thing now is that scaling does not "happen" as we stall on: >>> atomic_long_inc(&skb->dev->rx_dropped); >> >> More interesting observations with the mentioned script (now attached). >> >> On my system the scaling stopped a 24Mpps, when I increased the number >> of threads the collective scaling was stuck at 24Mpps. > > what was your config to start hitting that drop counter? > We can convert it to per_cpu, but I'm not sure it's worth doing. > If I send normal ip packets they don't go this path. Only > unknown protocol packets suppose to hit it ? I'm assuming it just has to be a packet that isn't claimed by any sockets or interfaces registered on top of the device. After all it isn't as if pktgen sends an unknown protocol so I would assume just enabling promiscuous mode on an interface without a bridge or raw socket would probably be enough to trigger this. The overhead itself would show up in __netif_receive_skb_core in perf since the atomic_inc would be inlined. I would think a common case where something like this might be seen would be if you registered enough macvlan interfaces to force a device to switch into promiscuous mode and then received traffic that wasn't meant for any of the other interfaces. - Alex ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' 2015-05-08 15:39 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Jesper Dangaard Brouer 2015-05-08 15:49 ` Multiqueue pktgen and ingress path (Was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') Jesper Dangaard Brouer @ 2015-05-08 15:57 ` Eric Dumazet 2015-05-08 16:50 ` Alexei Starovoitov 1 sibling, 1 reply; 38+ messages in thread From: Eric Dumazet @ 2015-05-08 15:57 UTC (permalink / raw) To: Jesper Dangaard Brouer; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev On Fri, 2015-05-08 at 17:39 +0200, Jesper Dangaard Brouer wrote: > The funny thing now is that scaling does not "happen" as we stall on: > atomic_long_inc(&skb->dev->rx_dropped); Note we already have percpu device refcnt. We could extend this to contain a per cpu rx_dropped, but I guess only these tests hit this path, so I simply used one atomic_long_t here. I guess an ingress action can do better, by stealing the packet before hitting this point. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' 2015-05-08 15:57 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Eric Dumazet @ 2015-05-08 16:50 ` Alexei Starovoitov 0 siblings, 0 replies; 38+ messages in thread From: Alexei Starovoitov @ 2015-05-08 16:50 UTC (permalink / raw) To: Eric Dumazet, Jesper Dangaard Brouer; +Cc: Daniel Borkmann, netdev On 5/8/15 8:57 AM, Eric Dumazet wrote: > > I guess an ingress action can do better, by stealing the packet before > hitting this point. unfortunately not yet :( We have spin_lock(&gact->tcf_lock); in tcf_gact, so simple 'action drop' doesn't scale. Shouldn't be hard to fix though. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v5 0/2] pktgen changes 2015-05-07 14:34 ` [PATCH v5 0/2] pktgen changes Jesper Dangaard Brouer 2015-05-07 14:34 ` [PATCH v5 1/2] pktgen: adjust flag NO_TIMESTAMP to be more pktgen compliant Jesper Dangaard Brouer 2015-05-07 14:35 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Jesper Dangaard Brouer @ 2015-05-10 2:26 ` David Miller 2 siblings, 0 replies; 38+ messages in thread From: David Miller @ 2015-05-10 2:26 UTC (permalink / raw) To: brouer; +Cc: netdev, eric.dumazet, ast From: Jesper Dangaard Brouer <brouer@redhat.com> Date: Thu, 07 May 2015 16:34:29 +0200 > The following series introduce some pktgen changes > > Patch01: > Cleanup my own work when I introduced NO_TIMESTAMP. > > Patch02: > Took over patch from Alexei, and addressed my own concerns, as Alexie > is too busy with other work, and this will provide an easy tool for > measuring ingress path performance, which is a hot topic ATM. > > Changes were primarily user interface related. Introduced a separate > "xmit_mode" setting, instead of stealing one of the dev flags like > Alexei did. Series applied, thanks Jesper. ^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2015-05-10 2:26 UTC | newest] Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-05-01 5:12 [PATCH v3 net-next] pktgen: introduce 'rx' mode Alexei Starovoitov 2015-05-01 16:54 ` Eric Dumazet 2015-05-02 8:46 ` Jesper Dangaard Brouer 2015-05-02 9:44 ` Daniel Borkmann 2015-05-02 9:54 ` Jesper Dangaard Brouer 2015-05-02 10:30 ` Daniel Borkmann 2015-05-02 16:01 ` Alexei Starovoitov 2015-05-02 16:46 ` Jesper Dangaard Brouer 2015-05-02 17:07 ` Alexei Starovoitov 2015-05-05 18:15 ` Jesper Dangaard Brouer 2015-05-05 18:30 ` Alexei Starovoitov 2015-05-05 20:29 ` [PATCH 0/2] pktgen changes Jesper Dangaard Brouer 2015-05-05 20:29 ` [PATCH 1/2] pktgen: adjust flag NO_TIMESTAMP to be more pktgen compliant Jesper Dangaard Brouer 2015-05-05 20:30 ` [PATCH 2/2] pktgen: introduce xmit_mode 'rx_inject' Jesper Dangaard Brouer 2015-05-06 4:33 ` Alexei Starovoitov 2015-05-06 5:24 ` Jesper Dangaard Brouer 2015-05-06 10:17 ` Daniel Borkmann 2015-05-06 11:22 ` Jesper Dangaard Brouer 2015-05-06 5:32 ` Alexander Duyck 2015-05-06 8:44 ` Jesper Dangaard Brouer 2015-05-06 14:35 ` Alexei Starovoitov 2015-05-07 14:34 ` [PATCH v5 0/2] pktgen changes Jesper Dangaard Brouer 2015-05-07 14:34 ` [PATCH v5 1/2] pktgen: adjust flag NO_TIMESTAMP to be more pktgen compliant Jesper Dangaard Brouer 2015-05-07 14:35 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Jesper Dangaard Brouer 2015-05-07 16:28 ` Alexei Starovoitov 2015-05-07 17:11 ` Daniel Borkmann 2015-05-07 17:16 ` Alexei Starovoitov 2015-05-07 17:20 ` Daniel Borkmann 2015-05-08 13:40 ` Multiqueue pktgen (was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') Jesper Dangaard Brouer 2015-05-08 15:39 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Jesper Dangaard Brouer 2015-05-08 15:49 ` Multiqueue pktgen and ingress path (Was: [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>') Jesper Dangaard Brouer 2015-05-08 15:56 ` Eric Dumazet 2015-05-08 16:53 ` Alexander Duyck 2015-05-08 17:00 ` Alexei Starovoitov 2015-05-08 18:21 ` Alexander Duyck 2015-05-08 15:57 ` [PATCH v5 2/2] pktgen: introduce xmit_mode '<start_xmit|netif_receive>' Eric Dumazet 2015-05-08 16:50 ` Alexei Starovoitov 2015-05-10 2:26 ` [PATCH v5 0/2] pktgen changes David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).