* Re: [net-next PATCH 0/5] be2net fixes
From: David Miller @ 2012-08-30 17:27 UTC (permalink / raw)
To: sathya.perla; +Cc: netdev
In-Reply-To: <68712d56-8bf1-48bf-9408-4a5081ea17ee@exht1.ad.emulex.com>
From: Sathya Perla <sathya.perla@emulex.com>
Date: Wed, 29 Aug 2012 12:07:39 +0530
> Pls apply.
>
> Sathya Perla (2):
> be2net: create RSS rings even in multi-channel configs
> be2net: fixup malloc/free of adapter->pmac_id
>
> Vasundhara Volam (3):
> be2net: fix max VFs reported by HW
> be2net: fix FW default for VF tx-rate
> be2net: modify log msg for lack of privilege error
All applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH 0/2]: ieee802154 drivers location
From: David Miller @ 2012-08-30 17:24 UTC (permalink / raw)
To: alex.bluesman.smirnov; +Cc: netdev, dbaryshkov
In-Reply-To: <1345993811-13803-1-git-send-email-alex.bluesman.smirnov@gmail.com>
From: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
Date: Sun, 26 Aug 2012 19:10:09 +0400
> Please review these two patches:
>
> 1. I want to keep 'ieee802154' drivers with other networking code in
> 'drivers/net' folder instead of current location in drivers root.
> I don't exactly know this historical reason, but lets keep networking
> code together.
> 2. Minor cosmetic improvement in at86rf230 driver
Both applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH 0/5] dev_<level> and dynamic_debug cleanups
From: David Miller @ 2012-08-30 17:16 UTC (permalink / raw)
To: joe; +Cc: akpm, netdev, gregkh, jbaron, jim.cromie, kay, linux-kernel
In-Reply-To: <cover.1345978012.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Sun, 26 Aug 2012 04:25:25 -0700
> The recent commit to fix dynamic_debug was a bit unclean.
> Neaten the style for dynamic_debug.
> Reduce the stack use of message logging that uses netdev_printk
> Add utility functions dev_printk_emit and dev_vprintk_emit for /dev/kmsg.
I think these should go through the generic device tree, feel free to
add my:
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [net PATCH 1/1] bnx2x: fix 57840_MF pci id
From: David Miller @ 2012-08-30 17:13 UTC (permalink / raw)
To: yuvalmin; +Cc: netdev, eilong, mchan
In-Reply-To: <1345977345-3721-1-git-send-email-yuvalmin@broadcom.com>
From: "Yuval Mintz" <yuvalmin@broadcom.com>
Date: Sun, 26 Aug 2012 13:35:45 +0300
> Commit c3def943c7117d42caaed3478731ea7c3c87190e have added support for
> new pci ids of the 57840 board, while failing to change the obsolete value
> in 'pci_ids.h'.
> This patch does so, allowing the probe of such devices.
>
> Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] netlink: add minlen validation for the new signed types
From: David Miller @ 2012-08-30 17:12 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <1345970877-4408-1-git-send-email-ja@ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Sun, 26 Aug 2012 11:47:57 +0300
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
Applied to net-next, thanks Julian.
^ permalink raw reply
* Re: [PATCH 2/2] drivers/net/ethernet/tundra/tsi108_eth.c: delete double assignment
From: David Miller @ 2012-08-30 17:09 UTC (permalink / raw)
To: Julia.Lawall; +Cc: netdev, kernel-janitors, linux-kernel
In-Reply-To: <1345969115-12977-2-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Sun, 26 Aug 2012 10:18:35 +0200
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Delete successive assignments to the same location.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Applied to net-next, thanks Julia.
^ permalink raw reply
* Re: [PATCH net-next v1 0/3] forcedeth: fix device lock-up for dual-port NICs
From: David Miller @ 2012-08-30 17:05 UTC (permalink / raw)
To: decot; +Cc: aabdulla, netdev, linux-kernel, edumazet, joe
In-Reply-To: <cover.1345864542.git.decot@googlers.com>
From: David Decotigny <decot@googlers.com>
Date: Fri, 24 Aug 2012 20:22:50 -0700
> On a dual port MCP55 10de:0373 (rev a3) NIC with both ports connected,
> we identified a configuration that does freeze the whole NIC: having
> autoneg & TX pause turned on while one port is physically connected
> but interface is down (eg. eth1) eventually causes the whole NIC to
> freeze (eth1 and... eth0). This triggers TX timeouts on the UP
> interface and, more generally, an unreachable network.
>
> In order to avoid the bug, all we have to do is make sure not to
> configure TX pause on the hardware while NIC is down. This is what the
> 2nd patch of the series does (details included).
>
> And, in case the NIC is in a bad state at reboot (should not happen
> anymore thanks to patch above), third patch basically always makes
> sure to fix the NIC when module is loaded.
>
> I could only test this with a MCP55 10de:0373 (rev a3) PCI device on a
> x86_64 host.
All applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH] [v3] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: David Miller @ 2012-08-30 16:55 UTC (permalink / raw)
To: timur; +Cc: afleming, swarren, ddaney.cavm, devicetree-discuss, netdev
In-Reply-To: <1345835453-8611-1-git-send-email-timur@freescale.com>
From: Timur Tabi <timur@freescale.com>
Date: Fri, 24 Aug 2012 14:10:53 -0500
> Add support for an MDIO bus multiplexer controlled by a simple memory-mapped
> device, like an FPGA. The device must be memory-mapped and contain only
> 8-bit registers (which keeps things simple).
>
> Tested on a Freescale P5020DS board which uses the "PIXIS" FPGA attached
> to the localbus.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH 1/1] net: ipv4: ipmr_expire_timer causes crash when removing net namespace
From: David Miller @ 2012-08-30 16:52 UTC (permalink / raw)
To: fruggeri; +Cc: netdev
In-Reply-To: <CA+HUmGhEaTD7QgYyfhDB22K1=Oj3RvDi2Ny217-YFXV03HJzqg@mail.gmail.com>
From: Francesco Ruggeri <fruggeri@aristanetworks.com>
Date: Fri, 24 Aug 2012 10:38:35 -0700
> When tearing down a net namespace, ipv4 mr_table structures are freed
> without first deactivating their timers. This can result in a crash in
> run_timer_softirq.
> This patch mimics the corresponding behaviour in ipv6.
> Locking and synchronization seem to be adequate.
> We are about to kfree mrt, so existing code should already make sure that
> no other references to mrt are pending or can be created by incoming traffic.
> The functions invoked here do not cause new references to mrt or other
> race conditions to be created.
> Invoking del_timer_sync guarantees that ipmr_expire_timer is inactive.
> Both ipmr_expire_process (whose completion we may have to wait in
> del_timer_sync) and mroute_clean_tables internally use mfc_unres_lock
> or other synchronizations when needed, and they both only modify mrt.
>
> Tested in Linux 3.4.8.
>
> Signed-off-by: Francesco Ruggeri <fruggeri@aristanetworks.com>
Applied, and queued up for -stable, thanks.
I think we have another problem. mroute_clean_tables() skips MRT_STATIC
cache entries, and does not purge them.
I can't see anything that will ever free those static entries up at all.
^ permalink raw reply
* Re: [RFC] Move in6_dev_hold under CONFIG_IPV6_PRIVACY
From: David Marchand @ 2012-08-30 16:52 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1346340321.2586.24.camel@edumazet-glaptop>
On 30/08/2012 17:25, Eric Dumazet wrote:
> Your patch was mangled by your email client, and its always safer to
> test a patch...
Sorry about that, I am not used to sending patch (tried again, I hope it is fine this time ...).
I did try my patch in my setup, and did not notice any problem at the moment.
I sent this patch as a rfc, mainly because I want to know if my analysis is correct (and if not, understand why).
> 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().
> */
>
Mmm, ok.
But if CONFIG_IPV6_PRIVACY is not selected, then I can't find where the associated in6_dev_put is done for this in6_dev_hold.
Besides, __ipv6_regen_rndid is a "CONFIG_IPV6_PRIVACY-only" stuff.
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6bc85f7..263fcf3 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -393,11 +393,6 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
return NULL;
}
- /* One reference from device. We must do this before
- * we invoke __ipv6_regen_rndid().
- */
- in6_dev_hold(ndev);
-
if (dev->flags & (IFF_NOARP | IFF_LOOPBACK))
ndev->cnf.accept_dad = -1;
@@ -410,6 +405,12 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
#ifdef CONFIG_IPV6_PRIVACY
INIT_LIST_HEAD(&ndev->tempaddr_list);
+
+ /* One reference from device. We must do this before
+ * we invoke ipv6_regen_rndid().
+ */
+ in6_dev_hold(ndev);
+
setup_timer(&ndev->regen_timer, ipv6_regen_rndid, (unsigned long)ndev);
if ((dev->flags&IFF_LOOPBACK) ||
dev->type == ARPHRD_TUNNEL ||
--
David Marchand
This message has been scanned for viruses by BlackSpider MailControl. - www.blackspider.com
^ permalink raw reply related
* Re: [PATCH 1/1] tcp: Wrong timeout for SYN segments
From: David Miller @ 2012-08-30 16:45 UTC (permalink / raw)
To: eric.dumazet; +Cc: hkjerry.chu, alex, netdev, linux-kernel
In-Reply-To: <1346332350.2586.10.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 30 Aug 2012 06:12:30 -0700
> On Wed, 2012-08-29 at 10:25 -0700, H.K. Jerry Chu wrote:
>
>> But it probably matter slightly more for TCP Fast Open (the server
>> side patch has
>> been completed and will be posted soon, after I finish breaking it up
>> into smaller
>> pieces for ease of review purpose), when a full socket will be created with data
>> passed to the app upon a valid SYN+data. Dropping a fully functioning socket
>> won't be the same as dropping a request_sock unknown to the app and letting
>> the other side retransmitting SYN (w/o data this time).
>>
>> >
>> > Sure, RFC numbers are what they are, but in practice, I doubt someone
>> > will really miss the extra SYNACK sent after ~32 seconds, since it would
>> > matter only for the last SYN attempted.
>>
>> I'd slightly prefer 1 extra retry plus longer wait time just to make
>> TCP Fast Open
>> a little more robust (even though the app protocol is required to be
>> idempotent).
>> But this is not a showstopper.
>
> Thats very good points indeed, thanks.
>
> Maybe we can increase SYNACK max retrans only if the FastOpen SYN cookie
> was validated.
>
> This way, we increase reliability without amplifying the effect of wild
> SYN packets.
Can we come to a final conclusion on this last point and arrive at a final
patch?
Thanks.
^ permalink raw reply
* Re: [net] e1000e: DoS while TSO enabled caused by link partner with small MSS
From: David Miller @ 2012-08-30 16:40 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: bruce.w.allan, netdev, gospo, sassmann, bhutchings, stable
In-Reply-To: <1345876691-22254-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 24 Aug 2012 23:38:11 -0700
> From: Bruce Allan <bruce.w.allan@intel.com>
>
> With a low enough MSS on the link partner and TSO enabled locally, the
> networking stack can periodically send a very large (e.g. 64KB) TCP
> message for which the driver will attempt to use more Tx descriptors than
> are available by default in the Tx ring. This is due to a workaround in
> the code that imposes a limit of only 4 MSS-sized segments per descriptor
> which appears to be a carry-over from the older e1000 driver and may be
> applicable only to some older PCI or PCIx parts which are not supported in
> e1000e. When the driver gets a message that is too large to fit across the
> configured number of Tx descriptors, it stops the upper stack from queueing
> any more and gets stuck in this state. After a timeout, the upper stack
> assumes the adapter is hung and calls the driver to reset it.
>
> Remove the unnecessary limitation of using up to only 4 MSS-sized segments
> per Tx descriptor, and put in a hard failure test to catch when attempting
> to check for message sizes larger than would fit in the whole Tx ring.
> Refactor the remaining logic that limits the size of data per Tx descriptor
> from a seemingly arbitrary 8KB to a limit based on the dynamic size of the
> Tx packet buffer as described in the hardware specification.
>
> Also, fix the logic in the check for space in the Tx ring for the next
> largest possible packet after the current one has been successfully queued
> for transmit, and use the appropriate defines for default ring sizes in
> e1000_probe instead of magic values.
>
> This issue goes back to the introduction of e1000e in 2.6.24 when it was
> split off from e1000.
>
> Reported-by: Ben Hutchings <bhutchings@solarflare.com>
> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Stable <stable@vger.kernel.org> [2.6.24+]
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 3.6.0-rc3 2/2] of/mdio-gpio: Simplify the way device tree support is implemented.
From: David Miller @ 2012-08-30 16:38 UTC (permalink / raw)
To: srinivas.kandagatla; +Cc: netdev, laurentp, devicetree-discuss, grant.likely
In-Reply-To: <1345809557-27727-1-git-send-email-srinivas.kandagatla@st.com>
From: Srinivas KANDAGATLA <srinivas.kandagatla@st.com>
Date: Fri, 24 Aug 2012 12:59:17 +0100
> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>
> This patch cleans up the way device tree support is added in mdio-gpio
> driver. I found lot of code duplication which is not necessary.
> Also strangely a new platform driver was also introduced for device tree
> support. All this forced me to do this cleanup patch.
> After this patch, the driver probe checks the of_node pointer to get the
> data from device tree.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH 3.6.0-rc3 1/2] of/mdio: Add dummy functions in of_mdio.h.
From: David Miller @ 2012-08-30 16:38 UTC (permalink / raw)
To: srinivas.kandagatla; +Cc: netdev, laurentp, devicetree-discuss, grant.likely
In-Reply-To: <1345809539-27684-1-git-send-email-srinivas.kandagatla@st.com>
From: Srinivas KANDAGATLA <srinivas.kandagatla@st.com>
Date: Fri, 24 Aug 2012 12:58:59 +0100
> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>
> This patch adds dummy functions in of_mdio.h, so that driver need not
> ifdef there code with CONFIG_OF.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Applied to net-next.
^ permalink raw reply
* [PATCH] net: ethernet: fix kernel OOPS when remove davinci_mdio module
From: Bin Liu @ 2012-08-30 16:37 UTC (permalink / raw)
To: mugunthanvnm, vaibhav.bedia, netdev, linux-kernel; +Cc: b-liu
davinci mdio device is not unregistered from mdiobus when removing
the module, which causes BUG_ON() when free the device from mdiobus.
Calling mdiobus_unregister() before mdiobus_free() fixes the issue.
Signed-off-by: Bin Liu <b-liu@ti.com>
---
drivers/net/ethernet/ti/davinci_mdio.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 0c2fcdd..7de19a5 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -391,8 +391,10 @@ static int __devexit davinci_mdio_remove(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct davinci_mdio_data *data = dev_get_drvdata(dev);
- if (data->bus)
+ if (data->bus) {
+ mdiobus_unregister(data->bus);
mdiobus_free(data->bus);
+ }
if (data->clk)
clk_put(data->clk);
--
1.7.0.4
^ permalink raw reply related
* 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
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