[ovs-dev] [PATCH] ofproto-dpif-sflow: avoid to specify agent device when configuring sFlow
Neil Mckee
neil.mckee at inmon.com
Tue Dec 6 21:43:25 UTC 2011
Hi Luca,
Just checking.... does this mean that the sFlow-agent-address might change just because the first collector in the list changed? Or because there was a routing change and the packet path to the collector went out a different interface?
The primary purpose of the sflow-agent-address is to provide a persistent identifier. One that can be used as a key to store the data under. It's helpful if that is also an address that can be talked to, but connectivity is not the main thing. The main thing is for it to stay the same across reboots, routing-changes and collector-changes.
It sounds like you can still set this address explicitly, which is good, but if you are choosing one on the fly then please consider ways to make that decision as "sticky" as possible. For example, one approach is to select the lowest-numbered loopback-address (if there are any) and fall back on the lowest numbered interface address (perhaps preferring IPv4 addresses over IPv6 addresses). Another approach is to choose one any way you like, but then keep it the same for as long as that IP belongs to the switch.
Neil
On Dec 6, 2011, at 12:42 PM, Luca Giraudo wrote:
> ---
> NEWS | 3 ++
> lib/netdev.c | 21 +++++++++++++++
> lib/netdev.h | 1 +
> ofproto/ofproto-dpif-sflow.c | 56 +++++++++++++++++++++++++++++++----------
> vswitchd/vswitch.xml | 4 ++-
> 5 files changed, 70 insertions(+), 15 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 8d7addf..6dc6a3a 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -26,6 +26,9 @@ v1.4.0 - xx xxx xxxx
> - A new distributed testing tool that allows one to diagnose performance
> and connectivity issues. This tool currently is not included in RH or
> Xen packages.
> + - ovs-vsctl:
> + - sFlow configuration without "agent" parameter in the case the routing
> + table contains a route to the first sFlow target in the list.
> - RHEL packaging now supports integration with Red Hat network scripts.
>
>
> diff --git a/lib/netdev.c b/lib/netdev.c
> index e77fcdf..053bf48 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -692,6 +692,27 @@ 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
> + true is successful, false on failure.
> + */
> +bool
> +netdev_get_in4_by_name(const char *device_name, struct in_addr *in4)
> +{
> + struct netdev *netdev;
> +
> + int error = netdev_open(device_name, "system", &netdev);
> + if (!error) {
> + error = netdev_get_in4(netdev, in4, NULL);
> + netdev_close(netdev);
> +
> + if(!error) {
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> /* 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..10923b6 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);
> +bool 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..a6c5517 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,9 +225,14 @@ 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 and at target
> + * list.
> + */
> 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)
> {
> struct in_addr in4;
> @@ -234,12 +241,28 @@ sflow_choose_agent_address(const char *agent_device, const char *control_ip,
> agent_addr->type = SFLADDRESSTYPE_IP_V4;
>
> if (agent_device) {
> - struct netdev *netdev;
> + bool ret = netdev_get_in4_by_name(agent_device, &in4);
> +
> + if (ret == true) {
> + goto success;
> + }
> + } else if (!sset_is_empty(targets)) {
> + const char *target;
> + ovs_be32 target_ip = 0;
> + char name[IFNAMSIZ];
> +
> + SSET_FOR_EACH (target, targets) {
> + struct sockaddr_in target_addr;
> + if (inet_parse_active(target, SFL_DEFAULT_COLLECTOR_PORT, &target_addr)) {
> + target_ip = target_addr.sin_addr.s_addr;
> + break;
> + }
> + }
>
> - if (!netdev_open(agent_device, "system", &netdev)) {
> - int error = netdev_get_in4(netdev, &in4, NULL);
> - netdev_close(netdev);
> - if (!error) {
> + if (target_ip && route_table_get_name(target_ip, name)) {
> + bool ret = netdev_get_in4_by_name(name, &in4);
> +
> + if (ret == true) {
> goto success;
> }
> }
> @@ -289,6 +312,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 +332,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 +456,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 +471,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 +599,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..948e756 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -2771,7 +2771,9 @@
>
> <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.
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list