The command "ovn-nbctl lrp-add" should not set the MAC address which length is invalid to logical router port. This patch updates the eth_addr_from_string() to check trailing characters. We should use the ovs_scan() to check the "addresses" owned by the logical port, instead of eth_addr_from_string(). This patch also updates the ovn-nbctl tests.
Signed-off-by: nickcooper-zhangtonghao <nickcooper-zhangtong...@opencloud.tech> --- lib/packets.c | 15 +++++++++------ ovn/northd/ovn-northd.c | 11 +++++++---- ovn/utilities/ovn-nbctl.c | 2 +- tests/ovn-nbctl.at | 7 +++++++ 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/lib/packets.c b/lib/packets.c index 11bb587..f661c34 100644 --- a/lib/packets.c +++ b/lib/packets.c @@ -142,16 +142,19 @@ eth_addr_is_reserved(const struct eth_addr ea) /* Attempts to parse 's' as an Ethernet address. If successful, stores the * address in 'ea' and returns true, otherwise zeros 'ea' and returns - * false. */ + * false. This function checks trailing characters. */ bool eth_addr_from_string(const char *s, struct eth_addr *ea) { - if (ovs_scan(s, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(*ea))) { - return true; - } else { - *ea = eth_addr_zero; - return false; + int n = 0; + if (ovs_scan_len(s, &n, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(*ea))) { + if (!s[n]) { + return true; + } } + + *ea = eth_addr_zero; + return false; } /* Fills 'b' with a Reverse ARP packet with Ethernet source address 'eth_src'. diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index eeeb41d..42b5423 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -2912,9 +2912,12 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, } for (size_t i = 0; i < op->nbsp->n_addresses; i++) { + /* Addresses are owned by the logical port. + * Ethernet address followed by zero or more IPv4 + * or IPv6 addresses (or both). */ struct eth_addr mac; - - if (eth_addr_from_string(op->nbsp->addresses[i], &mac)) { + if (ovs_scan(op->nbsp->addresses[i], + ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(mac))) { ds_clear(&match); ds_put_format(&match, "eth.dst == "ETH_ADDR_FMT, ETH_ADDR_ARGS(mac)); @@ -2930,8 +2933,8 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, } } else if (!strcmp(op->nbsp->addresses[i], "dynamic")) { if (!op->nbsp->dynamic_addresses - || !eth_addr_from_string(op->nbsp->dynamic_addresses, - &mac)) { + || !ovs_scan(op->nbsp->dynamic_addresses, + ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(mac))) { continue; } ds_clear(&match); diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index 7643241..453ff72 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -2299,7 +2299,7 @@ nbctl_lrp_add(struct ctl_context *ctx) } struct eth_addr ea; - if (!ovs_scan(mac, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(ea))) { + if (!eth_addr_from_string(mac, &ea)) { ctl_fatal("%s: invalid mac address %s", lrp_name, mac); } diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index d8331f8..115d781 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -530,6 +530,13 @@ AT_SETUP([ovn-nbctl - basic logical router port commands]) OVN_NBCTL_TEST_START AT_CHECK([ovn-nbctl lr-add lr0]) +AT_CHECK([ovn-nbctl lrp-add lr0 lrp0 00:00:00:01:02 192.168.1.1/24], [1], [], + [ovn-nbctl: lrp0: invalid mac address 00:00:00:01:02 +]) +AT_CHECK([ovn-nbctl lrp-add lr0 lrp0 00:00:00:01:02:03:04 192.168.1.1/24], [1], [], + [ovn-nbctl: lrp0: invalid mac address 00:00:00:01:02:03:04 +]) + AT_CHECK([ovn-nbctl lrp-add lr0 lrp0 00:00:00:01:02:03 192.168.1.1/24]) AT_CHECK([ovn-nbctl show lr0 | ${PERL} $srcdir/uuidfilt.pl], [0], [dnl -- 1.8.3.1 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev