| Message ID | 20220105130528.690-1-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] physical: Add remote parent ports to OFTABLE_REMOTE_OUTPUT flows. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Hi Dumitru, Looks good to me. Acked-by: Mark Michelson <mmichels@redhat.com> On 1/5/22 08:05, Dumitru Ceara wrote: > Even if it has no associated local OVS interface, for every port binding > of type 'LP_VIF' that is a parent lport of container lports, a 'struct > local_binding' object is created. When building flows to forward BUM > traffic keep this in mind when adding flows to table > OFTABLE_REMOTE_OUTPUT. > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2036970 > Fixes: 3ae8470edc64 ("I-P: Handle runtime data changes for pflow_output engine.") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > controller/physical.c | 10 ++++++---- > tests/ovn.at | 22 ++++++++++++++-------- > 2 files changed, 20 insertions(+), 12 deletions(-) > > diff --git a/controller/physical.c b/controller/physical.c > index 836fc769a..aa651b876 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -1477,10 +1477,12 @@ consider_mc_group(struct ovsdb_idl_index *sbrec_port_binding_by_name, > put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, > &remote_ofpacts); > put_resubmit(OFTABLE_CHECK_LOOPBACK, &remote_ofpacts); > - } else if (local_binding_get_primary_pb(local_bindings, lport_name) > - || simap_contains(patch_ofports, port->logical_port) > - || (!strcmp(port->type, "l3gateway") > - && port->chassis == chassis)) { > + } else if (port->chassis == chassis > + && (local_binding_get_primary_pb(local_bindings, lport_name) > + || !strcmp(port->type, "l3gateway"))) { > + put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); > + put_resubmit(OFTABLE_CHECK_LOOPBACK, &ofpacts); > + } else if (simap_contains(patch_ofports, port->logical_port)) { > put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); > put_resubmit(OFTABLE_CHECK_LOOPBACK, &ofpacts); > } else if (!strcmp(port->type, "chassisredirect") > diff --git a/tests/ovn.at b/tests/ovn.at > index 9ec62e321..92e284e8a 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -9658,8 +9658,8 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet > > # expected packet at foo2 > packet=${dst_mac}${src_mac}8100000208004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 > -echo $packet > expected > -OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected]) > +echo $packet > expected2 > +OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected2]) > > # Send ip packets between foo1 and bar2 (different switch, different HV) > src_mac="f00000010205" > @@ -9673,8 +9673,8 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet > src_mac="000000010204" > dst_mac="f00000010208" > packet=${dst_mac}${src_mac}8100000108004500001c000000003f110100${src_ip}${dst_ip}0035111100080000 > -echo $packet >> expected > -OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected]) > +echo $packet >> expected2 > +OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected2]) > > # Send ip packets between foo1 and bar1 > # (different switch, loopback to same vm but different tag) > @@ -9703,11 +9703,11 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet > > # expected packet at bar3 > packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 > -echo $packet > expected > -OVN_CHECK_PACKETS([hv1/bar3-tx.pcap], [expected]) > +echo $packet > expected3 > +OVN_CHECK_PACKETS([hv1/bar3-tx.pcap], [expected3]) > > # Send ip packets between foo1 and vm1. > -(different switch, container to the VM hosting it.) > +# (different switch, container to the VM hosting it.) > src_mac="f00000010205" > dst_mac="000000010203" > src_ip=`ip_to_hex 192 168 1 2` > @@ -9723,7 +9723,7 @@ echo $packet >> expected1 > OVN_CHECK_PACKETS([hv1/vm1-tx.pcap], [expected1]) > > # Send packets from vm1 to bar1. > -(different switch, A hosting VM to a container inside it) > +# (different switch, A hosting VM to a container inside it) > src_mac="f00000010203" > dst_mac="000000010202" > src_ip=`ip_to_hex 172 16 1 2` > @@ -9739,6 +9739,7 @@ echo $packet >> expected1 > OVN_CHECK_PACKETS([hv1/vm1-tx.pcap], [expected1]) > > # Send broadcast packet from foo1. foo1 should not receive the same packet. > +# But foo2 should. > src_mac="f00000010205" > dst_mac="ffffffffffff" > src_ip=`ip_to_hex 192 168 1 2` > @@ -9749,6 +9750,11 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet > # expected packet at VM1 > OVN_CHECK_PACKETS([hv1/vm1-tx.pcap], [expected1]) > > +# expected packet at foo2 > +packet=${dst_mac}${src_mac}8100000208004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 > +echo $packet >> expected2 > +OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected2]) > + > # Test binding of parent and container ports. > ovn-nbctl lsp-set-options vm1 requested-chassis=foo > >
On Wed, Jan 5, 2022 at 2:37 PM Mark Michelson <mmichels@redhat.com> wrote: > > Hi Dumitru, > > Looks good to me. > Acked-by: Mark Michelson <mmichels@redhat.com> Thanks. I applied the patch to main and backported to 21.12 and 21.09 branches. Numan > > On 1/5/22 08:05, Dumitru Ceara wrote: > > Even if it has no associated local OVS interface, for every port binding > > of type 'LP_VIF' that is a parent lport of container lports, a 'struct > > local_binding' object is created. When building flows to forward BUM > > traffic keep this in mind when adding flows to table > > OFTABLE_REMOTE_OUTPUT. > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2036970 > > Fixes: 3ae8470edc64 ("I-P: Handle runtime data changes for pflow_output engine.") > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > --- > > controller/physical.c | 10 ++++++---- > > tests/ovn.at | 22 ++++++++++++++-------- > > 2 files changed, 20 insertions(+), 12 deletions(-) > > > > diff --git a/controller/physical.c b/controller/physical.c > > index 836fc769a..aa651b876 100644 > > --- a/controller/physical.c > > +++ b/controller/physical.c > > @@ -1477,10 +1477,12 @@ consider_mc_group(struct ovsdb_idl_index *sbrec_port_binding_by_name, > > put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, > > &remote_ofpacts); > > put_resubmit(OFTABLE_CHECK_LOOPBACK, &remote_ofpacts); > > - } else if (local_binding_get_primary_pb(local_bindings, lport_name) > > - || simap_contains(patch_ofports, port->logical_port) > > - || (!strcmp(port->type, "l3gateway") > > - && port->chassis == chassis)) { > > + } else if (port->chassis == chassis > > + && (local_binding_get_primary_pb(local_bindings, lport_name) > > + || !strcmp(port->type, "l3gateway"))) { > > + put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); > > + put_resubmit(OFTABLE_CHECK_LOOPBACK, &ofpacts); > > + } else if (simap_contains(patch_ofports, port->logical_port)) { > > put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); > > put_resubmit(OFTABLE_CHECK_LOOPBACK, &ofpacts); > > } else if (!strcmp(port->type, "chassisredirect") > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 9ec62e321..92e284e8a 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -9658,8 +9658,8 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet > > > > # expected packet at foo2 > > packet=${dst_mac}${src_mac}8100000208004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 > > -echo $packet > expected > > -OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected]) > > +echo $packet > expected2 > > +OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected2]) > > > > # Send ip packets between foo1 and bar2 (different switch, different HV) > > src_mac="f00000010205" > > @@ -9673,8 +9673,8 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet > > src_mac="000000010204" > > dst_mac="f00000010208" > > packet=${dst_mac}${src_mac}8100000108004500001c000000003f110100${src_ip}${dst_ip}0035111100080000 > > -echo $packet >> expected > > -OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected]) > > +echo $packet >> expected2 > > +OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected2]) > > > > # Send ip packets between foo1 and bar1 > > # (different switch, loopback to same vm but different tag) > > @@ -9703,11 +9703,11 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet > > > > # expected packet at bar3 > > packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 > > -echo $packet > expected > > -OVN_CHECK_PACKETS([hv1/bar3-tx.pcap], [expected]) > > +echo $packet > expected3 > > +OVN_CHECK_PACKETS([hv1/bar3-tx.pcap], [expected3]) > > > > # Send ip packets between foo1 and vm1. > > -(different switch, container to the VM hosting it.) > > +# (different switch, container to the VM hosting it.) > > src_mac="f00000010205" > > dst_mac="000000010203" > > src_ip=`ip_to_hex 192 168 1 2` > > @@ -9723,7 +9723,7 @@ echo $packet >> expected1 > > OVN_CHECK_PACKETS([hv1/vm1-tx.pcap], [expected1]) > > > > # Send packets from vm1 to bar1. > > -(different switch, A hosting VM to a container inside it) > > +# (different switch, A hosting VM to a container inside it) > > src_mac="f00000010203" > > dst_mac="000000010202" > > src_ip=`ip_to_hex 172 16 1 2` > > @@ -9739,6 +9739,7 @@ echo $packet >> expected1 > > OVN_CHECK_PACKETS([hv1/vm1-tx.pcap], [expected1]) > > > > # Send broadcast packet from foo1. foo1 should not receive the same packet. > > +# But foo2 should. > > src_mac="f00000010205" > > dst_mac="ffffffffffff" > > src_ip=`ip_to_hex 192 168 1 2` > > @@ -9749,6 +9750,11 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet > > # expected packet at VM1 > > OVN_CHECK_PACKETS([hv1/vm1-tx.pcap], [expected1]) > > > > +# expected packet at foo2 > > +packet=${dst_mac}${src_mac}8100000208004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 > > +echo $packet >> expected2 > > +OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected2]) > > + > > # Test binding of parent and container ports. > > ovn-nbctl lsp-set-options vm1 requested-chassis=foo > > > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/physical.c b/controller/physical.c index 836fc769a..aa651b876 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -1477,10 +1477,12 @@ consider_mc_group(struct ovsdb_idl_index *sbrec_port_binding_by_name, put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &remote_ofpacts); put_resubmit(OFTABLE_CHECK_LOOPBACK, &remote_ofpacts); - } else if (local_binding_get_primary_pb(local_bindings, lport_name) - || simap_contains(patch_ofports, port->logical_port) - || (!strcmp(port->type, "l3gateway") - && port->chassis == chassis)) { + } else if (port->chassis == chassis + && (local_binding_get_primary_pb(local_bindings, lport_name) + || !strcmp(port->type, "l3gateway"))) { + put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); + put_resubmit(OFTABLE_CHECK_LOOPBACK, &ofpacts); + } else if (simap_contains(patch_ofports, port->logical_port)) { put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); put_resubmit(OFTABLE_CHECK_LOOPBACK, &ofpacts); } else if (!strcmp(port->type, "chassisredirect") diff --git a/tests/ovn.at b/tests/ovn.at index 9ec62e321..92e284e8a 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -9658,8 +9658,8 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet # expected packet at foo2 packet=${dst_mac}${src_mac}8100000208004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 -echo $packet > expected -OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected]) +echo $packet > expected2 +OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected2]) # Send ip packets between foo1 and bar2 (different switch, different HV) src_mac="f00000010205" @@ -9673,8 +9673,8 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet src_mac="000000010204" dst_mac="f00000010208" packet=${dst_mac}${src_mac}8100000108004500001c000000003f110100${src_ip}${dst_ip}0035111100080000 -echo $packet >> expected -OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected]) +echo $packet >> expected2 +OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected2]) # Send ip packets between foo1 and bar1 # (different switch, loopback to same vm but different tag) @@ -9703,11 +9703,11 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet # expected packet at bar3 packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 -echo $packet > expected -OVN_CHECK_PACKETS([hv1/bar3-tx.pcap], [expected]) +echo $packet > expected3 +OVN_CHECK_PACKETS([hv1/bar3-tx.pcap], [expected3]) # Send ip packets between foo1 and vm1. -(different switch, container to the VM hosting it.) +# (different switch, container to the VM hosting it.) src_mac="f00000010205" dst_mac="000000010203" src_ip=`ip_to_hex 192 168 1 2` @@ -9723,7 +9723,7 @@ echo $packet >> expected1 OVN_CHECK_PACKETS([hv1/vm1-tx.pcap], [expected1]) # Send packets from vm1 to bar1. -(different switch, A hosting VM to a container inside it) +# (different switch, A hosting VM to a container inside it) src_mac="f00000010203" dst_mac="000000010202" src_ip=`ip_to_hex 172 16 1 2` @@ -9739,6 +9739,7 @@ echo $packet >> expected1 OVN_CHECK_PACKETS([hv1/vm1-tx.pcap], [expected1]) # Send broadcast packet from foo1. foo1 should not receive the same packet. +# But foo2 should. src_mac="f00000010205" dst_mac="ffffffffffff" src_ip=`ip_to_hex 192 168 1 2` @@ -9749,6 +9750,11 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet # expected packet at VM1 OVN_CHECK_PACKETS([hv1/vm1-tx.pcap], [expected1]) +# expected packet at foo2 +packet=${dst_mac}${src_mac}8100000208004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 +echo $packet >> expected2 +OVN_CHECK_PACKETS([hv2/vm2-tx.pcap], [expected2]) + # Test binding of parent and container ports. ovn-nbctl lsp-set-options vm1 requested-chassis=foo
Even if it has no associated local OVS interface, for every port binding of type 'LP_VIF' that is a parent lport of container lports, a 'struct local_binding' object is created. When building flows to forward BUM traffic keep this in mind when adding flows to table OFTABLE_REMOTE_OUTPUT. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2036970 Fixes: 3ae8470edc64 ("I-P: Handle runtime data changes for pflow_output engine.") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- controller/physical.c | 10 ++++++---- tests/ovn.at | 22 ++++++++++++++-------- 2 files changed, 20 insertions(+), 12 deletions(-)