Hello Anil The only standing issue with API backward compatibility is whether to expose a reg_load2 action in the API and forcing changes on applications to use that for nsh fields, vs using the currently exposed reg_load action and internally encode that to reg_load2 for these nsh fields.
Even not worrying about backward API compatibility, the second option is still worthy of consideration IMHO. The only useful reason to expose reg_load2 in the API would be to be able for the user to set complex structured fields, which there are none right now. The fact that right now we need to encode that action for experimenter fields is an implementation detail that the user might not be concerned about. Even so, we might want to expose reg_load2 given that it may come the day when there is really a use for it. We do need to consider that the payload of a reg_load2 action could be any field and this does add a level of complexity both in the modelling and conversion logic, an extra effort that wouldn't be really useful until that day comes. At that time, it has probably more value to work on being able to use standard openflow action set_field (functionally equivalent) with extension fields. Something similar happens with output_reg action vs output_reg2 that supports experimenter fields. In this case, there is no functional difference between the two, so it looks like it makes sense to encode output_reg2 for all fields, and drop output_reg alltogether, without any change on APIs. BR Jaime. -----Original Message----- From: Anil Vishnoi <[email protected]> To: Hema Gopalkrishnan <[email protected]>, Prasanna Huddar <[email protected]>, Abhijit Kumbhare <abhijitkoss@gm ail.com>, Shuva Kar <[email protected]>, [email protected] Cc: openflowplugin-dev <[email protected]> Subject: Re: [openflowplugin-dev] Implementing nsh support available since OVS 2.8 Date: Sun, 20 May 2018 23:12:28 -0700 On Mon, May 14, 2018 at 11:35 AM, Jaime Caamaño Ruiz <[email protected]> wrote: > Hello all > > I am working on adding to openflowplugin the nsh support that has > been > available since OVS 2.8. Some of us had some discussions about this > on > DDF, that have continued on the project meetings and now continue on > this email. > > Openflowplugin already had nsh support for an OVS 2.5/2.6 unoficially > patched out of branch with [1], but now the official available > support > is somewhat different to that. > > Early discussions we had about this we talked about preserving > backward > compatibility, but it seems that some of us understood different > things > about backward compatibility: whether we wanted API backward > compatibility for applications or whether we want to support both old > OSV+nsh patch and new OVS nsh implementations at the same time. I > assumed the former, but needs to be further discussed. Afaik, user > applications are SFC, Netvirt and GBP. Unaware about downstreamers, > but > users unlikely due to being a patched OVS. > > The main difference between both implementations is that the same nsh > match fields are now experimenter oxm_class=0xFFFF with experimenter > ID > NXOXM_NSH=0x005ad650. These are the first of its kind, as existing > nicira fields up until now, including the previous version of nsh > fields already implemented for patched OVS, are all standard OXM > fields > under nicira vendor/class (NXM_OF=0x0000, NXM_NX=0x0001). For > reference, new nsh fields introduced with OVS 2.8 that did not exist > on > the patched OVS are already proposed on patch [2], where as > coincidental fields are to be migrated to the new enconding in a > future > patch. > > Aside from the fields themselves, some of the existing openflowplugin > nicira actions that were being used to manipulate these fields > require > changes to be able to support experimenter fields: > > - NXAST_REG_LOAD action is used to set values to nicira fields. This > action does not support oxm experimenter fields. NXAST_REG_LOAD2 > needs > to be used instead. There is already a patch proposed for this [3]. > This patch will keep intact the existing application API for reg load > action, and internally implement NXAST_REG_LOAD or NXAST_REG_LOAD2 > depending on whether the field to be set is experimenter or not. > > - NXAST_RAW_REG_MOVE action is used to copy values between nicira oxm > fields. It does support experimenter fields, but not as implemented > on > its current form as it allows only 4 byte for oxm headers. Needs to > be > extended to support 8 byte experimenter oxm headers. There is already > a > patch proposed for this [4]. > > - NXAST_RAW_OUTPUT_REG action is used to out to a port based on the > value of a field. Similarly to NXAST_REG_LOAD, it does not support > experimenter oxm headers, NXAST_RAW_OUTPUT_REG2 needs to be used > instead. > > Topics I would like to discuss: > > - What kind of backward compatibility would we like? OVS backward > compatibility to an unofficial out of branch patched OVS? API > backward > compatibility for applications? Both? None? Previous NSH related API's in openflowplugin where based on old NSH OVS (2.6) patches, and these API's were working with OVS 2.6 + NSH patch. Now if we implement new NSH API's and just keep the API compatibility, but it's breaks the functionality for OVS2.6, than keeping the API compatibility is pretty much useless because those API's were designed based on the old NSH OVS patch. So either we keep the end to end backward compatibility or nothing. Ideally deprecating the API takes the process of first deprecating the old API for one release and remove it during the next release, but given that these API's are very specific to OVS and related to NSH functionality that never existed in formally released OVS version, i believe we should remove the old API's and implement the new one. That will avoid any confusion for the end-user. Also as Brady mentioned, i doubt there will be any downstream consumer apart from the ODL projects (SFC, NetVirt, GBP), so removing these API's without deprecation process should be safe. @Abhijit, @Prasanna @Hema, @Shuva @jozef what is your take on it? > - As these are experimenter fields, we are extending the experimenter > id case in the model as proposed on [2], but Anil sugested if we > could > avoid this. I think we could but I would like to understand why. > - As propoosed in [3], is it accetable to keep a single API for > openflow actions that have two versions, non-experimenter vs > experimenter? Initially thought to be API backward compatible, but > even > regardless? Answer to these questions depends on how we answer the first question. > [1] https://github.com/yyang13/ovs_nsh_patches > [2] https://git.opendaylight.org/gerrit/#/c/71895/ > [3] https://git.opendaylight.org/gerrit/#/c/71917/ > [4] https://git.opendaylight.org/gerrit/#/c/71999/ > > Thanks and BR > Jaime. > > > _______________________________________________ > openflowplugin-dev mailing list > [email protected] > https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowplugin-dev mailing list [email protected] https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev
