On 4/25/14, 5:19 AM, Jesse Gross wrote:
+noethernet:
if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE,
output->eth.type))
goto nla_put_failure;
Does it still make sense to send the EtherType? It's not present in
the packet and I believe that the information is contained in the
attributes that we do send (i.e. IPv4 or v6 attributes).
We had a discussion about this in August last year:
One such decision is how to handle the flow key. I set all
fields
in
key->eth to 0, except the type, because we still need to know
what
kind
of L3 packet do we have. Since a lot of code is accessing
key->eth.type, this is easier than having this information in a
different place, although it would be more elegant to set this
field
to
0 as well.
Can we use skb->protocol for the cases where we currently access
the
EtherType? Are there cases that you are particularly concerned
about?
The only case I'm concerned about is in the action validation code,
particularly the validate_set() and validate_tp_port() functions,
since
they
don't have access to the skb, and need to know the layer 3 protocol
of
the
flow. In validate_set() we could relax the check for eth.type for
OVS_KEY_ATTR_IPV4 and OVS_KEY_ATTR_IPV6, but I'm not sure what to
do
about
validate_tp_port().
In general, I think it would be a good idea for the flow key to
have
a
field
specifying the layer 3 protocol, when an ethernet header is not
present.
That makes sense to me.
You mean that we keep eth.type as the L3 protocol field, or define a
new
one, separate from the eth union?
I think it's fine to keep using eth.type as the protocol field. I
think we can probably some holes to move the is_layer3 flag into the
non-tunnel portion of the flow though.
Should we revisit this discussion?
I was just referring to the Netlink encoding here. Can we populate the
flow key in the kernel when we translate the flow?
Not sure I understand the question.
Going through the code, I see that omitting OVS_KEY_ATTR_ETHERTYPE
currently
means an 802.2 packet, if the mask is set to 0xffff. Are you suggesting
to
omit OVS_KEY_ATTR_ETHERTYPE for layer 3 packets both in the flow key and
the
mask?
All I was trying to say is that is that the Netlink and struct
sw_flow_key representations don't necessarily have to be exactly the
same as long as we can translate back and forth. I'm not sure that the
previous discussion applies - this is more about Netlink and that
seemed to apply to struct sw_flow_key.
Ok, understand now. But validation would become even more complicated if we
didn't send the OVS_KEY_ATTR_ETHERTYPE over Netlink for layer 3 packets.
I think we would basically have to scan for L3 attributes and use that
to fill in the EtherType, right? Is there anything else?
It would be nice to fully break the link to Ethernet and it seems like
a half measure if we keep EtherType in. There have been various
requests for non-Ethernet protocols in the past so it seems like it
could get messy if we assume some elements of Ethernet.
I had a discussion about this with Thomas Morin, who proposed the
IP-over-GRE patches and MPLS-over-GRE patches based on this work, and we
think it would still be useful to keep the EtherType, even in the
Netlink attributes, to be able to support generic packet types in the
future.
Ben was involved in EXT-112 at the ONF, which has the objective to
"Support non-Ethernet packets throughout the pipeline". The current
version of the proposal, recently moved from incubation to prototyping,
defines a new "packet type" match field, which is used to determine the
type of the first header. There are 5 namespaces that can be used to
define the packet type, one of which is EtherType. With this match
type, the canonical representation for an IPv4 packet would be
(namespace=1,ns_type=0x800), namespace 1 being the EtherType.
Because of the potential future implementation of EXT-112 in OVS I think
keeping the EtherType around would be beneficial.
I'm starting to think that the path we went down with the megaflow
Netlink encoding is not particularly sustainable because it means that
while the keys can do something fairly reasonable, the masks must
always list all the protocols that it is not, such as Ethernet and
EtherType in this case.
I wonder if it actually makes more sense to switch over to something
similar to what Jarno is planning for the actions - a key and mask
paired together and then we can also fix some left over oddities from
the exact match days.
I wonder if we could refine this in later patches?
My hope is that we can at least get some other opinions on this first.
Sure, looking forward to that!
-Lori
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
