[ovs-dev] [PATCH v3 1/3] dpif-netdev: use dpif_packet structure for packets

Daniele Di Proietto ddiproietto at vmware.com
Sat Jun 7 05:03:28 UTC 2014


Hi Jarno,

> - dpif_packet should not be defined ofpbuf.h. If it is exposed in the dpif API, then it should be in dpif-provider.h, if it is private to dpif-netdev, then somewhere else.

It can’t be completely private to dpif-netdev, but we can surely place it elsewhere.

> - I would prefer not try to hide that dpif_packet contains an ofpbuf, i.e., do not introduce abstractions that duplicate ofpbuf functionality.  This patch would seem simpler if the ofpbuf member of the dpif_packet would be used directly in place of an accessor/getter function.

If you think it would be better, I can easily change this.

> - Related to the above, I don't see why cloning an ofpbuf to the dpif_packet should be necessary. Maybe later patches get rid of the copying?

It is necessary, for example in dpif_netdev_execute: we get an ofpbuf and we have to pass a dpif_packet to the dpif provider, somehow. The copy would have happened most of the times anyways, since dp_netdev_execute was called with ‘may_steal’ set to false (it is true now).

If you think we can fit uint32_t (the dp_hash) into ofpbuf (if NETDEV_DPDK is _not_ defined) we can avoid adding dpif_packet (for now).

Thanks,

Daniele


----- Original Message -----
From: "Jarno Rajahalme" <jrajahalme at nicira.com>
To: "Daniele Di Proietto" <ddiproietto at vmware.com>
Cc: dev at openvswitch.org
Sent: Friday, June 6, 2014 8:04:37 PM
Subject: Re: [ovs-dev] [PATCH v3 1/3] dpif-netdev: use dpif_packet structure for packets

Some high-level comments for now:

- dpif_packet should not be defined ofpbuf.h. If it is exposed in the dpif API, then it should be in dpif-provider.h, if it is private to dpif-netdev, then somewhere else.

- I would prefer not try to hide that dpif_packet contains an ofpbuf, i.e., do not introduce abstractions that duplicate ofpbuf functionality.  This patch would seem simpler if the ofpbuf member of the dpif_packet would be used directly in place of an accessor/getter function.

- Related to the above, I don't see why cloning an ofpbuf to the dpif_packet should be necessary. Maybe later patches get rid of the copying?

  Jarno

> On Jun 6, 2014, at 5:13 PM, Daniele Di Proietto <ddiproietto at 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 <ddiproietto at vmware.com>
> ---
> lib/dpif-netdev.c            | 42 ++++++++++++++-----------
> lib/dpif.c                   | 12 ++++---
> lib/netdev-bsd.c             | 10 ++++--
> lib/netdev-dpdk.c            | 11 ++++---
> lib/netdev-dummy.c           |  7 +++--
> lib/netdev-linux.c           | 10 ++++--
> lib/netdev-provider.h        |  2 +-
> lib/netdev.c                 |  2 +-
> lib/netdev.h                 |  4 ++-
> lib/odp-execute.c            | 52 +++++++++++++++++-------------
> lib/odp-execute.h            | 12 +++----
> lib/ofpbuf.h                 | 75 ++++++++++++++++++++++++++++++++++++++++++++
> ofproto/ofproto-dpif-xlate.c | 10 +++---
> 13 files changed, 177 insertions(+), 72 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index c89ae20..48d20f4 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -337,11 +337,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);
> @@ -1518,6 +1519,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;
> @@ -1533,7 +1535,9 @@ 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 = dpif_packet_clone_from_ofpbuf(execute->packet);
> +
> +    dp_netdev_execute_actions(dp, &key.flow, packet, true, md,
>                               execute->actions, execute->actions_len);
> 
>     return 0;
> @@ -1747,7 +1751,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);
> @@ -1992,27 +1996,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 = dpif_packet_to_ofpbuf(packet);
>     struct {
>         struct miniflow flow;
>         uint32_t buf[FLOW_U32S];
>     } key;
> 
> -    if (ofpbuf_size(packet) < ETH_HEADER_LEN) {
> -        ofpbuf_delete(packet);
> +    if (dpif_packet_size(packet) < ETH_HEADER_LEN) {
> +        dpif_packet_delete(packet);
>         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,
> @@ -2020,15 +2025,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();
> @@ -2098,7 +2102,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
> @@ -2112,7 +2116,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, dpif_packet_to_ofpbuf(packet), may_steal);
>         }
>         break;
> 
> @@ -2121,7 +2125,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
> +                           ? dpif_packet_to_ofpbuf(packet)
> +                           : ofpbuf_clone(dpif_packet_to_ofpbuf(packet));
> 
>         dp_netdev_output_userspace(aux->dp, userspace_packet,
>                                    miniflow_hash_5tuple(aux->key, 0)
> @@ -2156,9 +2162,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)++;
> @@ -2185,7 +2191,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 ac73be1..0c1bcc9 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1058,7 +1058,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)
> {
> @@ -1072,7 +1072,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 = dpif_packet_to_ofpbuf(packet);
>         execute.md = *md;
>         execute.needs_help = false;
>         aux->error = aux->dpif->dpif_class->execute(aux->dpif, &execute);
> @@ -1100,12 +1100,14 @@ 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 = dpif_packet_clone_from_ofpbuf(execute->packet);
> +
> +    odp_execute_actions(&aux, packet, true, &execute->md, execute->actions,
> +                        execute->actions_len, dpif_execute_helper_cb);
>     return aux.error;
> }
> 
> diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
> index 8dc33df..f0088b8 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..1e90fae 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -217,16 +217,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 +586,8 @@ dpdk_queue_flush(struct netdev_dpdk *dev, int qid)
> }
> 
> 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 501fb82..ad90f17 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -754,7 +754,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);
> @@ -778,7 +778,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 c1d9323..1dca9d5 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -984,10 +984,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 +998,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 = dpif_packet_to_ofpbuf(packet);
> 
>     retval = (rx->is_tap
>               ? netdev_linux_rxq_recv_tap(rx->fd, buffer)
> @@ -1010,7 +1014,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 dd800a4..009c159 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);
> 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..ceb4208 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -64,7 +64,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 +88,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(dpif_packet_to_ofpbuf(packet),
>                           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(dpif_packet_to_ofpbuf(packet), 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(dpif_packet_to_ofpbuf(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);
>         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(dpif_packet_to_ofpbuf(packet), 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(dpif_packet_to_ofpbuf(packet), 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(dpif_packet_to_ofpbuf(packet), 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(dpif_packet_to_ofpbuf(packet), 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(dpif_packet_to_ofpbuf(packet),
> +                nl_attr_get_unspec(a, sizeof(struct ovs_key_arp)));
>         break;
> 
>     case OVS_KEY_ATTR_DP_HASH:
> @@ -152,13 +158,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 +199,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 +236,7 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, bool steal,
>                 struct flow flow;
>                 uint32_t hash;
> 
> -                flow_extract(packet, md, &flow);
> +                flow_extract(dpif_packet_to_ofpbuf(packet), md, &flow);
>                 hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
>                 md->dp_hash = hash ? hash : 1;
>             } else {
> @@ -242,22 +248,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(dpif_packet_to_ofpbuf(packet),
> +                          htons(ETH_TYPE_VLAN), vlan->vlan_tci);
>             break;
>         }
> 
>         case OVS_ACTION_ATTR_POP_VLAN:
> -            eth_pop_vlan(packet);
> +            eth_pop_vlan(dpif_packet_to_ofpbuf(packet));
>             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(dpif_packet_to_ofpbuf(packet),
> +                      mpls->mpls_ethertype, mpls->mpls_lse);
>             break;
>          }
> 
>         case OVS_ACTION_ATTR_POP_MPLS:
> -            pop_mpls(packet, nl_attr_get_be16(a));
> +            pop_mpls(dpif_packet_to_ofpbuf(packet), nl_attr_get_be16(a));
>             break;
> 
>         case OVS_ACTION_ATTR_SET:
> @@ -277,7 +285,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 +295,6 @@ odp_execute_actions(void *dp, struct ofpbuf *packet, bool steal,
> 
>     if (!actions_len && steal) {
>         /* Drop action. */
> -        ofpbuf_delete(packet);
> +        dpif_packet_delete(packet);
>     }
> }
> 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/ofpbuf.h b/lib/ofpbuf.h
> index 13a3e9d..8d2a87c 100644
> --- a/lib/ofpbuf.h
> +++ b/lib/ofpbuf.h
> @@ -420,6 +420,81 @@ static inline void ofpbuf_set_size(struct ofpbuf *b, uint32_t v)
>     b->size_ = v;
> }
> #endif
> +
> +/* A packet received from a netdev and passed to a dpif. */
> +
> +struct dpif_packet {
> +    struct ofpbuf packet;       /* Packet data. */
> +};
> +
> +static inline 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->packet;
> +
> +    ofpbuf_init(b, size + headroom);
> +    ofpbuf_reserve(b, headroom);
> +
> +    return p;
> +}
> +
> +static inline struct ofpbuf *
> +dpif_packet_to_ofpbuf(struct dpif_packet * p)
> +{
> +    return &p->packet;
> +}
> +
> +static inline 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->packet, ofpbuf_size(b) + headroom);
> +    ofpbuf_reserve(&p->packet, headroom);
> +
> +    ofpbuf_put(&p->packet, ofpbuf_data(b), ofpbuf_size(b));
> +
> +    if (b->frame) {
> +        uintptr_t data_delta
> +            = (char *)ofpbuf_data(&p->packet) - (char *)ofpbuf_data(b);
> +
> +        p->packet.frame = (char *) b->frame + data_delta;
> +    }
> +    p->packet.l2_5_ofs = b->l2_5_ofs;
> +    p->packet.l3_ofs = b->l3_ofs;
> +    p->packet.l4_ofs = b->l4_ofs;
> +
> +    return p;
> +}
> +
> +static inline size_t
> +dpif_packet_size(struct dpif_packet * p)
> +{
> +    return ofpbuf_size(&p->packet);
> +}
> +
> +static inline void
> +dpif_packet_delete(struct dpif_packet * p)
> +{
> +    ofpbuf_delete(&p->packet);
> +}
> +
> +static inline struct dpif_packet *
> +dpif_packet_clone(struct dpif_packet * p)
> +{
> +    struct dpif_packet *newp;
> +
> +    newp = dpif_packet_clone_from_ofpbuf(dpif_packet_to_ofpbuf(p));
> +
> +    return newp;
> +}
> +static inline struct dpif_packet *
> +dpif_packet_cast_from_ofpbuf(struct ofpbuf *b)
> +{
> +    return CONTAINER_OF(b, struct dpif_packet, packet);
> +}
> 
> #ifdef  __cplusplus
> }
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index eded9d8..202084c 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -2643,7 +2643,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 +2651,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 +2662,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 = dpif_packet_size(packet);
> +    pin->up.packet = ofpbuf_steal_data(dpif_packet_to_ofpbuf(packet));
>     pin->up.reason = reason;
>     pin->up.table_id = ctx->table_id;
>     pin->up.cookie = (ctx->rule
> @@ -2691,7 +2691,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
>         }
>     }
>     ofproto_dpif_send_packet_in(ctx->xbridge->ofproto, pin);
> -    ofpbuf_delete(packet);
> +    dpif_packet_delete(packet);
> }
> 
> static void
> -- 
> 2.0.0.rc2
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=MV9BdLjtFIdhBDBaw5z%2BU6SSA2gAfY4L%2F1HCy3VjlKU%3D%0A&m=ElBoHEywzDVt3LbAhZLFTGdGUL50h7%2BKS5FLgsqbsww%3D%0A&s=b4ce3cac91a725aedea313c37d2a5c250f25a5417354f0cdf59f2a6ac7921058



More information about the dev mailing list