* [PATCH net-next-2.6] net: use helpers to access mc list
@ 2010-01-26 14:57 Jiri Pirko
2010-02-02 15:25 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2010-01-26 14:57 UTC (permalink / raw)
To: netdev; +Cc: davem
This patch introduces the similar helpers as those already done for uc list.
However multicast lists are no list_head lists but "mademanually". The three
macros added by this patch will make the transition of mc_list to list_head
smooth.
>From now on, drivers can (and should) use "netdev_for_each_mc_addr" to iterate
over the addresses with iterator of type "struct netdev_hw_addr". Also macros
"netdev_mc_count" and "netdev_mc_empty" to read list's length. This is the state
which should be reached in all drivers.
Temporary macro "netdev_for_each_mc_addr" works in the ugly way, I'm aware, but
it will be replaced. It uses iterator stored in "struct net_device". In every
iteration, it copies addr from the list to "struct netdev_hw_addr" instance
(also stored in "struct net_device"). Driver reads address stored in this
structure. All is protected by addr_list_lock held by a caller.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 93a32a5..e470b22 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -268,6 +268,18 @@ struct netdev_hw_addr_list {
#define netdev_for_each_uc_addr(ha, dev) \
list_for_each_entry(ha, &dev->uc.list, list)
+#define netdev_mc_count(dev) ((dev)->mc_count)
+#define netdev_mc_empty(dev) (netdev_mc_count(dev) == 0)
+
+/* This is ugly, but only temporary. */
+#define netdev_for_each_mc_addr(ha, dev) \
+ for ((dev)->tmp_mc_iter = (dev)->mc_list, \
+ netdev_tmp_mc_addr_cpy(dev), \
+ ha = &dev->tmp_mc_ha; \
+ (dev)->tmp_mc_iter; \
+ (dev)->tmp_mc_iter = (dev)->tmp_mc_iter->next, \
+ netdev_tmp_mc_addr_cpy(dev))
+
struct hh_cache {
struct hh_cache *hh_next; /* Next entry */
atomic_t hh_refcnt; /* number of users */
@@ -820,6 +832,8 @@ struct net_device {
mac addresses */
int uc_promisc;
spinlock_t addr_list_lock;
+ struct netdev_hw_addr tmp_mc_ha;
+ struct dev_addr_list *tmp_mc_iter;
struct dev_addr_list *mc_list; /* Multicast mac addresses */
int mc_count; /* Number of installed mcasts */
unsigned int promiscuity;
@@ -953,6 +967,15 @@ struct net_device {
#define NETDEV_ALIGN 32
+/* Used and to be used by netdev_for_each_mc_addr. This will disappear. */
+static inline void netdev_tmp_mc_addr_cpy(struct net_device *dev)
+{
+ if (dev->tmp_mc_iter)
+ memcpy(dev->tmp_mc_ha.addr,
+ dev->tmp_mc_iter->da_addr,
+ MAX_ADDR_LEN);
+}
+
static inline
struct netdev_queue *netdev_get_tx_queue(const struct net_device *dev,
unsigned int index)
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next-2.6] net: use helpers to access mc list
2010-01-26 14:57 [PATCH net-next-2.6] net: use helpers to access mc list Jiri Pirko
@ 2010-02-02 15:25 ` David Miller
2010-02-02 16:03 ` Jiri Pirko
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2010-02-02 15:25 UTC (permalink / raw)
To: jpirko; +Cc: netdev
From: Jiri Pirko <jpirko@redhat.com>
Date: Tue, 26 Jan 2010 15:57:39 +0100
> Temporary macro "netdev_for_each_mc_addr" works in the ugly way, I'm aware, but
> it will be replaced. It uses iterator stored in "struct net_device". In every
> iteration, it copies addr from the list to "struct netdev_hw_addr" instance
> (also stored in "struct net_device"). Driver reads address stored in this
> structure. All is protected by addr_list_lock held by a caller.
This kind of ugly hack is rarely necessary, so I'm not applying
this, sorry.
In the macros, use the iterator type the driver instances already use,
which is the mcaddr entry pointer.
Then by using list_for_each_entry() things should "just work".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next-2.6] net: use helpers to access mc list
2010-02-02 15:25 ` David Miller
@ 2010-02-02 16:03 ` Jiri Pirko
2010-02-02 16:06 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2010-02-02 16:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Tue, Feb 02, 2010 at 04:25:33PM CET, davem@davemloft.net wrote:
>From: Jiri Pirko <jpirko@redhat.com>
>Date: Tue, 26 Jan 2010 15:57:39 +0100
>
>> Temporary macro "netdev_for_each_mc_addr" works in the ugly way, I'm aware, but
>> it will be replaced. It uses iterator stored in "struct net_device". In every
>> iteration, it copies addr from the list to "struct netdev_hw_addr" instance
>> (also stored in "struct net_device"). Driver reads address stored in this
>> structure. All is protected by addr_list_lock held by a caller.
>
>This kind of ugly hack is rarely necessary, so I'm not applying
>this, sorry.
Well, I admit it's ugly but it's only temporary. It would go away once all
drivers would be converted to use it (a matter of weeks/month tops I hope).
>
>In the macros, use the iterator type the driver instances already use,
>which is the mcaddr entry pointer.
You mean "struct dev_mc_list"? But that would solve nothing. If I would still
use current structure in drivers, then still the migration to struct_hw_addr
would be all-at-once for all drivers :( This patch was exacly made to avoid
this.
>
>Then by using list_for_each_entry() things should "just work".
Maybe I do not uderstand you correctly. Would you please explain?
Thanks a lot.
Jirka
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next-2.6] net: use helpers to access mc list
2010-02-02 16:03 ` Jiri Pirko
@ 2010-02-02 16:06 ` David Miller
2010-02-02 16:31 ` Jiri Pirko
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2010-02-02 16:06 UTC (permalink / raw)
To: jpirko; +Cc: netdev
From: Jiri Pirko <jpirko@redhat.com>
Date: Tue, 2 Feb 2010 17:03:48 +0100
> You mean "struct dev_mc_list"? But that would solve nothing. If I would still
> use current structure in drivers, then still the migration to struct_hw_addr
> would be all-at-once for all drivers :( This patch was exacly made to avoid
> this.
I think changing the iterator type will have to be done
wholesale in one changeset, there is no reasonable way
to avoid it.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next-2.6] net: use helpers to access mc list
2010-02-02 16:06 ` David Miller
@ 2010-02-02 16:31 ` Jiri Pirko
2010-02-02 17:03 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2010-02-02 16:31 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Tue, Feb 02, 2010 at 05:06:01PM CET, davem@davemloft.net wrote:
>From: Jiri Pirko <jpirko@redhat.com>
>Date: Tue, 2 Feb 2010 17:03:48 +0100
>
>> You mean "struct dev_mc_list"? But that would solve nothing. If I would still
>> use current structure in drivers, then still the migration to struct_hw_addr
>> would be all-at-once for all drivers :( This patch was exacly made to avoid
>> this.
>
>I think changing the iterator type will have to be done
>wholesale in one changeset, there is no reasonable way
>to avoid it.
So what you are proposing is to change drivers' code to use macro to iterate
through lists as step n1 and then change the iterator type as step n2 right?
If we would use my patch, we would have this done in single step. But I understand
that a kind of ugliness patch introduces is simply not acceptable not even for a
short time period, right?
Thanks
Jirka
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next-2.6] net: use helpers to access mc list
2010-02-02 16:31 ` Jiri Pirko
@ 2010-02-02 17:03 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-02-02 17:03 UTC (permalink / raw)
To: jpirko; +Cc: netdev
From: Jiri Pirko <jpirko@redhat.com>
Date: Tue, 2 Feb 2010 17:31:08 +0100
> Tue, Feb 02, 2010 at 05:06:01PM CET, davem@davemloft.net wrote:
>>From: Jiri Pirko <jpirko@redhat.com>
>>Date: Tue, 2 Feb 2010 17:03:48 +0100
>>
>>> You mean "struct dev_mc_list"? But that would solve nothing. If I would still
>>> use current structure in drivers, then still the migration to struct_hw_addr
>>> would be all-at-once for all drivers :( This patch was exacly made to avoid
>>> this.
>>
>>I think changing the iterator type will have to be done
>>wholesale in one changeset, there is no reasonable way
>>to avoid it.
>
> So what you are proposing is to change drivers' code to use macro to iterate
> through lists as step n1 and then change the iterator type as step n2 right?
>
> If we would use my patch, we would have this done in single step. But I understand
> that a kind of ugliness patch introduces is simply not acceptable not even for a
> short time period, right?
Right.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-02-02 17:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-26 14:57 [PATCH net-next-2.6] net: use helpers to access mc list Jiri Pirko
2010-02-02 15:25 ` David Miller
2010-02-02 16:03 ` Jiri Pirko
2010-02-02 16:06 ` David Miller
2010-02-02 16:31 ` Jiri Pirko
2010-02-02 17:03 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).