On 9/19/2017 10:45 PM, Chas Williams wrote: > From: "Charles (Chas) Williams" <ciwil...@brocade.com> > > In certain situations, low speed interfaces, it may be desirable to > have the flow control provided by the kernel queueing disciplines.
Out of curiosity, do you have any compression of performance numbers with and without qdisc? > > Signed-off-by: Chas Williams <ch...@att.com> > --- > drivers/net/af_packet/rte_eth_af_packet.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/af_packet/rte_eth_af_packet.c > b/drivers/net/af_packet/rte_eth_af_packet.c > index 7aa26e5..e3858fa 100644 > --- a/drivers/net/af_packet/rte_eth_af_packet.c > +++ b/drivers/net/af_packet/rte_eth_af_packet.c > @@ -59,6 +59,7 @@ > #define ETH_AF_PACKET_BLOCKSIZE_ARG "blocksz" > #define ETH_AF_PACKET_FRAMESIZE_ARG "framesz" > #define ETH_AF_PACKET_FRAMECOUNT_ARG "framecnt" > +#define ETH_AF_PACKET_BYPASS_ARG "bypass" I thinks argument name "bypass" on its own is not clear what is bypassed, would you mind using something like "qdisc_bypass"? > > #define DFLT_BLOCK_SIZE (1 << 12) > #define DFLT_FRAME_SIZE (1 << 11) > @@ -115,6 +116,7 @@ static const char *valid_arguments[] = { > ETH_AF_PACKET_BLOCKSIZE_ARG, > ETH_AF_PACKET_FRAMESIZE_ARG, > ETH_AF_PACKET_FRAMECOUNT_ARG, > + ETH_AF_PACKET_BYPASS_ARG, Same comment here, what about "ETH_AF_PACKET_QDISC_BYPASS_ARG" ? > NULL > }; > > @@ -559,6 +561,7 @@ rte_pmd_init_internals(struct rte_vdev_device *dev, > unsigned int blockcnt, > unsigned int framesize, > unsigned int framecnt, > + unsigned int bypass, > struct pmd_internals **internals, > struct rte_eth_dev **eth_dev, > struct rte_kvargs *kvlist) > @@ -580,9 +583,6 @@ rte_pmd_init_internals(struct rte_vdev_device *dev, > #if defined(PACKET_FANOUT) > int fanout_arg; > #endif > -#if defined(PACKET_QDISC_BYPASS) > - int bypass; > -#endif > > for (k_idx = 0; k_idx < kvlist->count; k_idx++) { > pair = &kvlist->pairs[k_idx]; > @@ -698,7 +698,6 @@ rte_pmd_init_internals(struct rte_vdev_device *dev, > } > > #if defined(PACKET_QDISC_BYPASS) > - bypass = 1; > rc = setsockopt(qsockfd, SOL_PACKET, PACKET_QDISC_BYPASS, > &bypass, sizeof(bypass)); > if (rc == -1) { > @@ -851,6 +850,7 @@ rte_eth_from_packet(struct rte_vdev_device *dev, > unsigned int framesize = DFLT_FRAME_SIZE; > unsigned int framecount = DFLT_FRAME_COUNT; > unsigned int qpairs = 1; > + unsigned int bypass = 1; > > /* do some parameter checking */ > if (*sockfd < 0) > @@ -902,6 +902,16 @@ rte_eth_from_packet(struct rte_vdev_device *dev, > } > continue; > } > + if (strstr(pair->key, ETH_AF_PACKET_BYPASS_ARG) != NULL) { > + bypass = atoi(pair->value); > + if (bypass > 2) { This accepts "2" too . As far as I can see kernel side checks if this value is zero or not, so perhaps this check is not required at all. But if you want to keep it I guess intention is to limit the value to "0" and "1". > + RTE_LOG(ERR, PMD, > + "%s: invalid bypass value\n", > + name); > + return -1; > + } > + continue; > + } > } > > if (framesize > blocksize) { > @@ -927,6 +937,7 @@ rte_eth_from_packet(struct rte_vdev_device *dev, > if (rte_pmd_init_internals(dev, *sockfd, qpairs, > blocksize, blockcount, > framesize, framecount, > + bypass, > &internals, ð_dev, > kvlist) < 0) > return -1; > @@ -1021,4 +1032,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_af_packet, > "qpairs=<int> " > "blocksz=<int> " > "framesz=<int> " > - "framecnt=<int>"); > + "framecnt=<int> " > + "bypass=<int>" Although storage is integer, does is make sense to document it as "0|1" to clarify the usage? ); >