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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev