Looks Good to me.

Is there any reason not to simply always set packet->l2?  I think it's
fine how it is, just curious.

Ethan

On Fri, Mar 25, 2011 at 10:35 AM, Ben Pfaff <b...@nicira.com> wrote:
> This will soon be used in the upcoming bond library.
> ---
>  lib/dpif-netdev.c |   32 +-------------------------------
>  lib/packets.c     |   35 ++++++++++++++++++++++++++++++++++-
>  lib/packets.h     |    2 ++
>  3 files changed, 37 insertions(+), 32 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 035ceae..cffbc9b 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -1137,36 +1137,6 @@ dp_netdev_wait(void)
>  }
>
>
> -/* Modify the TCI field of 'packet'.  If a VLAN tag is present, its TCI field
> - * is replaced by 'tci'.  If a VLAN tag is not present, one is added with the
> - * TCI field set to 'tci'.
> - */
> -static void
> -dp_netdev_set_dl_tci(struct ofpbuf *packet, uint16_t tci)
> -{
> -    struct vlan_eth_header *veh;
> -    struct eth_header *eh;
> -
> -    eh = packet->l2;
> -    if (packet->size >= sizeof(struct vlan_eth_header)
> -        && eh->eth_type == htons(ETH_TYPE_VLAN)) {
> -        veh = packet->l2;
> -        veh->veth_tci = tci;
> -    } else {
> -        /* Insert new 802.1Q header. */
> -        struct vlan_eth_header tmp;
> -        memcpy(tmp.veth_dst, eh->eth_dst, ETH_ADDR_LEN);
> -        memcpy(tmp.veth_src, eh->eth_src, ETH_ADDR_LEN);
> -        tmp.veth_type = htons(ETH_TYPE_VLAN);
> -        tmp.veth_tci = tci;
> -        tmp.veth_next_type = eh->eth_type;
> -
> -        veh = ofpbuf_push_uninit(packet, VLAN_HEADER_LEN);
> -        memcpy(veh, &tmp, sizeof tmp);
> -        packet->l2 = (char*)packet->l2 - VLAN_HEADER_LEN;
> -    }
> -}
> -
>  static void
>  dp_netdev_strip_vlan(struct ofpbuf *packet)
>  {
> @@ -1369,7 +1339,7 @@ dp_netdev_execute_actions(struct dp_netdev *dp,
>             break;
>
>         case ODP_ACTION_ATTR_SET_DL_TCI:
> -            dp_netdev_set_dl_tci(packet, nl_attr_get_be16(a));
> +            eth_set_vlan_tci(packet, nl_attr_get_be16(a));
>             break;
>
>         case ODP_ACTION_ATTR_STRIP_VLAN:
> diff --git a/lib/packets.c b/lib/packets.c
> index f16e749..4c6cc48 100644
> --- a/lib/packets.c
> +++ b/lib/packets.c
> @@ -75,6 +75,40 @@ compose_benign_packet(struct ofpbuf *b, const char *tag, 
> uint16_t snap_type,
>     memcpy(payload + tag_size, eth_src, ETH_ADDR_LEN);
>  }
>
> +/* Modify the TCI field of 'packet', whose data must begin with an Ethernet
> + * header.  If a VLAN tag is present, its TCI field is replaced by 'tci'.  
> If a
> + * VLAN tag is not present, one is added with the TCI field set to 'tci'.
> + *
> + * If 'packet->l2' is set before the call, then it should point to the 
> Ethernet
> + * header.  This function will adjust it, if necessary, so that it still 
> points
> + * to the Ethernet header afterward.  afterward. */
> +void
> +eth_set_vlan_tci(struct ofpbuf *packet, ovs_be16 tci)
> +{
> +    struct eth_header *eh = packet->data;
> +    struct vlan_eth_header *veh;
> +
> +    if (packet->size >= sizeof(struct vlan_eth_header)
> +        && eh->eth_type == htons(ETH_TYPE_VLAN)) {
> +        veh = packet->data;
> +        veh->veth_tci = tci;
> +    } else {
> +        /* Insert new 802.1Q header. */
> +        struct vlan_eth_header tmp;
> +        memcpy(tmp.veth_dst, eh->eth_dst, ETH_ADDR_LEN);
> +        memcpy(tmp.veth_src, eh->eth_src, ETH_ADDR_LEN);
> +        tmp.veth_type = htons(ETH_TYPE_VLAN);
> +        tmp.veth_tci = tci;
> +        tmp.veth_next_type = eh->eth_type;
> +
> +        veh = ofpbuf_push_uninit(packet, VLAN_HEADER_LEN);
> +        memcpy(veh, &tmp, sizeof tmp);
> +        if (packet->l2) {
> +            packet->l2 = packet->data;
> +        }
> +    }
> +}
> +
>  /* Stores the string representation of the IPv6 address 'addr' into the
>  * character array 'addr_str', which must be at least INET6_ADDRSTRLEN
>  * bytes long. */
> @@ -163,7 +197,6 @@ ipv6_count_cidr_bits(const struct in6_addr *netmask)
>     return count;
>  }
>
> -
>  /* Returns true if 'netmask' is a CIDR netmask, that is, if it consists of N
>  * high-order 1-bits and 128-N low-order 0-bits. */
>  bool
> diff --git a/lib/packets.h b/lib/packets.h
> index 17ef5a6..0ca5631 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -131,6 +131,8 @@ void compose_benign_packet(struct ofpbuf *, const char 
> *tag,
>                            uint16_t snap_type,
>                            const uint8_t eth_src[ETH_ADDR_LEN]);
>
> +void eth_set_vlan_tci(struct ofpbuf *, ovs_be16 tci);
> +
>  /* Example:
>  *
>  * uint8_t mac[ETH_ADDR_LEN];
> --
> 1.7.1
>
> _______________________________________________
> 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