[ovs-dev] [PATCH] dpdk: Ditch MAX_PKT_BURST macro.

[Ethan Jackson]

Cool thanks for trying it out.  I'll merge soon.

Ethan

On Mon, May 18, 2015 at 1:22 PM, Traynor, Kevin <kevin.traynor at intel.com> wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Ethan Jackson
>> Sent: Monday, May 18, 2015 5:08 PM
>> To: dev at openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH] dpdk: Ditch MAX_PKT_BURST macro.
>>
>> This version of the patch breaks sparse, I sent out another.
>>
>> Ethan
>
> The change makes sense - I tested this version on various dpdk interfaces and
> as expected there was no performance issues.
>
>>
>> On Sat, May 16, 2015 at 11:24 AM, Ethan Jackson <ethan at nicira.com> wrote:
>> > The MAX_PKT_BURST and NETDEV_MAX_RX_BATCH macros had a confusing
>> > relationship.  They basically purport to do the same thing, making it
>> > unclear which is the source of truth.
>> >
>> > Furthermore, while NETDEV_MAX_RX_BATCH was 256, MAX_PKT_BURST was 32,
>> > meaning we never process a batch larger than 32 packets further adding
>> > to the confusion.
>> >
>> > This patch resolves the issue by removing MAX_PKT_BURST completely,
>> > and shrinking the new NETDEV_MAX_BURST macro to only 32.  This should
>> > have no change in the execution path except shrinking a couple of
>> > structs and memory allocations (can't hurt).
>> >
>> > Signed-off-by: Ethan Jackson <ethan at nicira.com>
>> > ---
>> >  lib/dpif-netdev.c | 10 +++++-----
>> >  lib/netdev-dpdk.c |  7 ++-----
>> >  lib/netdev.h      |  2 +-
>> >  3 files changed, 8 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> > index f1d65f5..4216865 100644
>> > --- a/lib/dpif-netdev.c
>> > +++ b/lib/dpif-netdev.c
>> > @@ -2500,7 +2500,7 @@ dp_netdev_process_rxq_port(struct
>> dp_netdev_pmd_thread *pmd,
>> >                             struct dp_netdev_port *port,
>> >                             struct netdev_rxq *rxq)
>> >  {
>> > -    struct dp_packet *packets[NETDEV_MAX_RX_BATCH];
>> > +    struct dp_packet *packets[NETDEV_MAX_BURST];
>> >      int error, cnt;
>> >
>> >      cycles_count_start(pmd);
>> > @@ -3027,7 +3027,7 @@ struct packet_batch {
>> >
>> >      struct dp_netdev_flow *flow;
>> >
>> > -    struct dp_packet *packets[NETDEV_MAX_RX_BATCH];
>> > +    struct dp_packet *packets[NETDEV_MAX_BURST];
>> >  };
>> >
>> >  static inline void
>> > @@ -3397,7 +3397,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets,
>> int cnt,
>> >
>> >      case OVS_ACTION_ATTR_TUNNEL_PUSH:
>> >          if (*depth < MAX_RECIRC_DEPTH) {
>> > -            struct dp_packet *tnl_pkt[NETDEV_MAX_RX_BATCH];
>> > +            struct dp_packet *tnl_pkt[NETDEV_MAX_BURST];
>> >              int err;
>> >
>> >              if (!may_steal) {
>> > @@ -3423,7 +3423,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets,
>> int cnt,
>> >
>> >              p = dp_netdev_lookup_port(dp, portno);
>> >              if (p) {
>> > -                struct dp_packet *tnl_pkt[NETDEV_MAX_RX_BATCH];
>> > +                struct dp_packet *tnl_pkt[NETDEV_MAX_BURST];
>> >                  int err;
>> >
>> >                  if (!may_steal) {
>> > @@ -3485,7 +3485,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets,
>> int cnt,
>> >
>> >      case OVS_ACTION_ATTR_RECIRC:
>> >          if (*depth < MAX_RECIRC_DEPTH) {
>> > -            struct dp_packet *recirc_pkts[NETDEV_MAX_RX_BATCH];
>> > +            struct dp_packet *recirc_pkts[NETDEV_MAX_BURST];
>> >
>> >              if (!may_steal) {
>> >                 dp_netdev_clone_pkt_batch(recirc_pkts, packets, cnt);
>> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> > index 505ab75..b06f92a 100644
>> > --- a/lib/netdev-dpdk.c
>> > +++ b/lib/netdev-dpdk.c
>> > @@ -99,8 +99,6 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
>> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
>> >  #define TX_HTHRESH 0  /* Default values of TX host threshold reg. */
>> >  #define TX_WTHRESH 0  /* Default values of TX write-back threshold reg. */
>> >
>> > -#define MAX_PKT_BURST 32           /* Max burst size for RX/TX */
>> > -
>> >  /* Character device cuse_dev_name. */
>> >  char *cuse_dev_name = NULL;
>> >
>> > @@ -862,7 +860,7 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq_,
>> >      nb_rx = rte_vhost_dequeue_burst(virtio_dev, qid,
>> >                                      vhost_dev->dpdk_mp->mp,
>> >                                      (struct rte_mbuf **)packets,
>> > -                                    MAX_PKT_BURST);
>> > +                                    NETDEV_MAX_BURST);
>> >      if (!nb_rx) {
>> >          return EAGAIN;
>> >      }
>> > @@ -889,8 +887,7 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, struct
>> dp_packet **packets,
>> >
>> >      nb_rx = rte_eth_rx_burst(rx->port_id, rxq_->queue_id,
>> >                               (struct rte_mbuf **) packets,
>> > -                             MIN((int) NETDEV_MAX_RX_BATCH,
>> > -                                 (int) MAX_PKT_BURST));
>> > +                             NETDEV_MAX_BURST);
>> >      if (!nb_rx) {
>> >          return EAGAIN;
>> >      }
>> > diff --git a/lib/netdev.h b/lib/netdev.h
>> > index 71c0af1..9d412ee 100644
>> > --- a/lib/netdev.h
>> > +++ b/lib/netdev.h
>> > @@ -338,7 +338,7 @@ typedef void netdev_dump_queue_stats_cb(unsigned int
>> queue_id,
>> >  int netdev_dump_queue_stats(const struct netdev *,
>> >                              netdev_dump_queue_stats_cb *, void *aux);
>> >
>> > -enum { NETDEV_MAX_RX_BATCH = 256 };     /* Maximum number packets in
>> rx_recv() batch. */
>> > +enum { NETDEV_MAX_BURST = 32 }; /* Maximum number packets in a batch. */
>> >  extern struct seq *tnl_conf_seq;
>> >
>> >  #ifdef  __cplusplus
>> > --
>> > 1.9.1
>> >
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev