On 27 May 2021, at 10:01, Dumitru Ceara wrote:
> On 5/27/21 9:26 AM, Eelco Chaudron wrote: >> >> >> On 25 May 2021, at 15:20, Aaron Conole wrote: >> >>> Dumitru Ceara <[email protected]> writes: >>> >>>> On 5/21/21 11:27 AM, Paolo Valerio wrote: >>>>> Aaron Conole <[email protected]> writes: >>>>> >>>>>> Dumitru Ceara <[email protected]> writes: >>>>>> >>>>>>> On 4/26/21 2:19 PM, Eelco Chaudron wrote: >>>>>>>> Currently, conntrack in the kernel has an undocumented feature referred >>>>>>>> to as all-zero IP address NULL SNAT. Basically, when a source port >>>>>>>> collision is detected during the commit, the source port will be >>>>>>>> translated to an ephemeral port. If there is no collision, no SNAT is >>>>>>>> performed. >>>>>>>> >>>>>>>> This patchset documents this behavior and adds a self-test to verify >>>>>>>> it's not changing. >>>>>>>> >>>>>>>> Signed-off-by: Eelco Chaudron <[email protected]> >>>>>>>> --- >>>>>>>> v3: Renamed NULL SNAT to all-zero IP SNAT. >>>>>>>> v2: Fixed NULL SNAT to only work in the -rpl state to be inline with >>>>>>>> OpenShift-SDN's behavior. >>>>>>> >>>>>>> Hi Eelco, >>>>>>> >>>>>>> Would it be possible to add this capability to the list of kernel >>>>>>> Datapath.capabilities ovsdb column? [0] >>>>>>> >>>>>>> Given that the patch to add userspace datapath support for all-zero IP >>>>>>> SNAT is not accepted yet [1], and even if it does it will likely not be >>>>>>> backported to LTS because it's a feature, this would make it easier for >>>>>>> OVN (for example ovn-controller) to determine at runtime if it should >>>>>>> use all-zero IP SNAT or not. >>>>>> >>>>>> I think it would be rather difficult to add this to the userspace patch >>>>>> being proposed. >>>>>> >>>>>> Actually, if we want something where datapath can detect whether such >>>>>> feature is supported, there isn't a good test. For instance, normally >>>>>> we would attempt to insert a flow that has the characteristics we desire >>>>>> and look for a failure to insert. In the case of all-zero SNAT, >>>>>> both datapaths can "accept" it, so it becomes difficult. >>>>>> >>>>> >>>>> I was wondering the same, a successful insertion doesn't ensure the >>>>> correctness and so the support of this feature. >>>>> >>>>>> Also, the netlink datapath under linux has had this support since it was >>>>>> introduced, so we're really just documenting it here. It can still be >>>>>> used in older releases. OTOH, userspace doesn't have such support. >>>>>> Both datapaths will accept flows with SNAT set to 0, as far as I can >>>>>> tell. This means there's not an easy way to distinguish them. >>>>>> >>>>>> Maybe we need a change to this patchset to reject such flows from the >>>>>> netdev datapath, and then we can modify [1] to remove such limitation. >>>> >>>> That would work, I guess. >>>> >>>>>> Then it can be detected by the datapath capabilities that already exist. >>>> >>>> Sounds good to me but which already existing datapath capability would >>>> you use? As far as I see the existing ones are: >>>> >>>> https://github.com/openvswitch/ovs/blob/13c0eaa7b4fc2694a8c6cc8e6487ec6538c607e4/ofproto/ofproto-dpif.c#L5567 >>>> >>>> Shouldn't we add a new one, e.g., "ct_zero_nat"? >>> >>> Yes, sorry - I think it was meant to use the mechanism that already >>> existed, rather than reuse an existing bit. >>> >> >> I’ll ping Paolo to see if he started the effort, if not I can do it today. >> >> Dumitru, so from OVN you will check to see if ct_zero_nat support field is >> in the database, if not you will support SNAT only on the kernel dpath. If >> it is present, you use it depending not the value set? (Which will be true >> for both kernel and userspace). Or do I miss something? >> > > OVN cannot differentiate between datapath types unless we hardcode > datapath-type names which doesn't seem OK to me. Can’t it use the datapath_type from the bridge table? > The plan is for OVN to use all-zero IP SNAT only if the ct_zero_nat > field is in the database. > > So, in order to use it with the kernel datapath, the field needs to be > set to 'true' when the kernel datapath is used. Is Paolo’s patch good enough? Meaning if we add the ct_zero_nat to his patch will that be enough? > Thanks, > Dumitru > >>>>>> >>>>>> Maybe I missed something, or misunderstood something. >>>>>> >>>>>> Thoughts? >>>>>> >>>>> >>>>> Considering that linux supports all-zero snat since day one, the >>>>> assumption seems solid enough to me for it. >>>>> >>>>> I have no details about it, but what about Windows dp? >>>>> >>>>>>> [0] >>>>>>> https://github.com/openvswitch/ovs/commit/27501802d09f782b8133031c1eae3394ae5ce147 >>>>>>> >>>>>>> [1] https://patchwork.ozlabs.org/project/openvswitch/list/?series=241223 >>>>>>> >>>>>>> Thanks, >>>>>>> Dumitru >>>>> >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
