Andy,
I will send out a v2 patch for your review soon.
Also, thanks for reviewing the patch in detail. My comments are inserted
below.
On 4/2/2014 3:54 PM, Andy Zhou wrote:
Thomas, thanks a lot for working on this.
The patch seems to be incomplete. Is this intentional? If yes, then we
may want delay mentioning it
in the news.
Don't mention it in news yet. I am still testing it. I plan to test it
in a "real" network today or tomorrow.
1) User space open flow PUSH_VLAN currently blocking 802.1ad header.
That needs to be fixed for
open flow versions that supports it.
I thought I caught everywhere it was blocking on non-8021Q. However, I
just found another place. I may have to
add tpid field to struct flow because it just assumes TPID is 8021Q if
the action is PUSH_VLAN.
2) User space VLAN flow handling may need adjustments as well
See comment above.
3) Kernel data receiving path needs to accept 802.1ad header.
I think this is OK on the receive side until we need to execute pop action.
Should this be && ?
Yes it should be. Thanks much for this catch!!
It looks odd that they use slightly different APIs.
Yes, it is odd and its a workaround for a problem where some nics with
HW acceleration won't work on 802.1ah Ethertype.
This is a bug in the upstream kernel too. 802.1ad packets are broken on
some nics. Its been discussed in netdev kernel list too.
Same API comment here.
See my comment above.
I am not sure if we need a separate else here, Why not just combine
with ETH_P_8021Q above?
This is a perhaps ugly way to get around problem mentioned above with
upstream kernel to make sure that
802.1ad packets don't get sent assuming HW Acceleration.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
