| Message ID | 161943952616.327630.4676425878426520994.stgit@ebuild |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v3] conntrack: document all-zero IP SNAT behavior and add a test case | expand |
Eelco Chaudron <echaudro@redhat.com> writes: > 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 <echaudro@redhat.com> > --- > 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. > > lib/ovs-actions.xml | 10 ++++++++ > tests/system-kmod-macros.at | 7 ++++++ > tests/system-traffic.at | 46 ++++++++++++++++++++++++++++++++++++++ > tests/system-userspace-macros.at | 10 ++++++++ > 4 files changed, 73 insertions(+) Acked-by: Paolo Valerio <pvalerio@redhat.com>
Eelco Chaudron <echaudro@redhat.com> writes: > 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 <echaudro@redhat.com> > --- Acked-by: Aaron Conole <aconole@redhat.com>
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 <echaudro@redhat.com> > --- > 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. [0] https://github.com/openvswitch/ovs/commit/27501802d09f782b8133031c1eae3394ae5ce147 [1] https://patchwork.ozlabs.org/project/openvswitch/list/?series=241223 Thanks, Dumitru
Hi Dumitru, That sounds like a good idea, guess we should probably do it as part of the user space patch[1], or else we have to update both patches ones more. Paolo, what do you think? Let me know if you want me to do the actual change, and we can update your patchset [1] with an additional patch? Cheers, Eelco On 17 May 2021, at 17:22, Dumitru Ceara wrote: > 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 <echaudro@redhat.com> >> --- >> 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. > > [0] > https://github.com/openvswitch/ovs/commit/27501802d09f782b8133031c1eae3394ae5ce147 > > [1] https://patchwork.ozlabs.org/project/openvswitch/list/?series=241223 > > Thanks, > Dumitru
On 5/17/21 5:22 PM, Dumitru Ceara wrote: > 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 <echaudro@redhat.com> >> --- >> 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. > > [0] > https://github.com/openvswitch/ovs/commit/27501802d09f782b8133031c1eae3394ae5ce147 I just realized that the Datapath record is not created automatically, I wonder if that's a change to be done in OVS? Or is it expected that the controller/CMS create the datapath record? > > [1] https://patchwork.ozlabs.org/project/openvswitch/list/?series=241223 > > Thanks, > Dumitru >
Dumitru Ceara <dceara@redhat.com> 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 <echaudro@redhat.com> >> --- >> 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. 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. Then it can be detected by the datapath capabilities that already exist. Maybe I missed something, or misunderstood something. Thoughts? > [0] > https://github.com/openvswitch/ovs/commit/27501802d09f782b8133031c1eae3394ae5ce147 > > [1] https://patchwork.ozlabs.org/project/openvswitch/list/?series=241223 > > Thanks, > Dumitru
Aaron Conole <aconole@redhat.com> writes: > Dumitru Ceara <dceara@redhat.com> 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 <echaudro@redhat.com> >>> --- >>> 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. > Then it can be detected by the datapath capabilities that already exist. > > 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
On 5/21/21 11:27 AM, Paolo Valerio wrote: > Aaron Conole <aconole@redhat.com> writes: > >> Dumitru Ceara <dceara@redhat.com> 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 <echaudro@redhat.com> >>>> --- >>>> 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"? >> >> 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 >
Dumitru Ceara <dceara@redhat.com> writes: > On 5/21/21 11:27 AM, Paolo Valerio wrote: >> Aaron Conole <aconole@redhat.com> writes: >> >>> Dumitru Ceara <dceara@redhat.com> 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 <echaudro@redhat.com> >>>>> --- >>>>> 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. >>> >>> 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 >>
On 25 May 2021, at 15:20, Aaron Conole wrote: > Dumitru Ceara <dceara@redhat.com> writes: > >> On 5/21/21 11:27 AM, Paolo Valerio wrote: >>> Aaron Conole <aconole@redhat.com> writes: >>> >>>> Dumitru Ceara <dceara@redhat.com> 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 <echaudro@redhat.com> >>>>>> --- >>>>>> 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? >>>> >>>> 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 >>>
On 5/27/21 9:26 AM, Eelco Chaudron wrote: > > > On 25 May 2021, at 15:20, Aaron Conole wrote: > >> Dumitru Ceara <dceara@redhat.com> writes: >> >>> On 5/21/21 11:27 AM, Paolo Valerio wrote: >>>> Aaron Conole <aconole@redhat.com> writes: >>>> >>>>> Dumitru Ceara <dceara@redhat.com> 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 <echaudro@redhat.com> >>>>>>> --- >>>>>>> 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. 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. 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 >>>> >
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 <dceara@redhat.com> writes: >>> >>>> On 5/21/21 11:27 AM, Paolo Valerio wrote: >>>>> Aaron Conole <aconole@redhat.com> writes: >>>>> >>>>>> Dumitru Ceara <dceara@redhat.com> 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 <echaudro@redhat.com> >>>>>>>> --- >>>>>>>> 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 >>>>> >>
On 5/27/21 10:09 AM, Eelco Chaudron wrote: > > > 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 <dceara@redhat.com> writes: >>>> >>>>> On 5/21/21 11:27 AM, Paolo Valerio wrote: >>>>>> Aaron Conole <aconole@redhat.com> writes: >>>>>> >>>>>>> Dumitru Ceara <dceara@redhat.com> 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 <echaudro@redhat.com> >>>>>>>>> --- >>>>>>>>> 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? Ideally we should avoid doing something like "if datapath_type == 'system' then generate flows that use zero-ip SNAT". Instead use the value of ct_zero_nat in the Datapath.capabilities to decide whether to use it or not. E.g., from the OVN RFC series I sent: http://patchwork.ozlabs.org/project/ovn/patch/20210526133652.16282.79459.stgit@dceara.remote.csb/ https://github.com/dceara/ovn/commit/9894af7660139d15fc649116ffa22fdc5189484b#diff-220cd89c1bf69b5cf68c6e9ea3771dd48da861c58aaf871f29f13d8cccd6cff1R481 > >> 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? I thought the agreed approach was: 1. Add a patch that sets ct_zero_nat in capabilities if the datapath accepts all-zero IP SNAT, similar to other action checks: https://github.com/openvswitch/ovs/blob/master/ofproto/ofproto-dpif.c#L1572 2. Add a patch that makes the userspace datapath refuse flows that use action ct(nat(src=0.0.0.0)). 3. Add Paolo's series to support all-zero IP SNAT, and revert the change in patch #2 above. Patches 1 & 2 could be backported to stable branches and patch 3 would be available in the next OVS release. That would allow OVN to be generic and work with any OVS release (upstream/downstream). Please correct me if I'm wrong. Thanks! > >> 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 >>>>>> >>> >
Hi Alin (or any other OVS windows expert), Do you know if the described all-zero IP address SNAT feature is supported by the windows datapath? If you don't but have a windows setup available, can you run the below self-test to find out? Thanks, Eelco On 26 Apr 2021, at 14:19, 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 <echaudro@redhat.com> > --- > 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. > > lib/ovs-actions.xml | 10 ++++++++ > tests/system-kmod-macros.at | 7 ++++++ > tests/system-traffic.at | 46 ++++++++++++++++++++++++++++++++++++++ > tests/system-userspace-macros.at | 10 ++++++++ > 4 files changed, 73 insertions(+) > > diff --git a/lib/ovs-actions.xml b/lib/ovs-actions.xml > index a2778de4b..a0070e6c6 100644 > --- a/lib/ovs-actions.xml > +++ b/lib/ovs-actions.xml > @@ -1833,6 +1833,16 @@ for <var>i</var> in [1,<var>n_members</var>]: > connection, will behave the same as a bare <code>nat</code>. > </p> > > + <p> > + For SNAT, there is a special case when the <code>src</code> IP > + address is configured as all 0's, i.e., > + <code>nat(src=0.0.0.0)</code>. In this case, 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. Note that this is currently only implemented in the > + Linux kernel datapath. > + </p> > + > <p> > Open vSwitch 2.6 introduced <code>nat</code>. Linux 4.6 was the > earliest upstream kernel that implemented <code>ct</code> support for > diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at > index 15628a7c6..812e7d99b 100644 > --- a/tests/system-kmod-macros.at > +++ b/tests/system-kmod-macros.at > @@ -99,6 +99,13 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP], > # > m4_define([CHECK_CONNTRACK_NAT]) > > +# CHECK_CONNTRACK_ZEROIP_SNAT() > +# > +# Perform requirements checks for running conntrack all-zero IP SNAT tests. > +# The kernel always supports all-zero IP SNAT, so no check is needed. > +# > +m4_define([CHECK_CONNTRACK_ZEROIP_SNAT]) > + > # CHECK_CONNTRACK_TIMEOUT() > # > # Perform requirements checks for running conntrack customized timeout tests. > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index fb5b9a36d..3d7f4751e 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -4433,6 +4433,52 @@ tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src= > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > + > +AT_SETUP([conntrack - all-zero IP SNAT]) > +AT_SKIP_IF([test $HAVE_NC = no]) > +CHECK_CONNTRACK() > +CHECK_CONNTRACK_ZEROIP_SNAT() > +OVS_TRAFFIC_VSWITCHD_START() > + > +ADD_NAMESPACES(at_ns0, at_ns1) > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") > +NS_CHECK_EXEC([at_ns0], [ip route add 172.1.1.0/24 via 10.1.1.2]) > + > +OVS_START_L7([at_ns1], [http]) > + > +AT_DATA([flows.txt], [dnl > +table=0,priority=30,ct_state=-trk,ip,action=ct(table=0) > +table=0,priority=20,ct_state=-rpl,ip,nw_dst=10.1.1.0/24,actions=ct(commit,nat(src=0.0.0.0),table=10) > +table=0,priority=20,ct_state=+rpl,ip,nw_dst=10.1.1.0/24,actions=resubmit(,10) > +table=0,priority=20,ip,nw_dst=172.1.1.2,actions=ct(commit,nat(dst=10.1.1.2),table=10) > +table=0,priority=10,arp,action=normal > +table=0,priority=1,action=drop > +table=10,priority=20,ct_state=+rpl,ip,nw_dst=10.1.1.0/24 actions=ct(table=20,nat) > +table=10,priority=10,ip,nw_dst=10.1.1.0/24 actions=resubmit(,20) > +table=20,priority=10,ip,nw_dst=10.1.1.1,action=1 > +table=20,priority=10,ip,nw_dst=10.1.1.2,action=2 > +]) > +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) > + > +dnl - Test to make sure src nat is NOT done when not needed > +NS_CHECK_EXEC([at_ns0], [echo "TEST" | nc -p 30000 10.1.1.2 80 > nc-1.log]) > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.1,"], [0], [dnl > +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=30000,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=30000),protoinfo=(state=TIME_WAIT) > +]) > + > +dnl - Test to make sure src nat is done when needed > +NS_CHECK_EXEC([at_ns0], [echo "TEST2" | nc -p 30001 172.1.1.2 80 > nc-2.log]) > +NS_CHECK_EXEC([at_ns0], [echo "TEST3" | nc -p 30001 10.1.1.2 80 > nc-3.log]) > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep 30001 | grep "orig=.src=10\.1\.1\.1," | sed -e 's/port=30001/port=<clnt_s_port>/g' -e 's/sport=80,dport=[[0-9]]\+/sport=80,dport=<rnd_port>/g' | sort], [0], [dnl > +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<clnt_s_port>,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=<rnd_port>),protoinfo=(state=TIME_WAIT) > +tcp,orig=(src=10.1.1.1,dst=172.1.1.2,sport=<clnt_s_port>,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=<clnt_s_port>),protoinfo=(state=TIME_WAIT) > +]) > + > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > + > AT_SETUP([conntrack - simple DNAT]) > CHECK_CONNTRACK() > CHECK_CONNTRACK_NAT() > diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at > index 34f82cee3..9f0d38dfb 100644 > --- a/tests/system-userspace-macros.at > +++ b/tests/system-userspace-macros.at > @@ -96,6 +96,16 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP]) > # > m4_define([CHECK_CONNTRACK_NAT]) > > +# CHECK_CONNTRACK_ZEROIP_SNAT() > +# > +# Perform requirements checks for running conntrack all-zero IP SNAT tests. > +# The userspace datapath does not support all-zero IP SNAT. > +# > +m4_define([CHECK_CONNTRACK_ZEROIP_SNAT], > +[ > + AT_SKIP_IF([:]) > +]) > + > # CHECK_CONNTRACK_TIMEOUT() > # > # Perform requirements checks for running conntrack customized timeout tests. > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/lib/ovs-actions.xml b/lib/ovs-actions.xml index a2778de4b..a0070e6c6 100644 --- a/lib/ovs-actions.xml +++ b/lib/ovs-actions.xml @@ -1833,6 +1833,16 @@ for <var>i</var> in [1,<var>n_members</var>]: connection, will behave the same as a bare <code>nat</code>. </p> + <p> + For SNAT, there is a special case when the <code>src</code> IP + address is configured as all 0's, i.e., + <code>nat(src=0.0.0.0)</code>. In this case, 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. Note that this is currently only implemented in the + Linux kernel datapath. + </p> + <p> Open vSwitch 2.6 introduced <code>nat</code>. Linux 4.6 was the earliest upstream kernel that implemented <code>ct</code> support for diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at index 15628a7c6..812e7d99b 100644 --- a/tests/system-kmod-macros.at +++ b/tests/system-kmod-macros.at @@ -99,6 +99,13 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP], # m4_define([CHECK_CONNTRACK_NAT]) +# CHECK_CONNTRACK_ZEROIP_SNAT() +# +# Perform requirements checks for running conntrack all-zero IP SNAT tests. +# The kernel always supports all-zero IP SNAT, so no check is needed. +# +m4_define([CHECK_CONNTRACK_ZEROIP_SNAT]) + # CHECK_CONNTRACK_TIMEOUT() # # Perform requirements checks for running conntrack customized timeout tests. diff --git a/tests/system-traffic.at b/tests/system-traffic.at index fb5b9a36d..3d7f4751e 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -4433,6 +4433,52 @@ tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src= OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([conntrack - all-zero IP SNAT]) +AT_SKIP_IF([test $HAVE_NC = no]) +CHECK_CONNTRACK() +CHECK_CONNTRACK_ZEROIP_SNAT() +OVS_TRAFFIC_VSWITCHD_START() + +ADD_NAMESPACES(at_ns0, at_ns1) +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") +NS_CHECK_EXEC([at_ns0], [ip route add 172.1.1.0/24 via 10.1.1.2]) + +OVS_START_L7([at_ns1], [http]) + +AT_DATA([flows.txt], [dnl +table=0,priority=30,ct_state=-trk,ip,action=ct(table=0) +table=0,priority=20,ct_state=-rpl,ip,nw_dst=10.1.1.0/24,actions=ct(commit,nat(src=0.0.0.0),table=10) +table=0,priority=20,ct_state=+rpl,ip,nw_dst=10.1.1.0/24,actions=resubmit(,10) +table=0,priority=20,ip,nw_dst=172.1.1.2,actions=ct(commit,nat(dst=10.1.1.2),table=10) +table=0,priority=10,arp,action=normal +table=0,priority=1,action=drop +table=10,priority=20,ct_state=+rpl,ip,nw_dst=10.1.1.0/24 actions=ct(table=20,nat) +table=10,priority=10,ip,nw_dst=10.1.1.0/24 actions=resubmit(,20) +table=20,priority=10,ip,nw_dst=10.1.1.1,action=1 +table=20,priority=10,ip,nw_dst=10.1.1.2,action=2 +]) +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) + +dnl - Test to make sure src nat is NOT done when not needed +NS_CHECK_EXEC([at_ns0], [echo "TEST" | nc -p 30000 10.1.1.2 80 > nc-1.log]) +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.1,"], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=30000,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=30000),protoinfo=(state=TIME_WAIT) +]) + +dnl - Test to make sure src nat is done when needed +NS_CHECK_EXEC([at_ns0], [echo "TEST2" | nc -p 30001 172.1.1.2 80 > nc-2.log]) +NS_CHECK_EXEC([at_ns0], [echo "TEST3" | nc -p 30001 10.1.1.2 80 > nc-3.log]) +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep 30001 | grep "orig=.src=10\.1\.1\.1," | sed -e 's/port=30001/port=<clnt_s_port>/g' -e 's/sport=80,dport=[[0-9]]\+/sport=80,dport=<rnd_port>/g' | sort], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<clnt_s_port>,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=<rnd_port>),protoinfo=(state=TIME_WAIT) +tcp,orig=(src=10.1.1.1,dst=172.1.1.2,sport=<clnt_s_port>,dport=80),reply=(src=10.1.1.2,dst=10.1.1.1,sport=80,dport=<clnt_s_port>),protoinfo=(state=TIME_WAIT) +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + + AT_SETUP([conntrack - simple DNAT]) CHECK_CONNTRACK() CHECK_CONNTRACK_NAT() diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at index 34f82cee3..9f0d38dfb 100644 --- a/tests/system-userspace-macros.at +++ b/tests/system-userspace-macros.at @@ -96,6 +96,16 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP]) # m4_define([CHECK_CONNTRACK_NAT]) +# CHECK_CONNTRACK_ZEROIP_SNAT() +# +# Perform requirements checks for running conntrack all-zero IP SNAT tests. +# The userspace datapath does not support all-zero IP SNAT. +# +m4_define([CHECK_CONNTRACK_ZEROIP_SNAT], +[ + AT_SKIP_IF([:]) +]) + # CHECK_CONNTRACK_TIMEOUT() # # Perform requirements checks for running conntrack customized timeout tests.
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 <echaudro@redhat.com> --- 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. lib/ovs-actions.xml | 10 ++++++++ tests/system-kmod-macros.at | 7 ++++++ tests/system-traffic.at | 46 ++++++++++++++++++++++++++++++++++++++ tests/system-userspace-macros.at | 10 ++++++++ 4 files changed, 73 insertions(+)