diff mbox

[OpenWrt-Devel,netifd] add support for setting send_redirects

Message ID 20160308122931.GA19488@makrotopia.org
State Changes Requested
Delegated to: Felix Fietkau
Headers show

Commit Message

Daniel Golle March 8, 2016, 12:29 p.m. UTC 
Setting /proc/sys/net/ipv4/conf/*/send_redirects is useful if a single
layer-2 domain is shared among routed subnets.
Sending redirects will prevents traffic from taking unnessesary detours
through a gateway in cases where direct connectivity on layer 2 exists.
This is commonly the case if an existing LAN infratructure with dump
switches is used to additionally carry routing protocols like OLSR
which are supported only by some nodes on the network.
It's important to note that the default value for send_redirects
differs for interface types (it's enabled on physical ethernet
interfaces, but disabled e.g. on VLANs), thus the default differs
also depending on the way an on-board switch is integrated on specific
boards. Having a way to explicitely enable or disable send_redirects is
thus desireable also to unify the default behaviour among different,
but seemingly similar devices supported by OpenWrt.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 device.c       |  9 +++++++++
 device.h       |  3 +++
 system-linux.c | 18 ++++++++++++++++++
 3 files changed, 30 insertions(+)

Comments

Daniel Golle March 8, 2016, 12:56 p.m. UTC | #1 
On Tue, Mar 08, 2016 at 01:29:36PM +0100, Daniel Golle wrote:
> It's important to note that the default value for send_redirects
> differs for interface types (it's enabled on physical ethernet
> interfaces, but disabled e.g. on VLANs), thus the default differs
> also depending on the way an on-board switch is integrated on specific
> boards. Having a way to explicitely enable or disable send_redirects is
> thus desireable also to unify the default behaviour among different,
> but seemingly similar devices supported by OpenWrt.

I forgot to mention that the different defaults for physical interfaces
vs. VLAN interfaces is due to olsrd changing the default in
/proc/sys/net/..., thus interfaces created by the kernel and not
touched by olsrd still got the original default from kernel
(send_redirects being switched on) while virtual interfaces being
created later on, eg. by network restart and such where VLANs on top
of a physical ethernet interface are being created, those will have
the new default previously set by olsrd (ie. send_redirects will be
switched off).
Due to this, the value of send_redirects after a network restart for
interfaces *not* used by olsrd suddenly differs depending on whether
they are 'physical' (e.g. eth0) interfaces or virtual interfaces
(eth0.1)...

At least allowing to explicitely configure send_redirects in netifd
seemed to be the easiest way to improve the situation significantly
without hurting anyone.
diff mbox

Patch

diff --git a/device.c b/device.c
index 9344e1b..2caf1e7 100644
--- a/device.c
+++ b/device.c
@@ -51,6 +51,7 @@  static const struct blobmsg_policy dev_attrs[__DEV_ATTR_MAX] = {
 	[DEV_ATTR_MULTICAST_TO_UNICAST] = { .name = "multicast_to_unicast", .type = BLOBMSG_TYPE_BOOL },
 	[DEV_ATTR_MULTICAST_ROUTER] = { .name = "multicast_router", .type = BLOBMSG_TYPE_INT32 },
 	[DEV_ATTR_MULTICAST] = { .name ="multicast", .type = BLOBMSG_TYPE_BOOL },
+	[DEV_ATTR_SENDREDIRECTS] = { .name = "sendredirects", .type = BLOBMSG_TYPE_BOOL },
 };
 
 const struct uci_blob_param_list device_attr_list = {
@@ -177,6 +178,7 @@  device_merge_settings(struct device *dev, struct device_settings *n)
 		s->multicast : os->multicast;
 	n->multicast_to_unicast = s->multicast_to_unicast;
 	n->multicast_router = s->multicast_router;
+	n->sendredirects = s->flags & DEV_OPT_SENDREDIRECTS ? s->sendredirects : os->sendredirects;
 	n->flags = s->flags | os->flags | os->valid_flags;
 }
 
@@ -295,6 +297,11 @@  device_init_settings(struct device *dev, struct blob_attr **tb)
 		s->flags |= DEV_OPT_MULTICAST;
 	}
 
+	if ((cur = tb[DEV_ATTR_SENDREDIRECTS])) {
+		s->sendredirects = blobmsg_get_bool(cur);
+		s->flags |= DEV_OPT_SENDREDIRECTS;
+	}
+
 	device_set_disabled(dev, disabled);
 }
 
@@ -939,6 +946,8 @@  device_dump_status(struct blob_buf *b, struct device *dev)
 			blobmsg_add_u32(b, "multicast_router", st.multicast_router);
 		if (st.flags & DEV_OPT_MULTICAST)
 			blobmsg_add_u8(b, "multicast", st.multicast);
+		if (st.flags & DEV_OPT_SENDREDIRECTS)
+			blobmsg_add_u8(b, "sendredirects", st.sendredirects);
 	}
 
 	s = blobmsg_open_table(b, "statistics");
diff --git a/device.h b/device.h
index ac77cfb..379a49f 100644
--- a/device.h
+++ b/device.h
@@ -45,6 +45,7 @@  enum {
 	DEV_ATTR_MULTICAST_TO_UNICAST,
 	DEV_ATTR_MULTICAST_ROUTER,
 	DEV_ATTR_MULTICAST,
+	DEV_ATTR_SENDREDIRECTS,
 	__DEV_ATTR_MAX,
 };
 
@@ -88,6 +89,7 @@  enum {
 	DEV_OPT_MULTICAST_TO_UNICAST	= (1 << 14),
 	DEV_OPT_MULTICAST_ROUTER	= (1 << 15),
 	DEV_OPT_MULTICAST		= (1 << 16),
+	DEV_OPT_SENDREDIRECTS		= (1 << 17),
 };
 
 /* events broadcasted to all users of a device */
@@ -149,6 +151,7 @@  struct device_settings {
 	bool multicast_to_unicast;
 	unsigned int multicast_router;
 	bool multicast;
+	bool sendredirects;
 };
 
 /*
diff --git a/system-linux.c b/system-linux.c
index 86e373c..e2f0761 100644
--- a/system-linux.c
+++ b/system-linux.c
@@ -332,6 +332,11 @@  static void system_bridge_set_multicast_router(struct device *dev, const char *v
 			      dev->ifname, val);
 }
 
+static void system_set_sendredirects(struct device *dev, const char *val)
+{
+	system_set_dev_sysctl("/proc/sys/net/ipv4/conf/%s/send_redirects", dev->ifname, val);
+}
+
 static int system_get_sysctl(const char *path, char *buf, const size_t buf_sz)
 {
 	int fd = -1, ret = -1;
@@ -408,6 +413,12 @@  static int system_get_dadtransmits(struct device *dev, char *buf, const size_t b
 			dev->ifname, buf, buf_sz);
 }
 
+static int system_get_sendredirects(struct device *dev, char *buf, const size_t buf_sz)
+{
+	return system_get_dev_sysctl("/proc/sys/net/ipv4/conf/%s/send_redirects",
+			dev->ifname, buf, buf_sz);
+}
+
 // Evaluate netlink messages
 static int cb_rtnl_event(struct nl_msg *msg, void *arg)
 {
@@ -1128,6 +1139,11 @@  system_if_get_settings(struct device *dev, struct device_settings *s)
 		s->dadtransmits = strtoul(buf, NULL, 0);
 		s->flags |= DEV_OPT_DADTRANSMITS;
 	}
+
+	if (!system_get_sendredirects(dev, buf, sizeof(buf))) {
+		s->sendredirects = strtoul(buf, NULL, 0);
+		s->flags |= DEV_OPT_SENDREDIRECTS;
+	}
 }
 
 static void
@@ -1227,6 +1243,8 @@  system_if_apply_settings(struct device *dev, struct device_settings *s, unsigned
 				    !s->multicast ? IFF_MULTICAST : 0) < 0)
 			s->flags &= ~DEV_OPT_MULTICAST;
 	}
+	if (s->flags & DEV_OPT_SENDREDIRECTS & apply_mask)
+		system_set_sendredirects(dev, s->sendredirects ? "1" : "0");
 
 	system_if_apply_rps_xps(dev, s);
 }