Message ID | 20210629162034.497992-1-ihrachys@redhat.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev,branch-21.06] Disable ARP/NA responders for vlan-passthru switches | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | warning | apply and check: warning |
ovsrobot/github-robot | success | github build: passed |
Bleep bloop. Greetings Ihar Hrachyshka, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Numan Siddique <numans@ovn.org> Lines checked: 204, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
I pushed this to branch-21.06. On 6/29/21 12:20 PM, Ihar Hrachyshka wrote: > When vlan-passthru is on, VIFs may attach different VLAN tags. In this > case, VIFs are not guaranteed to belong to the same L2 broadcast domain. > Because of that, we don't know if a peer port on the switch has the same > tag used and should not allow the local responder to generate neighbour > traffic. Instead, pass ARP and ND requests to the peer port owner and > allow it to reply, if needed. > > Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com> > Signed-off-by: Numan Siddique <numans@ovn.org> > (cherry picked from commit ea57f666f6eef1eb1d578f0e975baa14c5d23ec9) > --- > northd/ovn-northd.8.xml | 6 ++- > northd/ovn-northd.c | 4 ++ > northd/ovn_northd.dl | 6 ++- > tests/ovn.at | 112 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 124 insertions(+), 4 deletions(-) > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index 407464602..21ae0ca60 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -1072,8 +1072,10 @@ output; > <code>localport</code> ports) that are down (unless <code> > ignore_lsp_down</code> is configured as true in <code>options</code> > column of <code>NB_Global</code> table of the <code>Northbound</code> > - database), for logical ports of type <code>virtual</code> and for > - logical ports with 'unknown' address set. > + database), for logical ports of type <code>virtual</code>, for > + logical ports with 'unknown' address set and for logical ports of > + a logical switch configured with > + <code>other_config:vlan-passthru=true</code>. > </p> > </li> > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 3dae7bb1c..17bcede5a 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -7007,6 +7007,10 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, > return; > } > > + if (is_vlan_transparent(op->od)) { > + return; > + } > + > for (size_t i = 0; i < op->n_lsp_addrs; i++) { > for (size_t j = 0; j < op->lsp_addrs[i].n_ipv4_addrs; j++) { > ds_clear(match); > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl > index 3afa80a3b..a09aea6ee 100644 > --- a/northd/ovn_northd.dl > +++ b/northd/ovn_northd.dl > @@ -3309,7 +3309,8 @@ for (CheckLspIsUp[check_lsp_is_up]) { > ((lsp_is_up(lsp) or not check_lsp_is_up) > or lsp.__type == "router" or lsp.__type == "localport") and > lsp.__type != "external" and lsp.__type != "virtual" and > - not lsp.addresses.contains("unknown")) > + not lsp.addresses.contains("unknown") and > + not sw.is_vlan_transparent) > { > var __match = "arp.tpa == ${addr.addr} && arp.op == 1" in > { > @@ -3359,7 +3360,8 @@ for (SwitchPortIPv6Address(.port = &SwitchPort{.lsp = lsp, .json_name = json_nam > .ea = ea, .addr = addr) > if lsp.is_enabled() and > (lsp_is_up(lsp) or lsp.__type == "router" or lsp.__type == "localport") and > - lsp.__type != "external" and lsp.__type != "virtual") > + lsp.__type != "external" and lsp.__type != "virtual" and > + not sw.is_vlan_transparent) > { > var __match = "nd_ns && ip6.dst == {${addr.addr}, ${addr.solicited_node()}} && nd.target == ${addr.addr}" in > var actions = "${if (lsp.__type == \"router\") \"nd_na_router\" else \"nd_na\"} { " > diff --git a/tests/ovn.at b/tests/ovn.at > index b6523c328..811a05c5a 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -3169,6 +3169,118 @@ OVN_CLEANUP([hv-1],[hv-2]) > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn -- VLAN transparency, passthru=true, ARP responder disabled]) > +ovn_start > + > +net_add net > +check ovs-vsctl add-br br-phys > +ovn_attach net br-phys 192.168.0.1 > + > +check ovn-nbctl ls-add ls > +check ovn-nbctl --wait=sb add Logical-Switch ls other_config vlan-passthru=true > + > +for i in 1 2; do > + check ovn-nbctl lsp-add ls lsp$i > + check ovn-nbctl lsp-set-addresses lsp$i "f0:00:00:00:00:0$i 10.0.0.$i" > +done > + > +for i in 1 2; do > + check ovs-vsctl add-port br-int vif$i -- set Interface vif$i external-ids:iface-id=lsp$i \ > + options:tx_pcap=vif$i-tx.pcap \ > + options:rxq_pcap=vif$i-rx.pcap \ > + ofport-request=$i > +done > + > +wait_for_ports_up > + > +ovn-sbctl dump-flows ls > lsflows > +AT_CAPTURE_FILE([lsflows]) > + > +AT_CHECK([grep -w "ls_in_arp_rsp" lsflows | sort], [0], [dnl > + table=16(ls_in_arp_rsp ), priority=0 , match=(1), action=(next;) > +]) > + > +test_arp() { > + local inport=$1 outport=$2 sha=$3 spa=$4 tpa=$5 reply_ha=$6 > + tag=8100fefe > + local request=ffffffffffff${sha}${tag}08060001080006040001${sha}${spa}ffffffffffff${tpa} > + ovs-appctl netdev-dummy/receive vif$inport $request > + echo $request >> $outport.expected > + > + local reply=${sha}${reply_ha}${tag}08060001080006040002${reply_ha}${tpa}${sha}${spa} > + ovs-appctl netdev-dummy/receive vif$outport $reply > + echo $reply >> $inport.expected > +} > + > +test_arp 1 2 f00000000001 0a000001 0a000002 f00000000002 > +test_arp 2 1 f00000000002 0a000002 0a000001 f00000000001 > + > +for i in 1 2; do > + OVN_CHECK_PACKETS([vif$i-tx.pcap], [$i.expected]) > +done > + > +AT_CLEANUP > +]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn -- VLAN transparency, passthru=true, ND/NA responder disabled]) > +ovn_start > + > +net_add net > +check ovs-vsctl add-br br-phys > +ovn_attach net br-phys 192.168.0.1 > + > +check ovn-nbctl ls-add ls > +check ovn-nbctl --wait=sb add Logical-Switch ls other_config vlan-passthru=true > + > +for i in 1 2; do > + check ovn-nbctl lsp-add ls lsp$i > + check ovn-nbctl lsp-set-addresses lsp$i "f0:00:00:00:00:0$i fe00::$i" > +done > + > +for i in 1 2; do > + check ovs-vsctl add-port br-int vif$i -- set Interface vif$i external-ids:iface-id=lsp$i \ > + options:tx_pcap=vif$i-tx.pcap \ > + options:rxq_pcap=vif$i-rx.pcap \ > + ofport-request=$i > +done > + > +wait_for_ports_up > + > +ovn-sbctl dump-flows ls > lsflows > +AT_CAPTURE_FILE([lsflows]) > + > +AT_CHECK([grep -w "ls_in_arp_rsp" lsflows | sort], [0], [dnl > + table=16(ls_in_arp_rsp ), priority=0 , match=(1), action=(next;) > +]) > + > +test_nd_na() { > + local inport=$1 outport=$2 sha=$3 spa=$4 tpa=$5 reply_ha=$6 > + tag=8100fefe > + icmp_type=87 > + local request=ffffffffffff${sha}${tag}86dd6000000000183aff${spa}ff0200000000000000000001ff${tpa: -6}${icmp_type}007ea100000000${tpa} > + ovs-appctl netdev-dummy/receive vif$inport $request > + echo $request >> $outport.expected > + echo $request > + > + icmp_type=88 > + local reply=${sha}${reply_ha}${tag}86dd6000000000183aff${tpa}${spa}${icmp_type}003da540000000${tpa} > + ovs-appctl netdev-dummy/receive vif$outport $reply > + echo $reply >> $inport.expected > + echo $reply > +} > + > +test_nd_na 1 2 f00000000001 fe000000000000000000000000000001 fe000000000000000000000000000002 f00000000002 > +test_nd_na 2 1 f00000000002 fe000000000000000000000000000002 fe000000000000000000000000000001 f00000000001 > + > +for i in 1 2; do > + OVN_CHECK_PACKETS([vif$i-tx.pcap], [$i.expected]) > +done > + > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([ovn -- VLAN transparency, passthru=true, multiple hosts]) > ovn_start >
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 407464602..21ae0ca60 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -1072,8 +1072,10 @@ output; <code>localport</code> ports) that are down (unless <code> ignore_lsp_down</code> is configured as true in <code>options</code> column of <code>NB_Global</code> table of the <code>Northbound</code> - database), for logical ports of type <code>virtual</code> and for - logical ports with 'unknown' address set. + database), for logical ports of type <code>virtual</code>, for + logical ports with 'unknown' address set and for logical ports of + a logical switch configured with + <code>other_config:vlan-passthru=true</code>. </p> </li> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 3dae7bb1c..17bcede5a 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -7007,6 +7007,10 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, return; } + if (is_vlan_transparent(op->od)) { + return; + } + for (size_t i = 0; i < op->n_lsp_addrs; i++) { for (size_t j = 0; j < op->lsp_addrs[i].n_ipv4_addrs; j++) { ds_clear(match); diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index 3afa80a3b..a09aea6ee 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -3309,7 +3309,8 @@ for (CheckLspIsUp[check_lsp_is_up]) { ((lsp_is_up(lsp) or not check_lsp_is_up) or lsp.__type == "router" or lsp.__type == "localport") and lsp.__type != "external" and lsp.__type != "virtual" and - not lsp.addresses.contains("unknown")) + not lsp.addresses.contains("unknown") and + not sw.is_vlan_transparent) { var __match = "arp.tpa == ${addr.addr} && arp.op == 1" in { @@ -3359,7 +3360,8 @@ for (SwitchPortIPv6Address(.port = &SwitchPort{.lsp = lsp, .json_name = json_nam .ea = ea, .addr = addr) if lsp.is_enabled() and (lsp_is_up(lsp) or lsp.__type == "router" or lsp.__type == "localport") and - lsp.__type != "external" and lsp.__type != "virtual") + lsp.__type != "external" and lsp.__type != "virtual" and + not sw.is_vlan_transparent) { var __match = "nd_ns && ip6.dst == {${addr.addr}, ${addr.solicited_node()}} && nd.target == ${addr.addr}" in var actions = "${if (lsp.__type == \"router\") \"nd_na_router\" else \"nd_na\"} { " diff --git a/tests/ovn.at b/tests/ovn.at index b6523c328..811a05c5a 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -3169,6 +3169,118 @@ OVN_CLEANUP([hv-1],[hv-2]) AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn -- VLAN transparency, passthru=true, ARP responder disabled]) +ovn_start + +net_add net +check ovs-vsctl add-br br-phys +ovn_attach net br-phys 192.168.0.1 + +check ovn-nbctl ls-add ls +check ovn-nbctl --wait=sb add Logical-Switch ls other_config vlan-passthru=true + +for i in 1 2; do + check ovn-nbctl lsp-add ls lsp$i + check ovn-nbctl lsp-set-addresses lsp$i "f0:00:00:00:00:0$i 10.0.0.$i" +done + +for i in 1 2; do + check ovs-vsctl add-port br-int vif$i -- set Interface vif$i external-ids:iface-id=lsp$i \ + options:tx_pcap=vif$i-tx.pcap \ + options:rxq_pcap=vif$i-rx.pcap \ + ofport-request=$i +done + +wait_for_ports_up + +ovn-sbctl dump-flows ls > lsflows +AT_CAPTURE_FILE([lsflows]) + +AT_CHECK([grep -w "ls_in_arp_rsp" lsflows | sort], [0], [dnl + table=16(ls_in_arp_rsp ), priority=0 , match=(1), action=(next;) +]) + +test_arp() { + local inport=$1 outport=$2 sha=$3 spa=$4 tpa=$5 reply_ha=$6 + tag=8100fefe + local request=ffffffffffff${sha}${tag}08060001080006040001${sha}${spa}ffffffffffff${tpa} + ovs-appctl netdev-dummy/receive vif$inport $request + echo $request >> $outport.expected + + local reply=${sha}${reply_ha}${tag}08060001080006040002${reply_ha}${tpa}${sha}${spa} + ovs-appctl netdev-dummy/receive vif$outport $reply + echo $reply >> $inport.expected +} + +test_arp 1 2 f00000000001 0a000001 0a000002 f00000000002 +test_arp 2 1 f00000000002 0a000002 0a000001 f00000000001 + +for i in 1 2; do + OVN_CHECK_PACKETS([vif$i-tx.pcap], [$i.expected]) +done + +AT_CLEANUP +]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn -- VLAN transparency, passthru=true, ND/NA responder disabled]) +ovn_start + +net_add net +check ovs-vsctl add-br br-phys +ovn_attach net br-phys 192.168.0.1 + +check ovn-nbctl ls-add ls +check ovn-nbctl --wait=sb add Logical-Switch ls other_config vlan-passthru=true + +for i in 1 2; do + check ovn-nbctl lsp-add ls lsp$i + check ovn-nbctl lsp-set-addresses lsp$i "f0:00:00:00:00:0$i fe00::$i" +done + +for i in 1 2; do + check ovs-vsctl add-port br-int vif$i -- set Interface vif$i external-ids:iface-id=lsp$i \ + options:tx_pcap=vif$i-tx.pcap \ + options:rxq_pcap=vif$i-rx.pcap \ + ofport-request=$i +done + +wait_for_ports_up + +ovn-sbctl dump-flows ls > lsflows +AT_CAPTURE_FILE([lsflows]) + +AT_CHECK([grep -w "ls_in_arp_rsp" lsflows | sort], [0], [dnl + table=16(ls_in_arp_rsp ), priority=0 , match=(1), action=(next;) +]) + +test_nd_na() { + local inport=$1 outport=$2 sha=$3 spa=$4 tpa=$5 reply_ha=$6 + tag=8100fefe + icmp_type=87 + local request=ffffffffffff${sha}${tag}86dd6000000000183aff${spa}ff0200000000000000000001ff${tpa: -6}${icmp_type}007ea100000000${tpa} + ovs-appctl netdev-dummy/receive vif$inport $request + echo $request >> $outport.expected + echo $request + + icmp_type=88 + local reply=${sha}${reply_ha}${tag}86dd6000000000183aff${tpa}${spa}${icmp_type}003da540000000${tpa} + ovs-appctl netdev-dummy/receive vif$outport $reply + echo $reply >> $inport.expected + echo $reply +} + +test_nd_na 1 2 f00000000001 fe000000000000000000000000000001 fe000000000000000000000000000002 f00000000002 +test_nd_na 2 1 f00000000002 fe000000000000000000000000000002 fe000000000000000000000000000001 f00000000001 + +for i in 1 2; do + OVN_CHECK_PACKETS([vif$i-tx.pcap], [$i.expected]) +done + +AT_CLEANUP +]) + OVN_FOR_EACH_NORTHD([ AT_SETUP([ovn -- VLAN transparency, passthru=true, multiple hosts]) ovn_start