On Fri, Nov 14, 2014 at 3:51 AM, Lorand Jakab <loja...@cisco.com> wrote:
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 3607170..3ecb3cc 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -570,6 +558,23 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb,
> struct genl_info *info)
> if (err)
> goto err_flow_free;
>
> + skb_reset_mac_header(packet);
> +
> + if (flow->key.phy.is_layer3) {
> + skb_reset_network_header(packet);
I think the code that was resetting the layer pointers here was mostly
just so that we can access the Ethernet header. Now that it is
happening after the flow extract, I would prefer to centralize all of
this type of code there.
> + } else {
> + eth = eth_hdr(packet);
> +
> + /* Normally, setting the skb 'protocol' field would be handled
> + * by a call to eth_type_trans(), but it assumes there's a
> + * sending device, which we may not have.
> + */
> + if (ntohs(eth->h_proto) >= ETH_P_802_3_MIN)
> + packet->protocol = eth->h_proto;
> + else
> + packet->protocol = htons(ETH_P_802_2);
We already have similar logic in flow extraction, so maybe we can just
use the EtherType set there.
> + }
Don't we need to set packet->protocol in the is_layer3 case as well?
> diff --git a/datapath/flow.c b/datapath/flow.c
> index b01f7bd..1dd3ac8 100644
> --- a/datapath/flow.c
> +++ b/datapath/flow.c
> +static __be16 ethertype_from_ip_version(struct sk_buff *skb)
> +{
> + struct iphdr *iphdr = ip_hdr(skb);
> +
> + if (iphdr->version == 4)
> + return htons(ETH_P_IP);
> + else if (iphdr->version == 6)
> + return htons(ETH_P_IPV6);
> +
> + return 0;
> +}
I don't really like reaching into the IP header to get the type from
the nibble. In any case, I don't think that it will generalize into
other cases (like MPLS) so it seems better to use the attributes
coming from userspace to figure this out.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
