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" <jrajaha...@nicira.com>
To: "Daniele Di Proietto" <ddiproie...@vmware.com>
Cc: dev@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 <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/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@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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
