[ovs-dev] [PATCH] ofproto-dpif-sflow: avoid to specify agent device when configuring sFlow

[Ben Pfaff]

Thank you for the patch.  It's pretty close to ready, I think.

I think that you should add updates to NEWS and vswitchd/vswitch.xml,
to inform users of this improvement.

In sflow_choose_agent_address():

    I think that the 'default_port' variable could be removed, since it is
    a constant.  You can just write SFL_DEFAULT_COLLECTOR_PORT where you
    currently reference 'default_port'.

    You should update the comment above this function.

    I think that the "continue;" in the loop below should be a "break;".
    Either that, or you can just remove it:

> +        SSET_FOR_EACH (target, targets) {
> +            if (inet_parse_active(target, default_port, &target_addr)) {
> +                target_ip = target_addr.sin_addr.s_addr;
> +                continue;
> +            }
> +        }

    I think that there is no need to initialize the 'target' variable,
    since its value is used only inside the loop.

    I think that the 'target_addr' variable should be declared inside
    the loop instead of outside of it, since it is used only inside
    the loop.

    I see that the following code block now appears twice, could you
    please factor it out into a function or otherwize:

        struct netdev *netdev;

        if (!netdev_open(agent_device, "system", &netdev)) {
            int error = netdev_get_in4(netdev, &in4, NULL);
            netdev_close(netdev);

            if (!error) {
                goto success;
            }
        }

Thanks,

Ben.