It seems to work properly. No differences from the my patch.
Luca
On Tue, Dec 6, 2011 at 1:02 PM, Ben Pfaff <b...@nicira.com> wrote:
> From: Luca Giraudo <lgira...@nicira.com>
>
> Bug #2407.
> ---
> Hi Luca, here's a version of your patch that I tweaked a bit.
> I assume that you have a test case for this feature? Will you
> please try this version of the patch out to verify that I did
> not break it?
>
> Thanks,
>
> Ben.
>
> diff --git a/AUTHORS b/AUTHORS
> index 994fc46..db482c0 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -26,6 +26,7 @@ Joe Perches j...@perches.com
> Jun Nakajima jun.nakaj...@intel.com
> Justin Pettit jpet...@nicira.com
> Keith Amidon ke...@nicira.com
> +Luca Giraudo lgira...@nicira.com
> Martin Casado cas...@nicira.com
> Natasha Gude nata...@nicira.com
> Neil McKee neil.mc...@inmon.com
> diff --git a/NEWS b/NEWS
> index 8d7addf..61b74dd 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -20,6 +20,8 @@ v1.4.0 - xx xxx xxxx
> now be properly mirrored for any flows, regardless of their
> actions.
> - Track packet and byte statistics sent on mirrors.
> + - The sFlow implementation can now usually infer the correct agent
> + device instead of having to be told explicitly.
> - ovs-appctl:
> - New "fdb/flush" command to flush bridge's MAC learning table.
> - ovs-test:
> diff --git a/lib/netdev.c b/lib/netdev.c
> index e77fcdf..eff6e4c 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -692,6 +692,26 @@ netdev_set_in4(struct netdev *netdev, struct in_addr
> addr, struct in_addr mask)
> : EOPNOTSUPP);
> }
>
> +/* Obtains ad IPv4 address from device name and save the address in
> + * in4. Returns 0 if successful, otherwise a positive errno value.
> + */
> +int
> +netdev_get_in4_by_name(const char *device_name, struct in_addr *in4)
> +{
> + struct netdev *netdev;
> + int error;
> +
> + error = netdev_open(device_name, "system", &netdev);
> + if (error) {
> + in4->s_addr = htonl(0);
> + return error;
> + }
> +
> + error = netdev_get_in4(netdev, in4, NULL);
> + netdev_close(netdev);
> + return error;
> +}
> +
> /* Adds 'router' as a default IP gateway for the TCP/IP stack that
> corresponds
> * to 'netdev'. */
> int
> diff --git a/lib/netdev.h b/lib/netdev.h
> index 02271d4..24a9b64 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -131,6 +131,7 @@ int netdev_set_advertisements(struct netdev *,
> uint32_t advertise);
> int netdev_get_in4(const struct netdev *, struct in_addr *address,
> struct in_addr *netmask);
> int netdev_set_in4(struct netdev *, struct in_addr addr, struct in_addr
> mask);
> +int netdev_get_in4_by_name(const char *device_name, struct in_addr *in4);
> int netdev_get_in6(const struct netdev *, struct in6_addr *);
> int netdev_add_router(struct netdev *, struct in_addr router);
> int netdev_get_next_hop(const struct netdev *, const struct in_addr *host,
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index d17b0be..7dc6d28 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -18,6 +18,7 @@
> #include <config.h>
> #include "ofproto-dpif-sflow.h"
> #include <inttypes.h>
> +#include <net/if.h>
> #include <stdlib.h>
> #include "collectors.h"
> #include "compiler.h"
> @@ -30,6 +31,7 @@
> #include "ofproto.h"
> #include "packets.h"
> #include "poll-loop.h"
> +#include "route-table.h"
> #include "sflow_api.h"
> #include "socket-util.h"
> #include "timeval.h"
> @@ -223,25 +225,35 @@ sflow_agent_get_counters(void *ds_, SFLPoller
> *poller,
> * The sFlow agent address should be a local IP address that is persistent
> and
> * reachable over the network, if possible. The IP address associated with
> * 'agent_device' is used if it has one, and otherwise 'control_ip', the IP
> - * address used to talk to the controller. */
> + * address used to talk to the controller. If the agent device is not
> + * specified then it is figured out by taking a look at the routing table
> based
> + * on 'targets'. */
> static bool
> -sflow_choose_agent_address(const char *agent_device, const char
> *control_ip,
> +sflow_choose_agent_address(const char *agent_device,
> + const struct sset *targets,
> + const char *control_ip,
> SFLAddress *agent_addr)
> {
> + const char *target;
> struct in_addr in4;
>
> memset(agent_addr, 0, sizeof *agent_addr);
> agent_addr->type = SFLADDRESSTYPE_IP_V4;
>
> if (agent_device) {
> - 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;
> - }
> + if (!netdev_get_in4_by_name(agent_device, &in4)) {
> + goto success;
> + }
> + }
> +
> + SSET_FOR_EACH (target, targets) {
> + struct sockaddr_in sin;
> + char name[IFNAMSIZ];
> +
> + if (inet_parse_active(target, SFL_DEFAULT_COLLECTOR_PORT, &sin)
> + && route_table_get_name(sin.sin_addr.s_addr, name)
> + && !netdev_get_in4_by_name(name, &in4)) {
> + goto success;
> }
> }
>
> @@ -289,6 +301,8 @@ dpif_sflow_create(struct dpif *dpif)
> ds->next_tick = time_now() + 1;
> hmap_init(&ds->ports);
> ds->probability = 0;
> + route_table_register();
> +
> return ds;
> }
>
> @@ -307,6 +321,7 @@ dpif_sflow_destroy(struct dpif_sflow *ds)
> if (ds) {
> struct dpif_sflow_port *dsp, *next;
>
> + route_table_unregister();
> dpif_sflow_clear(ds);
> HMAP_FOR_EACH_SAFE (dsp, next, hmap_node, &ds->ports) {
> dpif_sflow_del_port__(ds, dsp);
> @@ -430,6 +445,14 @@ dpif_sflow_set_options(struct dpif_sflow *ds,
> }
> }
>
> + /* Choose agent IP address and agent device (if not yet setup) */
> + if (!sflow_choose_agent_address(options->agent_device,
> + &options->targets,
> + options->control_ip, &agentIP)) {
> + dpif_sflow_clear(ds);
> + return;
> + }
> +
> /* Avoid reconfiguring if options didn't change. */
> if (!options_changed) {
> return;
> @@ -437,13 +460,6 @@ dpif_sflow_set_options(struct dpif_sflow *ds,
> ofproto_sflow_options_destroy(ds->options);
> ds->options = ofproto_sflow_options_clone(options);
>
> - /* Choose agent IP address. */
> - if (!sflow_choose_agent_address(options->agent_device,
> - options->control_ip, &agentIP)) {
> - dpif_sflow_clear(ds);
> - return;
> - }
> -
> /* Create agent. */
> VLOG_INFO("creating sFlow agent %d", options->sub_id);
> if (ds->sflow_agent) {
> @@ -572,6 +588,7 @@ dpif_sflow_run(struct dpif_sflow *ds)
> {
> if (dpif_sflow_is_enabled(ds)) {
> time_t now = time_now();
> + route_table_run();
> if (now >= ds->next_tick) {
> sfl_agent_tick(ds->sflow_agent, time_wall());
> ds->next_tick = now + 1;
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 51309bf..84d91da 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -2771,10 +2771,12 @@
>
> <column name="agent">
> Name of the network device whose IP address should be reported as the
> - ``agent address'' to collectors. If not specified, the IP address
> + ``agent address'' to collectors. If not specified, the agent
> device is
> + figured from the first target address and the routing table. If the
> + routing table does not contain a route to the target, the IP address
> defaults to the <ref table="Controller" column="local_ip"/> in the
> collector's <ref table="Controller"/>. If an agent IP address
> cannot be
> - determined either way, sFlow is disabled.
> + determined any of these ways, sFlow is disabled.
> </column>
>
> <column name="header">
> --
> 1.7.4.4
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
