* Re: [PATCH net-next] netpoll: provide an IP ident in UDP frames
From: David Miller @ 2012-08-30 16:32 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1345808846.29722.133.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 24 Aug 2012 13:47:26 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> Let's fill IP header ident field with a meaningful value,
> it might help some setups.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH 1/1] l2tp: avoid to use synchronize_rcu in tunnel free function
From: David Miller @ 2012-08-30 16:31 UTC (permalink / raw)
To: xeb; +Cc: netdev, kleptog, jchapman
In-Reply-To: <1430615.YYms4uRCh4@dima>
From: Kozlov Dmitry <xeb@mail.ru>
Date: Fri, 24 Aug 2012 15:07:38 +0400
> Avoid to use synchronize_rcu in l2tp_tunnel_free because context may be
> atomic.
>
> Signed-off-by: Dmitry Kozlov <xeb@mail.ru>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] netvm: check for page == NULL when propogating the skb->pfmemalloc flag
From: David Miller @ 2012-08-30 16:24 UTC (permalink / raw)
To: konrad.wilk
Cc: Ian.Campbell, mgorman, linux-mm, linux-kernel, netdev, xen-devel,
konrad, akpm
In-Reply-To: <20120823141740.GA30305@phenom.dumpdata.com>
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Thu, 23 Aug 2012 10:17:40 -0400
> On Wed, Aug 22, 2012 at 11:26:47AM +0100, Ian Campbell wrote:
>> On Wed, 2012-08-08 at 23:50 +0100, David Miller wrote:
>> > Just use something like a call to __pskb_pull_tail(skb, len) and all
>> > that other crap around that area can simply be deleted.
>>
>> I think you mean something like this, which works for me, although I've
>> only lightly tested it.
>>
>
> I've tested it heavily and works great.
>
> Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> and I took a look at it too and:
>
> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Applied, thanks everyone.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH net-next v2] net: dev: fix the incorrect hold of net namespace's lo device
From: David Miller @ 2012-08-30 16:22 UTC (permalink / raw)
To: gaofeng; +Cc: netdev, eric.dumazet, ebiederm
In-Reply-To: <1345772215-30379-1-git-send-email-gaofeng@cn.fujitsu.com>
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Fri, 24 Aug 2012 09:36:55 +0800
> When moving a net device from one net namespace to another
> net namespace,dev_change_net_namespace calls NETDEV_DOWN
> event,so the original net namespace's dst entries which
> beloned to this net device will be put into dst_garbage
> list.
>
> then dev_change_net_namespace will set this net device's
> net to the new net namespace.
>
> If we unregister this net device's driver, this will trigger
> the NETDEV_UNREGISTER_FINAL event, dst_ifdown will be called,
> and get this net device's dst entries from dst_garbage list,
> put these entries' dev to the new net namespace's lo device.
>
> It's not what we want,actually we need these dst entries hold
> the original net namespace's lo device,this incorrect device
> holding will trigger emg message like below.
> unregister_netdevice: waiting for lo to become free. Usage count = 1
>
> so we should call NETDEV_UNREGISTER_FINAL event in
> dev_change_net_namespace too,in order to make sure dst entries
> already in the dst_garbage list, we need rcu_barrier before we
> call NETDEV_UNREGISTER_FINAL event.
>
> With help form Eric Dumazet.
>
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH RFC 1/2] tcp: add generic netlink support for tcp_metrics
From: David Miller @ 2012-08-30 16:20 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <1345734105-28328-2-git-send-email-ja@ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Thu, 23 Aug 2012 18:01:44 +0300
> Add support for genl "tcp_metrics". No locking
> is changed, only that now we can unlink and delete
> entries after grace period. We implement get/del for
> single entry and dump to support show/flush filtering
> in user space.
>
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
I like it.
Please address Eric Dumazet's feedback and resubmit, thanks!
^ permalink raw reply
* Re: [PATCH] rtnl_wilddump_request: fix alignment issue for embedded platforms
From: Lutz Jaenicke @ 2012-08-30 16:09 UTC (permalink / raw)
To: David Laight; +Cc: netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B6FC9@saturn3.aculab.com>
On Thu, Aug 30, 2012 at 04:51:49PM +0100, David Laight wrote:
> > + /* attribute has to be NLMSG aligned */
> > + struct rtattr ext_req
> __attribute__((aligned(NLMSG_ALIGNTO)));
>
> Would it be better to apply the attribute to the definition
> of 'struct rtattr' itself ?
'struct rtattr' is defined in a kernel header and I am not really sure
what kind of side effects it would have.
The problem I intend to solve with my page is rather caused by the use case:
The request is sent as a single structure, the processing is however
performed step by step: first the nlmsg is parsed, then the pointer
is moved on to the parsing of the content. This pointer is hoever not
oriented at the structure element but by using size information which
is then adjust by NLMSG_ALIGN() etc. Consequently one should actually
use the same method in sending (generate the header first, then addattr_()
which enforces protocol conform alignment). This is the way all other
instances in iproute2 generate their messages btw.
I hence consider my proposed patch to be the least intrusive solution.
Best regards,
Lutz
--
Dr.-Ing. Lutz Jänicke
CTO
Innominate Security Technologies AG /protecting industrial networks/
tel: +49.30.921028-200
fax: +49.30.921028-020
Rudower Chaussee 13
D-12489 Berlin, Germany
www.innominate.com
Register Court: AG Charlottenburg, HR B 81603
Management Board: Dirk Seewald
Chairman of the Supervisory Board: Christoph Leifer
^ permalink raw reply
* Re: [RFC PATCH bridge 2/5] bridge: Add vlan to unicast fdb entries
From: Vlad Yasevich @ 2012-08-30 16:07 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev
In-Reply-To: <20120830154521.GB21739@redhat.com>
On 08/30/2012 11:45 AM, Michael S. Tsirkin wrote:
> On Thu, Aug 30, 2012 at 10:48:57AM -0400, Vlad Yasevich wrote:
>> On 08/30/2012 10:33 AM, Michael S. Tsirkin wrote:
>>> On Thu, Aug 23, 2012 at 03:29:52PM -0400, Vlad Yasevich wrote:
>>>> This patch adds vlan to unicast fdb entries that are created for
>>>> learned addresses (not the manually configured ones). It adds
>>>> vlan id into the hash mix and uses vlan as an addditional parameter
>>>> for an entry match.
>>>>
>>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>>
>>> I wonder whether this conflicts with Eric's
>>> 05423b241311c9380b7280179295bac7794281b6
>>> which allowed null vid.
>>>
>>> Specifically this calls vlan_tx_tag_get without
>>> checking vlan_tx_tag_present. Why is this OK?
>>
>> We ignore the PRIO bits and just look at vlan deeper in the functions.
>> So if the tci is set, even to NULL/0 vid, we are still OK as we'll use
>> just the VID bits.
>
> I do not fully understand why does kernel allow NULL vid,
> thus my question.
> It seems that your patches treat packets with NULL vid
> same as untagged packets - is this OK?
>
I think they are. Technically they do not have a tag, just a specific
priority that a switch would interpret. So, they should be forwarded
the same as packets without a vlan header.
-vlad
>> Now, the other issue with this patch is that it doesn't work when
>> VLANs aren't supported by the kernel and skb->vlan_tci is never set.
>> I am working to address that.
>>
>> -vlad
>
> I guess if we special-case !vlan_tx_tag_present this will
> also treat untagged packets different from tagged ones?
>
>>>
>>>> ---
>>>> include/linux/if_bridge.h | 1 +
>>>> net/bridge/br_device.c | 2 +-
>>>> net/bridge/br_fdb.c | 71 ++++++++++++++++++++++++++------------------
>>>> net/bridge/br_input.c | 14 ++++++---
>>>> net/bridge/br_private.h | 7 +++-
>>>> 5 files changed, 58 insertions(+), 37 deletions(-)
>>>>
>>>> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
>>>> index dd3f201..288ff10 100644
>>>> --- a/include/linux/if_bridge.h
>>>> +++ b/include/linux/if_bridge.h
>>>> @@ -95,6 +95,7 @@ struct __fdb_entry {
>>>> __u8 port_hi;
>>>> __u8 pad0;
>>>> __u16 unused;
>>>> +#define fdb_vid unused
>>>> };
>>>>
>>>> #ifdef __KERNEL__
>>>> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
>>>> index 3334845..e321b9d 100644
>>>> --- a/net/bridge/br_device.c
>>>> +++ b/net/bridge/br_device.c
>>>> @@ -66,7 +66,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>>>> br_multicast_deliver(mdst, skb);
>>>> else
>>>> br_flood_deliver(br, skb);
>>>> - } else if ((dst = __br_fdb_get(br, dest)) != NULL)
>>>> + } else if ((dst = __br_fdb_get(br, dest, vlan_tx_tag_get(skb))) != NULL)
>>>> br_deliver(dst->dst, skb);
>>>> else
>>>> br_flood_deliver(br, skb);
>>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>>> index d21f323..478ca1d 100644
>>>> --- a/net/bridge/br_fdb.c
>>>> +++ b/net/bridge/br_fdb.c
>>>> @@ -23,6 +23,7 @@
>>>> #include <linux/slab.h>
>>>> #include <linux/atomic.h>
>>>> #include <asm/unaligned.h>
>>>> +#include <linux/if_vlan.h>
>>>> #include "br_private.h"
>>>>
>>>> static struct kmem_cache *br_fdb_cache __read_mostly;
>>>> @@ -67,11 +68,12 @@ static inline int has_expired(const struct net_bridge *br,
>>>> time_before_eq(fdb->updated + hold_time(br), jiffies);
>>>> }
>>>>
>>>> -static inline int br_mac_hash(const unsigned char *mac)
>>>> +static inline int br_mac_hash(const unsigned char *mac, __u16 vlan_tci)
>>>> {
>>>> - /* use 1 byte of OUI cnd 3 bytes of NIC */
>>>> + /* use 1 byte of OUI and 3 bytes of NIC */
>>>> u32 key = get_unaligned((u32 *)(mac + 2));
>>>> - return jhash_1word(key, fdb_salt) & (BR_HASH_SIZE - 1);
>>>> + return jhash_2words(key, (vlan_tci & VLAN_VID_MASK),
>>>> + fdb_salt) & (BR_HASH_SIZE - 1);
>>>> }
>>>>
>>>> static void fdb_rcu_free(struct rcu_head *head)
>>>> @@ -132,7 +134,7 @@ void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr)
>>>> struct net_bridge_fdb_entry *f;
>>>>
>>>> /* If old entry was unassociated with any port, then delete it. */
>>>> - f = __br_fdb_get(br, br->dev->dev_addr);
>>>> + f = __br_fdb_get(br, br->dev->dev_addr, 0);
>>>> if (f && f->is_local && !f->dst)
>>>> fdb_delete(br, f);
>>>>
>>>> @@ -231,13 +233,16 @@ void br_fdb_delete_by_port(struct net_bridge *br,
>>>>
>>>> /* No locking or refcounting, assumes caller has rcu_read_lock */
>>>> struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
>>>> - const unsigned char *addr)
>>>> + const unsigned char *addr,
>>>> + __u16 vlan_tci)
>>>> {
>>>> struct hlist_node *h;
>>>> struct net_bridge_fdb_entry *fdb;
>>>>
>>>> - hlist_for_each_entry_rcu(fdb, h, &br->hash[br_mac_hash(addr)], hlist) {
>>>> - if (ether_addr_equal(fdb->addr.addr, addr)) {
>>>> + hlist_for_each_entry_rcu(fdb, h,
>>>> + &br->hash[br_mac_hash(addr, vlan_tci)], hlist) {
>>>> + if (ether_addr_equal(fdb->addr.addr, addr) &&
>>>> + fdb->vlan_id == (vlan_tci & VLAN_VID_MASK) ) {
>>>> if (unlikely(has_expired(br, fdb)))
>>>> break;
>>>> return fdb;
>>>> @@ -261,7 +266,7 @@ int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
>>>> if (!port)
>>>> ret = 0;
>>>> else {
>>>> - fdb = __br_fdb_get(port->br, addr);
>>>> + fdb = __br_fdb_get(port->br, addr, 0);
>>>> ret = fdb && fdb->dst && fdb->dst->dev != dev &&
>>>> fdb->dst->state == BR_STATE_FORWARDING;
>>>> }
>>>> @@ -313,6 +318,7 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
>>>> fe->is_local = f->is_local;
>>>> if (!f->is_static)
>>>> fe->ageing_timer_value = jiffies_to_clock_t(jiffies - f->updated);
>>>> + fe->fdb_vid = f->vlan_id;
>>>> ++fe;
>>>> ++num;
>>>> }
>>>> @@ -325,26 +331,30 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
>>>> }
>>>>
>>>> static struct net_bridge_fdb_entry *fdb_find(struct hlist_head *head,
>>>> - const unsigned char *addr)
>>>> + const unsigned char *addr,
>>>> + __u16 vlan_tci)
>>>> {
>>>> struct hlist_node *h;
>>>> struct net_bridge_fdb_entry *fdb;
>>>>
>>>> hlist_for_each_entry(fdb, h, head, hlist) {
>>>> - if (ether_addr_equal(fdb->addr.addr, addr))
>>>> + if (ether_addr_equal(fdb->addr.addr, addr) &&
>>>> + fdb->vlan_id == (vlan_tci & VLAN_VID_MASK))
>>>> return fdb;
>>>> }
>>>> return NULL;
>>>> }
>>>>
>>>> static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head,
>>>> - const unsigned char *addr)
>>>> + const unsigned char *addr,
>>>> + __u16 vlan_tci)
>>>> {
>>>> struct hlist_node *h;
>>>> struct net_bridge_fdb_entry *fdb;
>>>>
>>>> hlist_for_each_entry_rcu(fdb, h, head, hlist) {
>>>> - if (ether_addr_equal(fdb->addr.addr, addr))
>>>> + if (ether_addr_equal(fdb->addr.addr, addr) &&
>>>> + fdb->vlan_id == (vlan_tci & VLAN_VID_MASK))
>>>> return fdb;
>>>> }
>>>> return NULL;
>>>> @@ -352,7 +362,8 @@ static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head,
>>>>
>>>> static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
>>>> struct net_bridge_port *source,
>>>> - const unsigned char *addr)
>>>> + const unsigned char *addr,
>>>> + __u16 vlan_tci)
>>>> {
>>>> struct net_bridge_fdb_entry *fdb;
>>>>
>>>> @@ -360,6 +371,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
>>>> if (fdb) {
>>>> memcpy(fdb->addr.addr, addr, ETH_ALEN);
>>>> fdb->dst = source;
>>>> + fdb->vlan_id = (vlan_tci & VLAN_VID_MASK);
>>>> fdb->is_local = 0;
>>>> fdb->is_static = 0;
>>>> fdb->updated = fdb->used = jiffies;
>>>> @@ -371,13 +383,13 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
>>>> static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>>>> const unsigned char *addr)
>>>> {
>>>> - struct hlist_head *head = &br->hash[br_mac_hash(addr)];
>>>> + struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
>>>> struct net_bridge_fdb_entry *fdb;
>>>>
>>>> if (!is_valid_ether_addr(addr))
>>>> return -EINVAL;
>>>>
>>>> - fdb = fdb_find(head, addr);
>>>> + fdb = fdb_find(head, addr, 0);
>>>> if (fdb) {
>>>> /* it is okay to have multiple ports with same
>>>> * address, just use the first one.
>>>> @@ -390,7 +402,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>>>> fdb_delete(br, fdb);
>>>> }
>>>>
>>>> - fdb = fdb_create(head, source, addr);
>>>> + fdb = fdb_create(head, source, addr, 0);
>>>> if (!fdb)
>>>> return -ENOMEM;
>>>>
>>>> @@ -412,9 +424,9 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>>>> }
>>>>
>>>> void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>>> - const unsigned char *addr)
>>>> + const unsigned char *addr, __u16 vlan_tci)
>>>> {
>>>> - struct hlist_head *head = &br->hash[br_mac_hash(addr)];
>>>> + struct hlist_head *head = &br->hash[br_mac_hash(addr, vlan_tci)];
>>>> struct net_bridge_fdb_entry *fdb;
>>>>
>>>> /* some users want to always flood. */
>>>> @@ -426,7 +438,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>>> source->state == BR_STATE_FORWARDING))
>>>> return;
>>>>
>>>> - fdb = fdb_find_rcu(head, addr);
>>>> + fdb = fdb_find_rcu(head, addr, vlan_tci);
>>>> if (likely(fdb)) {
>>>> /* attempt to update an entry for a local interface */
>>>> if (unlikely(fdb->is_local)) {
>>>> @@ -441,8 +453,8 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>>> }
>>>> } else {
>>>> spin_lock(&br->hash_lock);
>>>> - if (likely(!fdb_find(head, addr))) {
>>>> - fdb = fdb_create(head, source, addr);
>>>> + if (likely(!fdb_find(head, addr, vlan_tci))) {
>>>> + fdb = fdb_create(head, source, addr, vlan_tci);
>>>> if (fdb)
>>>> fdb_notify(br, fdb, RTM_NEWNEIGH);
>>>> }
>>>> @@ -571,18 +583,18 @@ out:
>>>>
>>>> /* Update (create or replace) forwarding database entry */
>>>> static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr,
>>>> - __u16 state, __u16 flags)
>>>> + __u16 state, __u16 flags, __u16 vlan_tci)
>>>> {
>>>> struct net_bridge *br = source->br;
>>>> - struct hlist_head *head = &br->hash[br_mac_hash(addr)];
>>>> + struct hlist_head *head = &br->hash[br_mac_hash(addr, vlan_tci)];
>>>> struct net_bridge_fdb_entry *fdb;
>>>>
>>>> - fdb = fdb_find(head, addr);
>>>> + fdb = fdb_find(head, addr, vlan_tci);
>>>> if (fdb == NULL) {
>>>> if (!(flags & NLM_F_CREATE))
>>>> return -ENOENT;
>>>>
>>>> - fdb = fdb_create(head, source, addr);
>>>> + fdb = fdb_create(head, source, addr, vlan_tci);
>>>> if (!fdb)
>>>> return -ENOMEM;
>>>> fdb_notify(br, fdb, RTM_NEWNEIGH);
>>>> @@ -628,11 +640,12 @@ int br_fdb_add(struct ndmsg *ndm, struct net_device *dev,
>>>>
>>>> if (ndm->ndm_flags & NTF_USE) {
>>>> rcu_read_lock();
>>>> - br_fdb_update(p->br, p, addr);
>>>> + br_fdb_update(p->br, p, addr, 0);
>>>> rcu_read_unlock();
>>>> } else {
>>>> spin_lock_bh(&p->br->hash_lock);
>>>> - err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags);
>>>> + err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags,
>>>> + 0);
>>>> spin_unlock_bh(&p->br->hash_lock);
>>>> }
>>>>
>>>> @@ -642,10 +655,10 @@ int br_fdb_add(struct ndmsg *ndm, struct net_device *dev,
>>>> static int fdb_delete_by_addr(struct net_bridge_port *p, u8 *addr)
>>>> {
>>>> struct net_bridge *br = p->br;
>>>> - struct hlist_head *head = &br->hash[br_mac_hash(addr)];
>>>> + struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
>>>> struct net_bridge_fdb_entry *fdb;
>>>>
>>>> - fdb = fdb_find(head, addr);
>>>> + fdb = fdb_find(head, addr, 0);
>>>> if (!fdb)
>>>> return -ENOENT;
>>>>
>>>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>>>> index b7ca3b3..a4579ee 100644
>>>> --- a/net/bridge/br_input.c
>>>> +++ b/net/bridge/br_input.c
>>>> @@ -62,12 +62,15 @@ int br_handle_frame_finish(struct sk_buff *skb)
>>>> * only traffic matching the VLAN filter.
>>>> */
>>>> vlan_map = rcu_dereference(p->vlan_map);
>>>> - if (vlan_map && !test_bit((skb->vlan_tci & VLAN_VID_MASK), vlan_map))
>>>> - goto drop;
>>>> + if (vlan_map && vlan_tx_tag_present(skb)) {
>>>> + unsigned short vid = vlan_tx_tag_get(skb) & VLAN_VID_MASK;
>>>> + if (!test_bit(vid+1, vlan_map))
>>>> + goto drop;
>>>> + }
>>>>
>>>> /* insert into forwarding database after filtering to avoid spoofing */
>>>> br = p->br;
>>>> - br_fdb_update(br, p, eth_hdr(skb)->h_source);
>>>> + br_fdb_update(br, p, eth_hdr(skb)->h_source, vlan_tx_tag_get(skb));
>>>>
>>>> if (!is_broadcast_ether_addr(dest) && is_multicast_ether_addr(dest) &&
>>>> br_multicast_rcv(br, p, skb))
>>>> @@ -102,7 +105,8 @@ int br_handle_frame_finish(struct sk_buff *skb)
>>>> skb2 = skb;
>>>>
>>>> br->dev->stats.multicast++;
>>>> - } else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
>>>> + } else if ((dst = __br_fdb_get(br, dest, vlan_tx_tag_get(skb))) &&
>>>> + dst->is_local) {
>>>> skb2 = skb;
>>>> /* Do not forward the packet since it's local. */
>>>> skb = NULL;
>>>> @@ -131,7 +135,7 @@ static int br_handle_local_finish(struct sk_buff *skb)
>>>> {
>>>> struct net_bridge_port *p = br_port_get_rcu(skb->dev);
>>>>
>>>> - br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
>>>> + br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vlan_tx_tag_get(skb));
>>>> return 0; /* process further */
>>>> }
>>>>
>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>> index 8da90e8..921b927 100644
>>>> --- a/net/bridge/br_private.h
>>>> +++ b/net/bridge/br_private.h
>>>> @@ -72,6 +72,7 @@ struct net_bridge_fdb_entry
>>>> unsigned long updated;
>>>> unsigned long used;
>>>> mac_addr addr;
>>>> + __u16 vlan_id;
>>>> unsigned char is_local;
>>>> unsigned char is_static;
>>>> };
>>>> @@ -352,7 +353,8 @@ extern void br_fdb_cleanup(unsigned long arg);
>>>> extern void br_fdb_delete_by_port(struct net_bridge *br,
>>>> const struct net_bridge_port *p, int do_all);
>>>> extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
>>>> - const unsigned char *addr);
>>>> + const unsigned char *addr,
>>>> + __u16 vlan_tci);
>>>> extern int br_fdb_test_addr(struct net_device *dev, unsigned char *addr);
>>>> extern int br_fdb_fillbuf(struct net_bridge *br, void *buf,
>>>> unsigned long count, unsigned long off);
>>>> @@ -361,7 +363,8 @@ extern int br_fdb_insert(struct net_bridge *br,
>>>> const unsigned char *addr);
>>>> extern void br_fdb_update(struct net_bridge *br,
>>>> struct net_bridge_port *source,
>>>> - const unsigned char *addr);
>>>> + const unsigned char *addr,
>>>> + __u16 vlan_tci);
>>>>
>>>> extern int br_fdb_delete(struct ndmsg *ndm,
>>>> struct net_device *dev,
>>>> --
>>>> 1.7.7.6
>>>>
>>>> --
>>>> 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
>>> --
>>> 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
>>>
^ permalink raw reply
* RE: [PATCH] rtnl_wilddump_request: fix alignment issue for embedded platforms
From: David Laight @ 2012-08-30 15:51 UTC (permalink / raw)
To: Lutz Jaenicke, netdev
In-Reply-To: <1346338894-12600-1-git-send-email-ljaenicke@innominate.com>
> + /* attribute has to be NLMSG aligned */
> + struct rtattr ext_req
__attribute__((aligned(NLMSG_ALIGNTO)));
Would it be better to apply the attribute to the definition
of 'struct rtattr' itself ?
David
^ permalink raw reply
* Re: [RFC PATCH bridge 5/5] bridge: Add sysfs interface to display VLANS
From: Michael S. Tsirkin @ 2012-08-30 15:58 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <503F8C3F.3000606@redhat.com>
On Thu, Aug 30, 2012 at 11:52:31AM -0400, Vlad Yasevich wrote:
> On 08/30/2012 11:47 AM, Michael S. Tsirkin wrote:
> >On Thu, Aug 30, 2012 at 11:07:24AM -0400, Vlad Yasevich wrote:
> >>On 08/30/2012 11:03 AM, Michael S. Tsirkin wrote:
> >>>On Thu, Aug 30, 2012 at 10:51:35AM -0400, Vlad Yasevich wrote:
> >>>>On 08/30/2012 10:44 AM, Michael S. Tsirkin wrote:
> >>>>>On Thu, Aug 30, 2012 at 10:36:34AM -0400, Vlad Yasevich wrote:
> >>>>>>On 08/30/2012 10:26 AM, Michael S. Tsirkin wrote:
> >>>>>>>On Thu, Aug 30, 2012 at 10:05:16AM -0400, Vlad Yasevich wrote:
> >>>>>>>>On 08/30/2012 08:27 AM, Michael S. Tsirkin wrote:
> >>>>>>>>>On Thu, Aug 23, 2012 at 03:29:55PM -0400, Vlad Yasevich wrote:
> >>>>>>>>>>Add a binary sysfs file that will dump out vlans currently configured on the
> >>>>>>>>>>port.
> >>>>>>
> >>>>>>I initially though of creating a sysfs object per vlan. That would
> >>>>>>have made it easy to see which vlans are configured without any
> >>>>>>tools.
> >>>>>>But that could result in a lot of objects being created, so I abandoned it.
> >>>>>>
> >>>>>>I did think about a text interface, but due to a page of output
> >>>>>>limitation, I didn't go that route. The reason is that if someone
> >>>>>>cats the file, they may not see all the vlans configured. So I
> >>>>>>decided on the binary interface, since a binary interface with a
> >>>>>>tool to read it could avoid the single page limitation.
> >>>>>>
> >>>>>>-vlad
> >>>>>
> >>>>>Maybe it's not needed in sysfs then - expose it to
> >>>>>brctl or whatever.
> >>>>>
> >>>>
> >>>>brctl uses sysfs for almost everything any more :)
> >>>>
> >>>>-vlad
> >>>
> >>>How about a long string of 0 and 1's?
> >>>And a separate one for untagged vlans.
> >>
> >>that would work too. You really don't like the binary interface, huh?
> >>
> >>-vlad
> >
> >Not in sysfs.
> >Another possibility: you are adding netlink command
> >to add/del a specific vid, add one to test a specific vid.
> >This means you will need to scan 4K possibilities if you
> >want to list them all, but in real life it's
> >probably just debugging tools that need to do this.
>
> Not really. A show command to see what has been configured is
> necessary not of only for debugging. It is no different then the
> show command to list fdb entries on the bridge. There needs to be
> an easy way to see which vlans are configured on which ports.
>
> -vlad
Yes. And
for (vid = 0; vid < 2096; ++vid)
ioctl(port,vid)
is easier than parsing a binary blob.
Just less efficient but that is I think OK
for the show command.
> >
> >>>
> >>>>>>>
> >>>>>>>
> >>>>>>>>>
> >>>>>>>>>>---
> >>>>>>>>>> include/linux/if_bridge.h | 1 +
> >>>>>>>>>> net/bridge/br_if.c | 34 ++++++++++++++++++++++++++++++++++
> >>>>>>>>>> net/bridge/br_private.h | 2 ++
> >>>>>>>>>> net/bridge/br_sysfs_if.c | 28 ++++++++++++++++++++++++++++
> >>>>>>>>>> 4 files changed, 65 insertions(+), 0 deletions(-)
> >>>>>>>>>>
> >>>>>>>>>>diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> >>>>>>>>>>index ab750dd..d0f869b 100644
> >>>>>>>>>>--- a/include/linux/if_bridge.h
> >>>>>>>>>>+++ b/include/linux/if_bridge.h
> >>>>>>>>>>@@ -20,6 +20,7 @@
> >>>>>>>>>> #define SYSFS_BRIDGE_PORT_SUBDIR "brif"
> >>>>>>>>>> #define SYSFS_BRIDGE_PORT_ATTR "brport"
> >>>>>>>>>> #define SYSFS_BRIDGE_PORT_LINK "bridge"
> >>>>>>>>>>+#define SYSFS_BRIDGE_PORT_VLANS "vlans"
> >>>>>>>>>>
> >>>>>>>>>> #define BRCTL_VERSION 1
> >>>>>>>>>>
> >>>>>>>>>>diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> >>>>>>>>>>index 90c1038..3963748 100644
> >>>>>>>>>>--- a/net/bridge/br_if.c
> >>>>>>>>>>+++ b/net/bridge/br_if.c
> >>>>>>>>>>@@ -510,6 +510,40 @@ int br_del_port_vlan(struct net_bridge_port *p, unsigned long vlan)
> >>>>>>>>>> return 0;
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>>+size_t br_port_fill_vlans(struct net_bridge_port *p, char* buf,
> >>>>>>>>>>+ unsigned long max, unsigned long skip)
> >>>>>>>>>>+{
> >>>>>>>>>>+ unsigned long *map;
> >>>>>>>>>>+ unsigned short *vid = (unsigned short *)buf;
> >>>>>>>>>>+ unsigned short i;
> >>>>>>>>>>+ int num = 0;
> >>>>>>>>>>+
> >>>>>>>>>>+ if (skip > (VLAN_N_VID+1))
> >>>>>>>>>>+ return -EINVAL;
> >>>>>>>>>>+
> >>>>>>>>>>+ memset(buf, 0, max * sizeof(unsigned short));
> >>>>>>>>>
> >>>>>>>>>Isn't max is in bytes? why is this safe?
> >>>>>>>>>
> >>>>>>>>>>+
> >>>>>>>>>>+ rcu_read_lock();
> >>>>>>>>>>+ map = rcu_dereference(p->vlan_map);
> >>>>>>>>>>+ if (!map)
> >>>>>>>>>>+ goto out;
> >>>>>>>>>>+
> >>>>>>>>>>+ for (i = skip + 1; i < VLAN_N_VID + 1; i++) {
> >>>>>>>>>
> >>>>>>>>>Isn't skip in bytes too? Why do you compare it to i which is
> >>>>>>>>>in dwords?
> >>>>>>>>>
> >>>>>>>>>>+ if (test_bit(i, map)) {
> >>>>>>>>>>+ if (num > max)
> >>>>>>>>>>+ goto out;
> >>>>>>>>>>+
> >>>>>>>>>>+ *vid = i-1;
> >>>>>>>>>>+ vid++;
> >>>>>>>>>>+ num++;
> >>>>>>>>>>+ }
> >>>>>>>>>>+ }
> >>>>>>>>>>+out:
> >>>>>>>>>>+ rcu_read_unlock();
> >>>>>>>>>>+
> >>>>>>>>>>+ return num*sizeof(unsigned short);
> >>>>>>>>>>+}
> >>>>>>>>>>+
> >>>>>>>>>> void __net_exit br_net_exit(struct net *net)
> >>>>>>>>>> {
> >>>>>>>>>> struct net_device *dev;
> >>>>>>>>>>diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> >>>>>>>>>>index 5639c1c..cf95cd7 100644
> >>>>>>>>>>--- a/net/bridge/br_private.h
> >>>>>>>>>>+++ b/net/bridge/br_private.h
> >>>>>>>>>>@@ -404,6 +404,8 @@ extern netdev_features_t br_features_recompute(struct net_bridge *br,
> >>>>>>>>>> netdev_features_t features);
> >>>>>>>>>> extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
> >>>>>>>>>> extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
> >>>>>>>>>>+extern size_t br_port_fill_vlans(struct net_bridge_port *p, char *buf,
> >>>>>>>>>>+ unsigned long max, unsigned long skip);
> >>>>>>>>>>
> >>>>>>>>>> /* br_input.c */
> >>>>>>>>>> extern int br_handle_frame_finish(struct sk_buff *skb);
> >>>>>>>>>>diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
> >>>>>>>>>>index 13b36bd..a81e2ef 100644
> >>>>>>>>>>--- a/net/bridge/br_sysfs_if.c
> >>>>>>>>>>+++ b/net/bridge/br_sysfs_if.c
> >>>>>>>>>>@@ -234,6 +234,29 @@ const struct sysfs_ops brport_sysfs_ops = {
> >>>>>>>>>> };
> >>>>>>>>>>
> >>>>>>>>>> /*
> >>>>>>>>>>+ * Export the vlan table for a given port as a binary file.
> >>>>>>>>>>+ * The records are unsgined shorts.
> >>>>>>>>>>+ *
> >>>>>>>>>>+ * Returns the number of bytes read.
> >>>>>>>>>>+ */
> >>>>>>>>>>+static ssize_t brport_vlans_read(struct file *filp, struct kobject *kobj,
> >>>>>>>>>>+ struct bin_attribute *bin_attr,
> >>>>>>>>>>+ char *buf, loff_t off, size_t count)
> >>>>>>>>>>+{
> >>>>>>>>>>+ struct net_bridge_port *p = to_brport(kobj);
> >>>>>>>>>>+
> >>>>>>>>>>+ return br_port_fill_vlans(p, buf,
> >>>>>>>>>>+ count/sizeof(unsigned short),
> >>>>>>>>>>+ off/sizeof(unsigned short));
> >>>>>>>>>>+}
> >>>>>>>>>>+
> >>>>>>>>>>+static struct bin_attribute port_vlans = {
> >>>>>>>>>>+ .attr = { .name = SYSFS_BRIDGE_PORT_VLANS,
> >>>>>>>>>>+ .mode = S_IRUGO, },
> >>>>>>>>>>+ .read = brport_vlans_read,
> >>>>>>>>>>+};
> >>>>>>>>>>+
> >>>>>>>>>>+/*
> >>>>>>>>>> * Add sysfs entries to ethernet device added to a bridge.
> >>>>>>>>>> * Creates a brport subdirectory with bridge attributes.
> >>>>>>>>>> * Puts symlink in bridge's brif subdirectory
> >>>>>>>>>>@@ -255,6 +278,11 @@ int br_sysfs_addif(struct net_bridge_port *p)
> >>>>>>>>>> return err;
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>>+ err = sysfs_create_bin_file(&p->kobj, &port_vlans);
> >>>>>>>>>>+ if (err) {
> >>>>>>>>>>+ return err;
> >>>>>>>>>>+ }
> >>>>>>>>>>+
> >>>>>>>>>> strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
> >>>>>>>>>> return sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
> >>>>>>>>>> }
> >>>>>>>>>>--
> >>>>>>>>>>1.7.7.6
> >>>>>>>>>>
> >>>>>>>>>>--
> >>>>>>>>>>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
> >>>--
> >>>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
> >>>
^ permalink raw reply
* Re: [RFC PATCH bridge 5/5] bridge: Add sysfs interface to display VLANS
From: Vlad Yasevich @ 2012-08-30 15:52 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev
In-Reply-To: <20120830154711.GC21739@redhat.com>
On 08/30/2012 11:47 AM, Michael S. Tsirkin wrote:
> On Thu, Aug 30, 2012 at 11:07:24AM -0400, Vlad Yasevich wrote:
>> On 08/30/2012 11:03 AM, Michael S. Tsirkin wrote:
>>> On Thu, Aug 30, 2012 at 10:51:35AM -0400, Vlad Yasevich wrote:
>>>> On 08/30/2012 10:44 AM, Michael S. Tsirkin wrote:
>>>>> On Thu, Aug 30, 2012 at 10:36:34AM -0400, Vlad Yasevich wrote:
>>>>>> On 08/30/2012 10:26 AM, Michael S. Tsirkin wrote:
>>>>>>> On Thu, Aug 30, 2012 at 10:05:16AM -0400, Vlad Yasevich wrote:
>>>>>>>> On 08/30/2012 08:27 AM, Michael S. Tsirkin wrote:
>>>>>>>>> On Thu, Aug 23, 2012 at 03:29:55PM -0400, Vlad Yasevich wrote:
>>>>>>>>>> Add a binary sysfs file that will dump out vlans currently configured on the
>>>>>>>>>> port.
>>>>>>
>>>>>> I initially though of creating a sysfs object per vlan. That would
>>>>>> have made it easy to see which vlans are configured without any
>>>>>> tools.
>>>>>> But that could result in a lot of objects being created, so I abandoned it.
>>>>>>
>>>>>> I did think about a text interface, but due to a page of output
>>>>>> limitation, I didn't go that route. The reason is that if someone
>>>>>> cats the file, they may not see all the vlans configured. So I
>>>>>> decided on the binary interface, since a binary interface with a
>>>>>> tool to read it could avoid the single page limitation.
>>>>>>
>>>>>> -vlad
>>>>>
>>>>> Maybe it's not needed in sysfs then - expose it to
>>>>> brctl or whatever.
>>>>>
>>>>
>>>> brctl uses sysfs for almost everything any more :)
>>>>
>>>> -vlad
>>>
>>> How about a long string of 0 and 1's?
>>> And a separate one for untagged vlans.
>>
>> that would work too. You really don't like the binary interface, huh?
>>
>> -vlad
>
> Not in sysfs.
> Another possibility: you are adding netlink command
> to add/del a specific vid, add one to test a specific vid.
> This means you will need to scan 4K possibilities if you
> want to list them all, but in real life it's
> probably just debugging tools that need to do this.
Not really. A show command to see what has been configured is necessary
not of only for debugging. It is no different then the show command to
list fdb entries on the bridge. There needs to be an easy way to see
which vlans are configured on which ports.
-vlad
>
>>>
>>>>>>>
>>>>>>>
>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>> include/linux/if_bridge.h | 1 +
>>>>>>>>>> net/bridge/br_if.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>>>>>> net/bridge/br_private.h | 2 ++
>>>>>>>>>> net/bridge/br_sysfs_if.c | 28 ++++++++++++++++++++++++++++
>>>>>>>>>> 4 files changed, 65 insertions(+), 0 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
>>>>>>>>>> index ab750dd..d0f869b 100644
>>>>>>>>>> --- a/include/linux/if_bridge.h
>>>>>>>>>> +++ b/include/linux/if_bridge.h
>>>>>>>>>> @@ -20,6 +20,7 @@
>>>>>>>>>> #define SYSFS_BRIDGE_PORT_SUBDIR "brif"
>>>>>>>>>> #define SYSFS_BRIDGE_PORT_ATTR "brport"
>>>>>>>>>> #define SYSFS_BRIDGE_PORT_LINK "bridge"
>>>>>>>>>> +#define SYSFS_BRIDGE_PORT_VLANS "vlans"
>>>>>>>>>>
>>>>>>>>>> #define BRCTL_VERSION 1
>>>>>>>>>>
>>>>>>>>>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>>>>>>>>>> index 90c1038..3963748 100644
>>>>>>>>>> --- a/net/bridge/br_if.c
>>>>>>>>>> +++ b/net/bridge/br_if.c
>>>>>>>>>> @@ -510,6 +510,40 @@ int br_del_port_vlan(struct net_bridge_port *p, unsigned long vlan)
>>>>>>>>>> return 0;
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> +size_t br_port_fill_vlans(struct net_bridge_port *p, char* buf,
>>>>>>>>>> + unsigned long max, unsigned long skip)
>>>>>>>>>> +{
>>>>>>>>>> + unsigned long *map;
>>>>>>>>>> + unsigned short *vid = (unsigned short *)buf;
>>>>>>>>>> + unsigned short i;
>>>>>>>>>> + int num = 0;
>>>>>>>>>> +
>>>>>>>>>> + if (skip > (VLAN_N_VID+1))
>>>>>>>>>> + return -EINVAL;
>>>>>>>>>> +
>>>>>>>>>> + memset(buf, 0, max * sizeof(unsigned short));
>>>>>>>>>
>>>>>>>>> Isn't max is in bytes? why is this safe?
>>>>>>>>>
>>>>>>>>>> +
>>>>>>>>>> + rcu_read_lock();
>>>>>>>>>> + map = rcu_dereference(p->vlan_map);
>>>>>>>>>> + if (!map)
>>>>>>>>>> + goto out;
>>>>>>>>>> +
>>>>>>>>>> + for (i = skip + 1; i < VLAN_N_VID + 1; i++) {
>>>>>>>>>
>>>>>>>>> Isn't skip in bytes too? Why do you compare it to i which is
>>>>>>>>> in dwords?
>>>>>>>>>
>>>>>>>>>> + if (test_bit(i, map)) {
>>>>>>>>>> + if (num > max)
>>>>>>>>>> + goto out;
>>>>>>>>>> +
>>>>>>>>>> + *vid = i-1;
>>>>>>>>>> + vid++;
>>>>>>>>>> + num++;
>>>>>>>>>> + }
>>>>>>>>>> + }
>>>>>>>>>> +out:
>>>>>>>>>> + rcu_read_unlock();
>>>>>>>>>> +
>>>>>>>>>> + return num*sizeof(unsigned short);
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> void __net_exit br_net_exit(struct net *net)
>>>>>>>>>> {
>>>>>>>>>> struct net_device *dev;
>>>>>>>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>>>>>>>> index 5639c1c..cf95cd7 100644
>>>>>>>>>> --- a/net/bridge/br_private.h
>>>>>>>>>> +++ b/net/bridge/br_private.h
>>>>>>>>>> @@ -404,6 +404,8 @@ extern netdev_features_t br_features_recompute(struct net_bridge *br,
>>>>>>>>>> netdev_features_t features);
>>>>>>>>>> extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
>>>>>>>>>> extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
>>>>>>>>>> +extern size_t br_port_fill_vlans(struct net_bridge_port *p, char *buf,
>>>>>>>>>> + unsigned long max, unsigned long skip);
>>>>>>>>>>
>>>>>>>>>> /* br_input.c */
>>>>>>>>>> extern int br_handle_frame_finish(struct sk_buff *skb);
>>>>>>>>>> diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
>>>>>>>>>> index 13b36bd..a81e2ef 100644
>>>>>>>>>> --- a/net/bridge/br_sysfs_if.c
>>>>>>>>>> +++ b/net/bridge/br_sysfs_if.c
>>>>>>>>>> @@ -234,6 +234,29 @@ const struct sysfs_ops brport_sysfs_ops = {
>>>>>>>>>> };
>>>>>>>>>>
>>>>>>>>>> /*
>>>>>>>>>> + * Export the vlan table for a given port as a binary file.
>>>>>>>>>> + * The records are unsgined shorts.
>>>>>>>>>> + *
>>>>>>>>>> + * Returns the number of bytes read.
>>>>>>>>>> + */
>>>>>>>>>> +static ssize_t brport_vlans_read(struct file *filp, struct kobject *kobj,
>>>>>>>>>> + struct bin_attribute *bin_attr,
>>>>>>>>>> + char *buf, loff_t off, size_t count)
>>>>>>>>>> +{
>>>>>>>>>> + struct net_bridge_port *p = to_brport(kobj);
>>>>>>>>>> +
>>>>>>>>>> + return br_port_fill_vlans(p, buf,
>>>>>>>>>> + count/sizeof(unsigned short),
>>>>>>>>>> + off/sizeof(unsigned short));
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static struct bin_attribute port_vlans = {
>>>>>>>>>> + .attr = { .name = SYSFS_BRIDGE_PORT_VLANS,
>>>>>>>>>> + .mode = S_IRUGO, },
>>>>>>>>>> + .read = brport_vlans_read,
>>>>>>>>>> +};
>>>>>>>>>> +
>>>>>>>>>> +/*
>>>>>>>>>> * Add sysfs entries to ethernet device added to a bridge.
>>>>>>>>>> * Creates a brport subdirectory with bridge attributes.
>>>>>>>>>> * Puts symlink in bridge's brif subdirectory
>>>>>>>>>> @@ -255,6 +278,11 @@ int br_sysfs_addif(struct net_bridge_port *p)
>>>>>>>>>> return err;
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> + err = sysfs_create_bin_file(&p->kobj, &port_vlans);
>>>>>>>>>> + if (err) {
>>>>>>>>>> + return err;
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>> strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
>>>>>>>>>> return sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
>>>>>>>>>> }
>>>>>>>>>> --
>>>>>>>>>> 1.7.7.6
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> 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
>>> --
>>> 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
>>>
^ permalink raw reply
* [Stlinux-devel] [PATCH linux-stm 4/4] net:stmmac: convert driver to use devm_request_and_ioremap.
From: Srinivas KANDAGATLA @ 2012-08-30 15:51 UTC (permalink / raw)
To: netdev; +Cc: peppe.cavallaro
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
This patch moves calls to ioremap and request_mem_region to
devm_request_and_ioremap call.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 40 ++++----------------
1 files changed, 8 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index b4ffdc7..ed112b5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -78,6 +78,7 @@ static int __devinit stmmac_pltfr_probe(struct platform_device *pdev)
{
int ret = 0;
struct resource *res;
+ struct device *dev = &pdev->dev;
void __iomem *addr = NULL;
struct stmmac_priv *priv = NULL;
struct plat_stmmacenet_data *plat_dat = NULL;
@@ -87,18 +88,10 @@ static int __devinit stmmac_pltfr_probe(struct platform_device *pdev)
if (!res)
return -ENODEV;
- if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
- pr_err("%s: ERROR: memory allocation failed"
- "cannot get the I/O addr 0x%x\n",
- __func__, (unsigned int)res->start);
- return -EBUSY;
- }
-
- addr = ioremap(res->start, resource_size(res));
+ addr = devm_request_and_ioremap(dev, res);
if (!addr) {
pr_err("%s: ERROR: memory mapping failed", __func__);
- ret = -ENOMEM;
- goto out_release_region;
+ return -ENOMEM;
}
if (pdev->dev.of_node) {
@@ -107,14 +100,13 @@ static int __devinit stmmac_pltfr_probe(struct platform_device *pdev)
GFP_KERNEL);
if (!plat_dat) {
pr_err("%s: ERROR: no memory", __func__);
- ret = -ENOMEM;
- goto out_unmap;
+ return -ENOMEM;
}
ret = stmmac_probe_config_dt(pdev, plat_dat, &mac);
if (ret) {
pr_err("%s: main dt probe failed", __func__);
- goto out_unmap;
+ return ret;
}
} else {
plat_dat = pdev->dev.platform_data;
@@ -124,13 +116,13 @@ static int __devinit stmmac_pltfr_probe(struct platform_device *pdev)
if (plat_dat->init) {
ret = plat_dat->init(pdev);
if (unlikely(ret))
- goto out_unmap;
+ return ret;
}
priv = stmmac_dvr_probe(&(pdev->dev), plat_dat, addr);
if (!priv) {
pr_err("%s: main driver probe failed", __func__);
- goto out_unmap;
+ return -ENODEV;
}
/* Get MAC address if available (DT) */
@@ -142,8 +134,7 @@ static int __devinit stmmac_pltfr_probe(struct platform_device *pdev)
if (priv->dev->irq == -ENXIO) {
pr_err("%s: ERROR: MAC IRQ configuration "
"information not found\n", __func__);
- ret = -ENXIO;
- goto out_unmap;
+ return -ENXIO;
}
/*
@@ -165,15 +156,6 @@ static int __devinit stmmac_pltfr_probe(struct platform_device *pdev)
pr_debug("STMMAC platform driver registration completed");
return 0;
-
-out_unmap:
- iounmap(addr);
- platform_set_drvdata(pdev, NULL);
-
-out_release_region:
- release_mem_region(res->start, resource_size(res));
-
- return ret;
}
/**
@@ -186,8 +168,6 @@ static int stmmac_pltfr_remove(struct platform_device *pdev)
{
struct net_device *ndev = platform_get_drvdata(pdev);
struct stmmac_priv *priv = netdev_priv(ndev);
- void __iomem *addr = priv->ioaddr;
- struct resource *res;
int ret = stmmac_dvr_remove(ndev);
if (priv->plat->exit)
@@ -195,10 +175,6 @@ static int stmmac_pltfr_remove(struct platform_device *pdev)
platform_set_drvdata(pdev, NULL);
- iounmap(addr);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- release_mem_region(res->start, resource_size(res));
-
return ret;
}
--
1.7.0.4
^ permalink raw reply related
* [Stlinux-devel] [PATCH linux-stm 3/4] net/stmmac: Remove bus_id from mdio platform data.
From: Srinivas KANDAGATLA @ 2012-08-30 15:50 UTC (permalink / raw)
To: netdev; +Cc: peppe.cavallaro
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
This patch removes bus_id from mdio platform data, The reason to remove
bus_id is, stmmac mdio bus_id is always same as stmmac bus-id, so there
is no point in passing this in different variable.
Also stmmac ethernet driver connects to phy with bus_id passed its
platform data.
So, having single bus-id is much simpler.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
Documentation/networking/stmmac.txt | 5 -----
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 8 +++-----
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 1 -
include/linux/stmmac.h | 1 -
4 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/Documentation/networking/stmmac.txt b/Documentation/networking/stmmac.txt
index c676b9c..ef9ee71 100644
--- a/Documentation/networking/stmmac.txt
+++ b/Documentation/networking/stmmac.txt
@@ -173,7 +173,6 @@ Where:
For MDIO bus The we have:
struct stmmac_mdio_bus_data {
- int bus_id;
int (*phy_reset)(void *priv);
unsigned int phy_mask;
int *irqs;
@@ -181,7 +180,6 @@ For MDIO bus The we have:
};
Where:
- o bus_id: bus identifier;
o phy_reset: hook to reset the phy device attached to the bus.
o phy_mask: phy mask passed when register the MDIO bus within the driver.
o irqs: list of IRQs, one per PHY.
@@ -230,9 +228,6 @@ there are two MAC cores: one MAC is for MDIO Bus/PHY emulation
with fixed_link support.
static struct stmmac_mdio_bus_data stmmac1_mdio_bus = {
- .bus_id = 1,
- |
- |-> phy device on the bus_id 1
.phy_reset = phy_reset;
|
|-> function to provide the phy_reset on this board
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index e1f3d04..0376a5e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -177,7 +177,7 @@ int stmmac_mdio_register(struct net_device *ndev)
new_bus->write = &stmmac_mdio_write;
new_bus->reset = &stmmac_mdio_reset;
snprintf(new_bus->id, MII_BUS_ID_SIZE, "%s-%x",
- new_bus->name, mdio_bus_data->bus_id);
+ new_bus->name, priv->plat->bus_id);
new_bus->priv = ndev;
new_bus->irq = irqlist;
new_bus->phy_mask = mdio_bus_data->phy_mask;
@@ -213,12 +213,10 @@ int stmmac_mdio_register(struct net_device *ndev)
* and no PHY number was provided to the MAC,
* use the one probed here.
*/
- if ((priv->plat->bus_id == mdio_bus_data->bus_id) &&
- (priv->plat->phy_addr == -1))
+ if (priv->plat->phy_addr == -1)
priv->plat->phy_addr = addr;
- act = (priv->plat->bus_id == mdio_bus_data->bus_id) &&
- (priv->plat->phy_addr == addr);
+ act = (priv->plat->phy_addr == addr);
switch (phydev->irq) {
case PHY_POLL:
irq_str = "POLL";
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 13afb8e..1f069b0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -40,7 +40,6 @@ static void stmmac_default_data(void)
plat_dat.has_gmac = 1;
plat_dat.force_sf_dma_mode = 1;
- mdio_data.bus_id = 1;
mdio_data.phy_reset = NULL;
mdio_data.phy_mask = 0;
plat_dat.mdio_bus_data = &mdio_data;
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index b69bdb1..a1547ea 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -76,7 +76,6 @@
/* Platfrom data for platform device structure's platform_data field */
struct stmmac_mdio_bus_data {
- int bus_id;
int (*phy_reset)(void *priv);
unsigned int phy_mask;
int *irqs;
--
1.7.0.4
^ permalink raw reply related
* [Stlinux-devel] [PATCH linux-stm 2/4] net:stmmac: fix broken stmmac_pltfr_remove.
From: Srinivas KANDAGATLA @ 2012-08-30 15:50 UTC (permalink / raw)
To: netdev; +Cc: peppe.cavallaro
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
This patch fixes stmmac_pltfr_remove function, which is broken because,
it is accessing plat variable via freed memory priv pointer which gets
freed by free_netdev called from stmmac_dvr_remove.
In short this patch caches the plat pointer in local variable before
calling stmmac_dvr_remove to prevent code accessing freed memory.
Without this patch any attempt to remove the stmmac device will fail as
below:
Unregistering eth 0 ...
Unable to handle kernel paging request at virtual address 6b6b6bab
pgd = de5dc000
[6b6b6bab] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP
Modules linked in: cdev(O+)
CPU: 0 Tainted: G O (3.3.1_stm24_0210-b2000+ #25)
PC is at stmmac_pltfr_remove+0x2c/0xa0
LR is at stmmac_pltfr_remove+0x28/0xa0
pc : [<c01b8908>] lr : [<c01b8904>] psr: 60000013
sp : def6be78 ip : de6c5a00 fp : 00000000
r10: 00000028 r9 : c082d81d r8 : 00000001
r7 : de65a600 r6 : df81b240 r5 : c0413fd8 r4 : 00000000
r3 : 6b6b6b6b r2 : def6be6c r1 : c0355e2b r0 : 00000020
Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 10c53c7d Table: 5e5dc04a DAC: 00000015
Process insmod (pid: 738, stack limit = 0xdef6a2f0)
Stack: (0xdef6be78 to 0xdef6c000)
be60: c0413fe0
c0403658
be80: c0400bb0 c019270c c01926f8 c0191478 00000000 c0414014 c0413fe0
c01914d8
bea0: 00000000 c0413fe0 df8045d0 c019109c c0413fe0 c0400bf0 c0413fd8
c018f04c
bec0: 00000000 bf000000 c0413fd8 c01929a0 c0413fd8 bf000000 00000000
c0192bfc
bee0: bf00009c bf000014 def6a000 c000859c 00000000 00000001 bf00009c
bf00009c
bf00: 00000001 bf00009c 00000001 bf0000e4 de65a600 00000001 c082d81d
c0058cd0
bf20: bf0000a8 c004fbd8 c0056414 c082d815 c02aea20 bf0001f0 00b0b008
e0846208
bf40: c03ec8a0 e0846000 0000db0d e0850604 e08504de e0853a24 00000204
000002d4
bf60: 00000000 00000000 0000001c 0000001d 00000009 00000000 00000006
00000000
bf80: 00000003 f63d4e2e 0000db0d bef02ed8 00000080 c000d2e8 def6a000
00000000
bfa0: 00000000 c000d140 f63d4e2e 0000db0d 00b0b018 0000db0d 00b0b008
b6f4f298
bfc0: f63d4e2e 0000db0d bef02ed8 00000080 00000003 00000000 00010000
00000000
bfe0: 00b0b008 bef02c64 00008d20 b6ef3784 60000010 00b0b018 5a5a5a5a
5a5a5a5a
[<c01b8908>] (stmmac_pltfr_remove+0x2c/0xa0) from [<c019270c>]
(platform_drv_remove+0x14/0x18)
[<c019270c>] (platform_drv_remove+0x14/0x18) from [<c0191478>]
(__device_release_driver+0x64/0xa4)
[<c0191478>] (__device_release_driver+0x64/0xa4) from [<c01914d8>]
(device_release_driver+0x20/0x2c)
[<c01914d8>] (device_release_driver+0x20/0x2c) from [<c019109c>]
(bus_remove_device+0xcc/0xdc)
[<c019109c>] (bus_remove_device+0xcc/0xdc) from [<c018f04c>]
(device_del+0x104/0x160)
[<c018f04c>] (device_del+0x104/0x160) from [<c01929a0>]
(platform_device_del+0x18/0x58)
[<c01929a0>] (platform_device_del+0x18/0x58) from [<c0192bfc>]
(platform_device_unregister+0xc/0x18)
[<c0192bfc>] (platform_device_unregister+0xc/0x18) from [<bf000014>]
(r_init+0x14/0x2c [cdev])
[<bf000014>] (r_init+0x14/0x2c [cdev]) from [<c000859c>]
(do_one_initcall+0x90/0x160)
[<c000859c>] (do_one_initcall+0x90/0x160) from [<c0058cd0>]
(sys_init_module+0x15c4/0x1794)
[<c0058cd0>] (sys_init_module+0x15c4/0x1794) from [<c000d140>]
(ret_fast_syscall+0x0/0x30)
Code: e1a04000 e59f0070 eb039b65 e59636e4 (e5933040)
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index b93245c..b4ffdc7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -186,6 +186,7 @@ static int stmmac_pltfr_remove(struct platform_device *pdev)
{
struct net_device *ndev = platform_get_drvdata(pdev);
struct stmmac_priv *priv = netdev_priv(ndev);
+ void __iomem *addr = priv->ioaddr;
struct resource *res;
int ret = stmmac_dvr_remove(ndev);
@@ -194,7 +195,7 @@ static int stmmac_pltfr_remove(struct platform_device *pdev)
platform_set_drvdata(pdev, NULL);
- iounmap((void __force __iomem *)priv->ioaddr);
+ iounmap(addr);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
release_mem_region(res->start, resource_size(res));
--
1.7.0.4
^ permalink raw reply related
* [Stlinux-devel] [PATCH linux-stm 1/4] net:stmmac: Add check if mdiobus is registered in stmmac_mdio_unregister
From: Srinivas KANDAGATLA @ 2012-08-30 15:49 UTC (permalink / raw)
To: netdev; +Cc: peppe.cavallaro
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
This patch adds a basic check in stmmac_mdio_unregister to see if mdio
bus registeration for this driver was actually sucessfull or not.
Use case here is, if BSP considers using mdio-gpio bus along with stmmac
driver by passing mdio_bus_data as NULL in platform data.
Call to stmmac_mdio_register with mdio_bus_data as NULL returns 0, which
is a considered sucessfull call form stmmac. Then again when we unload
the driver we just call stmmac_mdio_unregister, this is were the actual
problem is stmmac-mdio code dont really know at this instance of calling
that stmmac_mdio_register was actually successful.
So Adding a check in stmmac_mdio_unregister is always safe.
Without this patch stmmac driver calls stmmac_mdio_register from
stmmac_release which Segfaults as mii bus was never registered at the
first point.
Originally the this bug was found when unloading an stmmac driver
instance which uses mdio-gpio for smi access.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index ade1082..e1f3d04 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -258,6 +258,9 @@ int stmmac_mdio_unregister(struct net_device *ndev)
{
struct stmmac_priv *priv = netdev_priv(ndev);
+ if (!priv->mii)
+ return 0;
+
mdiobus_unregister(priv->mii);
priv->mii->priv = NULL;
mdiobus_free(priv->mii);
--
1.7.0.4
^ permalink raw reply related
* Re: [RFC PATCH bridge 5/5] bridge: Add sysfs interface to display VLANS
From: Michael S. Tsirkin @ 2012-08-30 15:47 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <503F81AC.2090201@redhat.com>
On Thu, Aug 30, 2012 at 11:07:24AM -0400, Vlad Yasevich wrote:
> On 08/30/2012 11:03 AM, Michael S. Tsirkin wrote:
> >On Thu, Aug 30, 2012 at 10:51:35AM -0400, Vlad Yasevich wrote:
> >>On 08/30/2012 10:44 AM, Michael S. Tsirkin wrote:
> >>>On Thu, Aug 30, 2012 at 10:36:34AM -0400, Vlad Yasevich wrote:
> >>>>On 08/30/2012 10:26 AM, Michael S. Tsirkin wrote:
> >>>>>On Thu, Aug 30, 2012 at 10:05:16AM -0400, Vlad Yasevich wrote:
> >>>>>>On 08/30/2012 08:27 AM, Michael S. Tsirkin wrote:
> >>>>>>>On Thu, Aug 23, 2012 at 03:29:55PM -0400, Vlad Yasevich wrote:
> >>>>>>>>Add a binary sysfs file that will dump out vlans currently configured on the
> >>>>>>>>port.
> >>>>
> >>>>I initially though of creating a sysfs object per vlan. That would
> >>>>have made it easy to see which vlans are configured without any
> >>>>tools.
> >>>>But that could result in a lot of objects being created, so I abandoned it.
> >>>>
> >>>>I did think about a text interface, but due to a page of output
> >>>>limitation, I didn't go that route. The reason is that if someone
> >>>>cats the file, they may not see all the vlans configured. So I
> >>>>decided on the binary interface, since a binary interface with a
> >>>>tool to read it could avoid the single page limitation.
> >>>>
> >>>>-vlad
> >>>
> >>>Maybe it's not needed in sysfs then - expose it to
> >>>brctl or whatever.
> >>>
> >>
> >>brctl uses sysfs for almost everything any more :)
> >>
> >>-vlad
> >
> >How about a long string of 0 and 1's?
> >And a separate one for untagged vlans.
>
> that would work too. You really don't like the binary interface, huh?
>
> -vlad
Not in sysfs.
Another possibility: you are adding netlink command
to add/del a specific vid, add one to test a specific vid.
This means you will need to scan 4K possibilities if you
want to list them all, but in real life it's
probably just debugging tools that need to do this.
> >
> >>>>>
> >>>>>
> >>>>>>>
> >>>>>>>>---
> >>>>>>>> include/linux/if_bridge.h | 1 +
> >>>>>>>> net/bridge/br_if.c | 34 ++++++++++++++++++++++++++++++++++
> >>>>>>>> net/bridge/br_private.h | 2 ++
> >>>>>>>> net/bridge/br_sysfs_if.c | 28 ++++++++++++++++++++++++++++
> >>>>>>>> 4 files changed, 65 insertions(+), 0 deletions(-)
> >>>>>>>>
> >>>>>>>>diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> >>>>>>>>index ab750dd..d0f869b 100644
> >>>>>>>>--- a/include/linux/if_bridge.h
> >>>>>>>>+++ b/include/linux/if_bridge.h
> >>>>>>>>@@ -20,6 +20,7 @@
> >>>>>>>> #define SYSFS_BRIDGE_PORT_SUBDIR "brif"
> >>>>>>>> #define SYSFS_BRIDGE_PORT_ATTR "brport"
> >>>>>>>> #define SYSFS_BRIDGE_PORT_LINK "bridge"
> >>>>>>>>+#define SYSFS_BRIDGE_PORT_VLANS "vlans"
> >>>>>>>>
> >>>>>>>> #define BRCTL_VERSION 1
> >>>>>>>>
> >>>>>>>>diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> >>>>>>>>index 90c1038..3963748 100644
> >>>>>>>>--- a/net/bridge/br_if.c
> >>>>>>>>+++ b/net/bridge/br_if.c
> >>>>>>>>@@ -510,6 +510,40 @@ int br_del_port_vlan(struct net_bridge_port *p, unsigned long vlan)
> >>>>>>>> return 0;
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>>+size_t br_port_fill_vlans(struct net_bridge_port *p, char* buf,
> >>>>>>>>+ unsigned long max, unsigned long skip)
> >>>>>>>>+{
> >>>>>>>>+ unsigned long *map;
> >>>>>>>>+ unsigned short *vid = (unsigned short *)buf;
> >>>>>>>>+ unsigned short i;
> >>>>>>>>+ int num = 0;
> >>>>>>>>+
> >>>>>>>>+ if (skip > (VLAN_N_VID+1))
> >>>>>>>>+ return -EINVAL;
> >>>>>>>>+
> >>>>>>>>+ memset(buf, 0, max * sizeof(unsigned short));
> >>>>>>>
> >>>>>>>Isn't max is in bytes? why is this safe?
> >>>>>>>
> >>>>>>>>+
> >>>>>>>>+ rcu_read_lock();
> >>>>>>>>+ map = rcu_dereference(p->vlan_map);
> >>>>>>>>+ if (!map)
> >>>>>>>>+ goto out;
> >>>>>>>>+
> >>>>>>>>+ for (i = skip + 1; i < VLAN_N_VID + 1; i++) {
> >>>>>>>
> >>>>>>>Isn't skip in bytes too? Why do you compare it to i which is
> >>>>>>>in dwords?
> >>>>>>>
> >>>>>>>>+ if (test_bit(i, map)) {
> >>>>>>>>+ if (num > max)
> >>>>>>>>+ goto out;
> >>>>>>>>+
> >>>>>>>>+ *vid = i-1;
> >>>>>>>>+ vid++;
> >>>>>>>>+ num++;
> >>>>>>>>+ }
> >>>>>>>>+ }
> >>>>>>>>+out:
> >>>>>>>>+ rcu_read_unlock();
> >>>>>>>>+
> >>>>>>>>+ return num*sizeof(unsigned short);
> >>>>>>>>+}
> >>>>>>>>+
> >>>>>>>> void __net_exit br_net_exit(struct net *net)
> >>>>>>>> {
> >>>>>>>> struct net_device *dev;
> >>>>>>>>diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> >>>>>>>>index 5639c1c..cf95cd7 100644
> >>>>>>>>--- a/net/bridge/br_private.h
> >>>>>>>>+++ b/net/bridge/br_private.h
> >>>>>>>>@@ -404,6 +404,8 @@ extern netdev_features_t br_features_recompute(struct net_bridge *br,
> >>>>>>>> netdev_features_t features);
> >>>>>>>> extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
> >>>>>>>> extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
> >>>>>>>>+extern size_t br_port_fill_vlans(struct net_bridge_port *p, char *buf,
> >>>>>>>>+ unsigned long max, unsigned long skip);
> >>>>>>>>
> >>>>>>>> /* br_input.c */
> >>>>>>>> extern int br_handle_frame_finish(struct sk_buff *skb);
> >>>>>>>>diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
> >>>>>>>>index 13b36bd..a81e2ef 100644
> >>>>>>>>--- a/net/bridge/br_sysfs_if.c
> >>>>>>>>+++ b/net/bridge/br_sysfs_if.c
> >>>>>>>>@@ -234,6 +234,29 @@ const struct sysfs_ops brport_sysfs_ops = {
> >>>>>>>> };
> >>>>>>>>
> >>>>>>>> /*
> >>>>>>>>+ * Export the vlan table for a given port as a binary file.
> >>>>>>>>+ * The records are unsgined shorts.
> >>>>>>>>+ *
> >>>>>>>>+ * Returns the number of bytes read.
> >>>>>>>>+ */
> >>>>>>>>+static ssize_t brport_vlans_read(struct file *filp, struct kobject *kobj,
> >>>>>>>>+ struct bin_attribute *bin_attr,
> >>>>>>>>+ char *buf, loff_t off, size_t count)
> >>>>>>>>+{
> >>>>>>>>+ struct net_bridge_port *p = to_brport(kobj);
> >>>>>>>>+
> >>>>>>>>+ return br_port_fill_vlans(p, buf,
> >>>>>>>>+ count/sizeof(unsigned short),
> >>>>>>>>+ off/sizeof(unsigned short));
> >>>>>>>>+}
> >>>>>>>>+
> >>>>>>>>+static struct bin_attribute port_vlans = {
> >>>>>>>>+ .attr = { .name = SYSFS_BRIDGE_PORT_VLANS,
> >>>>>>>>+ .mode = S_IRUGO, },
> >>>>>>>>+ .read = brport_vlans_read,
> >>>>>>>>+};
> >>>>>>>>+
> >>>>>>>>+/*
> >>>>>>>> * Add sysfs entries to ethernet device added to a bridge.
> >>>>>>>> * Creates a brport subdirectory with bridge attributes.
> >>>>>>>> * Puts symlink in bridge's brif subdirectory
> >>>>>>>>@@ -255,6 +278,11 @@ int br_sysfs_addif(struct net_bridge_port *p)
> >>>>>>>> return err;
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>>+ err = sysfs_create_bin_file(&p->kobj, &port_vlans);
> >>>>>>>>+ if (err) {
> >>>>>>>>+ return err;
> >>>>>>>>+ }
> >>>>>>>>+
> >>>>>>>> strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
> >>>>>>>> return sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
> >>>>>>>> }
> >>>>>>>>--
> >>>>>>>>1.7.7.6
> >>>>>>>>
> >>>>>>>>--
> >>>>>>>>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
> >--
> >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
> >
^ permalink raw reply
* Re: [RFC PATCH bridge 2/5] bridge: Add vlan to unicast fdb entries
From: Michael S. Tsirkin @ 2012-08-30 15:45 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <503F7D59.4030906@redhat.com>
On Thu, Aug 30, 2012 at 10:48:57AM -0400, Vlad Yasevich wrote:
> On 08/30/2012 10:33 AM, Michael S. Tsirkin wrote:
> >On Thu, Aug 23, 2012 at 03:29:52PM -0400, Vlad Yasevich wrote:
> >>This patch adds vlan to unicast fdb entries that are created for
> >>learned addresses (not the manually configured ones). It adds
> >>vlan id into the hash mix and uses vlan as an addditional parameter
> >>for an entry match.
> >>
> >>Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> >
> >I wonder whether this conflicts with Eric's
> >05423b241311c9380b7280179295bac7794281b6
> >which allowed null vid.
> >
> >Specifically this calls vlan_tx_tag_get without
> >checking vlan_tx_tag_present. Why is this OK?
>
> We ignore the PRIO bits and just look at vlan deeper in the functions.
> So if the tci is set, even to NULL/0 vid, we are still OK as we'll use
> just the VID bits.
I do not fully understand why does kernel allow NULL vid,
thus my question.
It seems that your patches treat packets with NULL vid
same as untagged packets - is this OK?
> Now, the other issue with this patch is that it doesn't work when
> VLANs aren't supported by the kernel and skb->vlan_tci is never set.
> I am working to address that.
>
> -vlad
I guess if we special-case !vlan_tx_tag_present this will
also treat untagged packets different from tagged ones?
> >
> >>---
> >> include/linux/if_bridge.h | 1 +
> >> net/bridge/br_device.c | 2 +-
> >> net/bridge/br_fdb.c | 71 ++++++++++++++++++++++++++------------------
> >> net/bridge/br_input.c | 14 ++++++---
> >> net/bridge/br_private.h | 7 +++-
> >> 5 files changed, 58 insertions(+), 37 deletions(-)
> >>
> >>diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> >>index dd3f201..288ff10 100644
> >>--- a/include/linux/if_bridge.h
> >>+++ b/include/linux/if_bridge.h
> >>@@ -95,6 +95,7 @@ struct __fdb_entry {
> >> __u8 port_hi;
> >> __u8 pad0;
> >> __u16 unused;
> >>+#define fdb_vid unused
> >> };
> >>
> >> #ifdef __KERNEL__
> >>diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> >>index 3334845..e321b9d 100644
> >>--- a/net/bridge/br_device.c
> >>+++ b/net/bridge/br_device.c
> >>@@ -66,7 +66,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
> >> br_multicast_deliver(mdst, skb);
> >> else
> >> br_flood_deliver(br, skb);
> >>- } else if ((dst = __br_fdb_get(br, dest)) != NULL)
> >>+ } else if ((dst = __br_fdb_get(br, dest, vlan_tx_tag_get(skb))) != NULL)
> >> br_deliver(dst->dst, skb);
> >> else
> >> br_flood_deliver(br, skb);
> >>diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> >>index d21f323..478ca1d 100644
> >>--- a/net/bridge/br_fdb.c
> >>+++ b/net/bridge/br_fdb.c
> >>@@ -23,6 +23,7 @@
> >> #include <linux/slab.h>
> >> #include <linux/atomic.h>
> >> #include <asm/unaligned.h>
> >>+#include <linux/if_vlan.h>
> >> #include "br_private.h"
> >>
> >> static struct kmem_cache *br_fdb_cache __read_mostly;
> >>@@ -67,11 +68,12 @@ static inline int has_expired(const struct net_bridge *br,
> >> time_before_eq(fdb->updated + hold_time(br), jiffies);
> >> }
> >>
> >>-static inline int br_mac_hash(const unsigned char *mac)
> >>+static inline int br_mac_hash(const unsigned char *mac, __u16 vlan_tci)
> >> {
> >>- /* use 1 byte of OUI cnd 3 bytes of NIC */
> >>+ /* use 1 byte of OUI and 3 bytes of NIC */
> >> u32 key = get_unaligned((u32 *)(mac + 2));
> >>- return jhash_1word(key, fdb_salt) & (BR_HASH_SIZE - 1);
> >>+ return jhash_2words(key, (vlan_tci & VLAN_VID_MASK),
> >>+ fdb_salt) & (BR_HASH_SIZE - 1);
> >> }
> >>
> >> static void fdb_rcu_free(struct rcu_head *head)
> >>@@ -132,7 +134,7 @@ void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr)
> >> struct net_bridge_fdb_entry *f;
> >>
> >> /* If old entry was unassociated with any port, then delete it. */
> >>- f = __br_fdb_get(br, br->dev->dev_addr);
> >>+ f = __br_fdb_get(br, br->dev->dev_addr, 0);
> >> if (f && f->is_local && !f->dst)
> >> fdb_delete(br, f);
> >>
> >>@@ -231,13 +233,16 @@ void br_fdb_delete_by_port(struct net_bridge *br,
> >>
> >> /* No locking or refcounting, assumes caller has rcu_read_lock */
> >> struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
> >>- const unsigned char *addr)
> >>+ const unsigned char *addr,
> >>+ __u16 vlan_tci)
> >> {
> >> struct hlist_node *h;
> >> struct net_bridge_fdb_entry *fdb;
> >>
> >>- hlist_for_each_entry_rcu(fdb, h, &br->hash[br_mac_hash(addr)], hlist) {
> >>- if (ether_addr_equal(fdb->addr.addr, addr)) {
> >>+ hlist_for_each_entry_rcu(fdb, h,
> >>+ &br->hash[br_mac_hash(addr, vlan_tci)], hlist) {
> >>+ if (ether_addr_equal(fdb->addr.addr, addr) &&
> >>+ fdb->vlan_id == (vlan_tci & VLAN_VID_MASK) ) {
> >> if (unlikely(has_expired(br, fdb)))
> >> break;
> >> return fdb;
> >>@@ -261,7 +266,7 @@ int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
> >> if (!port)
> >> ret = 0;
> >> else {
> >>- fdb = __br_fdb_get(port->br, addr);
> >>+ fdb = __br_fdb_get(port->br, addr, 0);
> >> ret = fdb && fdb->dst && fdb->dst->dev != dev &&
> >> fdb->dst->state == BR_STATE_FORWARDING;
> >> }
> >>@@ -313,6 +318,7 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
> >> fe->is_local = f->is_local;
> >> if (!f->is_static)
> >> fe->ageing_timer_value = jiffies_to_clock_t(jiffies - f->updated);
> >>+ fe->fdb_vid = f->vlan_id;
> >> ++fe;
> >> ++num;
> >> }
> >>@@ -325,26 +331,30 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
> >> }
> >>
> >> static struct net_bridge_fdb_entry *fdb_find(struct hlist_head *head,
> >>- const unsigned char *addr)
> >>+ const unsigned char *addr,
> >>+ __u16 vlan_tci)
> >> {
> >> struct hlist_node *h;
> >> struct net_bridge_fdb_entry *fdb;
> >>
> >> hlist_for_each_entry(fdb, h, head, hlist) {
> >>- if (ether_addr_equal(fdb->addr.addr, addr))
> >>+ if (ether_addr_equal(fdb->addr.addr, addr) &&
> >>+ fdb->vlan_id == (vlan_tci & VLAN_VID_MASK))
> >> return fdb;
> >> }
> >> return NULL;
> >> }
> >>
> >> static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head,
> >>- const unsigned char *addr)
> >>+ const unsigned char *addr,
> >>+ __u16 vlan_tci)
> >> {
> >> struct hlist_node *h;
> >> struct net_bridge_fdb_entry *fdb;
> >>
> >> hlist_for_each_entry_rcu(fdb, h, head, hlist) {
> >>- if (ether_addr_equal(fdb->addr.addr, addr))
> >>+ if (ether_addr_equal(fdb->addr.addr, addr) &&
> >>+ fdb->vlan_id == (vlan_tci & VLAN_VID_MASK))
> >> return fdb;
> >> }
> >> return NULL;
> >>@@ -352,7 +362,8 @@ static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head,
> >>
> >> static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
> >> struct net_bridge_port *source,
> >>- const unsigned char *addr)
> >>+ const unsigned char *addr,
> >>+ __u16 vlan_tci)
> >> {
> >> struct net_bridge_fdb_entry *fdb;
> >>
> >>@@ -360,6 +371,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
> >> if (fdb) {
> >> memcpy(fdb->addr.addr, addr, ETH_ALEN);
> >> fdb->dst = source;
> >>+ fdb->vlan_id = (vlan_tci & VLAN_VID_MASK);
> >> fdb->is_local = 0;
> >> fdb->is_static = 0;
> >> fdb->updated = fdb->used = jiffies;
> >>@@ -371,13 +383,13 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
> >> static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
> >> const unsigned char *addr)
> >> {
> >>- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
> >>+ struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
> >> struct net_bridge_fdb_entry *fdb;
> >>
> >> if (!is_valid_ether_addr(addr))
> >> return -EINVAL;
> >>
> >>- fdb = fdb_find(head, addr);
> >>+ fdb = fdb_find(head, addr, 0);
> >> if (fdb) {
> >> /* it is okay to have multiple ports with same
> >> * address, just use the first one.
> >>@@ -390,7 +402,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
> >> fdb_delete(br, fdb);
> >> }
> >>
> >>- fdb = fdb_create(head, source, addr);
> >>+ fdb = fdb_create(head, source, addr, 0);
> >> if (!fdb)
> >> return -ENOMEM;
> >>
> >>@@ -412,9 +424,9 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
> >> }
> >>
> >> void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
> >>- const unsigned char *addr)
> >>+ const unsigned char *addr, __u16 vlan_tci)
> >> {
> >>- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
> >>+ struct hlist_head *head = &br->hash[br_mac_hash(addr, vlan_tci)];
> >> struct net_bridge_fdb_entry *fdb;
> >>
> >> /* some users want to always flood. */
> >>@@ -426,7 +438,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
> >> source->state == BR_STATE_FORWARDING))
> >> return;
> >>
> >>- fdb = fdb_find_rcu(head, addr);
> >>+ fdb = fdb_find_rcu(head, addr, vlan_tci);
> >> if (likely(fdb)) {
> >> /* attempt to update an entry for a local interface */
> >> if (unlikely(fdb->is_local)) {
> >>@@ -441,8 +453,8 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
> >> }
> >> } else {
> >> spin_lock(&br->hash_lock);
> >>- if (likely(!fdb_find(head, addr))) {
> >>- fdb = fdb_create(head, source, addr);
> >>+ if (likely(!fdb_find(head, addr, vlan_tci))) {
> >>+ fdb = fdb_create(head, source, addr, vlan_tci);
> >> if (fdb)
> >> fdb_notify(br, fdb, RTM_NEWNEIGH);
> >> }
> >>@@ -571,18 +583,18 @@ out:
> >>
> >> /* Update (create or replace) forwarding database entry */
> >> static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr,
> >>- __u16 state, __u16 flags)
> >>+ __u16 state, __u16 flags, __u16 vlan_tci)
> >> {
> >> struct net_bridge *br = source->br;
> >>- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
> >>+ struct hlist_head *head = &br->hash[br_mac_hash(addr, vlan_tci)];
> >> struct net_bridge_fdb_entry *fdb;
> >>
> >>- fdb = fdb_find(head, addr);
> >>+ fdb = fdb_find(head, addr, vlan_tci);
> >> if (fdb == NULL) {
> >> if (!(flags & NLM_F_CREATE))
> >> return -ENOENT;
> >>
> >>- fdb = fdb_create(head, source, addr);
> >>+ fdb = fdb_create(head, source, addr, vlan_tci);
> >> if (!fdb)
> >> return -ENOMEM;
> >> fdb_notify(br, fdb, RTM_NEWNEIGH);
> >>@@ -628,11 +640,12 @@ int br_fdb_add(struct ndmsg *ndm, struct net_device *dev,
> >>
> >> if (ndm->ndm_flags & NTF_USE) {
> >> rcu_read_lock();
> >>- br_fdb_update(p->br, p, addr);
> >>+ br_fdb_update(p->br, p, addr, 0);
> >> rcu_read_unlock();
> >> } else {
> >> spin_lock_bh(&p->br->hash_lock);
> >>- err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags);
> >>+ err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags,
> >>+ 0);
> >> spin_unlock_bh(&p->br->hash_lock);
> >> }
> >>
> >>@@ -642,10 +655,10 @@ int br_fdb_add(struct ndmsg *ndm, struct net_device *dev,
> >> static int fdb_delete_by_addr(struct net_bridge_port *p, u8 *addr)
> >> {
> >> struct net_bridge *br = p->br;
> >>- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
> >>+ struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
> >> struct net_bridge_fdb_entry *fdb;
> >>
> >>- fdb = fdb_find(head, addr);
> >>+ fdb = fdb_find(head, addr, 0);
> >> if (!fdb)
> >> return -ENOENT;
> >>
> >>diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> >>index b7ca3b3..a4579ee 100644
> >>--- a/net/bridge/br_input.c
> >>+++ b/net/bridge/br_input.c
> >>@@ -62,12 +62,15 @@ int br_handle_frame_finish(struct sk_buff *skb)
> >> * only traffic matching the VLAN filter.
> >> */
> >> vlan_map = rcu_dereference(p->vlan_map);
> >>- if (vlan_map && !test_bit((skb->vlan_tci & VLAN_VID_MASK), vlan_map))
> >>- goto drop;
> >>+ if (vlan_map && vlan_tx_tag_present(skb)) {
> >>+ unsigned short vid = vlan_tx_tag_get(skb) & VLAN_VID_MASK;
> >>+ if (!test_bit(vid+1, vlan_map))
> >>+ goto drop;
> >>+ }
> >>
> >> /* insert into forwarding database after filtering to avoid spoofing */
> >> br = p->br;
> >>- br_fdb_update(br, p, eth_hdr(skb)->h_source);
> >>+ br_fdb_update(br, p, eth_hdr(skb)->h_source, vlan_tx_tag_get(skb));
> >>
> >> if (!is_broadcast_ether_addr(dest) && is_multicast_ether_addr(dest) &&
> >> br_multicast_rcv(br, p, skb))
> >>@@ -102,7 +105,8 @@ int br_handle_frame_finish(struct sk_buff *skb)
> >> skb2 = skb;
> >>
> >> br->dev->stats.multicast++;
> >>- } else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
> >>+ } else if ((dst = __br_fdb_get(br, dest, vlan_tx_tag_get(skb))) &&
> >>+ dst->is_local) {
> >> skb2 = skb;
> >> /* Do not forward the packet since it's local. */
> >> skb = NULL;
> >>@@ -131,7 +135,7 @@ static int br_handle_local_finish(struct sk_buff *skb)
> >> {
> >> struct net_bridge_port *p = br_port_get_rcu(skb->dev);
> >>
> >>- br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
> >>+ br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vlan_tx_tag_get(skb));
> >> return 0; /* process further */
> >> }
> >>
> >>diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> >>index 8da90e8..921b927 100644
> >>--- a/net/bridge/br_private.h
> >>+++ b/net/bridge/br_private.h
> >>@@ -72,6 +72,7 @@ struct net_bridge_fdb_entry
> >> unsigned long updated;
> >> unsigned long used;
> >> mac_addr addr;
> >>+ __u16 vlan_id;
> >> unsigned char is_local;
> >> unsigned char is_static;
> >> };
> >>@@ -352,7 +353,8 @@ extern void br_fdb_cleanup(unsigned long arg);
> >> extern void br_fdb_delete_by_port(struct net_bridge *br,
> >> const struct net_bridge_port *p, int do_all);
> >> extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
> >>- const unsigned char *addr);
> >>+ const unsigned char *addr,
> >>+ __u16 vlan_tci);
> >> extern int br_fdb_test_addr(struct net_device *dev, unsigned char *addr);
> >> extern int br_fdb_fillbuf(struct net_bridge *br, void *buf,
> >> unsigned long count, unsigned long off);
> >>@@ -361,7 +363,8 @@ extern int br_fdb_insert(struct net_bridge *br,
> >> const unsigned char *addr);
> >> extern void br_fdb_update(struct net_bridge *br,
> >> struct net_bridge_port *source,
> >>- const unsigned char *addr);
> >>+ const unsigned char *addr,
> >>+ __u16 vlan_tci);
> >>
> >> extern int br_fdb_delete(struct ndmsg *ndm,
> >> struct net_device *dev,
> >>--
> >>1.7.7.6
> >>
> >>--
> >>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
> >--
> >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
> >
^ permalink raw reply
* Re: [RFC] Move in6_dev_hold under CONFIG_IPV6_PRIVACY
From: Eric Dumazet @ 2012-08-30 15:25 UTC (permalink / raw)
To: David Marchand; +Cc: netdev
In-Reply-To: <503F3589.5070804@pace.com>
On Thu, 2012-08-30 at 11:42 +0200, David Marchand wrote:
> Hello,
>
> I am currently looking at a problem with in6 interface refcnt on a
> really old kernel and I have just noticed something I find suspicious in
> all kernels until now.
>
> The comment at net/ipv6/addrconf.c:396 suggests that the call to
> in6_dev_hold is only for ipv6_regen_rndid.
> As a consequence, if CONFIG_IPV6_PRIVACY is not set, then this
> in6_dev_hold will leak a refcnt.
>
> Can someone look at this ?
> I did not test this patch, yet it looks reasonable to me.
Your patch was mangled by your email client, and its always safer to
test a patch...
Dont trust the comment, its a bit misleading.
Comment intent was to say :
/* One reference from device. We must do this before
* we eventually invoke __ipv6_regen_rndid().
*/
^ permalink raw reply
* [BUG] TIPC handling of -ERESTARTSYS in connect()
From: Chris Friesen @ 2012-08-30 15:20 UTC (permalink / raw)
To: netdev, ying.xue, Allan Stephens, Jon Maloy
Hi,
I'm seeing some behaviour that looks unintentional in the TIPC connect() call.
I'm running TIPC 1.7.7. My userspace code (stripped of error handling) looks
like this:
int sd = socket (AF_TIPC, SOCK_SEQPACKET,0);
connect(sd,(struct sockaddr*)&topsrv,sizeof(topsrv));
where topsrv is the address of the TIPC topology server. The thing that's weird
is that intermittently we get a EISCONN error on the connect call.
Looking at the TIPC connect() code, I think what is happening is that
wait_event_interruptible_timeout() is being interrupted by a signal and returns
-ERESTARTSYS. This sets sock->state to SS_DISCONNECTING and exits. Userspace
sees the ERESTARTSYS and retries the syscall, then we hit the
"sock->state != SS_UNCONNECTED" check and exit with -EISCONN.
I think current mainline is susceptible to this as well.
I'm not sure what the proper fix would be--can we detect coming back in that we were
waiting for a message and just skip down to the wait_event_interruptible_timeout()
call?
Chris
--
Chris Friesen
Software Designer
3500 Carling Avenue
Ottawa, Ontario K2H 8E9
www.genband.com
^ permalink raw reply
* Re: [RFC PATCH bridge 5/5] bridge: Add sysfs interface to display VLANS
From: Vlad Yasevich @ 2012-08-30 15:07 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev
In-Reply-To: <20120830150351.GA21739@redhat.com>
On 08/30/2012 11:03 AM, Michael S. Tsirkin wrote:
> On Thu, Aug 30, 2012 at 10:51:35AM -0400, Vlad Yasevich wrote:
>> On 08/30/2012 10:44 AM, Michael S. Tsirkin wrote:
>>> On Thu, Aug 30, 2012 at 10:36:34AM -0400, Vlad Yasevich wrote:
>>>> On 08/30/2012 10:26 AM, Michael S. Tsirkin wrote:
>>>>> On Thu, Aug 30, 2012 at 10:05:16AM -0400, Vlad Yasevich wrote:
>>>>>> On 08/30/2012 08:27 AM, Michael S. Tsirkin wrote:
>>>>>>> On Thu, Aug 23, 2012 at 03:29:55PM -0400, Vlad Yasevich wrote:
>>>>>>>> Add a binary sysfs file that will dump out vlans currently configured on the
>>>>>>>> port.
>>>>
>>>> I initially though of creating a sysfs object per vlan. That would
>>>> have made it easy to see which vlans are configured without any
>>>> tools.
>>>> But that could result in a lot of objects being created, so I abandoned it.
>>>>
>>>> I did think about a text interface, but due to a page of output
>>>> limitation, I didn't go that route. The reason is that if someone
>>>> cats the file, they may not see all the vlans configured. So I
>>>> decided on the binary interface, since a binary interface with a
>>>> tool to read it could avoid the single page limitation.
>>>>
>>>> -vlad
>>>
>>> Maybe it's not needed in sysfs then - expose it to
>>> brctl or whatever.
>>>
>>
>> brctl uses sysfs for almost everything any more :)
>>
>> -vlad
>
> How about a long string of 0 and 1's?
> And a separate one for untagged vlans.
that would work too. You really don't like the binary interface, huh?
-vlad
>
>>>>>
>>>>>
>>>>>>>
>>>>>>>> ---
>>>>>>>> include/linux/if_bridge.h | 1 +
>>>>>>>> net/bridge/br_if.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>>>> net/bridge/br_private.h | 2 ++
>>>>>>>> net/bridge/br_sysfs_if.c | 28 ++++++++++++++++++++++++++++
>>>>>>>> 4 files changed, 65 insertions(+), 0 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
>>>>>>>> index ab750dd..d0f869b 100644
>>>>>>>> --- a/include/linux/if_bridge.h
>>>>>>>> +++ b/include/linux/if_bridge.h
>>>>>>>> @@ -20,6 +20,7 @@
>>>>>>>> #define SYSFS_BRIDGE_PORT_SUBDIR "brif"
>>>>>>>> #define SYSFS_BRIDGE_PORT_ATTR "brport"
>>>>>>>> #define SYSFS_BRIDGE_PORT_LINK "bridge"
>>>>>>>> +#define SYSFS_BRIDGE_PORT_VLANS "vlans"
>>>>>>>>
>>>>>>>> #define BRCTL_VERSION 1
>>>>>>>>
>>>>>>>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>>>>>>>> index 90c1038..3963748 100644
>>>>>>>> --- a/net/bridge/br_if.c
>>>>>>>> +++ b/net/bridge/br_if.c
>>>>>>>> @@ -510,6 +510,40 @@ int br_del_port_vlan(struct net_bridge_port *p, unsigned long vlan)
>>>>>>>> return 0;
>>>>>>>> }
>>>>>>>>
>>>>>>>> +size_t br_port_fill_vlans(struct net_bridge_port *p, char* buf,
>>>>>>>> + unsigned long max, unsigned long skip)
>>>>>>>> +{
>>>>>>>> + unsigned long *map;
>>>>>>>> + unsigned short *vid = (unsigned short *)buf;
>>>>>>>> + unsigned short i;
>>>>>>>> + int num = 0;
>>>>>>>> +
>>>>>>>> + if (skip > (VLAN_N_VID+1))
>>>>>>>> + return -EINVAL;
>>>>>>>> +
>>>>>>>> + memset(buf, 0, max * sizeof(unsigned short));
>>>>>>>
>>>>>>> Isn't max is in bytes? why is this safe?
>>>>>>>
>>>>>>>> +
>>>>>>>> + rcu_read_lock();
>>>>>>>> + map = rcu_dereference(p->vlan_map);
>>>>>>>> + if (!map)
>>>>>>>> + goto out;
>>>>>>>> +
>>>>>>>> + for (i = skip + 1; i < VLAN_N_VID + 1; i++) {
>>>>>>>
>>>>>>> Isn't skip in bytes too? Why do you compare it to i which is
>>>>>>> in dwords?
>>>>>>>
>>>>>>>> + if (test_bit(i, map)) {
>>>>>>>> + if (num > max)
>>>>>>>> + goto out;
>>>>>>>> +
>>>>>>>> + *vid = i-1;
>>>>>>>> + vid++;
>>>>>>>> + num++;
>>>>>>>> + }
>>>>>>>> + }
>>>>>>>> +out:
>>>>>>>> + rcu_read_unlock();
>>>>>>>> +
>>>>>>>> + return num*sizeof(unsigned short);
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> void __net_exit br_net_exit(struct net *net)
>>>>>>>> {
>>>>>>>> struct net_device *dev;
>>>>>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>>>>>> index 5639c1c..cf95cd7 100644
>>>>>>>> --- a/net/bridge/br_private.h
>>>>>>>> +++ b/net/bridge/br_private.h
>>>>>>>> @@ -404,6 +404,8 @@ extern netdev_features_t br_features_recompute(struct net_bridge *br,
>>>>>>>> netdev_features_t features);
>>>>>>>> extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
>>>>>>>> extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
>>>>>>>> +extern size_t br_port_fill_vlans(struct net_bridge_port *p, char *buf,
>>>>>>>> + unsigned long max, unsigned long skip);
>>>>>>>>
>>>>>>>> /* br_input.c */
>>>>>>>> extern int br_handle_frame_finish(struct sk_buff *skb);
>>>>>>>> diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
>>>>>>>> index 13b36bd..a81e2ef 100644
>>>>>>>> --- a/net/bridge/br_sysfs_if.c
>>>>>>>> +++ b/net/bridge/br_sysfs_if.c
>>>>>>>> @@ -234,6 +234,29 @@ const struct sysfs_ops brport_sysfs_ops = {
>>>>>>>> };
>>>>>>>>
>>>>>>>> /*
>>>>>>>> + * Export the vlan table for a given port as a binary file.
>>>>>>>> + * The records are unsgined shorts.
>>>>>>>> + *
>>>>>>>> + * Returns the number of bytes read.
>>>>>>>> + */
>>>>>>>> +static ssize_t brport_vlans_read(struct file *filp, struct kobject *kobj,
>>>>>>>> + struct bin_attribute *bin_attr,
>>>>>>>> + char *buf, loff_t off, size_t count)
>>>>>>>> +{
>>>>>>>> + struct net_bridge_port *p = to_brport(kobj);
>>>>>>>> +
>>>>>>>> + return br_port_fill_vlans(p, buf,
>>>>>>>> + count/sizeof(unsigned short),
>>>>>>>> + off/sizeof(unsigned short));
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static struct bin_attribute port_vlans = {
>>>>>>>> + .attr = { .name = SYSFS_BRIDGE_PORT_VLANS,
>>>>>>>> + .mode = S_IRUGO, },
>>>>>>>> + .read = brport_vlans_read,
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +/*
>>>>>>>> * Add sysfs entries to ethernet device added to a bridge.
>>>>>>>> * Creates a brport subdirectory with bridge attributes.
>>>>>>>> * Puts symlink in bridge's brif subdirectory
>>>>>>>> @@ -255,6 +278,11 @@ int br_sysfs_addif(struct net_bridge_port *p)
>>>>>>>> return err;
>>>>>>>> }
>>>>>>>>
>>>>>>>> + err = sysfs_create_bin_file(&p->kobj, &port_vlans);
>>>>>>>> + if (err) {
>>>>>>>> + return err;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
>>>>>>>> return sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
>>>>>>>> }
>>>>>>>> --
>>>>>>>> 1.7.7.6
>>>>>>>>
>>>>>>>> --
>>>>>>>> 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
> --
> 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
>
^ permalink raw reply
* Re: [RFC PATCH bridge 5/5] bridge: Add sysfs interface to display VLANS
From: Michael S. Tsirkin @ 2012-08-30 15:03 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <503F7DF7.5020606@redhat.com>
On Thu, Aug 30, 2012 at 10:51:35AM -0400, Vlad Yasevich wrote:
> On 08/30/2012 10:44 AM, Michael S. Tsirkin wrote:
> >On Thu, Aug 30, 2012 at 10:36:34AM -0400, Vlad Yasevich wrote:
> >>On 08/30/2012 10:26 AM, Michael S. Tsirkin wrote:
> >>>On Thu, Aug 30, 2012 at 10:05:16AM -0400, Vlad Yasevich wrote:
> >>>>On 08/30/2012 08:27 AM, Michael S. Tsirkin wrote:
> >>>>>On Thu, Aug 23, 2012 at 03:29:55PM -0400, Vlad Yasevich wrote:
> >>>>>>Add a binary sysfs file that will dump out vlans currently configured on the
> >>>>>>port.
> >>
> >>I initially though of creating a sysfs object per vlan. That would
> >>have made it easy to see which vlans are configured without any
> >>tools.
> >>But that could result in a lot of objects being created, so I abandoned it.
> >>
> >>I did think about a text interface, but due to a page of output
> >>limitation, I didn't go that route. The reason is that if someone
> >>cats the file, they may not see all the vlans configured. So I
> >>decided on the binary interface, since a binary interface with a
> >>tool to read it could avoid the single page limitation.
> >>
> >>-vlad
> >
> >Maybe it's not needed in sysfs then - expose it to
> >brctl or whatever.
> >
>
> brctl uses sysfs for almost everything any more :)
>
> -vlad
How about a long string of 0 and 1's?
And a separate one for untagged vlans.
> >>>
> >>>
> >>>>>
> >>>>>>---
> >>>>>> include/linux/if_bridge.h | 1 +
> >>>>>> net/bridge/br_if.c | 34 ++++++++++++++++++++++++++++++++++
> >>>>>> net/bridge/br_private.h | 2 ++
> >>>>>> net/bridge/br_sysfs_if.c | 28 ++++++++++++++++++++++++++++
> >>>>>> 4 files changed, 65 insertions(+), 0 deletions(-)
> >>>>>>
> >>>>>>diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> >>>>>>index ab750dd..d0f869b 100644
> >>>>>>--- a/include/linux/if_bridge.h
> >>>>>>+++ b/include/linux/if_bridge.h
> >>>>>>@@ -20,6 +20,7 @@
> >>>>>> #define SYSFS_BRIDGE_PORT_SUBDIR "brif"
> >>>>>> #define SYSFS_BRIDGE_PORT_ATTR "brport"
> >>>>>> #define SYSFS_BRIDGE_PORT_LINK "bridge"
> >>>>>>+#define SYSFS_BRIDGE_PORT_VLANS "vlans"
> >>>>>>
> >>>>>> #define BRCTL_VERSION 1
> >>>>>>
> >>>>>>diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> >>>>>>index 90c1038..3963748 100644
> >>>>>>--- a/net/bridge/br_if.c
> >>>>>>+++ b/net/bridge/br_if.c
> >>>>>>@@ -510,6 +510,40 @@ int br_del_port_vlan(struct net_bridge_port *p, unsigned long vlan)
> >>>>>> return 0;
> >>>>>> }
> >>>>>>
> >>>>>>+size_t br_port_fill_vlans(struct net_bridge_port *p, char* buf,
> >>>>>>+ unsigned long max, unsigned long skip)
> >>>>>>+{
> >>>>>>+ unsigned long *map;
> >>>>>>+ unsigned short *vid = (unsigned short *)buf;
> >>>>>>+ unsigned short i;
> >>>>>>+ int num = 0;
> >>>>>>+
> >>>>>>+ if (skip > (VLAN_N_VID+1))
> >>>>>>+ return -EINVAL;
> >>>>>>+
> >>>>>>+ memset(buf, 0, max * sizeof(unsigned short));
> >>>>>
> >>>>>Isn't max is in bytes? why is this safe?
> >>>>>
> >>>>>>+
> >>>>>>+ rcu_read_lock();
> >>>>>>+ map = rcu_dereference(p->vlan_map);
> >>>>>>+ if (!map)
> >>>>>>+ goto out;
> >>>>>>+
> >>>>>>+ for (i = skip + 1; i < VLAN_N_VID + 1; i++) {
> >>>>>
> >>>>>Isn't skip in bytes too? Why do you compare it to i which is
> >>>>>in dwords?
> >>>>>
> >>>>>>+ if (test_bit(i, map)) {
> >>>>>>+ if (num > max)
> >>>>>>+ goto out;
> >>>>>>+
> >>>>>>+ *vid = i-1;
> >>>>>>+ vid++;
> >>>>>>+ num++;
> >>>>>>+ }
> >>>>>>+ }
> >>>>>>+out:
> >>>>>>+ rcu_read_unlock();
> >>>>>>+
> >>>>>>+ return num*sizeof(unsigned short);
> >>>>>>+}
> >>>>>>+
> >>>>>> void __net_exit br_net_exit(struct net *net)
> >>>>>> {
> >>>>>> struct net_device *dev;
> >>>>>>diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> >>>>>>index 5639c1c..cf95cd7 100644
> >>>>>>--- a/net/bridge/br_private.h
> >>>>>>+++ b/net/bridge/br_private.h
> >>>>>>@@ -404,6 +404,8 @@ extern netdev_features_t br_features_recompute(struct net_bridge *br,
> >>>>>> netdev_features_t features);
> >>>>>> extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
> >>>>>> extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
> >>>>>>+extern size_t br_port_fill_vlans(struct net_bridge_port *p, char *buf,
> >>>>>>+ unsigned long max, unsigned long skip);
> >>>>>>
> >>>>>> /* br_input.c */
> >>>>>> extern int br_handle_frame_finish(struct sk_buff *skb);
> >>>>>>diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
> >>>>>>index 13b36bd..a81e2ef 100644
> >>>>>>--- a/net/bridge/br_sysfs_if.c
> >>>>>>+++ b/net/bridge/br_sysfs_if.c
> >>>>>>@@ -234,6 +234,29 @@ const struct sysfs_ops brport_sysfs_ops = {
> >>>>>> };
> >>>>>>
> >>>>>> /*
> >>>>>>+ * Export the vlan table for a given port as a binary file.
> >>>>>>+ * The records are unsgined shorts.
> >>>>>>+ *
> >>>>>>+ * Returns the number of bytes read.
> >>>>>>+ */
> >>>>>>+static ssize_t brport_vlans_read(struct file *filp, struct kobject *kobj,
> >>>>>>+ struct bin_attribute *bin_attr,
> >>>>>>+ char *buf, loff_t off, size_t count)
> >>>>>>+{
> >>>>>>+ struct net_bridge_port *p = to_brport(kobj);
> >>>>>>+
> >>>>>>+ return br_port_fill_vlans(p, buf,
> >>>>>>+ count/sizeof(unsigned short),
> >>>>>>+ off/sizeof(unsigned short));
> >>>>>>+}
> >>>>>>+
> >>>>>>+static struct bin_attribute port_vlans = {
> >>>>>>+ .attr = { .name = SYSFS_BRIDGE_PORT_VLANS,
> >>>>>>+ .mode = S_IRUGO, },
> >>>>>>+ .read = brport_vlans_read,
> >>>>>>+};
> >>>>>>+
> >>>>>>+/*
> >>>>>> * Add sysfs entries to ethernet device added to a bridge.
> >>>>>> * Creates a brport subdirectory with bridge attributes.
> >>>>>> * Puts symlink in bridge's brif subdirectory
> >>>>>>@@ -255,6 +278,11 @@ int br_sysfs_addif(struct net_bridge_port *p)
> >>>>>> return err;
> >>>>>> }
> >>>>>>
> >>>>>>+ err = sysfs_create_bin_file(&p->kobj, &port_vlans);
> >>>>>>+ if (err) {
> >>>>>>+ return err;
> >>>>>>+ }
> >>>>>>+
> >>>>>> strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
> >>>>>> return sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
> >>>>>> }
> >>>>>>--
> >>>>>>1.7.7.6
> >>>>>>
> >>>>>>--
> >>>>>>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
^ permalink raw reply
* Re: [iproute2][PATCH] tc: mirred target: do not report non-existing devices
From: Eric Dumazet @ 2012-08-30 15:01 UTC (permalink / raw)
To: Dan Kenigsberg; +Cc: netdev, Stephen Hemminger
In-Reply-To: <1346338277-8395-1-git-send-email-danken@redhat.com>
On Thu, 2012-08-30 at 17:51 +0300, Dan Kenigsberg wrote:
> Currently, if a mirred target device is removed, `tc filter show`
> does not reveal the fact. Instead, it replaces the original name of the
> device with the default output of ll_map:ll_idx_n2a().
>
> This is unfortunate, since one cannot differ between this case and a valid
> mirroring target device named 'if17'.
>
> It seems that the original code meant to report an error message in this
> case, but it does not, since ll_index_to_name() never returns 0. I would
> not like to bail out in case of an error, since the user would still be
> interested to know what are the other details of the action.
>
> Signed-off-by: Dan Kenigsberg <danken@redhat.com>
> ---
> lib/ll_map.c | 13 +++++++++++++
> tc/m_mirred.c | 10 ++++------
> 2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/lib/ll_map.c b/lib/ll_map.c
> index 1ca781e..8ceef41 100644
> --- a/lib/ll_map.c
> +++ b/lib/ll_map.c
> @@ -108,6 +108,19 @@ const char *ll_idx_n2a(unsigned idx, char *buf)
> return buf;
> }
>
> +char *ll_index_exists(unsigned idx)
> +{
> + const struct ll_cache *im;
> +
> + if (idx == 0)
> + return 0;
> +
> + for (im = idxhead(idx); im; im = im->idx_next)
> + if (im->index == idx)
> + return 1;
> +
> + return 0;
> +}
>
I am curious to know what compiler accepted this.
return type should be a "int", not a "char *"
> const char *ll_index_to_name(unsigned idx)
> {
> diff --git a/tc/m_mirred.c b/tc/m_mirred.c
> index 0d771bc..eba1240 100644
> --- a/tc/m_mirred.c
> +++ b/tc/m_mirred.c
> @@ -268,13 +268,11 @@ print_mirred(struct action_util *au,FILE * f, struct rtattr *arg)
> ll_init_map(&rth);
> */
>
> + dev = ll_index_to_name(p->ifindex);
>
> - if ((dev = ll_index_to_name(p->ifindex)) == 0) {
> - fprintf(stderr, "Cannot find device %d\n", p->ifindex);
> - return -1;
> - }
> -
> - fprintf(f, "mirred (%s to device %s) %s", mirred_n2a(p->eaction), dev,action_n2a(p->action, b1, sizeof (b1)));
> + fprintf(f, "mirred (%s to %sdevice %s) %s", mirred_n2a(p->eaction),
> + ll_index_exists(p->ifindex) ? "" : "missing-",
> + dev, action_n2a(p->action, b1, sizeof (b1)));
>
> fprintf(f, "\n ");
> fprintf(f, "\tindex %d ref %d bind %d",p->index,p->refcnt,p->bindcnt);
How this can compile without triggering a warning, as the
ll_index_exists() is not in an include file ?
^ permalink raw reply
* [PATCH] rtnl_wilddump_request: fix alignment issue for embedded platforms
From: Lutz Jaenicke @ 2012-08-30 15:01 UTC (permalink / raw)
To: netdev
In-Reply-To: <1345621133-23583-1-git-send-email-ljaenicke@innominate.com>
Platforms have different alignment requirements which need to be
fulfilled by the compiler. If the structure elements are already
4 byte (NLMGS_ALIGNTO) aligned by the compiler adding an explicit
padding element (align_rta) is not allowed.
Use __attribute__ ((aligned (NLMSG_ALIGNTO))) in order to achieve
the required alignment.
Experienced on ARM (xscale) with symptom
netlink: 12 bytes leftover after parsing attributes
Tested on:
ARM (32bit Big Endian)
PowerPC (32bit Big Endian)
x86_64 (64bit Little Endian)
Each with different aligment requirments.
Signed-off-by: Lutz Jaenicke <ljaenicke@innominate.com>
---
lib/libnetlink.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 8e8c8b9..09b4277 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -94,8 +94,8 @@ int rtnl_wilddump_request(struct rtnl_handle *rth, int family, int type)
struct {
struct nlmsghdr nlh;
struct rtgenmsg g;
- __u16 align_rta; /* attribute has to be 32bit aligned */
- struct rtattr ext_req;
+ /* attribute has to be NLMSG aligned */
+ struct rtattr ext_req __attribute__ ((aligned(NLMSG_ALIGNTO)));
__u32 ext_filter_mask;
} req;
--
1.7.2.5
^ permalink raw reply related
* Re: [RFC PATCH bridge 5/5] bridge: Add sysfs interface to display VLANS
From: Vlad Yasevich @ 2012-08-30 14:51 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev
In-Reply-To: <20120830144443.GD21646@redhat.com>
On 08/30/2012 10:44 AM, Michael S. Tsirkin wrote:
> On Thu, Aug 30, 2012 at 10:36:34AM -0400, Vlad Yasevich wrote:
>> On 08/30/2012 10:26 AM, Michael S. Tsirkin wrote:
>>> On Thu, Aug 30, 2012 at 10:05:16AM -0400, Vlad Yasevich wrote:
>>>> On 08/30/2012 08:27 AM, Michael S. Tsirkin wrote:
>>>>> On Thu, Aug 23, 2012 at 03:29:55PM -0400, Vlad Yasevich wrote:
>>>>>> Add a binary sysfs file that will dump out vlans currently configured on the
>>>>>> port.
>>
>> I initially though of creating a sysfs object per vlan. That would
>> have made it easy to see which vlans are configured without any
>> tools.
>> But that could result in a lot of objects being created, so I abandoned it.
>>
>> I did think about a text interface, but due to a page of output
>> limitation, I didn't go that route. The reason is that if someone
>> cats the file, they may not see all the vlans configured. So I
>> decided on the binary interface, since a binary interface with a
>> tool to read it could avoid the single page limitation.
>>
>> -vlad
>
> Maybe it's not needed in sysfs then - expose it to
> brctl or whatever.
>
brctl uses sysfs for almost everything any more :)
-vlad
>>>
>>>
>>>>>
>>>>>> ---
>>>>>> include/linux/if_bridge.h | 1 +
>>>>>> net/bridge/br_if.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>> net/bridge/br_private.h | 2 ++
>>>>>> net/bridge/br_sysfs_if.c | 28 ++++++++++++++++++++++++++++
>>>>>> 4 files changed, 65 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
>>>>>> index ab750dd..d0f869b 100644
>>>>>> --- a/include/linux/if_bridge.h
>>>>>> +++ b/include/linux/if_bridge.h
>>>>>> @@ -20,6 +20,7 @@
>>>>>> #define SYSFS_BRIDGE_PORT_SUBDIR "brif"
>>>>>> #define SYSFS_BRIDGE_PORT_ATTR "brport"
>>>>>> #define SYSFS_BRIDGE_PORT_LINK "bridge"
>>>>>> +#define SYSFS_BRIDGE_PORT_VLANS "vlans"
>>>>>>
>>>>>> #define BRCTL_VERSION 1
>>>>>>
>>>>>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>>>>>> index 90c1038..3963748 100644
>>>>>> --- a/net/bridge/br_if.c
>>>>>> +++ b/net/bridge/br_if.c
>>>>>> @@ -510,6 +510,40 @@ int br_del_port_vlan(struct net_bridge_port *p, unsigned long vlan)
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>> +size_t br_port_fill_vlans(struct net_bridge_port *p, char* buf,
>>>>>> + unsigned long max, unsigned long skip)
>>>>>> +{
>>>>>> + unsigned long *map;
>>>>>> + unsigned short *vid = (unsigned short *)buf;
>>>>>> + unsigned short i;
>>>>>> + int num = 0;
>>>>>> +
>>>>>> + if (skip > (VLAN_N_VID+1))
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + memset(buf, 0, max * sizeof(unsigned short));
>>>>>
>>>>> Isn't max is in bytes? why is this safe?
>>>>>
>>>>>> +
>>>>>> + rcu_read_lock();
>>>>>> + map = rcu_dereference(p->vlan_map);
>>>>>> + if (!map)
>>>>>> + goto out;
>>>>>> +
>>>>>> + for (i = skip + 1; i < VLAN_N_VID + 1; i++) {
>>>>>
>>>>> Isn't skip in bytes too? Why do you compare it to i which is
>>>>> in dwords?
>>>>>
>>>>>> + if (test_bit(i, map)) {
>>>>>> + if (num > max)
>>>>>> + goto out;
>>>>>> +
>>>>>> + *vid = i-1;
>>>>>> + vid++;
>>>>>> + num++;
>>>>>> + }
>>>>>> + }
>>>>>> +out:
>>>>>> + rcu_read_unlock();
>>>>>> +
>>>>>> + return num*sizeof(unsigned short);
>>>>>> +}
>>>>>> +
>>>>>> void __net_exit br_net_exit(struct net *net)
>>>>>> {
>>>>>> struct net_device *dev;
>>>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>>>> index 5639c1c..cf95cd7 100644
>>>>>> --- a/net/bridge/br_private.h
>>>>>> +++ b/net/bridge/br_private.h
>>>>>> @@ -404,6 +404,8 @@ extern netdev_features_t br_features_recompute(struct net_bridge *br,
>>>>>> netdev_features_t features);
>>>>>> extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
>>>>>> extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
>>>>>> +extern size_t br_port_fill_vlans(struct net_bridge_port *p, char *buf,
>>>>>> + unsigned long max, unsigned long skip);
>>>>>>
>>>>>> /* br_input.c */
>>>>>> extern int br_handle_frame_finish(struct sk_buff *skb);
>>>>>> diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
>>>>>> index 13b36bd..a81e2ef 100644
>>>>>> --- a/net/bridge/br_sysfs_if.c
>>>>>> +++ b/net/bridge/br_sysfs_if.c
>>>>>> @@ -234,6 +234,29 @@ const struct sysfs_ops brport_sysfs_ops = {
>>>>>> };
>>>>>>
>>>>>> /*
>>>>>> + * Export the vlan table for a given port as a binary file.
>>>>>> + * The records are unsgined shorts.
>>>>>> + *
>>>>>> + * Returns the number of bytes read.
>>>>>> + */
>>>>>> +static ssize_t brport_vlans_read(struct file *filp, struct kobject *kobj,
>>>>>> + struct bin_attribute *bin_attr,
>>>>>> + char *buf, loff_t off, size_t count)
>>>>>> +{
>>>>>> + struct net_bridge_port *p = to_brport(kobj);
>>>>>> +
>>>>>> + return br_port_fill_vlans(p, buf,
>>>>>> + count/sizeof(unsigned short),
>>>>>> + off/sizeof(unsigned short));
>>>>>> +}
>>>>>> +
>>>>>> +static struct bin_attribute port_vlans = {
>>>>>> + .attr = { .name = SYSFS_BRIDGE_PORT_VLANS,
>>>>>> + .mode = S_IRUGO, },
>>>>>> + .read = brport_vlans_read,
>>>>>> +};
>>>>>> +
>>>>>> +/*
>>>>>> * Add sysfs entries to ethernet device added to a bridge.
>>>>>> * Creates a brport subdirectory with bridge attributes.
>>>>>> * Puts symlink in bridge's brif subdirectory
>>>>>> @@ -255,6 +278,11 @@ int br_sysfs_addif(struct net_bridge_port *p)
>>>>>> return err;
>>>>>> }
>>>>>>
>>>>>> + err = sysfs_create_bin_file(&p->kobj, &port_vlans);
>>>>>> + if (err) {
>>>>>> + return err;
>>>>>> + }
>>>>>> +
>>>>>> strlcpy(p->sysfs_name, p->dev->name, IFNAMSIZ);
>>>>>> return sysfs_create_link(br->ifobj, &p->kobj, p->sysfs_name);
>>>>>> }
>>>>>> --
>>>>>> 1.7.7.6
>>>>>>
>>>>>> --
>>>>>> 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
^ permalink raw reply
* [iproute2][PATCH] tc: mirred target: do not report non-existing devices
From: Dan Kenigsberg @ 2012-08-30 14:51 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, Dan Kenigsberg
Currently, if a mirred target device is removed, `tc filter show`
does not reveal the fact. Instead, it replaces the original name of the
device with the default output of ll_map:ll_idx_n2a().
This is unfortunate, since one cannot differ between this case and a valid
mirroring target device named 'if17'.
It seems that the original code meant to report an error message in this
case, but it does not, since ll_index_to_name() never returns 0. I would
not like to bail out in case of an error, since the user would still be
interested to know what are the other details of the action.
Signed-off-by: Dan Kenigsberg <danken@redhat.com>
---
lib/ll_map.c | 13 +++++++++++++
tc/m_mirred.c | 10 ++++------
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/lib/ll_map.c b/lib/ll_map.c
index 1ca781e..8ceef41 100644
--- a/lib/ll_map.c
+++ b/lib/ll_map.c
@@ -108,6 +108,19 @@ const char *ll_idx_n2a(unsigned idx, char *buf)
return buf;
}
+char *ll_index_exists(unsigned idx)
+{
+ const struct ll_cache *im;
+
+ if (idx == 0)
+ return 0;
+
+ for (im = idxhead(idx); im; im = im->idx_next)
+ if (im->index == idx)
+ return 1;
+
+ return 0;
+}
const char *ll_index_to_name(unsigned idx)
{
diff --git a/tc/m_mirred.c b/tc/m_mirred.c
index 0d771bc..eba1240 100644
--- a/tc/m_mirred.c
+++ b/tc/m_mirred.c
@@ -268,13 +268,11 @@ print_mirred(struct action_util *au,FILE * f, struct rtattr *arg)
ll_init_map(&rth);
*/
+ dev = ll_index_to_name(p->ifindex);
- if ((dev = ll_index_to_name(p->ifindex)) == 0) {
- fprintf(stderr, "Cannot find device %d\n", p->ifindex);
- return -1;
- }
-
- fprintf(f, "mirred (%s to device %s) %s", mirred_n2a(p->eaction), dev,action_n2a(p->action, b1, sizeof (b1)));
+ fprintf(f, "mirred (%s to %sdevice %s) %s", mirred_n2a(p->eaction),
+ ll_index_exists(p->ifindex) ? "" : "missing-",
+ dev, action_n2a(p->action, b1, sizeof (b1)));
fprintf(f, "\n ");
fprintf(f, "\tindex %d ref %d bind %d",p->index,p->refcnt,p->bindcnt);
--
1.7.7.6
^ permalink raw reply related
* Re: [RFC PATCH bridge 2/5] bridge: Add vlan to unicast fdb entries
From: Vlad Yasevich @ 2012-08-30 14:48 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev
In-Reply-To: <20120830143303.GA21630@redhat.com>
On 08/30/2012 10:33 AM, Michael S. Tsirkin wrote:
> On Thu, Aug 23, 2012 at 03:29:52PM -0400, Vlad Yasevich wrote:
>> This patch adds vlan to unicast fdb entries that are created for
>> learned addresses (not the manually configured ones). It adds
>> vlan id into the hash mix and uses vlan as an addditional parameter
>> for an entry match.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>
> I wonder whether this conflicts with Eric's
> 05423b241311c9380b7280179295bac7794281b6
> which allowed null vid.
>
> Specifically this calls vlan_tx_tag_get without
> checking vlan_tx_tag_present. Why is this OK?
We ignore the PRIO bits and just look at vlan deeper in the functions.
So if the tci is set, even to NULL/0 vid, we are still OK as we'll use
just the VID bits.
Now, the other issue with this patch is that it doesn't work when VLANs
aren't supported by the kernel and skb->vlan_tci is never set.
I am working to address that.
-vlad
>
>> ---
>> include/linux/if_bridge.h | 1 +
>> net/bridge/br_device.c | 2 +-
>> net/bridge/br_fdb.c | 71 ++++++++++++++++++++++++++------------------
>> net/bridge/br_input.c | 14 ++++++---
>> net/bridge/br_private.h | 7 +++-
>> 5 files changed, 58 insertions(+), 37 deletions(-)
>>
>> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
>> index dd3f201..288ff10 100644
>> --- a/include/linux/if_bridge.h
>> +++ b/include/linux/if_bridge.h
>> @@ -95,6 +95,7 @@ struct __fdb_entry {
>> __u8 port_hi;
>> __u8 pad0;
>> __u16 unused;
>> +#define fdb_vid unused
>> };
>>
>> #ifdef __KERNEL__
>> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
>> index 3334845..e321b9d 100644
>> --- a/net/bridge/br_device.c
>> +++ b/net/bridge/br_device.c
>> @@ -66,7 +66,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>> br_multicast_deliver(mdst, skb);
>> else
>> br_flood_deliver(br, skb);
>> - } else if ((dst = __br_fdb_get(br, dest)) != NULL)
>> + } else if ((dst = __br_fdb_get(br, dest, vlan_tx_tag_get(skb))) != NULL)
>> br_deliver(dst->dst, skb);
>> else
>> br_flood_deliver(br, skb);
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index d21f323..478ca1d 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -23,6 +23,7 @@
>> #include <linux/slab.h>
>> #include <linux/atomic.h>
>> #include <asm/unaligned.h>
>> +#include <linux/if_vlan.h>
>> #include "br_private.h"
>>
>> static struct kmem_cache *br_fdb_cache __read_mostly;
>> @@ -67,11 +68,12 @@ static inline int has_expired(const struct net_bridge *br,
>> time_before_eq(fdb->updated + hold_time(br), jiffies);
>> }
>>
>> -static inline int br_mac_hash(const unsigned char *mac)
>> +static inline int br_mac_hash(const unsigned char *mac, __u16 vlan_tci)
>> {
>> - /* use 1 byte of OUI cnd 3 bytes of NIC */
>> + /* use 1 byte of OUI and 3 bytes of NIC */
>> u32 key = get_unaligned((u32 *)(mac + 2));
>> - return jhash_1word(key, fdb_salt) & (BR_HASH_SIZE - 1);
>> + return jhash_2words(key, (vlan_tci & VLAN_VID_MASK),
>> + fdb_salt) & (BR_HASH_SIZE - 1);
>> }
>>
>> static void fdb_rcu_free(struct rcu_head *head)
>> @@ -132,7 +134,7 @@ void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr)
>> struct net_bridge_fdb_entry *f;
>>
>> /* If old entry was unassociated with any port, then delete it. */
>> - f = __br_fdb_get(br, br->dev->dev_addr);
>> + f = __br_fdb_get(br, br->dev->dev_addr, 0);
>> if (f && f->is_local && !f->dst)
>> fdb_delete(br, f);
>>
>> @@ -231,13 +233,16 @@ void br_fdb_delete_by_port(struct net_bridge *br,
>>
>> /* No locking or refcounting, assumes caller has rcu_read_lock */
>> struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
>> - const unsigned char *addr)
>> + const unsigned char *addr,
>> + __u16 vlan_tci)
>> {
>> struct hlist_node *h;
>> struct net_bridge_fdb_entry *fdb;
>>
>> - hlist_for_each_entry_rcu(fdb, h, &br->hash[br_mac_hash(addr)], hlist) {
>> - if (ether_addr_equal(fdb->addr.addr, addr)) {
>> + hlist_for_each_entry_rcu(fdb, h,
>> + &br->hash[br_mac_hash(addr, vlan_tci)], hlist) {
>> + if (ether_addr_equal(fdb->addr.addr, addr) &&
>> + fdb->vlan_id == (vlan_tci & VLAN_VID_MASK) ) {
>> if (unlikely(has_expired(br, fdb)))
>> break;
>> return fdb;
>> @@ -261,7 +266,7 @@ int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
>> if (!port)
>> ret = 0;
>> else {
>> - fdb = __br_fdb_get(port->br, addr);
>> + fdb = __br_fdb_get(port->br, addr, 0);
>> ret = fdb && fdb->dst && fdb->dst->dev != dev &&
>> fdb->dst->state == BR_STATE_FORWARDING;
>> }
>> @@ -313,6 +318,7 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
>> fe->is_local = f->is_local;
>> if (!f->is_static)
>> fe->ageing_timer_value = jiffies_to_clock_t(jiffies - f->updated);
>> + fe->fdb_vid = f->vlan_id;
>> ++fe;
>> ++num;
>> }
>> @@ -325,26 +331,30 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
>> }
>>
>> static struct net_bridge_fdb_entry *fdb_find(struct hlist_head *head,
>> - const unsigned char *addr)
>> + const unsigned char *addr,
>> + __u16 vlan_tci)
>> {
>> struct hlist_node *h;
>> struct net_bridge_fdb_entry *fdb;
>>
>> hlist_for_each_entry(fdb, h, head, hlist) {
>> - if (ether_addr_equal(fdb->addr.addr, addr))
>> + if (ether_addr_equal(fdb->addr.addr, addr) &&
>> + fdb->vlan_id == (vlan_tci & VLAN_VID_MASK))
>> return fdb;
>> }
>> return NULL;
>> }
>>
>> static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head,
>> - const unsigned char *addr)
>> + const unsigned char *addr,
>> + __u16 vlan_tci)
>> {
>> struct hlist_node *h;
>> struct net_bridge_fdb_entry *fdb;
>>
>> hlist_for_each_entry_rcu(fdb, h, head, hlist) {
>> - if (ether_addr_equal(fdb->addr.addr, addr))
>> + if (ether_addr_equal(fdb->addr.addr, addr) &&
>> + fdb->vlan_id == (vlan_tci & VLAN_VID_MASK))
>> return fdb;
>> }
>> return NULL;
>> @@ -352,7 +362,8 @@ static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head,
>>
>> static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
>> struct net_bridge_port *source,
>> - const unsigned char *addr)
>> + const unsigned char *addr,
>> + __u16 vlan_tci)
>> {
>> struct net_bridge_fdb_entry *fdb;
>>
>> @@ -360,6 +371,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
>> if (fdb) {
>> memcpy(fdb->addr.addr, addr, ETH_ALEN);
>> fdb->dst = source;
>> + fdb->vlan_id = (vlan_tci & VLAN_VID_MASK);
>> fdb->is_local = 0;
>> fdb->is_static = 0;
>> fdb->updated = fdb->used = jiffies;
>> @@ -371,13 +383,13 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
>> static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>> const unsigned char *addr)
>> {
>> - struct hlist_head *head = &br->hash[br_mac_hash(addr)];
>> + struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
>> struct net_bridge_fdb_entry *fdb;
>>
>> if (!is_valid_ether_addr(addr))
>> return -EINVAL;
>>
>> - fdb = fdb_find(head, addr);
>> + fdb = fdb_find(head, addr, 0);
>> if (fdb) {
>> /* it is okay to have multiple ports with same
>> * address, just use the first one.
>> @@ -390,7 +402,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>> fdb_delete(br, fdb);
>> }
>>
>> - fdb = fdb_create(head, source, addr);
>> + fdb = fdb_create(head, source, addr, 0);
>> if (!fdb)
>> return -ENOMEM;
>>
>> @@ -412,9 +424,9 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>> }
>>
>> void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>> - const unsigned char *addr)
>> + const unsigned char *addr, __u16 vlan_tci)
>> {
>> - struct hlist_head *head = &br->hash[br_mac_hash(addr)];
>> + struct hlist_head *head = &br->hash[br_mac_hash(addr, vlan_tci)];
>> struct net_bridge_fdb_entry *fdb;
>>
>> /* some users want to always flood. */
>> @@ -426,7 +438,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>> source->state == BR_STATE_FORWARDING))
>> return;
>>
>> - fdb = fdb_find_rcu(head, addr);
>> + fdb = fdb_find_rcu(head, addr, vlan_tci);
>> if (likely(fdb)) {
>> /* attempt to update an entry for a local interface */
>> if (unlikely(fdb->is_local)) {
>> @@ -441,8 +453,8 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>> }
>> } else {
>> spin_lock(&br->hash_lock);
>> - if (likely(!fdb_find(head, addr))) {
>> - fdb = fdb_create(head, source, addr);
>> + if (likely(!fdb_find(head, addr, vlan_tci))) {
>> + fdb = fdb_create(head, source, addr, vlan_tci);
>> if (fdb)
>> fdb_notify(br, fdb, RTM_NEWNEIGH);
>> }
>> @@ -571,18 +583,18 @@ out:
>>
>> /* Update (create or replace) forwarding database entry */
>> static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr,
>> - __u16 state, __u16 flags)
>> + __u16 state, __u16 flags, __u16 vlan_tci)
>> {
>> struct net_bridge *br = source->br;
>> - struct hlist_head *head = &br->hash[br_mac_hash(addr)];
>> + struct hlist_head *head = &br->hash[br_mac_hash(addr, vlan_tci)];
>> struct net_bridge_fdb_entry *fdb;
>>
>> - fdb = fdb_find(head, addr);
>> + fdb = fdb_find(head, addr, vlan_tci);
>> if (fdb == NULL) {
>> if (!(flags & NLM_F_CREATE))
>> return -ENOENT;
>>
>> - fdb = fdb_create(head, source, addr);
>> + fdb = fdb_create(head, source, addr, vlan_tci);
>> if (!fdb)
>> return -ENOMEM;
>> fdb_notify(br, fdb, RTM_NEWNEIGH);
>> @@ -628,11 +640,12 @@ int br_fdb_add(struct ndmsg *ndm, struct net_device *dev,
>>
>> if (ndm->ndm_flags & NTF_USE) {
>> rcu_read_lock();
>> - br_fdb_update(p->br, p, addr);
>> + br_fdb_update(p->br, p, addr, 0);
>> rcu_read_unlock();
>> } else {
>> spin_lock_bh(&p->br->hash_lock);
>> - err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags);
>> + err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags,
>> + 0);
>> spin_unlock_bh(&p->br->hash_lock);
>> }
>>
>> @@ -642,10 +655,10 @@ int br_fdb_add(struct ndmsg *ndm, struct net_device *dev,
>> static int fdb_delete_by_addr(struct net_bridge_port *p, u8 *addr)
>> {
>> struct net_bridge *br = p->br;
>> - struct hlist_head *head = &br->hash[br_mac_hash(addr)];
>> + struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
>> struct net_bridge_fdb_entry *fdb;
>>
>> - fdb = fdb_find(head, addr);
>> + fdb = fdb_find(head, addr, 0);
>> if (!fdb)
>> return -ENOENT;
>>
>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>> index b7ca3b3..a4579ee 100644
>> --- a/net/bridge/br_input.c
>> +++ b/net/bridge/br_input.c
>> @@ -62,12 +62,15 @@ int br_handle_frame_finish(struct sk_buff *skb)
>> * only traffic matching the VLAN filter.
>> */
>> vlan_map = rcu_dereference(p->vlan_map);
>> - if (vlan_map && !test_bit((skb->vlan_tci & VLAN_VID_MASK), vlan_map))
>> - goto drop;
>> + if (vlan_map && vlan_tx_tag_present(skb)) {
>> + unsigned short vid = vlan_tx_tag_get(skb) & VLAN_VID_MASK;
>> + if (!test_bit(vid+1, vlan_map))
>> + goto drop;
>> + }
>>
>> /* insert into forwarding database after filtering to avoid spoofing */
>> br = p->br;
>> - br_fdb_update(br, p, eth_hdr(skb)->h_source);
>> + br_fdb_update(br, p, eth_hdr(skb)->h_source, vlan_tx_tag_get(skb));
>>
>> if (!is_broadcast_ether_addr(dest) && is_multicast_ether_addr(dest) &&
>> br_multicast_rcv(br, p, skb))
>> @@ -102,7 +105,8 @@ int br_handle_frame_finish(struct sk_buff *skb)
>> skb2 = skb;
>>
>> br->dev->stats.multicast++;
>> - } else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
>> + } else if ((dst = __br_fdb_get(br, dest, vlan_tx_tag_get(skb))) &&
>> + dst->is_local) {
>> skb2 = skb;
>> /* Do not forward the packet since it's local. */
>> skb = NULL;
>> @@ -131,7 +135,7 @@ static int br_handle_local_finish(struct sk_buff *skb)
>> {
>> struct net_bridge_port *p = br_port_get_rcu(skb->dev);
>>
>> - br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
>> + br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vlan_tx_tag_get(skb));
>> return 0; /* process further */
>> }
>>
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 8da90e8..921b927 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -72,6 +72,7 @@ struct net_bridge_fdb_entry
>> unsigned long updated;
>> unsigned long used;
>> mac_addr addr;
>> + __u16 vlan_id;
>> unsigned char is_local;
>> unsigned char is_static;
>> };
>> @@ -352,7 +353,8 @@ extern void br_fdb_cleanup(unsigned long arg);
>> extern void br_fdb_delete_by_port(struct net_bridge *br,
>> const struct net_bridge_port *p, int do_all);
>> extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
>> - const unsigned char *addr);
>> + const unsigned char *addr,
>> + __u16 vlan_tci);
>> extern int br_fdb_test_addr(struct net_device *dev, unsigned char *addr);
>> extern int br_fdb_fillbuf(struct net_bridge *br, void *buf,
>> unsigned long count, unsigned long off);
>> @@ -361,7 +363,8 @@ extern int br_fdb_insert(struct net_bridge *br,
>> const unsigned char *addr);
>> extern void br_fdb_update(struct net_bridge *br,
>> struct net_bridge_port *source,
>> - const unsigned char *addr);
>> + const unsigned char *addr,
>> + __u16 vlan_tci);
>>
>> extern int br_fdb_delete(struct ndmsg *ndm,
>> struct net_device *dev,
>> --
>> 1.7.7.6
>>
>> --
>> 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
> --
> 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
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox