| Message ID | 20160629200306.16491.65359.stgit@john-Precision-Tower-5810 |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 29 Jun 2016 13:03:06 -0700 John Fastabend <john.fastabend@gmail.com> wrote: > Add another xmit_mode to pktgen to allow testing xmit functionality > of qdiscs. The new mode "queue_xmit" injects packets at > __dev_queue_xmit() so that qdisc is called. > > Signed-off-by: John Fastabend <john.r.fastabend@intel.com> I generally like this. > net/core/pktgen.c | 42 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 2 deletions(-) > > diff --git a/net/core/pktgen.c b/net/core/pktgen.c > index f74ab9c..4b3d467 100644 > --- a/net/core/pktgen.c > +++ b/net/core/pktgen.c > @@ -213,6 +213,7 @@ > /* Xmit modes */ > #define M_START_XMIT 0 /* Default normal TX */ > #define M_NETIF_RECEIVE 1 /* Inject packets into stack */ > +#define M_QUEUE_XMIT 2 /* Inject packet into qdisc */ > > /* If lock -- protects updating of if_list */ > #define if_lock(t) spin_lock(&(t->if_lock)); > @@ -626,6 +627,8 @@ static int pktgen_if_show(struct seq_file *seq, void *v) > > if (pkt_dev->xmit_mode == M_NETIF_RECEIVE) > seq_puts(seq, " xmit_mode: netif_receive\n"); > + else if (pkt_dev->xmit_mode == M_QUEUE_XMIT) > + seq_puts(seq, " xmit_mode: xmit_queue\n"); > > seq_puts(seq, " Flags: "); > > @@ -1142,8 +1145,10 @@ static ssize_t pktgen_if_write(struct file *file, > return len; > > i += len; > - if ((value > 1) && (pkt_dev->xmit_mode == M_START_XMIT) && > - (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) > + if ((value > 1) && > + ((pkt_dev->xmit_mode == M_QUEUE_XMIT) || > + ((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; > sprintf(pg_result, "OK: burst=%d", pkt_dev->burst); > @@ -1198,6 +1203,9 @@ static ssize_t pktgen_if_write(struct file *file, > * at module loading time > */ > pkt_dev->clone_skb = 0; > + } else if (strcmp(f, "queue_xmit") == 0) { > + pkt_dev->xmit_mode = M_QUEUE_XMIT; > + pkt_dev->last_ok = 1; > } else { > sprintf(pg_result, > "xmit_mode -:%s:- unknown\nAvailable modes: %s", > @@ -3434,6 +3442,36 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) > #endif > } while (--burst > 0); > goto out; /* Skips xmit_mode M_START_XMIT */ > + } else if (pkt_dev->xmit_mode == M_QUEUE_XMIT) { > + local_bh_disable(); > + atomic_add(burst, &pkt_dev->skb->users); Reading the code, people might think that "burst" is allowed for this mode, which it is not. (You do handle this earlier in this patch when configuring this mode). > + ret = dev_queue_xmit(pkt_dev->skb); > + switch (ret) { > + case NET_XMIT_SUCCESS: > + pkt_dev->sofar++; > + pkt_dev->seq_num++; > + pkt_dev->tx_bytes += pkt_dev->last_pkt_size; > + break; > + case NET_XMIT_DROP: > + case NET_XMIT_CN: > + /* These are all valid return codes for a qdisc but > + * indicate packets are being dropped or will likely > + * be dropped soon. > + */ > + case NETDEV_TX_BUSY: > + /* qdisc may call dev_hard_start_xmit directly in cases > + * where no queues exist e.g. loopback device, virtual > + * devices, etc. In this case we need to handle > + * NETDEV_TX_ codes. > + */ > + default: > + pkt_dev->errors++; > + net_info_ratelimited("%s xmit error: %d\n", > + pkt_dev->odevname, ret); > + break; > + } > + goto out; > } > > txq = skb_get_tx_queue(odev, pkt_dev->skb); >
On 16-06-30 01:37 AM, Jesper Dangaard Brouer wrote: > On Wed, 29 Jun 2016 13:03:06 -0700 > John Fastabend <john.fastabend@gmail.com> wrote: > >> Add another xmit_mode to pktgen to allow testing xmit functionality >> of qdiscs. The new mode "queue_xmit" injects packets at >> __dev_queue_xmit() so that qdisc is called. >> >> Signed-off-by: John Fastabend <john.r.fastabend@intel.com> > > I generally like this. > [...] >> @@ -3434,6 +3442,36 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) >> #endif >> } while (--burst > 0); >> goto out; /* Skips xmit_mode M_START_XMIT */ >> + } else if (pkt_dev->xmit_mode == M_QUEUE_XMIT) { >> + local_bh_disable(); >> + atomic_add(burst, &pkt_dev->skb->users); > > Reading the code, people might think that "burst" is allowed for this > mode, which it is not. (You do handle this earlier in this patch when > configuring this mode). Right we never get here without burst == 1 but sure it does read a bit strange I'll use atomic_inc(). Thanks, John
diff --git a/net/core/pktgen.c b/net/core/pktgen.c index f74ab9c..4b3d467 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -213,6 +213,7 @@ /* Xmit modes */ #define M_START_XMIT 0 /* Default normal TX */ #define M_NETIF_RECEIVE 1 /* Inject packets into stack */ +#define M_QUEUE_XMIT 2 /* Inject packet into qdisc */ /* If lock -- protects updating of if_list */ #define if_lock(t) spin_lock(&(t->if_lock)); @@ -626,6 +627,8 @@ static int pktgen_if_show(struct seq_file *seq, void *v) if (pkt_dev->xmit_mode == M_NETIF_RECEIVE) seq_puts(seq, " xmit_mode: netif_receive\n"); + else if (pkt_dev->xmit_mode == M_QUEUE_XMIT) + seq_puts(seq, " xmit_mode: xmit_queue\n"); seq_puts(seq, " Flags: "); @@ -1142,8 +1145,10 @@ static ssize_t pktgen_if_write(struct file *file, return len; i += len; - if ((value > 1) && (pkt_dev->xmit_mode == M_START_XMIT) && - (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING))) + if ((value > 1) && + ((pkt_dev->xmit_mode == M_QUEUE_XMIT) || + ((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; sprintf(pg_result, "OK: burst=%d", pkt_dev->burst); @@ -1198,6 +1203,9 @@ static ssize_t pktgen_if_write(struct file *file, * at module loading time */ pkt_dev->clone_skb = 0; + } else if (strcmp(f, "queue_xmit") == 0) { + pkt_dev->xmit_mode = M_QUEUE_XMIT; + pkt_dev->last_ok = 1; } else { sprintf(pg_result, "xmit_mode -:%s:- unknown\nAvailable modes: %s", @@ -3434,6 +3442,36 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) #endif } while (--burst > 0); goto out; /* Skips xmit_mode M_START_XMIT */ + } else if (pkt_dev->xmit_mode == M_QUEUE_XMIT) { + local_bh_disable(); + atomic_add(burst, &pkt_dev->skb->users); + + ret = dev_queue_xmit(pkt_dev->skb); + switch (ret) { + case NET_XMIT_SUCCESS: + pkt_dev->sofar++; + pkt_dev->seq_num++; + pkt_dev->tx_bytes += pkt_dev->last_pkt_size; + break; + case NET_XMIT_DROP: + case NET_XMIT_CN: + /* These are all valid return codes for a qdisc but + * indicate packets are being dropped or will likely + * be dropped soon. + */ + case NETDEV_TX_BUSY: + /* qdisc may call dev_hard_start_xmit directly in cases + * where no queues exist e.g. loopback device, virtual + * devices, etc. In this case we need to handle + * NETDEV_TX_ codes. + */ + default: + pkt_dev->errors++; + net_info_ratelimited("%s xmit error: %d\n", + pkt_dev->odevname, ret); + break; + } + goto out; } txq = skb_get_tx_queue(odev, pkt_dev->skb);
Add another xmit_mode to pktgen to allow testing xmit functionality of qdiscs. The new mode "queue_xmit" injects packets at __dev_queue_xmit() so that qdisc is called. Signed-off-by: John Fastabend <john.r.fastabend@intel.com> --- net/core/pktgen.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-)