[B.A.T.M.A.N.] [PATCHv2] batman-adv: send each broadcast only once on non-wireless interfaces
While it makes sense to send each broadcast thrice on 802.11 (WLAN) interfaces as broadcasts are often unreliable on there, there is no reason to do so on other interface types.
The increased the overhead can be harmful on low-bandwidth links like VPN connections over slow internet lines, therefore it is better to reduce the number of broadcast packets sent on non-wireless links to one.
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net --- hard-interface.c | 11 +++++++++++ main.h | 5 +++++ send.c | 5 ++++- types.h | 1 + 4 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/hard-interface.c b/hard-interface.c index fd99e42..3f3ff9f 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -515,6 +515,17 @@ batadv_hardif_add_interface(struct net_device *net_dev) INIT_WORK(&hard_iface->cleanup_work, batadv_hardif_remove_interface_finish);
+ hard_iface->num_bcasts = BATADV_NUM_BCASTS_DEFAULT; + +#if IS_ENABLED(CONFIG_WIRELESS_EXT) || IS_ENABLED(CONFIG_CFG80211) + if (net_dev->ieee80211_ptr) + hard_iface->num_bcasts = BATADV_NUM_BCASTS_WIRELESS; +#if IS_ENABLED(CONFIG_WIRELESS_EXT) + else if (net_dev->wireless_handlers) + hard_iface->num_bcasts = BATADV_NUM_BCASTS_WIRELESS; +#endif +#endif + /* extra reference for return */ atomic_set(&hard_iface->refcount, 2);
diff --git a/main.h b/main.h index e6eaf43..2c751c6 100644 --- a/main.h +++ b/main.h @@ -76,6 +76,11 @@
#define BATADV_LOG_BUF_LEN 8192 /* has to be a power of 2 */
+/* number of packets to send for broadcasts on different interface types */ +#define BATADV_NUM_BCASTS_DEFAULT 1 +#define BATADV_NUM_BCASTS_WIRELESS 3 +#define BATADV_NUM_BCASTS_MAX 3 + /* msecs after which an ARP_REQUEST is sent in broadcast as fallback */ #define ARP_REQ_DELAY 250 /* numbers of originator to contact for any PUT/GET DHT operation */ diff --git a/send.c b/send.c index 0a0bb45..e21138f 100644 --- a/send.c +++ b/send.c @@ -260,6 +260,9 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) if (hard_iface->soft_iface != soft_iface) continue;
+ if (forw_packet->num_packets >= hard_iface->num_bcasts) + continue; + /* send a copy of the saved skb */ skb1 = skb_clone(forw_packet->skb, GFP_ATOMIC); if (skb1) @@ -271,7 +274,7 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) forw_packet->num_packets++;
/* if we still have some more bcasts to send */ - if (forw_packet->num_packets < 3) { + if (forw_packet->num_packets < BATADV_NUM_BCASTS_MAX) { _batadv_add_bcast_packet_to_list(bat_priv, forw_packet, msecs_to_jiffies(5)); return; diff --git a/types.h b/types.h index aba8364..0c5009f 100644 --- a/types.h +++ b/types.h @@ -76,6 +76,7 @@ struct batadv_hard_iface { char if_status; struct net_device *net_dev; atomic_t frag_seqno; + uint8_t num_bcasts; struct kobject *hardif_obj; atomic_t refcount; struct packet_type batman_adv_ptype;
While it makes sense to send each broadcast thrice on 802.11 (WLAN) interfaces as broadcasts are often unreliable on these, there is no reason to do so on other interface types.
The increased the overhead can be harmful on low-bandwidth links like VPN connections over slow internet lines, therefore it is better to reduce the number of broadcast packets sent on non-wireless links to one.
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net --- v3: fixed typo in commit message
hard-interface.c | 11 +++++++++++ main.h | 5 +++++ send.c | 5 ++++- types.h | 1 + 4 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/hard-interface.c b/hard-interface.c index fd99e42..3f3ff9f 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -515,6 +515,17 @@ batadv_hardif_add_interface(struct net_device *net_dev) INIT_WORK(&hard_iface->cleanup_work, batadv_hardif_remove_interface_finish);
+ hard_iface->num_bcasts = BATADV_NUM_BCASTS_DEFAULT; + +#if IS_ENABLED(CONFIG_WIRELESS_EXT) || IS_ENABLED(CONFIG_CFG80211) + if (net_dev->ieee80211_ptr) + hard_iface->num_bcasts = BATADV_NUM_BCASTS_WIRELESS; +#if IS_ENABLED(CONFIG_WIRELESS_EXT) + else if (net_dev->wireless_handlers) + hard_iface->num_bcasts = BATADV_NUM_BCASTS_WIRELESS; +#endif +#endif + /* extra reference for return */ atomic_set(&hard_iface->refcount, 2);
diff --git a/main.h b/main.h index e6eaf43..2c751c6 100644 --- a/main.h +++ b/main.h @@ -76,6 +76,11 @@
#define BATADV_LOG_BUF_LEN 8192 /* has to be a power of 2 */
+/* number of packets to send for broadcasts on different interface types */ +#define BATADV_NUM_BCASTS_DEFAULT 1 +#define BATADV_NUM_BCASTS_WIRELESS 3 +#define BATADV_NUM_BCASTS_MAX 3 + /* msecs after which an ARP_REQUEST is sent in broadcast as fallback */ #define ARP_REQ_DELAY 250 /* numbers of originator to contact for any PUT/GET DHT operation */ diff --git a/send.c b/send.c index 0a0bb45..e21138f 100644 --- a/send.c +++ b/send.c @@ -260,6 +260,9 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) if (hard_iface->soft_iface != soft_iface) continue;
+ if (forw_packet->num_packets >= hard_iface->num_bcasts) + continue; + /* send a copy of the saved skb */ skb1 = skb_clone(forw_packet->skb, GFP_ATOMIC); if (skb1) @@ -271,7 +274,7 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) forw_packet->num_packets++;
/* if we still have some more bcasts to send */ - if (forw_packet->num_packets < 3) { + if (forw_packet->num_packets < BATADV_NUM_BCASTS_MAX) { _batadv_add_bcast_packet_to_list(bat_priv, forw_packet, msecs_to_jiffies(5)); return; diff --git a/types.h b/types.h index aba8364..0c5009f 100644 --- a/types.h +++ b/types.h @@ -76,6 +76,7 @@ struct batadv_hard_iface { char if_status; struct net_device *net_dev; atomic_t frag_seqno; + uint8_t num_bcasts; struct kobject *hardif_obj; atomic_t refcount; struct packet_type batman_adv_ptype;
Hi Matthias,
On Sat, Mar 09, 2013 at 11:40:26AM +0100, Matthias Schiffer wrote:
@@ -515,6 +515,17 @@ batadv_hardif_add_interface(struct net_device *net_dev) INIT_WORK(&hard_iface->cleanup_work, batadv_hardif_remove_interface_finish);
- hard_iface->num_bcasts = BATADV_NUM_BCASTS_DEFAULT;
+#if IS_ENABLED(CONFIG_WIRELESS_EXT) || IS_ENABLED(CONFIG_CFG80211)
- if (net_dev->ieee80211_ptr)
hard_iface->num_bcasts = BATADV_NUM_BCASTS_WIRELESS;+#if IS_ENABLED(CONFIG_WIRELESS_EXT)
- else if (net_dev->wireless_handlers)
hard_iface->num_bcasts = BATADV_NUM_BCASTS_WIRELESS;+#endif +#endif
- /* extra reference for return */ atomic_set(&hard_iface->refcount, 2);
What about re-using batadv_is_wifi_iface() instead of adding the same code here? :)
batadv_is_wifi_iface() takes an ifindex as parameter and then gets the net_device by means of dev_get_by_index. You may want to split it up so that you can avoid the research since you already have the related net_device object.
Cheers,
Previously batadv_is_wifi_iface() did two things at once: looking up a net_device from an interface index, and determining if it is a wifi device.
The second part is useful itself when the caller already has a net_device reference.
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net --- hard-interface.c | 73 +++++++++++++++++++++++++++++++------------------------- 1 file changed, 41 insertions(+), 32 deletions(-)
diff --git a/hard-interface.c b/hard-interface.c index fd99e42..333a504 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -117,6 +117,47 @@ static int batadv_is_valid_iface(const struct net_device *net_dev) return 1; }
+/* This function returns true if the net device is a 802.11 wireless device */ +static bool batadv_is_wifi_net_device(struct net_device *net_device) +{ +#ifdef CONFIG_WIRELESS_EXT + /* pre-cfg80211 drivers have to implement WEXT, so it is possible to + * check for wireless_handlers != NULL + */ + if (net_device->wireless_handlers) + return true; +#endif + + /* cfg80211 drivers have to set ieee80211_ptr */ + if (net_device->ieee80211_ptr) + return true; + + return false; +} + +/* This function returns true if the interface represented by ifindex is a + * 802.11 wireless device + */ +bool batadv_is_wifi_iface(int ifindex) +{ + struct net_device *net_device = NULL; + bool ret = false; + + if (ifindex == BATADV_NULL_IFINDEX) + goto out; + + net_device = dev_get_by_index(&init_net, ifindex); + if (!net_device) + goto out; + + ret = batadv_is_wifi_net_device(net_device); + +out: + if (net_device) + dev_put(net_device); + return ret; +} + static struct batadv_hard_iface * batadv_hardif_get_active(const struct net_device *soft_iface) { @@ -631,38 +672,6 @@ out: return NOTIFY_DONE; }
-/* This function returns true if the interface represented by ifindex is a - * 802.11 wireless device - */ -bool batadv_is_wifi_iface(int ifindex) -{ - struct net_device *net_device = NULL; - bool ret = false; - - if (ifindex == BATADV_NULL_IFINDEX) - goto out; - - net_device = dev_get_by_index(&init_net, ifindex); - if (!net_device) - goto out; - -#ifdef CONFIG_WIRELESS_EXT - /* pre-cfg80211 drivers have to implement WEXT, so it is possible to - * check for wireless_handlers != NULL - */ - if (net_device->wireless_handlers) - ret = true; - else -#endif - /* cfg80211 drivers have to set ieee80211_ptr */ - if (net_device->ieee80211_ptr) - ret = true; -out: - if (net_device) - dev_put(net_device); - return ret; -} - struct notifier_block batadv_hard_if_notifier = { .notifier_call = batadv_hard_if_event, };
While it makes sense to send each broadcast thrice on 802.11 (WLAN) interfaces as broadcasts are often unreliable on these, there is no reason to do so on other interface types.
The increased the overhead can be harmful on low-bandwidth links like VPN connections over slow internet lines, therefore it is better to reduce the number of broadcast packets sent on non-wireless links to one.
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net --- hard-interface.c | 5 +++++ main.h | 5 +++++ send.c | 5 ++++- types.h | 1 + 4 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/hard-interface.c b/hard-interface.c index 333a504..0eaf02c 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -556,6 +556,11 @@ batadv_hardif_add_interface(struct net_device *net_dev) INIT_WORK(&hard_iface->cleanup_work, batadv_hardif_remove_interface_finish);
+ if (batadv_is_wifi_net_device(net_dev)) + hard_iface->num_bcasts = BATADV_NUM_BCASTS_WIRELESS; + else + hard_iface->num_bcasts = BATADV_NUM_BCASTS_DEFAULT; + /* extra reference for return */ atomic_set(&hard_iface->refcount, 2);
diff --git a/main.h b/main.h index e6eaf43..2c751c6 100644 --- a/main.h +++ b/main.h @@ -76,6 +76,11 @@
#define BATADV_LOG_BUF_LEN 8192 /* has to be a power of 2 */
+/* number of packets to send for broadcasts on different interface types */ +#define BATADV_NUM_BCASTS_DEFAULT 1 +#define BATADV_NUM_BCASTS_WIRELESS 3 +#define BATADV_NUM_BCASTS_MAX 3 + /* msecs after which an ARP_REQUEST is sent in broadcast as fallback */ #define ARP_REQ_DELAY 250 /* numbers of originator to contact for any PUT/GET DHT operation */ diff --git a/send.c b/send.c index 0a0bb45..e21138f 100644 --- a/send.c +++ b/send.c @@ -260,6 +260,9 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) if (hard_iface->soft_iface != soft_iface) continue;
+ if (forw_packet->num_packets >= hard_iface->num_bcasts) + continue; + /* send a copy of the saved skb */ skb1 = skb_clone(forw_packet->skb, GFP_ATOMIC); if (skb1) @@ -271,7 +274,7 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) forw_packet->num_packets++;
/* if we still have some more bcasts to send */ - if (forw_packet->num_packets < 3) { + if (forw_packet->num_packets < BATADV_NUM_BCASTS_MAX) { _batadv_add_bcast_packet_to_list(bat_priv, forw_packet, msecs_to_jiffies(5)); return; diff --git a/types.h b/types.h index aba8364..0c5009f 100644 --- a/types.h +++ b/types.h @@ -76,6 +76,7 @@ struct batadv_hard_iface { char if_status; struct net_device *net_dev; atomic_t frag_seqno; + uint8_t num_bcasts; struct kobject *hardif_obj; atomic_t refcount; struct packet_type batman_adv_ptype;
Previously batadv_is_wifi_iface() did two things at once: looking up a net_device from an interface index, and determining if it is a wifi device.
The second part is useful itself when the caller already has a net_device reference.
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net --- hard-interface.c | 73 +++++++++++++++++++++++++++++++------------------------- 1 file changed, 41 insertions(+), 32 deletions(-)
diff --git a/hard-interface.c b/hard-interface.c index fd99e42..333a504 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -117,6 +117,47 @@ static int batadv_is_valid_iface(const struct net_device *net_dev) return 1; }
+/* This function returns true if the net device is a 802.11 wireless device */ +static bool batadv_is_wifi_net_device(struct net_device *net_device) +{ +#ifdef CONFIG_WIRELESS_EXT + /* pre-cfg80211 drivers have to implement WEXT, so it is possible to + * check for wireless_handlers != NULL + */ + if (net_device->wireless_handlers) + return true; +#endif + + /* cfg80211 drivers have to set ieee80211_ptr */ + if (net_device->ieee80211_ptr) + return true; + + return false; +} + +/* This function returns true if the interface represented by ifindex is a + * 802.11 wireless device + */ +bool batadv_is_wifi_iface(int ifindex) +{ + struct net_device *net_device = NULL; + bool ret = false; + + if (ifindex == BATADV_NULL_IFINDEX) + goto out; + + net_device = dev_get_by_index(&init_net, ifindex); + if (!net_device) + goto out; + + ret = batadv_is_wifi_net_device(net_device); + +out: + if (net_device) + dev_put(net_device); + return ret; +} + static struct batadv_hard_iface * batadv_hardif_get_active(const struct net_device *soft_iface) { @@ -631,38 +672,6 @@ out: return NOTIFY_DONE; }
-/* This function returns true if the interface represented by ifindex is a - * 802.11 wireless device - */ -bool batadv_is_wifi_iface(int ifindex) -{ - struct net_device *net_device = NULL; - bool ret = false; - - if (ifindex == BATADV_NULL_IFINDEX) - goto out; - - net_device = dev_get_by_index(&init_net, ifindex); - if (!net_device) - goto out; - -#ifdef CONFIG_WIRELESS_EXT - /* pre-cfg80211 drivers have to implement WEXT, so it is possible to - * check for wireless_handlers != NULL - */ - if (net_device->wireless_handlers) - ret = true; - else -#endif - /* cfg80211 drivers have to set ieee80211_ptr */ - if (net_device->ieee80211_ptr) - ret = true; -out: - if (net_device) - dev_put(net_device); - return ret; -} - struct notifier_block batadv_hard_if_notifier = { .notifier_call = batadv_hard_if_event, };
While it makes sense to send each broadcast thrice on 802.11 (WLAN) interfaces as broadcasts are often unreliable on these, there is no reason to do so on other interface types.
The increased the overhead can be harmful on low-bandwidth links like VPN connections over slow internet lines, therefore it is better to reduce the number of broadcast packets sent on non-wireless links to one.
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net --- hard-interface.c | 4 ++++ main.h | 5 +++++ send.c | 5 ++++- types.h | 1 + 4 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/hard-interface.c b/hard-interface.c index 333a504..e4eb77e 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -556,6 +556,10 @@ batadv_hardif_add_interface(struct net_device *net_dev) INIT_WORK(&hard_iface->cleanup_work, batadv_hardif_remove_interface_finish);
+ hard_iface->num_bcasts = BATADV_NUM_BCASTS_DEFAULT; + if (batadv_is_wifi_net_device(net_dev)) + hard_iface->num_bcasts = BATADV_NUM_BCASTS_WIRELESS; + /* extra reference for return */ atomic_set(&hard_iface->refcount, 2);
diff --git a/main.h b/main.h index e6eaf43..2c751c6 100644 --- a/main.h +++ b/main.h @@ -76,6 +76,11 @@
#define BATADV_LOG_BUF_LEN 8192 /* has to be a power of 2 */
+/* number of packets to send for broadcasts on different interface types */ +#define BATADV_NUM_BCASTS_DEFAULT 1 +#define BATADV_NUM_BCASTS_WIRELESS 3 +#define BATADV_NUM_BCASTS_MAX 3 + /* msecs after which an ARP_REQUEST is sent in broadcast as fallback */ #define ARP_REQ_DELAY 250 /* numbers of originator to contact for any PUT/GET DHT operation */ diff --git a/send.c b/send.c index 0a0bb45..e21138f 100644 --- a/send.c +++ b/send.c @@ -260,6 +260,9 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) if (hard_iface->soft_iface != soft_iface) continue;
+ if (forw_packet->num_packets >= hard_iface->num_bcasts) + continue; + /* send a copy of the saved skb */ skb1 = skb_clone(forw_packet->skb, GFP_ATOMIC); if (skb1) @@ -271,7 +274,7 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work) forw_packet->num_packets++;
/* if we still have some more bcasts to send */ - if (forw_packet->num_packets < 3) { + if (forw_packet->num_packets < BATADV_NUM_BCASTS_MAX) { _batadv_add_bcast_packet_to_list(bat_priv, forw_packet, msecs_to_jiffies(5)); return; diff --git a/types.h b/types.h index aba8364..0c5009f 100644 --- a/types.h +++ b/types.h @@ -76,6 +76,7 @@ struct batadv_hard_iface { char if_status; struct net_device *net_dev; atomic_t frag_seqno; + uint8_t num_bcasts; struct kobject *hardif_obj; atomic_t refcount; struct packet_type batman_adv_ptype;
On Sunday, March 10, 2013 06:14:23 Matthias Schiffer wrote:
While it makes sense to send each broadcast thrice on 802.11 (WLAN) interfaces as broadcasts are often unreliable on these, there is no reason to do so on other interface types.
The increased the overhead can be harmful on low-bandwidth links like VPN connections over slow internet lines, therefore it is better to reduce the number of broadcast packets sent on non-wireless links to one.
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net
hard-interface.c | 4 ++++ main.h | 5 +++++ send.c | 5 ++++- types.h | 1 + 4 files changed, 14 insertions(+), 1 deletion(-)
Applied in revision cc1fde3.
Thanks, Marek
PS: Also struct definitions require kernel doc ..
On Sunday, March 10, 2013 06:14:22 Matthias Schiffer wrote:
Previously batadv_is_wifi_iface() did two things at once: looking up a net_device from an interface index, and determining if it is a wifi device.
The second part is useful itself when the caller already has a net_device reference.
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net
hard-interface.c | 73 +++++++++++++++++++++++++++++++------------------------- 1 file changed, 41 insertions(+), 32 deletions(-)
Applied in revision a75dbf4.
Thanks, Marek
PS: Every new function is required to have kernel doc. This time I filled in the missing pieces but please do so in the future.
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Marek Lindner -
Matthias Schiffer