| Message ID | 20240524080004.29355-1-vsaienko@mirantis.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] Do not reply on unicast arps for IPv4 targets. | 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 |
On Fri, May 24, 2024 at 4:00 AM Vasyl Saienko <vsaienko@mirantis.com> wrote: > > Reply only if target ethernet address is broadcast, if > address is specified explicitly do noting to let target > reply by itself. This technique allows to monitor target > aliveness with arping. > > Closes #239 > > Signed-off-by: Vasyl Saienko <vsaienko@mirantis.com> Sorry for the late reviews. Acked-by: Numan Siddique <numans@ovn.org> Numan > --- > northd/northd.c | 11 +++++++++-- > northd/ovn-northd.8.xml | 7 ++++--- > tests/ovn-northd.at | 4 ++-- > 3 files changed, 15 insertions(+), 7 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 37f443e70..e80e1885d 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -8832,8 +8832,15 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, > 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); > - ds_put_format(match, "arp.tpa == %s && arp.op == 1", > - op->lsp_addrs[i].ipv4_addrs[j].addr_s); > + /* NOTE(vsaienko): Do not reply on unicast ARPs, forward > + * them to the target to have ability to monitor target > + * aliveness via ARPs. > + */ > + ds_put_format(match, > + "arp.tpa == %s && " > + "arp.op == 1 && " > + "eth.dst == ff:ff:ff:ff:ff:ff", > + op->lsp_addrs[i].ipv4_addrs[j].addr_s); > ds_clear(actions); > ds_put_format(actions, > "eth.dst = eth.src; " > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index b14a30285..ffdd67895 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -1435,9 +1435,10 @@ > > <li> > <p> > - Priority-50 flows that match ARP requests to each known IP address > - <var>A</var> of every logical switch port, and respond with ARP > - replies directly with corresponding Ethernet address <var>E</var>: > + Priority-50 flows that match only broadcast ARP requests to each > + known IPv4 address <var>A</var> of every logical switch port, and > + respond with ARP replies directly with corresponding Ethernet > + address <var>E</var>: > </p> > > <pre> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index be006fb32..4162196f4 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -9283,9 +9283,9 @@ AT_CAPTURE_FILE([S1flows]) > > AT_CHECK([grep -e "ls_in_arp_rsp" S1flows | ovn_strip_lflows], [0], [dnl > table=??(ls_in_arp_rsp ), priority=0 , match=(1), action=(next;) > - table=??(ls_in_arp_rsp ), priority=100 , match=(arp.tpa == 192.168.0.10 && arp.op == 1 && inport == "S1-vm1"), action=(next;) > + table=??(ls_in_arp_rsp ), priority=100 , match=(arp.tpa == 192.168.0.10 && arp.op == 1 && eth.dst == ff:ff:ff:ff:ff:ff && inport == "S1-vm1"), action=(next;) > table=??(ls_in_arp_rsp ), priority=100 , match=(nd_ns && ip6.dst == {fd00::10, ff02::1:ff00:10} && nd.target == fd00::10 && inport == "S1-vm1"), action=(next;) > - table=??(ls_in_arp_rsp ), priority=50 , match=(arp.tpa == 192.168.0.10 && arp.op == 1), action=(eth.dst = eth.src; eth.src = 50:54:00:00:00:10; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 50:54:00:00:00:10; arp.tpa = arp.spa; arp.spa = 192.168.0.10; outport = inport; flags.loopback = 1; output;) > + table=??(ls_in_arp_rsp ), priority=50 , match=(arp.tpa == 192.168.0.10 && arp.op == 1 && eth.dst == ff:ff:ff:ff:ff:ff), action=(eth.dst = eth.src; eth.src = 50:54:00:00:00:10; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 50:54:00:00:00:10; arp.tpa = arp.spa; arp.spa = 192.168.0.10; outport = inport; flags.loopback = 1; output;) > table=??(ls_in_arp_rsp ), priority=50 , match=(nd_ns && ip6.dst == {fd00::10, ff02::1:ff00:10} && nd.target == fd00::10), action=(nd_na { eth.src = 50:54:00:00:00:10; ip6.src = fd00::10; nd.target = fd00::10; nd.tll = 50:54:00:00:00:10; outport = inport; flags.loopback = 1; output; };) > ]) > > -- > 2.43.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 6/27/24 16:18, Numan Siddique wrote: > On Fri, May 24, 2024 at 4:00 AM Vasyl Saienko <vsaienko@mirantis.com> wrote: >> >> Reply only if target ethernet address is broadcast, if >> address is specified explicitly do noting to let target >> reply by itself. This technique allows to monitor target >> aliveness with arping. >> >> Closes #239 >> >> Signed-off-by: Vasyl Saienko <vsaienko@mirantis.com> > > Sorry for the late reviews. > > Acked-by: Numan Siddique <numans@ovn.org> > > Numan > >> --- >> northd/northd.c | 11 +++++++++-- >> northd/ovn-northd.8.xml | 7 ++++--- >> tests/ovn-northd.at | 4 ++-- >> 3 files changed, 15 insertions(+), 7 deletions(-) >> >> diff --git a/northd/northd.c b/northd/northd.c >> index 37f443e70..e80e1885d 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -8832,8 +8832,15 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, >> 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); >> - ds_put_format(match, "arp.tpa == %s && arp.op == 1", >> - op->lsp_addrs[i].ipv4_addrs[j].addr_s); >> + /* NOTE(vsaienko): Do not reply on unicast ARPs, forward >> + * them to the target to have ability to monitor target >> + * aliveness via ARPs. >> + */ Thanks, Vasyl and Numan! I changed this to: /* Do not reply on unicast ARPs, forward them to the target * to have ability to monitor target liveness via unicast * ARP requests. */ And applied the patch to main. Best regards, Dumitru
On 6/28/24 11:02, Dumitru Ceara wrote: [...] > > And applied the patch to main. > I also added Vasyl to the AUTHORS list. Regards, Dumitru
diff --git a/northd/northd.c b/northd/northd.c index 37f443e70..e80e1885d 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -8832,8 +8832,15 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, 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); - ds_put_format(match, "arp.tpa == %s && arp.op == 1", - op->lsp_addrs[i].ipv4_addrs[j].addr_s); + /* NOTE(vsaienko): Do not reply on unicast ARPs, forward + * them to the target to have ability to monitor target + * aliveness via ARPs. + */ + ds_put_format(match, + "arp.tpa == %s && " + "arp.op == 1 && " + "eth.dst == ff:ff:ff:ff:ff:ff", + op->lsp_addrs[i].ipv4_addrs[j].addr_s); ds_clear(actions); ds_put_format(actions, "eth.dst = eth.src; " diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index b14a30285..ffdd67895 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -1435,9 +1435,10 @@ <li> <p> - Priority-50 flows that match ARP requests to each known IP address - <var>A</var> of every logical switch port, and respond with ARP - replies directly with corresponding Ethernet address <var>E</var>: + Priority-50 flows that match only broadcast ARP requests to each + known IPv4 address <var>A</var> of every logical switch port, and + respond with ARP replies directly with corresponding Ethernet + address <var>E</var>: </p> <pre> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index be006fb32..4162196f4 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -9283,9 +9283,9 @@ AT_CAPTURE_FILE([S1flows]) AT_CHECK([grep -e "ls_in_arp_rsp" S1flows | ovn_strip_lflows], [0], [dnl table=??(ls_in_arp_rsp ), priority=0 , match=(1), action=(next;) - table=??(ls_in_arp_rsp ), priority=100 , match=(arp.tpa == 192.168.0.10 && arp.op == 1 && inport == "S1-vm1"), action=(next;) + table=??(ls_in_arp_rsp ), priority=100 , match=(arp.tpa == 192.168.0.10 && arp.op == 1 && eth.dst == ff:ff:ff:ff:ff:ff && inport == "S1-vm1"), action=(next;) table=??(ls_in_arp_rsp ), priority=100 , match=(nd_ns && ip6.dst == {fd00::10, ff02::1:ff00:10} && nd.target == fd00::10 && inport == "S1-vm1"), action=(next;) - table=??(ls_in_arp_rsp ), priority=50 , match=(arp.tpa == 192.168.0.10 && arp.op == 1), action=(eth.dst = eth.src; eth.src = 50:54:00:00:00:10; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 50:54:00:00:00:10; arp.tpa = arp.spa; arp.spa = 192.168.0.10; outport = inport; flags.loopback = 1; output;) + table=??(ls_in_arp_rsp ), priority=50 , match=(arp.tpa == 192.168.0.10 && arp.op == 1 && eth.dst == ff:ff:ff:ff:ff:ff), action=(eth.dst = eth.src; eth.src = 50:54:00:00:00:10; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 50:54:00:00:00:10; arp.tpa = arp.spa; arp.spa = 192.168.0.10; outport = inport; flags.loopback = 1; output;) table=??(ls_in_arp_rsp ), priority=50 , match=(nd_ns && ip6.dst == {fd00::10, ff02::1:ff00:10} && nd.target == fd00::10), action=(nd_na { eth.src = 50:54:00:00:00:10; ip6.src = fd00::10; nd.target = fd00::10; nd.tll = 50:54:00:00:00:10; outport = inport; flags.loopback = 1; output; };) ])
Reply only if target ethernet address is broadcast, if address is specified explicitly do noting to let target reply by itself. This technique allows to monitor target aliveness with arping. Closes #239 Signed-off-by: Vasyl Saienko <vsaienko@mirantis.com> --- northd/northd.c | 11 +++++++++-- northd/ovn-northd.8.xml | 7 ++++--- tests/ovn-northd.at | 4 ++-- 3 files changed, 15 insertions(+), 7 deletions(-)