On Fri, Nov 09, 2012 at 01:34:30PM -0800, Jesse Gross wrote:
> On Thu, Nov 8, 2012 at 1:06 AM, JieYue Ma <[email protected]> wrote:
>
> > Hi,
> >
> > I am reading ovs code, while in function ovs_flow_to_nlattrs, we have
> >
> > ...
> > if (swkey->eth.tci || swkey->eth.type == htons(ETH_P_8021Q)) {
> > if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, htons(ETH_P_8021Q))
> > ||
> > nla_put_be16(skb, OVS_KEY_ATTR_VLAN, swkey->eth.tci))
> > goto nla_put_failure;
> > encap = nla_nest_start(skb, OVS_KEY_ATTR_ENCAP);
> > if (!swkey->eth.tci)
> > goto unencap;
> > } else {
> > encap = NULL;
> > }
> > ...
> >
> > here's the line: if swkey->eth.tci || swkey->eth.type ==
> > htons(ETH_P_8021Q), well, is it POSSIBLE swkey->eth.tci being zero
> > while swkey->eth.type == htons(ETH_P_8021Q)? Why we need the latter
> > condition judgement?
> > On my stand, from ovs_flow_extract, swkey->eth.type, in most
> > situations, is ETH_P_IP, ETH_P_IPV6, or ETH_P_ARP. It can be
> > ETH_P_8021Q when in nested VLAN scenarios, but swkey->eth.tci also
> > non-zero. So, maybe swkey->eth.type == htons(ETH_P_8021Q) is
> > unnecesary, isn't it?
>
>
> It can happen if the packet is truncated so the EtherType is 0x8100 but it
> is not long enough to contain a vlan header.
It's even documented in datapath/README:
Handling malformed packets
--------------------------
Don't drop packets in the kernel for malformed protocol headers, bad
checksums, etc. This would prevent userspace from implementing a
simple Ethernet switch that forwards every packet.
Instead, in such a case, include an attribute with "empty" content.
It doesn't matter if the empty content could be valid protocol values,
as long as those values are rarely seen in practice, because userspace
can always forward all packets with those values to userspace and
handle them individually.
For example, consider a packet that contains an IP header that
indicates protocol 6 for TCP, but which is truncated just after the IP
header, so that the TCP header is missing. The flow key for this
packet would include a tcp attribute with all-zero src and dst, like
this:
eth(...), eth_type(0x0800), ip(proto=6, ...), tcp(src=0, dst=0)
As another example, consider a packet with an Ethernet type of 0x8100,
indicating that a VLAN TCI should follow, but which is truncated just
after the Ethernet type. The flow key for this packet would include
an all-zero-bits vlan and an empty encap attribute, like this:
eth(...), eth_type(0x8100), vlan(0), encap()
Unlike a TCP packet with source and destination ports 0, an
all-zero-bits VLAN TCI is not that rare, so the CFI bit (aka
VLAN_TAG_PRESENT inside the kernel) is ordinarily set in a vlan
attribute expressly to allow this situation to be distinguished.
Thus, the flow key in this second example unambiguously indicates a
missing or malformed VLAN TCI.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
