Hi Ben, All your code review comments have been taken care for MPLS. I did retest the code after incorporating code review comments. I also modified the code for QinQ (common comments which apply to QinQ code as well), added additional automation tests in odp.at for QinQ. In addition, patch to support MPLS stack bit match is ready as well.
Atleast I feel we can now continue on QinQ (userspace + openflow) changes and later on I can work with you and Jesse on kernel code review for both MPLS and QinQ. Please let me know how you want to proceed. Thanks, Ravi On Tue, May 29, 2012 at 11:01 AM, <ravi.ke...@telekom.com> wrote: > > -----Original Message----- > From: Ben Pfaff [mailto:b...@nicira.com] > Sent: Friday, May 25, 2012 9:32 AM > To: Kerur, Ravi > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] MPLS and VLAN QinQ patch > > On Fri, May 25, 2012 at 01:24:00AM +0200, ravi.ke...@telekom.com wrote: >> On Thu, May 24, 2012 at 11:58:53PM +0200, ravi.ke...@telekom.com wrote: >> > Ben Pfaff writes: >> > > get_l3_ttl_and_tos() >> > > -------------------- >> > > >> > > Why does get_l3_ttl_and_tos() consider an unknown IP version as >> > > success, with a default? This seems odd. >> > >> > To handle non-IP traffic i.e push/pop MPLS over non-IP traffic e.g. >> > ARP(practical?), VPLS. I had also mentioned in previous discussion >> > that MPLS ttl actions will not work for these type of traffic but >> > other actions lke push_mpls,pop_mpls, set_mpls_label/set_mpls_tc >> > would be. In this function I keep them to default for non-IP traffic. >> >> But that only comes up in one situation: copying the TTL inward or outward >> when there is exactly one MPLS label. In other situations (pushing an MPLS >> label or popping one) we know the correct inner Ethertype either because >> it's in the packet or because it's an argument to the action. When we know >> it, we should use it. >> >> <rk> Cases for multiple MPLS headers currently happen only at Provider core >> routers and yes in that case we don't have to worry, I was thinking more in >> terms of Provider-Edge or MPLS termination point. For those multiple MPLS >> label case it is already handled and I use existing mpls header to copy. I >> will closely look at it one more time probably I am missing your point. I >> think you are probably saying extract ethtype from packet and use it to >> derive ttl and tos? > > Yes, that's what I had in mind. > > Thanks, > > Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
