On 3/1/23 20:17, Mark Michelson wrote:
> Hi Ilya, I have a small suggestion below.
>
> On 2/27/23 15:03, Ilya Maximets wrote:
>> For non-dynamic address, current version performs 1 strcmp + 4 attempts
>> for ovs_scan. Updated code perfroms 1 strncmp + 1 ovs_scan.
>>
>> Also, for ports with both IPv4 and IPv6 specified, new version doesn't
>> re-parse IPv4 twice.
>>
>> Signed-off-by: Ilya Maximets <[email protected]>
>> ---
>> lib/ovn-util.c | 47 +++++++++++++++++++++++++++++++++++------------
>> 1 file changed, 35 insertions(+), 12 deletions(-)
>>
>> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
>> index 47484ef01..e683abb45 100644
>> --- a/lib/ovn-util.c
>> +++ b/lib/ovn-util.c
>> @@ -105,18 +105,41 @@ is_dynamic_lsp_address(const char *address)
>> char ipv6_s[IPV6_SCAN_LEN + 1];
>> struct eth_addr ea;
>> ovs_be32 ip;
>> - int n;
>> - return (!strcmp(address, "dynamic")
>> - || (ovs_scan(address, "dynamic "IP_SCAN_FMT"%n",
>> - IP_SCAN_ARGS(&ip), &n)
>> - && address[n] == '\0')
>> - || (ovs_scan(address, "dynamic "IP_SCAN_FMT" "IPV6_SCAN_FMT"%n",
>> - IP_SCAN_ARGS(&ip), ipv6_s, &n)
>> - && address[n] == '\0')
>> - || (ovs_scan(address, "dynamic "IPV6_SCAN_FMT"%n",
>> - ipv6_s, &n) && address[n] == '\0')
>> - || (ovs_scan(address, ETH_ADDR_SCAN_FMT" dynamic%n",
>> - ETH_ADDR_SCAN_ARGS(ea), &n) && address[n] ==
>> '\0'));
>> + int n = 0;
>> +
>> + if (!strncmp(address, "dynamic", 7)) {
>> + n = 7;
>> + if (!address[n]) {
>> + /* "dynamic" */
>> + return true;
>> + }
>
> marker (see below)
>
>> + if (ovs_scan_len(address, &n, " "IP_SCAN_FMT, IP_SCAN_ARGS(&ip))) {
>> + if (!address[n]) {
>> + /* "dynamic x.x.x.x" */
>> + return true;
>> + }
>> + if (ovs_scan_len(address, &n, " "IPV6_SCAN_FMT, ipv6_s)
>> + && !address[n]) {
>> + /* "dynamic x.x.x.x xxxx::xxxx" */
>> + return true;
>> + }
>> + return false;
I guess, 6 lines above can be just removed without any logic change, right?
I mean, we'll fall-through to the exactly same 6 lines below, except for
the comment.
>> + }
>> + if (ovs_scan_len(address, &n, " "IPV6_SCAN_FMT, ipv6_s)
>> + && !address[n]) {
>> + /* "dynamic xxxx::xxxx" */
>> + return true;
>> + }
>> + return false;
>> + }
>
>
> I think duplication can be reduced from the above marker to here:
see above.
>
> const char *addr = address;
> if (ovs_scan_len(addr, &n, " "IP_SCAN_FMT, IP_SCAN_ARGS(&ip))) {
> if (!addr[n]) {
> /* "dynamic x.x.x.x" */
> return true;
> }
> addr += n;
We would also need to reset 'n' here.
> }
> if (ovs_scan_len(addr, &n, " "IPV6_SCAN_FMT, ipv6_s) && !address[n]) {
> /* Either "dynamic xxxx::xxxx" or "dynamic x.x.x.x xxxx::xxxx"
> return true;
> }
> return false;
>
>> +
>> + if (ovs_scan_len(address, &n, ETH_ADDR_SCAN_FMT" dynamic",
>> + ETH_ADDR_SCAN_ARGS(ea)) && !address[n]) {
>> + /* "xx:xx:xx:xx:xx:xx dynamic" */
>> + return true;
>> + } > +
>> + return false;
>> }
>> static bool
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
