On Sat, Aug 19, 2017 at 06:20:23AM +0800, Yang, Yi wrote:
> On Sat, Aug 19, 2017 at 01:36:57AM +0800, Ben Pfaff wrote:
> > On Fri, Aug 18, 2017 at 07:38:56AM +0800, Yi Yang wrote:
> > > v1->v2
> > >   - Rework per kernel datapath review comments
> > >   - Add new NSH key ttl
> > >   - Add many helpers in nsh.h and replace much code
> > >     with these helpers
> > >   - nsh.h includes the lasted NSH spec
> > >   - bits of flags and mdtype have a change
> > > 
> > > This patch seires reworks NSH netlink keys and actions
> > > per kernel datapath requirments. OVS_KEY_ATTR_NSH is
> > > changed as a nested key, encap_nsh and decap_nsh are
> > > renamed as push_nsh and pop_nsh. PUSH_NSH used nested
> > > OVS_KEY_ATTR_NSH keys to transfer NSH header data.
> > > 
> > > It also adds new NSH key 'ttl' to follow the lasted
> > > IETF NSH draft:
> > > 
> > > https://datatracker.ietf.org/doc/draft-ietf-sfc-nsh/
> > > 
> > > I have double confirmed from one of its authors, this
> > > is a final version which will be approved as IETF RFC,
> > > the NSH header format won't be change anymore.
> > 
> > I think it's very wise to make 2.8 compliant with the latest NSH draft.
> > It looks like that's patch 2.
> > 
> > It seems less valuable to me to rework the internals other than to gain
> > this compliance, since as far as I can tell the kernel-side details are
> > still under review and have not been applied to net-next.  I don't think
> > it's worth closely tracking the proposals there.
> > 
> > Would you mind sending a patch that just makes the NSH implementation
> > compliant with the latest Internet-Draft, without irrelevant changes?
> > That is more suitable for a stable branch.
> > 
> > Thanks,
> 
> Forgot addressing patch 1, I think all the people have agreed we need to
> do the below changes:
> 
> 1. change ecanp_nsh and decap_nsh to push_nsh and pop_nsh
> 2. Use nested OVS_KEY_ATTR_NSH to handle push_nsh.
> 
> Patch 1 is precisely doing this way.

Since this is targeted at 2.8, I'm only planning to take actual bug
fixes, or changes that affect Open vSwitch external interfaces where we
historically maintain a high degree of backward compatibility.

In patch 1, it looks like the change to nsh_hdr_len() is a bug fix.  At
a glance, I am not sure whether any of the other changes in patch 1 are
bug fixes.

Also in patch 1, renaming and Netlink restructuring isn't a bug fix,
doesn't appear to affect, say, OpenFlow action names, and it doesn't
affect any ABIs, so it's not appropriate for 2.8.

Making 2.8 compliant with the NSH draft makes sense.

Can you edit this series so that patch 1 is just bug fixes, and then
re-send it?

Thanks,

Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to