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