On Fri, Jun 13, 2014 at 5:00 PM, Daniele Di Proietto
<ddiproie...@vmware.com> wrote:
> This commit introduces a new data structure used for receiving packets from
> netdevs and passing them to dpifs.
> The purpose of this change is to allow storing some private data for each
> packet. The subsequent commits make use of it.
>
> Signed-off-by: Daniele Di Proietto <ddiproie...@vmware.com>
> ---
>  lib/automake.mk              |  2 ++
>  lib/dpif-netdev.c            | 48 ++++++++++++++++++++------------
>  lib/dpif.c                   | 19 +++++++++----
>  lib/netdev-bsd.c             | 10 +++++--
>  lib/netdev-dpdk.c            | 12 ++++----
>  lib/netdev-dummy.c           |  8 ++++--
>  lib/netdev-linux.c           | 11 ++++++--
>  lib/netdev-provider.h        |  2 +-
>  lib/netdev.c                 |  2 +-
>  lib/netdev.h                 |  4 ++-
>  lib/odp-execute.c            | 53 ++++++++++++++++++++---------------
>  lib/odp-execute.h            | 12 ++++----
>  lib/packet-dpif.c            | 66 
> ++++++++++++++++++++++++++++++++++++++++++++
>  lib/packet-dpif.h            | 41 +++++++++++++++++++++++++++
>  ofproto/ofproto-dpif-xlate.c | 11 ++++----
>  15 files changed, 229 insertions(+), 72 deletions(-)
>  create mode 100644 lib/packet-dpif.c
>  create mode 100644 lib/packet-dpif.h
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index dc2ca0e..3f984d9 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -165,6 +165,8 @@ lib_libopenvswitch_la_SOURCES = \
>         lib/ovsdb-parser.h \
>         lib/ovsdb-types.c \
>         lib/ovsdb-types.h \
> +       lib/packet-dpif.c \
> +       lib/packet-dpif.h \
>         lib/packets.c \
>         lib/packets.h \
>         lib/pcap-file.c \
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 6c281fe..332bbda 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -52,6 +52,7 @@
>  #include "ofp-print.h"
>  #include "ofpbuf.h"
>  #include "ovs-rcu.h"
> +#include "packet-dpif.h"
>  #include "packets.h"
>  #include "poll-loop.h"
>  #include "random.h"
> @@ -337,11 +338,12 @@ static int dp_netdev_output_userspace(struct dp_netdev 
> *dp, struct ofpbuf *,
>                                        const struct nlattr *userdata);
>  static void dp_netdev_execute_actions(struct dp_netdev *dp,
>                                        const struct miniflow *,
> -                                      struct ofpbuf *, bool may_steal,
> +                                      struct dpif_packet *, bool may_steal,
>                                        struct pkt_metadata *,
>                                        const struct nlattr *actions,
>                                        size_t actions_len);
> -static void dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf *packet,
> +static void dp_netdev_port_input(struct dp_netdev *dp,
> +                                 struct dpif_packet *packet,
>                                   struct pkt_metadata *);
>
>  static void dp_netdev_set_pmd_threads(struct dp_netdev *, int n);
> @@ -1522,6 +1524,7 @@ static int
>  dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute)
>  {
>      struct dp_netdev *dp = get_dp_netdev(dpif);
> +    struct dpif_packet packet;
>      struct pkt_metadata *md = &execute->md;
>      struct {
>          struct miniflow flow;
> @@ -1537,9 +1540,16 @@ dpif_netdev_execute(struct dpif *dpif, struct 
> dpif_execute *execute)
>      miniflow_initialize(&key.flow, key.buf);
>      miniflow_extract(execute->packet, md, &key.flow);
>
> -    dp_netdev_execute_actions(dp, &key.flow, execute->packet, false, md,
> +    packet.ofpbuf = *execute->packet;
> +
> +    dp_netdev_execute_actions(dp, &key.flow, &packet, false, md,
>                                execute->actions, execute->actions_len);
>
> +    /* Even though may_steal is set to false, some actions could modify or
> +     * reallocate the ofpbuf memory. We need to pass those changes to the
> +     * caller */
> +    *execute->packet = packet.ofpbuf;
> +
>      return 0;
>  }
>
> @@ -1751,7 +1761,7 @@ dp_netdev_process_rxq_port(struct dp_netdev *dp,
>                            struct dp_netdev_port *port,
>                            struct netdev_rxq *rxq)
>  {
> -    struct ofpbuf *packet[NETDEV_MAX_RX_BATCH];
> +    struct dpif_packet *packet[NETDEV_MAX_RX_BATCH];
>      int error, c;
>
>      error = netdev_rxq_recv(rxq, packet, &c);
> @@ -1993,27 +2003,28 @@ dp_netdev_count_packet(struct dp_netdev *dp, enum 
> dp_stat_type type)
>  }
>
>  static void
> -dp_netdev_input(struct dp_netdev *dp, struct ofpbuf *packet,
> +dp_netdev_input(struct dp_netdev *dp, struct dpif_packet *packet,
>                  struct pkt_metadata *md)
>  {
>      struct dp_netdev_flow *netdev_flow;
> +    struct ofpbuf * ofp = &packet->ofpbuf;

extra space while declaring ofp, There are multiple instances of this
coding style issue in this patch series.

>      struct {
>          struct miniflow flow;
>          uint32_t buf[FLOW_U32S];
>      } key;
>
> -    if (ofpbuf_size(packet) < ETH_HEADER_LEN) {
> -        ofpbuf_delete(packet);
> +    if (ofpbuf_size(ofp) < ETH_HEADER_LEN) {
> +        ofpbuf_delete(ofp);
>          return;
>      }
>      miniflow_initialize(&key.flow, key.buf);
> -    miniflow_extract(packet, md, &key.flow);
> +    miniflow_extract(ofp, md, &key.flow);
>
>      netdev_flow = dp_netdev_lookup_flow(dp, &key.flow);
>      if (netdev_flow) {
>          struct dp_netdev_actions *actions;
>
> -        dp_netdev_flow_used(netdev_flow, packet, &key.flow);
> +        dp_netdev_flow_used(netdev_flow, ofp, &key.flow);
>
>          actions = dp_netdev_flow_get_actions(netdev_flow);
>          dp_netdev_execute_actions(dp, &key.flow, packet, true, md,
> @@ -2021,15 +2032,14 @@ dp_netdev_input(struct dp_netdev *dp, struct ofpbuf 
> *packet,
>          dp_netdev_count_packet(dp, DP_STAT_HIT);
>      } else if (dp->handler_queues) {
>          dp_netdev_count_packet(dp, DP_STAT_MISS);
> -        dp_netdev_output_userspace(dp, packet,
> -                                   miniflow_hash_5tuple(&key.flow, 0)
> +        dp_netdev_output_userspace(dp, ofp, miniflow_hash_5tuple(&key.flow, 
> 0)
>                                     % dp->n_handlers,
>                                     DPIF_UC_MISS, &key.flow, NULL);
>      }
>  }
>
>  static void
> -dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf *packet,
> +dp_netdev_port_input(struct dp_netdev *dp, struct dpif_packet *packet,
>                       struct pkt_metadata *md)
>  {
>      uint32_t *recirc_depth = recirc_depth_get();
> @@ -2099,7 +2109,7 @@ struct dp_netdev_execute_aux {
>  };
>
>  static void
> -dp_execute_cb(void *aux_, struct ofpbuf *packet,
> +dp_execute_cb(void *aux_, struct dpif_packet *packet,
>                struct pkt_metadata *md,
>                const struct nlattr *a, bool may_steal)
>      OVS_NO_THREAD_SAFETY_ANALYSIS
> @@ -2113,7 +2123,7 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet,
>      case OVS_ACTION_ATTR_OUTPUT:
>          p = dp_netdev_lookup_port(aux->dp, u32_to_odp(nl_attr_get_u32(a)));
>          if (p) {
> -            netdev_send(p->netdev, packet, may_steal);
> +            netdev_send(p->netdev, &packet->ofpbuf, may_steal);
>          }
>          break;
>
> @@ -2122,7 +2132,9 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet,
>          const struct nlattr *userdata;
>
>          userdata = nl_attr_find_nested(a, OVS_USERSPACE_ATTR_USERDATA);
> -        userspace_packet = may_steal ? packet : ofpbuf_clone(packet);
> +        userspace_packet = may_steal
> +                           ? &packet->ofpbuf
> +                           : ofpbuf_clone(&packet->ofpbuf);
>
>          dp_netdev_output_userspace(aux->dp, userspace_packet,
>                                     miniflow_hash_5tuple(aux->key, 0)
> @@ -2157,9 +2169,9 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet,
>      case OVS_ACTION_ATTR_RECIRC:
>          if (*depth < MAX_RECIRC_DEPTH) {
>              struct pkt_metadata recirc_md = *md;
> -            struct ofpbuf *recirc_packet;
> +            struct dpif_packet *recirc_packet;
>
> -            recirc_packet = may_steal ? packet : ofpbuf_clone(packet);
> +            recirc_packet = may_steal ? packet : dpif_packet_clone(packet);
>              recirc_md.recirc_id = nl_attr_get_u32(a);
>
>              (*depth)++;
> @@ -2186,7 +2198,7 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet,
>
>  static void
>  dp_netdev_execute_actions(struct dp_netdev *dp, const struct miniflow *key,
> -                          struct ofpbuf *packet, bool may_steal,
> +                          struct dpif_packet *packet, bool may_steal,
>                            struct pkt_metadata *md,
>                            const struct nlattr *actions, size_t actions_len)
>  {
> diff --git a/lib/dpif.c b/lib/dpif.c
> index cace47b..99d5a52 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -34,6 +34,7 @@
>  #include "ofp-print.h"
>  #include "ofp-util.h"
>  #include "ofpbuf.h"
> +#include "packet-dpif.h"
>  #include "packets.h"
>  #include "poll-loop.h"
>  #include "shash.h"
> @@ -1060,7 +1061,7 @@ struct dpif_execute_helper_aux {
>  /* This is called for actions that need the context of the datapath to be
>   * meaningful. */
>  static void
> -dpif_execute_helper_cb(void *aux_, struct ofpbuf *packet,
> +dpif_execute_helper_cb(void *aux_, struct dpif_packet *packet,
>                         struct pkt_metadata *md,
>                         const struct nlattr *action, bool may_steal 
> OVS_UNUSED)
>  {
> @@ -1074,7 +1075,7 @@ dpif_execute_helper_cb(void *aux_, struct ofpbuf 
> *packet,
>      case OVS_ACTION_ATTR_RECIRC:
>          execute.actions = action;
>          execute.actions_len = NLA_ALIGN(action->nla_len);
> -        execute.packet = packet;
> +        execute.packet = &packet->ofpbuf;
>          execute.md = *md;
>          execute.needs_help = false;
>          aux->error = aux->dpif->dpif_class->execute(aux->dpif, &execute);
> @@ -1102,12 +1103,20 @@ static int
>  dpif_execute_with_help(struct dpif *dpif, struct dpif_execute *execute)
>  {
>      struct dpif_execute_helper_aux aux = {dpif, 0};
> +    struct dpif_packet packet;
>
>      COVERAGE_INC(dpif_execute_with_help);
>
> -    odp_execute_actions(&aux, execute->packet, false, &execute->md,
> -                        execute->actions, execute->actions_len,
> -                        dpif_execute_helper_cb);
> +    packet.ofpbuf = *execute->packet;
> +
> +    odp_execute_actions(&aux, &packet, false, &execute->md, execute->actions,
> +                        execute->actions_len, dpif_execute_helper_cb);
> +
> +    /* Even though may_steal is set to false, some actions could modify or
> +     * reallocate the ofpbuf memory. We need to pass those changes to the
> +     * caller */
> +    *execute->packet = packet.ofpbuf;
> +
>      return aux.error;
>  }
>
> diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
> index 35a8da4..27d90f0 100644
> --- a/lib/netdev-bsd.c
> +++ b/lib/netdev-bsd.c
> @@ -620,10 +620,12 @@ netdev_rxq_bsd_recv_tap(struct netdev_rxq_bsd *rxq, 
> struct ofpbuf *buffer)
>  }
>
>  static int
> -netdev_bsd_rxq_recv(struct netdev_rxq *rxq_, struct ofpbuf **packet, int *c)
> +netdev_bsd_rxq_recv(struct netdev_rxq *rxq_, struct dpif_packet **packets,
> +                    int *c)
>  {
>      struct netdev_rxq_bsd *rxq = netdev_rxq_bsd_cast(rxq_);
>      struct netdev *netdev = rxq->up.netdev;
> +    struct dpif_packet *packet;
>      struct ofpbuf *buffer;
>      ssize_t retval;
>      int mtu;
> @@ -632,7 +634,9 @@ netdev_bsd_rxq_recv(struct netdev_rxq *rxq_, struct 
> ofpbuf **packet, int *c)
>          mtu = ETH_PAYLOAD_MAX;
>      }
>
> -    buffer = ofpbuf_new_with_headroom(VLAN_ETH_HEADER_LEN + mtu, 
> DP_NETDEV_HEADROOM);
> +    packet = dpif_packet_new_with_headroom(VLAN_ETH_HEADER_LEN + mtu,
> +                                           DP_NETDEV_HEADROOM);
> +    buffer = dpif_packet_to_ofpbuf(packet);
>
>      retval = (rxq->pcap_handle
>              ? netdev_rxq_bsd_recv_pcap(rxq, buffer)
> @@ -642,7 +646,7 @@ netdev_bsd_rxq_recv(struct netdev_rxq *rxq_, struct 
> ofpbuf **packet, int *c)
>          ofpbuf_delete(buffer);
>      } else {
>          dp_packet_pad(buffer);
> -        packet[0] = buffer;
> +        packets[0] = packet;
>          *c = 1;
>      }
>      return retval;
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index fbdb6b3..ec6565a 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -38,6 +38,7 @@
>  #include "ofpbuf.h"
>  #include "ovs-thread.h"
>  #include "ovs-rcu.h"
> +#include "packet-dpif.h"
>  #include "packets.h"
>  #include "shash.h"
>  #include "sset.h"
> @@ -217,16 +218,16 @@ __rte_pktmbuf_init(struct rte_mempool *mp,
>                     unsigned i OVS_UNUSED)
>  {
>      struct rte_mbuf *m = _m;
> -    uint32_t buf_len = mp->elt_size - sizeof(struct ofpbuf);
> +    uint32_t buf_len = mp->elt_size - sizeof(struct dpif_packet);
>
> -    RTE_MBUF_ASSERT(mp->elt_size >= sizeof(struct ofpbuf));
> +    RTE_MBUF_ASSERT(mp->elt_size >= sizeof(struct dpif_packet));
>
>      memset(m, 0, mp->elt_size);
>
>      /* start of buffer is just after mbuf structure */
> -    m->buf_addr = (char *)m + sizeof(struct ofpbuf);
> +    m->buf_addr = (char *)m + sizeof(struct dpif_packet);
>      m->buf_physaddr = rte_mempool_virt2phy(mp, m) +
> -                    sizeof(struct ofpbuf);
> +                    sizeof(struct dpif_packet);
>      m->buf_len = (uint16_t)buf_len;
>
>      /* keep some headroom between start of buffer and data */
> @@ -586,7 +587,8 @@ dpdk_queue_flush(struct netdev_dpdk *dev, int qid)
>  }
>
Since rte_pktmbuf_init() allocate dpif-packet, free_dpdk_buf() should
take dpif-packet, rather than ofpbuf.

>  static int
> -netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, struct ofpbuf **packets, int 
> *c)
> +netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, struct dpif_packet **packets,
> +                     int *c)
>  {
>      struct netdev_rxq_dpdk *rx = netdev_rxq_dpdk_cast(rxq_);
>      struct netdev *netdev = rx->up.netdev;
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> index b087ed1..1464d29 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -29,6 +29,7 @@
>  #include "odp-util.h"
>  #include "ofp-print.h"
>  #include "ofpbuf.h"
> +#include "packet-dpif.h"
>  #include "packets.h"
>  #include "pcap-file.h"
>  #include "poll-loop.h"
> @@ -779,7 +780,7 @@ netdev_dummy_rxq_dealloc(struct netdev_rxq *rxq_)
>  }
>
>  static int
> -netdev_dummy_rxq_recv(struct netdev_rxq *rxq_, struct ofpbuf **arr, int *c)
> +netdev_dummy_rxq_recv(struct netdev_rxq *rxq_, struct dpif_packet **arr, int 
> *c)
>  {
>      struct netdev_rxq_dummy *rx = netdev_rxq_dummy_cast(rxq_);
>      struct netdev_dummy *netdev = netdev_dummy_cast(rx->up.netdev);
> @@ -803,7 +804,10 @@ netdev_dummy_rxq_recv(struct netdev_rxq *rxq_, struct 
> ofpbuf **arr, int *c)
>      ovs_mutex_unlock(&netdev->mutex);
>
>      dp_packet_pad(packet);
> -    arr[0] = packet;
> +
> +    /* This performs a (sometimes unnecessary) copy */
> +    arr[0] = dpif_packet_clone_from_ofpbuf(packet);
> +    ofpbuf_delete(packet);
>      *c = 1;
>      return 0;
>  }
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 840022d..074a061 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -62,6 +62,7 @@
>  #include "ofpbuf.h"
>  #include "openflow/openflow.h"
>  #include "ovs-atomic.h"
> +#include "packet-dpif.h"
>  #include "packets.h"
>  #include "poll-loop.h"
>  #include "rtnetlink-link.h"
> @@ -984,10 +985,12 @@ netdev_linux_rxq_recv_tap(int fd, struct ofpbuf *buffer)
>  }
>
>  static int
> -netdev_linux_rxq_recv(struct netdev_rxq *rxq_, struct ofpbuf **packet, int 
> *c)
> +netdev_linux_rxq_recv(struct netdev_rxq *rxq_, struct dpif_packet **packets,
> +                      int *c)
>  {
>      struct netdev_rxq_linux *rx = netdev_rxq_linux_cast(rxq_);
>      struct netdev *netdev = rx->up.netdev;
> +    struct dpif_packet *packet;
>      struct ofpbuf *buffer;
>      ssize_t retval;
>      int mtu;
> @@ -996,7 +999,9 @@ netdev_linux_rxq_recv(struct netdev_rxq *rxq_, struct 
> ofpbuf **packet, int *c)
>          mtu = ETH_PAYLOAD_MAX;
>      }
>
> -    buffer = ofpbuf_new_with_headroom(VLAN_ETH_HEADER_LEN + mtu, 
> DP_NETDEV_HEADROOM);
> +    packet = dpif_packet_new_with_headroom(VLAN_ETH_HEADER_LEN + mtu,
> +                                           DP_NETDEV_HEADROOM);
> +    buffer = &packet->ofpbuf;
>
>      retval = (rx->is_tap
>                ? netdev_linux_rxq_recv_tap(rx->fd, buffer)
> @@ -1010,7 +1015,7 @@ netdev_linux_rxq_recv(struct netdev_rxq *rxq_, struct 
> ofpbuf **packet, int *c)
>          ofpbuf_delete(buffer);
>      } else {
>          dp_packet_pad(buffer);
> -        packet[0] = buffer;
> +        packets[0] = packet;
>          *c = 1;
>      }
>
> diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
> index 37b9da3..42c0012 100644
> --- a/lib/netdev-provider.h
> +++ b/lib/netdev-provider.h
> @@ -672,7 +672,7 @@ struct netdev_class {
>       * Caller is expected to pass array of size MAX_RX_BATCH.
>       * This function may be set to null if it would always return EOPNOTSUPP
>       * anyhow. */
> -    int (*rxq_recv)(struct netdev_rxq *rx, struct ofpbuf **pkt, int *cnt);
> +    int (*rxq_recv)(struct netdev_rxq *rx, struct dpif_packet **pkt, int 
> *cnt);
>
>      /* Registers with the poll loop to wake up from the next call to
>       * poll_block() when a packet is ready to be received with 
> netdev_rxq_recv()
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 07cda42..6a2ad51 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -622,7 +622,7 @@ netdev_rxq_close(struct netdev_rxq *rx)
>   * This function may be set to null if it would always return EOPNOTSUPP
>   * anyhow. */
>  int
> -netdev_rxq_recv(struct netdev_rxq *rx, struct ofpbuf **buffers, int *cnt)
> +netdev_rxq_recv(struct netdev_rxq *rx, struct dpif_packet **buffers, int 
> *cnt)
>  {
>      int retval;
>
> diff --git a/lib/netdev.h b/lib/netdev.h
> index a4bd01a..c8880a4 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -59,6 +59,7 @@ extern "C" {
>   *      netdev and access each of those from a different thread.)
>   */
>
> +struct dpif_packet;
>  struct netdev;
>  struct netdev_class;
>  struct netdev_rxq;
> @@ -166,7 +167,8 @@ void netdev_rxq_close(struct netdev_rxq *);
>
>  const char *netdev_rxq_get_name(const struct netdev_rxq *);
>
> -int netdev_rxq_recv(struct netdev_rxq *rx, struct ofpbuf **buffers, int 
> *cnt);
> +int netdev_rxq_recv(struct netdev_rxq *rx, struct dpif_packet **buffers,
> +                    int *cnt);
If you going to use dpif-packet for recv, then we should do same for
sending it. netdev_send should take dpif-packet as parameter.

>  void netdev_rxq_wait(struct netdev_rxq *);
>  int netdev_rxq_drain(struct netdev_rxq *);
>
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index cc18536..15cc406 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -25,6 +25,7 @@
>  #include "netlink.h"
>  #include "ofpbuf.h"
>  #include "odp-util.h"
> +#include "packet-dpif.h"
>  #include "packets.h"
>  #include "flow.h"
>  #include "unaligned.h"
> @@ -64,7 +65,7 @@ set_arp(struct ofpbuf *packet, const struct ovs_key_arp 
> *arp_key)
>  }
>
>  static void
> -odp_execute_set_action(struct ofpbuf *packet, const struct nlattr *a,
> +odp_execute_set_action(struct dpif_packet *packet, const struct nlattr *a,
>                         struct pkt_metadata *md)
>  {
>      enum ovs_key_attr type = nl_attr_type(a);
> @@ -88,44 +89,50 @@ odp_execute_set_action(struct ofpbuf *packet, const 
> struct nlattr *a,
>          break;
>
>      case OVS_KEY_ATTR_ETHERNET:
> -        odp_eth_set_addrs(packet,
> +        odp_eth_set_addrs(&packet->ofpbuf,
>                            nl_attr_get_unspec(a, sizeof(struct 
> ovs_key_ethernet)));
>          break;
>
>      case OVS_KEY_ATTR_IPV4:
>          ipv4_key = nl_attr_get_unspec(a, sizeof(struct ovs_key_ipv4));
> -        packet_set_ipv4(packet, ipv4_key->ipv4_src, ipv4_key->ipv4_dst,
> -                        ipv4_key->ipv4_tos, ipv4_key->ipv4_ttl);
> +        packet_set_ipv4(&packet->ofpbuf, ipv4_key->ipv4_src,
> +                        ipv4_key->ipv4_dst, ipv4_key->ipv4_tos,
> +                        ipv4_key->ipv4_ttl);
>          break;
>
>      case OVS_KEY_ATTR_IPV6:
>          ipv6_key = nl_attr_get_unspec(a, sizeof(struct ovs_key_ipv6));
> -        packet_set_ipv6(packet, ipv6_key->ipv6_proto, ipv6_key->ipv6_src,
> -                        ipv6_key->ipv6_dst, ipv6_key->ipv6_tclass,
> -                        ipv6_key->ipv6_label, ipv6_key->ipv6_hlimit);
> +        packet_set_ipv6(&packet->ofpbuf, ipv6_key->ipv6_proto,
> +                        ipv6_key->ipv6_src, ipv6_key->ipv6_dst,
> +                        ipv6_key->ipv6_tclass, ipv6_key->ipv6_label,
> +                        ipv6_key->ipv6_hlimit);
>          break;
>
>      case OVS_KEY_ATTR_TCP:
>          tcp_key = nl_attr_get_unspec(a, sizeof(struct ovs_key_tcp));
> -        packet_set_tcp_port(packet, tcp_key->tcp_src, tcp_key->tcp_dst);
> +        packet_set_tcp_port(&packet->ofpbuf, tcp_key->tcp_src,
> +                            tcp_key->tcp_dst);
>          break;
>
>      case OVS_KEY_ATTR_UDP:
>          udp_key = nl_attr_get_unspec(a, sizeof(struct ovs_key_udp));
> -        packet_set_udp_port(packet, udp_key->udp_src, udp_key->udp_dst);
> +        packet_set_udp_port(&packet->ofpbuf, udp_key->udp_src,
> +                            udp_key->udp_dst);
>          break;
>
>      case OVS_KEY_ATTR_SCTP:
>          sctp_key = nl_attr_get_unspec(a, sizeof(struct ovs_key_sctp));
> -        packet_set_sctp_port(packet, sctp_key->sctp_src, sctp_key->sctp_dst);
> +        packet_set_sctp_port(&packet->ofpbuf, sctp_key->sctp_src,
> +                             sctp_key->sctp_dst);
>          break;
>
>      case OVS_KEY_ATTR_MPLS:
> -         set_mpls_lse(packet, nl_attr_get_be32(a));
> +         set_mpls_lse(&packet->ofpbuf, nl_attr_get_be32(a));
>           break;
>
>      case OVS_KEY_ATTR_ARP:
> -        set_arp(packet, nl_attr_get_unspec(a, sizeof(struct ovs_key_arp)));
> +        set_arp(&packet->ofpbuf,
> +                nl_attr_get_unspec(a, sizeof(struct ovs_key_arp)));
>          break;
>
>      case OVS_KEY_ATTR_DP_HASH:
> @@ -152,13 +159,13 @@ odp_execute_set_action(struct ofpbuf *packet, const 
> struct nlattr *a,
>  }
>
>  static void
> -odp_execute_actions__(void *dp, struct ofpbuf *packet, bool steal,
> +odp_execute_actions__(void *dp, struct dpif_packet *packet, bool steal,
>                        struct pkt_metadata *,
>                        const struct nlattr *actions, size_t actions_len,
>                        odp_execute_cb dp_execute_action, bool more_actions);
>
>  static void
> -odp_execute_sample(void *dp, struct ofpbuf *packet, bool steal,
> +odp_execute_sample(void *dp, struct dpif_packet *packet, bool steal,
>                     struct pkt_metadata *md, const struct nlattr *action,
>                     odp_execute_cb dp_execute_action, bool more_actions)
>  {
> @@ -193,7 +200,7 @@ odp_execute_sample(void *dp, struct ofpbuf *packet, bool 
> steal,
>  }
>
>  static void
> -odp_execute_actions__(void *dp, struct ofpbuf *packet, bool steal,
> +odp_execute_actions__(void *dp, struct dpif_packet *packet, bool steal,
>                        struct pkt_metadata *md,
>                        const struct nlattr *actions, size_t actions_len,
>                        odp_execute_cb dp_execute_action, bool more_actions)
> @@ -230,7 +237,7 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, 
> bool steal,
>                  struct flow flow;
>                  uint32_t hash;
>
> -                flow_extract(packet, md, &flow);
> +                flow_extract(&packet->ofpbuf, md, &flow);
>                  hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
>                  md->dp_hash = hash ? hash : 1;
>              } else {
> @@ -242,22 +249,24 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, 
> bool steal,
>
>          case OVS_ACTION_ATTR_PUSH_VLAN: {
>              const struct ovs_action_push_vlan *vlan = nl_attr_get(a);
> -            eth_push_vlan(packet, htons(ETH_TYPE_VLAN), vlan->vlan_tci);
> +            eth_push_vlan(&packet->ofpbuf,
> +                          htons(ETH_TYPE_VLAN), vlan->vlan_tci);
>              break;
>          }
>
>          case OVS_ACTION_ATTR_POP_VLAN:
> -            eth_pop_vlan(packet);
> +            eth_pop_vlan(&packet->ofpbuf);
>              break;
>
>          case OVS_ACTION_ATTR_PUSH_MPLS: {
>              const struct ovs_action_push_mpls *mpls = nl_attr_get(a);
> -            push_mpls(packet, mpls->mpls_ethertype, mpls->mpls_lse);
> +            push_mpls(&packet->ofpbuf,
> +                      mpls->mpls_ethertype, mpls->mpls_lse);
>              break;
>           }
>
>          case OVS_ACTION_ATTR_POP_MPLS:
> -            pop_mpls(packet, nl_attr_get_be16(a));
> +            pop_mpls(&packet->ofpbuf, nl_attr_get_be16(a));
>              break;
>
>          case OVS_ACTION_ATTR_SET:
> @@ -277,7 +286,7 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, 
> bool steal,
>  }
>
>  void
> -odp_execute_actions(void *dp, struct ofpbuf *packet, bool steal,
> +odp_execute_actions(void *dp, struct dpif_packet *packet, bool steal,
>                      struct pkt_metadata *md,
>                      const struct nlattr *actions, size_t actions_len,
>                      odp_execute_cb dp_execute_action)
> @@ -287,6 +296,6 @@ odp_execute_actions(void *dp, struct ofpbuf *packet, bool 
> steal,
>
>      if (!actions_len && steal) {
>          /* Drop action. */
> -        ofpbuf_delete(packet);
> +        ofpbuf_delete(&packet->ofpbuf);
>      }
>  }
> diff --git a/lib/odp-execute.h b/lib/odp-execute.h
> index 91f0c51..1f836d5 100644
> --- a/lib/odp-execute.h
> +++ b/lib/odp-execute.h
> @@ -24,10 +24,10 @@
>  #include "openvswitch/types.h"
>
>  struct nlattr;
> -struct ofpbuf;
> +struct dpif_packet;
>  struct pkt_metadata;
>
> -typedef void (*odp_execute_cb)(void *dp, struct ofpbuf *packet,
> +typedef void (*odp_execute_cb)(void *dp, struct dpif_packet *packet,
>                                 struct pkt_metadata *,
>                                 const struct nlattr *action, bool may_steal);
>
> @@ -35,8 +35,8 @@ typedef void (*odp_execute_cb)(void *dp, struct ofpbuf 
> *packet,
>   * to 'dp_execute_action', if non-NULL.  Currently this is called only for
>   * actions OVS_ACTION_ATTR_OUTPUT and OVS_ACTION_ATTR_USERSPACE so
>   * 'dp_execute_action' needs to handle only these. */
> -void odp_execute_actions(void *dp, struct ofpbuf *packet, bool steal,
> -                    struct pkt_metadata *,
> -                    const struct nlattr *actions, size_t actions_len,
> -                    odp_execute_cb dp_execute_action);
> +void odp_execute_actions(void *dp, struct dpif_packet *packet, bool steal,
> +                         struct pkt_metadata *,
> +                         const struct nlattr *actions, size_t actions_len,
> +                         odp_execute_cb dp_execute_action);
>  #endif
> diff --git a/lib/packet-dpif.c b/lib/packet-dpif.c
> new file mode 100644
> index 0000000..779332b
> --- /dev/null
> +++ b/lib/packet-dpif.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (c) 2014 Nicira, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +
> +#include "ofpbuf.h"
> +#include "packet-dpif.h"
> +
> +struct dpif_packet *
> +dpif_packet_new_with_headroom(size_t size, size_t headroom)
> +{
> +    struct dpif_packet *p = xmalloc(sizeof *p);
> +    struct ofpbuf *b = &p->ofpbuf;
> +
> +    ofpbuf_init(b, size + headroom);
> +    ofpbuf_reserve(b, headroom);
> +
> +    return p;
> +}
> +
> +struct dpif_packet *
> +dpif_packet_clone_from_ofpbuf(const struct ofpbuf * b)
> +{
> +    struct dpif_packet *p = xmalloc(sizeof *p);
> +    size_t headroom = ofpbuf_headroom(b);
> +
> +    ofpbuf_init(&p->ofpbuf, ofpbuf_size(b) + headroom);
> +    ofpbuf_reserve(&p->ofpbuf, headroom);
> +
> +    ofpbuf_put(&p->ofpbuf, ofpbuf_data(b), ofpbuf_size(b));
> +
> +    if (b->frame) {
> +        uintptr_t data_delta
> +            = (char *)ofpbuf_data(&p->ofpbuf) - (char *)ofpbuf_data(b);
> +
> +        p->ofpbuf.frame = (char *) b->frame + data_delta;
> +    }
> +    p->ofpbuf.l2_5_ofs = b->l2_5_ofs;
> +    p->ofpbuf.l3_ofs = b->l3_ofs;
> +    p->ofpbuf.l4_ofs = b->l4_ofs;
> +
> +    return p;
> +}
> +
> +struct dpif_packet *
> +dpif_packet_clone(struct dpif_packet * p)
> +{
> +    struct dpif_packet *newp;
> +
> +    newp = dpif_packet_clone_from_ofpbuf(&p->ofpbuf);
> +
> +    return newp;
> +}
> diff --git a/lib/packet-dpif.h b/lib/packet-dpif.h
> new file mode 100644
> index 0000000..0c04d14
> --- /dev/null
> +++ b/lib/packet-dpif.h
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright (c) 2014 Nicira, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef PACKET_DPIF_H
> +#define PACKET_DPIF_H 1
> +
> +#ifdef  __cplusplus
> +extern "C" {
> +#endif
> +
> +/* A packet received from a netdev and passed to a dpif. */
> +
> +struct dpif_packet {
> +    struct ofpbuf ofpbuf;       /* Packet data. */
> +};
> +
> +struct dpif_packet * dpif_packet_new_with_headroom(size_t size,
> +                                                   size_t headroom);
> +
> +struct dpif_packet * dpif_packet_clone_from_ofpbuf(const struct ofpbuf * b);
> +
> +struct dpif_packet * dpif_packet_clone(struct dpif_packet * p);
> +
> +#ifdef  __cplusplus
> +}
> +#endif
> +
> +#endif /* packet-dpif.h */
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 71eaad1..30ac1dd 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -46,6 +46,7 @@
>  #include "ofproto/ofproto-dpif-sflow.h"
>  #include "ofproto/ofproto-dpif.h"
>  #include "ofproto/ofproto-provider.h"
> +#include "packet-dpif.h"
>  #include "tunnel.h"
>  #include "vlog.h"
>
> @@ -2643,7 +2644,7 @@ execute_controller_action(struct xlate_ctx *ctx, int 
> len,
>                            uint16_t controller_id)
>  {
>      struct ofproto_packet_in *pin;
> -    struct ofpbuf *packet;
> +    struct dpif_packet *packet;
>      struct pkt_metadata md = PKT_METADATA_INITIALIZER(0);
>
>      ctx->xout->slow |= SLOW_CONTROLLER;
> @@ -2651,7 +2652,7 @@ execute_controller_action(struct xlate_ctx *ctx, int 
> len,
>          return;
>      }
>
> -    packet = ofpbuf_clone(ctx->xin->packet);
> +    packet = dpif_packet_clone_from_ofpbuf(ctx->xin->packet);
>
>      ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
>                                            &ctx->xout->odp_actions,
> @@ -2662,8 +2663,8 @@ execute_controller_action(struct xlate_ctx *ctx, int 
> len,
>                          ofpbuf_size(&ctx->xout->odp_actions), NULL);
>
>      pin = xmalloc(sizeof *pin);
> -    pin->up.packet_len = ofpbuf_size(packet);
> -    pin->up.packet = ofpbuf_steal_data(packet);
> +    pin->up.packet_len = ofpbuf_size(&packet->ofpbuf);
> +    pin->up.packet = ofpbuf_steal_data(&packet->ofpbuf);
>      pin->up.reason = reason;
>      pin->up.table_id = ctx->table_id;
>      pin->up.cookie = (ctx->rule
> @@ -2691,7 +2692,7 @@ execute_controller_action(struct xlate_ctx *ctx, int 
> len,
>          }
>      }
>      ofproto_dpif_send_packet_in(ctx->xbridge->ofproto, pin);
> -    ofpbuf_delete(packet);
> +    ofpbuf_delete(&packet->ofpbuf);

This is confusing to see a separate API to allocate dpif_packet, but
we are using ofpbuf delete to delete it, Can you add new API to delete
dpif-packet?

>  }
>
>  static void
> --
> 2.0.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to