Patchwork [net-next-2.6,1/4] net: introduce mc list helpers

login
register
mail settings
Submitter Jiri Pirko
Date Oct. 22, 2009, 1:52 p.m.
Message ID <20091022135220.GD2868@psychotron.lab.eng.brq.redhat.com>
Download mbox | patch
Permalink /patch/36659/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Jiri Pirko - Oct. 22, 2009, 1:52 p.m.
This helpers should be used by network drivers to access to netdev
multicast lists.

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 include/linux/netdevice.h |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)
Ben Hutchings - Oct. 22, 2009, 2:18 p.m.
On Thu, 2009-10-22 at 15:52 +0200, Jiri Pirko wrote:
> This helpers should be used by network drivers to access to netdev
> multicast lists.
[...]
> +static inline void netdev_mc_walk(struct net_device *dev,
> +				  void (*func)(void *, unsigned char *),
> +				  void *data)
> +{
> +	struct dev_addr_list *mclist;
> +	int i;
> +
> +	for (i = 0, mclist = dev->mc_list; mclist && i < dev->mc_count;
> +	     i++, mclist = mclist->next)
> +		func(data, mclist->dmi_addr);
> +}
[...]

We usually implement iteration as macros so that any context doesn't
have to be squeezed through a single untyped (void *) variable.  A macro
for this would look something like:

#define netdev_for_each_mc_addr(dev, addr)						\
	for (addr = (dev)->mc_list ? (dev)->mc_list->dmi_addr : NULL;			\
	     addr;									\
	     addr = (container_of(addr, struct dev_addr_list, dmi_addr)->next ?		\
		     container_of(addr, struct dev_addr_list, dmi_addr)->next->dmi_addr : \
		     NULL))

Once you change the list type this can presumably be made less ugly.

Ben.
Jiri Pirko - Oct. 22, 2009, 2:28 p.m.
Thu, Oct 22, 2009 at 04:18:32PM CEST, bhutchings@solarflare.com wrote:
>On Thu, 2009-10-22 at 15:52 +0200, Jiri Pirko wrote:
>> This helpers should be used by network drivers to access to netdev
>> multicast lists.
>[...]
>> +static inline void netdev_mc_walk(struct net_device *dev,
>> +				  void (*func)(void *, unsigned char *),
>> +				  void *data)
>> +{
>> +	struct dev_addr_list *mclist;
>> +	int i;
>> +
>> +	for (i = 0, mclist = dev->mc_list; mclist && i < dev->mc_count;
>> +	     i++, mclist = mclist->next)
>> +		func(data, mclist->dmi_addr);
>> +}
>[...]
>
>We usually implement iteration as macros so that any context doesn't
>have to be squeezed through a single untyped (void *) variable.  A macro
>for this would look something like:
>
>#define netdev_for_each_mc_addr(dev, addr)						\
>	for (addr = (dev)->mc_list ? (dev)->mc_list->dmi_addr : NULL;			\
>	     addr;									\
>	     addr = (container_of(addr, struct dev_addr_list, dmi_addr)->next ?		\
>		     container_of(addr, struct dev_addr_list, dmi_addr)->next->dmi_addr : \
>		     NULL))

I admit this would look better. Going to change this and then repost.

Thanks Ben

>
>Once you change the list type this can presumably be made less ugly.
>
>Ben.
>
>-- 
>Ben Hutchings, Senior Software Engineer, Solarflare Communications
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Pirko - Oct. 29, 2009, 3:19 p.m.
Thu, Oct 22, 2009 at 04:18:32PM CEST, bhutchings@solarflare.com wrote:
>On Thu, 2009-10-22 at 15:52 +0200, Jiri Pirko wrote:
>> This helpers should be used by network drivers to access to netdev
>> multicast lists.
>[...]
>> +static inline void netdev_mc_walk(struct net_device *dev,
>> +				  void (*func)(void *, unsigned char *),
>> +				  void *data)
>> +{
>> +	struct dev_addr_list *mclist;
>> +	int i;
>> +
>> +	for (i = 0, mclist = dev->mc_list; mclist && i < dev->mc_count;
>> +	     i++, mclist = mclist->next)
>> +		func(data, mclist->dmi_addr);
>> +}
>[...]
>
>We usually implement iteration as macros so that any context doesn't
>have to be squeezed through a single untyped (void *) variable.  A macro
>for this would look something like:
>
>#define netdev_for_each_mc_addr(dev, addr)						\
>	for (addr = (dev)->mc_list ? (dev)->mc_list->dmi_addr : NULL;			\
>	     addr;									\
>	     addr = (container_of(addr, struct dev_addr_list, dmi_addr)->next ?		\
>		     container_of(addr, struct dev_addr_list, dmi_addr)->next->dmi_addr : \
>		     NULL))
>
>Once you change the list type this can presumably be made less ugly.

Looking at this, I'm not sure how to deal with this macro once we need to
convert it to work with list_head. I see two options:

1) traverse through the list by hand in this macro (ugly)
2) introduce something like "list_for_each_struct_entry" which takes pointer of
   the structure member as a cursor. Then netdev_for_each_mc_addr would be just
   wrap-up of this.

What do you think?

Thanks

Jirka
>
>Ben.
>
>-- 
>Ben Hutchings, Senior Software Engineer, Solarflare Communications
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8380009..7edc4a6 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -921,6 +921,28 @@  struct net_device
 
 #define	NETDEV_ALIGN		32
 
+static inline int netdev_mc_count(struct net_device *dev)
+{
+	return dev->mc_count;
+}
+
+static inline bool netdev_mc_empty(struct net_device *dev)
+{
+	return netdev_mc_count(dev) == 0;
+}
+
+static inline void netdev_mc_walk(struct net_device *dev,
+				  void (*func)(void *, unsigned char *),
+				  void *data)
+{
+	struct dev_addr_list *mclist;
+	int i;
+
+	for (i = 0, mclist = dev->mc_list; mclist && i < dev->mc_count;
+	     i++, mclist = mclist->next)
+		func(data, mclist->dmi_addr);
+}
+
 static inline
 struct netdev_queue *netdev_get_tx_queue(const struct net_device *dev,
 					 unsigned int index)