* [PATCH net-next v1 2/2] bridge: export multicast database via netlink
From: Cong Wang @ 2012-11-30 9:58 UTC (permalink / raw)
To: netdev
Cc: Cong Wang, bridge, Herbert Xu, Jesper Dangaard Brouer,
Thomas Graf, Stephen Hemminger, David S. Miller
In-Reply-To: <1354269514-1863-1-git-send-email-amwang@redhat.com>
This patch exports bridge multicast database via netlink
message type RTM_GETMDB. Similar to fdb, but currently bridge-specific.
We may need to support modify multicast database too (RTM_{ADD,DEL}MDB).
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Graf <tgraf@suug.ch>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
include/uapi/linux/if_bridge.h | 26 ++++++
include/uapi/linux/rtnetlink.h | 3 +
net/bridge/Makefile | 2 +-
net/bridge/br_mdb.c | 166 ++++++++++++++++++++++++++++++++++++++++
net/bridge/br_multicast.c | 2 +
net/bridge/br_private.h | 2 +
6 files changed, 200 insertions(+), 1 deletions(-)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index b388579..c30c236 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -116,4 +116,30 @@ enum {
__IFLA_BRIDGE_MAX,
};
#define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
+
+/* Bridge multicast database attributes
+ * [MDBA_MDB] = {
+ * [MDBA_MCADDR]
+ * [MDBA_BRPORT_NO]
+ * }
+ * [MDBA_ROUTER] = {
+ * [MDBA_BRPORT_NO]
+ * }
+ */
+enum {
+ MDBA_UNSPEC,
+ MDBA_MDB,
+ MDBA_ROUTER,
+ __MDBA_MAX,
+};
+#define MDBA_MAX (__MDBA_MAX - 1)
+
+enum {
+ MDBA_MDB_UNSPEC,
+ MDBA_MCADDR,
+ MDBA_BRPORT_NO,
+ __MDBA_MDB_MAX,
+};
+#define MDBA_MDB_MAX (__MDBA_MDB_MAX - 1)
+
#endif /* _UAPI_LINUX_IF_BRIDGE_H */
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 3dee071..0df623f 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -125,6 +125,9 @@ enum {
RTM_GETNETCONF = 82,
#define RTM_GETNETCONF RTM_GETNETCONF
+ RTM_GETMDB = 86,
+#define RTM_GETMDB RTM_GETMDB
+
__RTM_MAX,
#define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
};
diff --git a/net/bridge/Makefile b/net/bridge/Makefile
index d0359ea..e859098 100644
--- a/net/bridge/Makefile
+++ b/net/bridge/Makefile
@@ -12,6 +12,6 @@ bridge-$(CONFIG_SYSFS) += br_sysfs_if.o br_sysfs_br.o
bridge-$(CONFIG_BRIDGE_NETFILTER) += br_netfilter.o
-bridge-$(CONFIG_BRIDGE_IGMP_SNOOPING) += br_multicast.o
+bridge-$(CONFIG_BRIDGE_IGMP_SNOOPING) += br_multicast.o br_mdb.o
obj-$(CONFIG_BRIDGE_NF_EBTABLES) += netfilter/
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
new file mode 100644
index 0000000..3751f7d
--- /dev/null
+++ b/net/bridge/br_mdb.c
@@ -0,0 +1,166 @@
+#include <linux/err.h>
+#include <linux/if_ether.h>
+#include <linux/igmp.h>
+#include <linux/kernel.h>
+#include <linux/netdevice.h>
+#include <linux/rculist.h>
+#include <linux/skbuff.h>
+#include <linux/slab.h>
+#include <net/ip.h>
+#if IS_ENABLED(CONFIG_IPV6)
+#include <net/ipv6.h>
+#include <net/mld.h>
+#include <net/addrconf.h>
+#include <net/ip6_checksum.h>
+#endif
+
+#include "br_private.h"
+
+struct br_port_msg {
+ int ifindex;
+};
+
+static int br_rports_fill_info(struct sk_buff *skb, struct netlink_callback *cb,
+ u32 seq, struct net_device *dev)
+{
+ struct net_bridge *br = netdev_priv(dev);
+ struct net_bridge_port *p;
+ struct hlist_node *n;
+ struct nlattr *nest;
+
+ if (!br->multicast_router || hlist_empty(&br->router_list)) {
+ printk(KERN_INFO "no router on bridge\n");
+ return 0;
+ }
+
+ nest = nla_nest_start(skb, MDBA_ROUTER);
+ if (nest == NULL)
+ return -EMSGSIZE;
+
+ hlist_for_each_entry_rcu(p, n, &br->router_list, rlist) {
+ if (p && nla_put_u16(skb, MDBA_BRPORT_NO, p->port_no))
+ goto fail;
+ }
+
+ nla_nest_end(skb, nest);
+ return 0;
+fail:
+ nla_nest_cancel(skb, nest);
+ return -EMSGSIZE;
+}
+
+static int br_mdb_fill_info(struct sk_buff *skb, struct netlink_callback *cb,
+ u32 seq, struct net_device *dev)
+{
+ struct net_bridge *br = netdev_priv(dev);
+ struct net_bridge_mdb_htable *mdb;
+ struct nlattr *nest;
+ unsigned int i;
+ int s_idx;
+
+ if (br->multicast_disabled) {
+ printk(KERN_INFO "multicast is disabled on bridge\n");
+ return 0;
+ }
+
+ mdb = rcu_dereference(br->mdb);
+ if (!mdb) {
+ printk(KERN_INFO "no mdb on bridge\n");
+ return 0;
+ }
+
+ cb->seq = mdb->seq;
+ s_idx = cb->args[1];
+ if (s_idx >= mdb->max)
+ return 0;
+
+ nest = nla_nest_start(skb, MDBA_MDB);
+ if (nest == NULL)
+ return -EMSGSIZE;
+
+ for (i = s_idx; i < mdb->max; i++) {
+ struct hlist_node *h;
+ struct net_bridge_mdb_entry *mp;
+ struct net_bridge_port_group *p, **pp;
+ struct net_bridge_port *port;
+
+ hlist_for_each_entry_rcu(mp, h, &mdb->mhash[i], hlist[mdb->ver]) {
+ for (pp = &mp->ports;
+ (p = rcu_dereference(*pp)) != NULL;
+ pp = &p->next) {
+ port = p->port;
+ if (port) {
+ printk(KERN_INFO "port %u, mcaddr: %pI4\n", port->port_no, &mp->addr.u.ip4);
+ if (nla_put_u16(skb, MDBA_BRPORT_NO, port->port_no) ||
+ nla_put_be32(skb, MDBA_MCADDR, p->addr.u.ip4))
+ goto fail;
+ }
+ }
+ }
+ }
+
+ cb->args[1] = i;
+ nla_nest_end(skb, nest);
+ return 0;
+fail:
+ cb->args[1] = i;
+ nla_nest_cancel(skb, nest);
+ return -EMSGSIZE;
+}
+
+static int br_mdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ struct net_device *dev;
+ struct net *net = sock_net(skb->sk);
+ struct nlmsghdr *nlh;
+ u32 seq = cb->nlh->nlmsg_seq;
+ int idx = 0, s_idx;
+
+ s_idx = cb->args[0];
+
+ rcu_read_lock();
+
+ for_each_netdev_rcu(net, dev) {
+ if (dev->priv_flags & IFF_EBRIDGE) {
+ struct br_port_msg *bpm;
+
+ if (idx < s_idx)
+ goto cont;
+
+ nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid,
+ seq, RTM_GETMDB,
+ sizeof(*bpm), NLM_F_MULTI);
+ if (nlh == NULL)
+ break;
+
+ bpm = nlmsg_data(nlh);
+ bpm->ifindex = dev->ifindex;
+ if (br_mdb_fill_info(skb, cb, seq, dev) < 0) {
+ printk(KERN_INFO "br_mdb_fill_info failed\n");
+ goto fail;
+ }
+ if (br_rports_fill_info(skb, cb, seq, dev) < 0) {
+ printk(KERN_INFO "br_rports_fill_info failed\n");
+ goto fail;
+ }
+
+ nlmsg_end(skb, nlh);
+cont:
+ idx++;
+ }
+ }
+
+ rcu_read_unlock();
+ cb->args[0] = idx;
+ return skb->len;
+
+fail:
+ rcu_read_unlock();
+ nlmsg_cancel(skb, nlh);
+ return skb->len;
+}
+
+void br_mdb_init(void)
+{
+ rtnl_register(PF_BRIDGE, RTM_GETMDB, NULL, br_mdb_dump, NULL);
+}
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 2417434..d53e4f4 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -322,6 +322,7 @@ static int br_mdb_rehash(struct net_bridge_mdb_htable __rcu **mdbp, int max,
mdb->size = old ? old->size : 0;
mdb->ver = old ? old->ver ^ 1 : 0;
+ mdb->seq = old ? (old->seq + 1): 0;
if (!old || elasticity)
get_random_bytes(&mdb->secret, sizeof(mdb->secret));
@@ -1584,6 +1585,7 @@ void br_multicast_init(struct net_bridge *br)
br_multicast_querier_expired, (unsigned long)br);
setup_timer(&br->multicast_query_timer, br_multicast_query_expired,
(unsigned long)br);
+ br_mdb_init();
}
void br_multicast_open(struct net_bridge *br)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index eb9cd42..6484069 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -105,6 +105,7 @@ struct net_bridge_mdb_htable
u32 max;
u32 secret;
u32 ver;
+ u32 seq;
};
struct net_bridge_port
@@ -432,6 +433,7 @@ extern int br_multicast_set_port_router(struct net_bridge_port *p,
extern int br_multicast_toggle(struct net_bridge *br, unsigned long val);
extern int br_multicast_set_querier(struct net_bridge *br, unsigned long val);
extern int br_multicast_set_hash_max(struct net_bridge *br, unsigned long val);
+extern void br_mdb_init(void);
static inline bool br_multicast_is_router(struct net_bridge *br)
{
^ permalink raw reply related
* Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc
From: Krzysztof Mazur @ 2012-11-30 9:53 UTC (permalink / raw)
To: David Woodhouse
Cc: Chas Williams (CONTRACTOR), David Laight, davem, netdev,
linux-kernel, nathan
In-Reply-To: <1354263922.21562.270.camel@shinybook.infradead.org>
On Fri, Nov 30, 2012 at 08:25:22AM +0000, David Woodhouse wrote:
> On Fri, 2012-11-30 at 01:57 +0000, David Woodhouse wrote:
> > I think it's actually fixed for pppoatm by the bh_lock_sock() and the
> > sock_owned_by_user() check. As soon as vcc_release() calls lock_sock(),
> > pppoatm stops accepting packets.
> >
> > It should be simple enough to do the same in br2684.
>
> Um... but now I come to look at it... Krzysztof, doesn't your 'pppoatm:
> take ATM socket lock in pppoatm_send()' patch actually *break* the case
> of sending via vcc_sendmsg()?
no, in case of
pppoatm_send()
vcc_sendmsg()
the vcc_sendmsg() will just wait for releasing sk->sk_lock.slock.
When the vcc_sendmsg() gets lock first
vcc_sendmsg()
pppoatm_send()
The pppoatm_send() might spin for a while for sk->sk_lock.slock, but
after lock_sock() the vcc_sendmsg() releases that lock and
pppoatm_send() will acquire it and notice that locked is locked
(sock_owned_by_user() returns true) and will just block pppoatm,
and will be woken up in release_sock() (fix was fixed by your 10/17
patch).
>
> Why did you include the sock_owned_by_user() check in there and not just
> use bh_lock_sock()?
because bh_lock_sock() will succeeds even with concurrent vcc_sendmsg()
and will have some races in that case.
>
> With the sock_owned_by_user() check, it'll *always* drop packets
> submitted through vcc_sendmsg(), won't it?
No, sock_owned_by_user() is just in pppoatm_send() and instead of
dropping packets we block pppd.
>
> Admittedly, for PPPoATM and BR2684 we never do want to have packets
> submitted directly from userspace that way; they should all come via the
> PPP channel or the netdev respectively. So we might want to keep the
> sock_owned_by_user() check because it fixes the close race, and
> explicitly document it.
It fixes also races with vcc_sendmsg(). If we really don't wont
vcc_sendmsg() with pppoatm and br2684 we must do some protection
than vcc_sendmsg() will fail instead of racing with pppoatm_send()
and crashing with some drivers that does not support concurent
->send().
>
> But it doesn't necessarily work for other protocols, so we may need a
> better solution for the general case. Perhaps drop the
> sock_owned_by_user() check, and put bh_lock_sock() around the beginning
> of vcc_destroy_socket() where it clears ATM_VF_READY? That'll ensure
> that no ->push() is *currently* operating on a skb having seen that the
> VCC is still open.
>
> Or maybe we just make the *devices* check the ATM_VF_CLOSE flag and
> refuse to send the skb? Put the entire thing into their domain. Although
> that may involve extra locking in the driver to synchronise send() and
> close() sufficiently.
We need some additional synchronizization with pppoatm_send(), now
we use:
tasklet_kill(&pvcc->wakeup_tasklet);
ppp_unregister_channel(&pvcc->chan);
In ppp_unregister_channel() we will synchronize with the function
calling pppoatm_send() using "downl" lock.
And this must be done in pppoatm.
>
> I'm still reluctant to swap the order of the device/protocol close in
> vcc_destroy_socket(). I think that'll just swap one set of problems
> which is now fairly well-understood and mostly solved, for another set.
> In particular, I think the device needs to see the close first, because
> *it* can actually abort or flush any pending TX and RX (including
> synchronising with its tasklet as solos-pci does, etc.). Only then does
> the protocol tear its data structures down. But I suppose the new set of
> problems could be found and overcome, if Chas wants to propose an
> alternative patch set...
>
I think that the current order is good, now we have:
1. stop_sending_fames to protocol
now TX is shut down
(currently done by
set_bit(ATM_VF_CLOSE, &vcc->flags);
clear_bit(ATM_VF_READY, &vcc->flags);
)
2. close_device to device
now RX is shut down
3. device_was_closed to protocol
ugly push(NULL), but we can add some other callback.
we also can do:
1. disable RX to device
now RX is shut down
2. detach to protocol
now TX is shut down
(now protocol can fully detach because RX is disabled)
3. close_device to device
(device is not used anymore)
Krzysiek
^ permalink raw reply
* [PATCH v2]realtek:r8169: Bugfix or workaround for missing extended GigaMAC registers settings
From: Wang YanQing @ 2012-11-30 9:38 UTC (permalink / raw)
To: nic_swsd; +Cc: romieu, netdev, linux-kernel
I get a board with 8168e-vl(10ec:8168 with RTL_GIGA_MAC_VER_34),
everything looks well first, I can use ifconfig to set ip, netmask,
etc. And the rx/tx statistics show by ifconfig looks good when I
ping another host or ping it from another host. But it don't work,
I can't get ICMP REPLAY from both sides, although the RX/TX statistics
seem good.
After add some debug code, I found this NIC only accept ethernet
broadcast package, it can't filter out the package send to its
MAC address, but it works good for sending.So ifconfig show the
RX/TX status means it can receive ARP package.(It don't know its
MAC address, so below)
I have try the driver provided by realtek's website, it have the
same problem at the first time. BUT IT WORK AFTER I REBOOT with
CRTL-ALT-DEL, the reason is that realtek's driver call rtl8168_rar_set
in the .shutdown function register with pci_register_driver. Yes,
the really reason to make it work is rtl8689_rar_set, this function
set extended GigaMAC registers, so after reboot without lost the power,
NIC keep the status before reboot.
I haven't see any code to set GigaMAC registers in kernel when boot,
so I guess BIOS or NIC's circuit make it, but of course one miss
the extended GigaMAC registers in this problem. The probe code can
get MAC address right, so MAC{0,4} must had been setted, but some
guys forget the extended GigaMAC registers.
This patch fix it.
[ I don't known whether others' realtek's NIC with extended GigaMAC
reigisters have the same problem, I meet it in 8168e-vl with
RTL_GIGA_MAC_VER_34, so I make this patch just for it.]
Changes:
V1-V2:
I follow Francois Romieu 's below opinion to make this patch oneline:
I'd rather see the GigaMAC registers written through a call to
rtl_rar_set when the mac address is read in rtl_init_one instead of
duplicating most of rtl_rar_set in a quite different place.
Signed-off-by: Wang YanQing <udknight@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 927aa33..cb1dce4 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6903,6 +6903,7 @@ rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
dev->dev_addr[i] = RTL_R8(MAC0 + i);
memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
+ rtl_rar_set(tp, dev->dev_addr);
SET_ETHTOOL_OPS(dev, &rtl8169_ethtool_ops);
dev->watchdog_timeo = RTL8169_TX_TIMEOUT;
--
1.7.11.1.116.g8228a23
^ permalink raw reply related
* [PATCH] sctp: fix CONFIG_SCTP_DBG_MSG=y null pointer dereference in sctp_v6_get_dst()
From: Tommi Rantala @ 2012-11-30 9:17 UTC (permalink / raw)
To: linux-sctp, netdev
Cc: Neil Horman, Vlad Yasevich, Sridhar Samudrala, David S. Miller,
Dave Jones, Tommi Rantala
Trinity (the syscall fuzzer) triggered the following BUG, reproducible
only when the kernel is configured with CONFIG_SCTP_DBG_MSG=y.
When CONFIG_SCTP_DBG_MSG is not set, the null pointer is never
dereferenced.
---[ end trace a4de0bfcb38a3642 ]---
BUG: unable to handle kernel NULL pointer dereference at 0000000000000100
IP: [<ffffffff8136796e>] ip6_string+0x1e/0xa0
PGD 4eead067 PUD 4e472067 PMD 0
Oops: 0000 [#1] PREEMPT SMP
Modules linked in:
CPU 3
Pid: 21324, comm: trinity-child11 Tainted: G W 3.7.0-rc7+ #61 ASUSTeK Computer INC. EB1012/EB1012
RIP: 0010:[<ffffffff8136796e>] [<ffffffff8136796e>] ip6_string+0x1e/0xa0
RSP: 0018:ffff88004e4637a0 EFLAGS: 00010046
RAX: ffff88004e4637da RBX: ffff88004e4637da RCX: 0000000000000000
RDX: ffffffff8246e92a RSI: 0000000000000100 RDI: ffff88004e4637da
RBP: ffff88004e4637a8 R08: 000000000000ffff R09: 000000000000ffff
R10: 0000000000000000 R11: 0000000000000000 R12: ffffffff8289d600
R13: ffffffff8289d230 R14: ffffffff8246e928 R15: ffffffff8289d600
FS: 00007fed95153700(0000) GS:ffff88005fd80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000100 CR3: 000000004eeac000 CR4: 00000000000007e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process trinity-child11 (pid: 21324, threadinfo ffff88004e462000, task ffff8800524b0000)
Stack:
ffff88004e4637da ffff88004e463828 ffffffff81368eee 000000004e4637d8
ffffffff0000ffff ffff88000000ffff 0000000000000000 000000004e4637f8
ffffffff826285d8 ffff88004e4637f8 0000000000000000 ffff8800524b06b0
Call Trace:
[<ffffffff81368eee>] ip6_addr_string.isra.11+0x3e/0xa0
[<ffffffff81369183>] pointer.isra.12+0x233/0x2d0
[<ffffffff810a413a>] ? vprintk_emit+0x1ba/0x450
[<ffffffff8110953d>] ? trace_hardirqs_on_caller+0x10d/0x1a0
[<ffffffff81369757>] vsnprintf+0x187/0x5d0
[<ffffffff81369c62>] vscnprintf+0x12/0x30
[<ffffffff810a4028>] vprintk_emit+0xa8/0x450
[<ffffffff81e5cb00>] printk+0x49/0x4b
[<ffffffff81d17221>] sctp_v6_get_dst+0x731/0x780
[<ffffffff81d16e15>] ? sctp_v6_get_dst+0x325/0x780
[<ffffffff81d00a96>] sctp_transport_route+0x46/0x120
[<ffffffff81cff0f1>] sctp_assoc_add_peer+0x161/0x350
[<ffffffff81d0fd8d>] sctp_sendmsg+0x6cd/0xcb0
[<ffffffff81b55bf0>] ? inet_create+0x670/0x670
[<ffffffff81b55cfb>] inet_sendmsg+0x10b/0x220
[<ffffffff81b55bf0>] ? inet_create+0x670/0x670
[<ffffffff81a72a64>] ? sock_update_classid+0xa4/0x2b0
[<ffffffff81a72ab0>] ? sock_update_classid+0xf0/0x2b0
[<ffffffff81a6ac1c>] sock_sendmsg+0xdc/0xf0
[<ffffffff8118e9e5>] ? might_fault+0x85/0x90
[<ffffffff8118e99c>] ? might_fault+0x3c/0x90
[<ffffffff81a6e12a>] sys_sendto+0xfa/0x130
[<ffffffff810a9887>] ? do_setitimer+0x197/0x380
[<ffffffff81e960d5>] ? sysret_check+0x22/0x5d
[<ffffffff81e960a9>] system_call_fastpath+0x16/0x1b
Code: 01 eb 89 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 f8 31 c9 48 89 e5 53 eb 12 0f 1f 40 00 48 83 c1 01 48 83 c0 04 48 83 f9 08 74 70 <0f> b6 3c 4e 89 fb 83 e7 0f c0 eb 04 41 89 d8 41 83 e0 0f 0f b6
RIP [<ffffffff8136796e>] ip6_string+0x1e/0xa0
RSP <ffff88004e4637a0>
CR2: 0000000000000100
---[ end trace a4de0bfcb38a3643 ]---
Signed-off-by: Tommi Rantala <tt.rantala@gmail.com>
---
net/sctp/ipv6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index ea14cb4..f3f0f4d 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -345,7 +345,7 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
}
out:
- if (!IS_ERR(dst)) {
+ if (!IS_ERR_OR_NULL(dst)) {
struct rt6_info *rt;
rt = (struct rt6_info *)dst;
t->dst = dst;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH]realtek:r8169: Bugfix or workaround for missing extended GigaMAC registers settings
From: Wang YanQing @ 2012-11-30 9:04 UTC (permalink / raw)
To: Francois Romieu; +Cc: nic_swsd, netdev, linux-kernel, Hayes Wang
In-Reply-To: <20121130063500.GA9436@electric-eye.fr.zoreil.com>
On Fri, Nov 30, 2012 at 07:35:00AM +0100, Francois Romieu wrote:
> Which kernel version is it ?
I have done the test and debug with mainline
e23739b4ade80a3a7f87198f008f6c44a7cbc9fd, v3.7-rc7-51-ge23739b
> I'd rather see the GigaMAC registers written through a call to
> rtl_rar_set when the mac address is read in rtl_init_one instead
> of duplicating most of rtl_rar_set in a quite different place.
I have the same feeling with you, I will resend the patch follow
your opinion if everything pass test and work.
Thanks
^ permalink raw reply
* Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc
From: David Woodhouse @ 2012-11-30 8:25 UTC (permalink / raw)
To: Chas Williams (CONTRACTOR)
Cc: Krzysztof Mazur, David Laight, davem, netdev, linux-kernel,
nathan
In-Reply-To: <1354240620.21562.256.camel@shinybook.infradead.org>
[-- Attachment #1: Type: text/plain, Size: 2212 bytes --]
On Fri, 2012-11-30 at 01:57 +0000, David Woodhouse wrote:
> I think it's actually fixed for pppoatm by the bh_lock_sock() and the
> sock_owned_by_user() check. As soon as vcc_release() calls lock_sock(),
> pppoatm stops accepting packets.
>
> It should be simple enough to do the same in br2684.
Um... but now I come to look at it... Krzysztof, doesn't your 'pppoatm:
take ATM socket lock in pppoatm_send()' patch actually *break* the case
of sending via vcc_sendmsg()?
Why did you include the sock_owned_by_user() check in there and not just
use bh_lock_sock()?
With the sock_owned_by_user() check, it'll *always* drop packets
submitted through vcc_sendmsg(), won't it?
Admittedly, for PPPoATM and BR2684 we never do want to have packets
submitted directly from userspace that way; they should all come via the
PPP channel or the netdev respectively. So we might want to keep the
sock_owned_by_user() check because it fixes the close race, and
explicitly document it.
But it doesn't necessarily work for other protocols, so we may need a
better solution for the general case. Perhaps drop the
sock_owned_by_user() check, and put bh_lock_sock() around the beginning
of vcc_destroy_socket() where it clears ATM_VF_READY? That'll ensure
that no ->push() is *currently* operating on a skb having seen that the
VCC is still open.
Or maybe we just make the *devices* check the ATM_VF_CLOSE flag and
refuse to send the skb? Put the entire thing into their domain. Although
that may involve extra locking in the driver to synchronise send() and
close() sufficiently.
I'm still reluctant to swap the order of the device/protocol close in
vcc_destroy_socket(). I think that'll just swap one set of problems
which is now fairly well-understood and mostly solved, for another set.
In particular, I think the device needs to see the close first, because
*it* can actually abort or flush any pending TX and RX (including
synchronising with its tasklet as solos-pci does, etc.). Only then does
the protocol tear its data structures down. But I suppose the new set of
problems could be found and overcome, if Chas wants to propose an
alternative patch set...
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply
* Re: [PATCH]realtek:r8169: Bugfix or workaround for missing extended GigaMAC registers settings
From: Francois Romieu @ 2012-11-30 6:35 UTC (permalink / raw)
To: Wang YanQing; +Cc: nic_swsd, netdev, linux-kernel, Hayes Wang
In-Reply-To: <20121130050619.GA12862@udknight>
Wang YanQing <udknight@gmail.com> :
[...]
> After add some debug code, I found this NIC only accept ethernet
> broadcast package, it can't filter out the package send to its
> MAC address, but it works good for sending.So ifconfig show the
> RX/TX status means it can receive ARP package.(It don't its MAC
> address, so below)
Which kernel version is it ?
[...]
> I haven't see any code to set GigaMAC registers in kernel when boot,
> so I guess BIOS or NIC's circuit make it, but of course one miss
I'd appreciate to figure it out (and understand why I did not notice
it when testing).
> the extended GigaMAC registers in this problem. The probe code can
> get MAC address right, so MAC{0,4} must had been setted, but some
> guys forget the extended GigaMAC registers.
>
> This patch fix it.
It is a good analysis job.
I'd rather see the GigaMAC registers written through a call to
rtl_rar_set when the mac address is read in rtl_init_one instead
of duplicating most of rtl_rar_set in a quite different place.
Hayes, can you specify if it would work or if it may mess the registers
init sequence ordering ?
Thanks.
--
Ueimor
^ permalink raw reply
* Re: iputils: ping -I <iface>
From: Jan Synacek @ 2012-11-30 6:06 UTC (permalink / raw)
To: Ben Greear; +Cc: YOSHIFUJI Hideaki, netdev
In-Reply-To: <50B7BC0F.6000709@candelatech.com>
On 11/29/2012 08:48 PM, Ben Greear wrote:
> On 11/29/2012 06:12 AM, Jan Synacek wrote:
>> Hello,
>>
>> There seems to be a bug(?) when calling ping with -I lo:
>>
>> $ ping -I lo kernel.org
>>
>> PING kernel.org (149.20.4.69) from 192.168.1.10 lo: 56(84) bytes of data.
>> ^C
>>
>> Note that 192.168.1.10 is my primary interface's address (em1). However, no
>> replies are coming back.
>>
>> $ ping -I em1 kernel.org
>>
>> PING kernel.org (149.20.4.69) from 192.168.1.10 em1: 56(84) bytes of data.
>> 64 bytes from pub2.kernel.org (149.20.4.69): icmp_seq=1 ttl=42 time=202 ms
>> 64 bytes from pub2.kernel.org (149.20.4.69): icmp_seq=2 ttl=42 time=187 ms
>> ^C
>>
>> Works as expected.
>>
>> I know that binding to loopback probably doesn't make much sense, but I think
>> that ping should be able to cope with that.
>
> I think it would be wrong if ping worked as you suggest. Binding to an
> interface means use that interface as the source of your packets, and having
> it bind hard helps when using systems with multiple NICs on same subnet
> (or possibly, same IP).
I just wanted to point out that if I call ping with -I lo, its 'from' address is
wrong (in my case 192.168.1.10) and nothing happens (that's, I guess, expected
if it really bound to loopback). If I call ping with the -I <the same address>
or -I em1 (the same address again), it works as expected. I'm sorry if I wasn't
clear enough.
>
>> Also, it would be nice to mention the difference between -I <ip> and -I <iface>
>> in the manpage.
>
> In my opinion, -I <iface> should use SO_BINDTODEVICE, but at least in
> older versions of ping it did not.
Ping does use SO_BINDTODEVICE.
>
> Thanks,
> Ben
>
Regards,
--
Jan Synacek
Software Engineer, BaseOS team Brno, Red Hat
^ permalink raw reply
* [PATCH]realtek:r8169: Bugfix or workaround for missing extended GigaMAC registers settings
From: Wang YanQing @ 2012-11-30 5:06 UTC (permalink / raw)
To: nic_swsd; +Cc: romieu, netdev, linux-kernel
I get a board with 8168e-vl(10ec:8168 with RTL_GIGA_MAC_VER_34),
everything looks well first, I can use ifconfig to set ip, netmask,
etc. And the rx/tx statistics show by ifconfig looks good when I
ping another host or ping it from another host. But it don't work,
I can't get ICMP REPLAY from both sides, although the RX/TX statistics
seem good.
After add some debug code, I found this NIC only accept ethernet
broadcast package, it can't filter out the package send to its
MAC address, but it works good for sending.So ifconfig show the
RX/TX status means it can receive ARP package.(It don't its MAC
address, so below)
I have try the driver provided by realtek's website, it have the
same problem at the first time. BUT IT WORK AFTER I REBOOT with
CRTL-ALT-DEL, the reason is that realtek's driver call rtl8168_rar_set
in the .shutdown function register with pci_register_driver. Yes,
the really reason to make it work is rtl8689_rar_set, this function
set extended GigaMAC registers, so after reboot without lost the power,
NIC keep the status before reboot.
I haven't see any code to set GigaMAC registers in kernel when boot,
so I guess BIOS or NIC's circuit make it, but of course one miss
the extended GigaMAC registers in this problem. The probe code can
get MAC address right, so MAC{0,4} must had been setted, but some
guys forget the extended GigaMAC registers.
This patch fix it.
[ I don't known whether others' realtek's NIC with extended GigaMAC
reigisters have the same problem, I meet it in 8168e-vl with
RTL_GIGA_MAC_VER_34, so I make this patch just for it.]
Signed-off-by: Wang YanQing <udknight@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 927aa33..e49c08d 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -3095,7 +3095,30 @@ static void rtl8168e_1_hw_phy_config(struct rtl8169_private *tp)
rtl_writephy(tp, 0x0e, 0x0000);
rtl_writephy(tp, 0x0d, 0x0000);
}
+static void rtl8168e_2_workaround(struct rtl8169_private *tp, u8 *addr)
+{
+ void __iomem *ioaddr = tp->mmio_addr;
+ u32 high;
+ u32 low;
+
+ low = addr[0] | (addr[1] << 8) | (addr[2] << 16) | (addr[3] << 24);
+ high = addr[4] | (addr[5] << 8);
+
+
+ RTL_W8(Cfg9346, Cfg9346_Unlock);
+ if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
+ const struct exgmac_reg e[] = {
+ { .addr = 0xe0, ERIAR_MASK_1111, .val = low },
+ { .addr = 0xe4, ERIAR_MASK_1111, .val = high },
+ { .addr = 0xf0, ERIAR_MASK_1111, .val = low << 16 },
+ { .addr = 0xf4, ERIAR_MASK_1111, .val = high << 16 |
+ low >> 16 },
+ };
+ rtl_write_exgmac_batch(tp, e, ARRAY_SIZE(e));
+ }
+ RTL_W8(Cfg9346, Cfg9346_Lock);
+}
static void rtl8168e_2_hw_phy_config(struct rtl8169_private *tp)
{
static const struct phy_reg phy_reg_init[] = {
@@ -3178,6 +3201,7 @@ static void rtl8168e_2_hw_phy_config(struct rtl8169_private *tp)
rtl_w1w0_phy(tp, 0x19, 0x0000, 0x0001);
rtl_w1w0_phy(tp, 0x10, 0x0000, 0x0400);
rtl_writephy(tp, 0x1f, 0x0000);
+ rtl8168e_2_workaround(tp, tp->dev->dev_addr);
}
static void rtl8168f_hw_phy_config(struct rtl8169_private *tp)
--
1.7.11.1.116.g8228a23
^ permalink raw reply related
* [PATCH 2/2] mac802154: use kfree_skb() instead of dev_kfree_skb()
From: Alan Ott @ 2012-11-30 4:25 UTC (permalink / raw)
To: Alexander Smirnov, Dmitry Eremin-Solenikov, David S. Miller,
Eric Dumazet
Cc: linux-zigbee-devel, netdev, linux-kernel, Alan Ott
In-Reply-To: <1354249511-8086-1-git-send-email-alan@signal11.us>
kfree_skb() indicates failure, which is where this is being used.
Signed-off-by: Alan Ott <alan@signal11.us>
---
net/mac802154/tx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c
index db63914..4e09d07 100644
--- a/net/mac802154/tx.c
+++ b/net/mac802154/tx.c
@@ -99,7 +99,7 @@ netdev_tx_t mac802154_tx(struct mac802154_priv *priv, struct sk_buff *skb,
}
if (skb_cow_head(skb, priv->hw.extra_tx_headroom)) {
- dev_kfree_skb(skb);
+ kfree_skb(skb);
return NETDEV_TX_OK;
}
--
1.7.11.2
^ permalink raw reply related
* [PATCH 1/2] mac802154: fix memory leaks
From: Alan Ott @ 2012-11-30 4:25 UTC (permalink / raw)
To: Alexander Smirnov, Dmitry Eremin-Solenikov, David S. Miller,
Eric Dumazet
Cc: linux-zigbee-devel, netdev, linux-kernel, Alan Ott
In-Reply-To: <504EA95C.9010003@signal11.us>
kfree_skb() was not getting called in the case of some failures.
This was pointed out by Eric Dumazet.
Signed-off-by: Alan Ott <alan@signal11.us>
---
net/mac802154/tx.c | 5 ++++-
net/mac802154/wpan.c | 4 +++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c
index 1a4df39..db63914 100644
--- a/net/mac802154/tx.c
+++ b/net/mac802154/tx.c
@@ -85,6 +85,7 @@ netdev_tx_t mac802154_tx(struct mac802154_priv *priv, struct sk_buff *skb,
if (!(priv->phy->channels_supported[page] & (1 << chan))) {
WARN_ON(1);
+ kfree_skb(skb);
return NETDEV_TX_OK;
}
@@ -103,8 +104,10 @@ netdev_tx_t mac802154_tx(struct mac802154_priv *priv, struct sk_buff *skb,
}
work = kzalloc(sizeof(struct xmit_work), GFP_ATOMIC);
- if (!work)
+ if (!work) {
+ kfree_skb(skb);
return NETDEV_TX_BUSY;
+ }
INIT_WORK(&work->work, mac802154_xmit_worker);
work->skb = skb;
diff --git a/net/mac802154/wpan.c b/net/mac802154/wpan.c
index f30f6d4..1191039 100644
--- a/net/mac802154/wpan.c
+++ b/net/mac802154/wpan.c
@@ -327,8 +327,10 @@ mac802154_wpan_xmit(struct sk_buff *skb, struct net_device *dev)
if (chan == MAC802154_CHAN_NONE ||
page >= WPAN_NUM_PAGES ||
- chan >= WPAN_NUM_CHANNELS)
+ chan >= WPAN_NUM_CHANNELS) {
+ kfree_skb(skb);
return NETDEV_TX_OK;
+ }
skb->skb_iif = dev->ifindex;
dev->stats.tx_packets++;
--
1.7.11.2
^ permalink raw reply related
* Re: [PATCH] 6lowpan: consider checksum bytes in fragmentation threshold
From: Alan Ott @ 2012-11-30 1:58 UTC (permalink / raw)
To: Alan Ott
Cc: Alexander Smirnov, Dmitry Eremin-Solenikov, David S. Miller,
linux-zigbee-devel, netdev, linux-kernel
In-Reply-To: <1354240544-22214-1-git-send-email-alan@signal11.us>
On 11/29/2012 08:55 PM, Alan Ott wrote:
> Change the threshold for framentation of a lowpan packet from
> using the MTU size to now use the MTU size minus the checksum length,
> which is added by the hardware. For IEEE 802.15.4, this effectively
> changes it from 127 bytes to 125 bytes.
>
Sorry, this was put in the wrong thread. One day I'll get one of these
first-try. :(
^ permalink raw reply
* Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc
From: David Woodhouse @ 2012-11-30 1:57 UTC (permalink / raw)
To: Chas Williams (CONTRACTOR)
Cc: Krzysztof Mazur, David Laight, davem, netdev, linux-kernel,
nathan
In-Reply-To: <201211300138.qAU1c8sE003388@thirdoffive.cmf.nrl.navy.mil>
[-- Attachment #1: Type: text/plain, Size: 732 bytes --]
On Thu, 2012-11-29 at 20:38 -0500, Chas Williams (CONTRACTOR) wrote:
> it isnt clear to me that fixes the race entirely either.
> vcc_destroy_socket() and any of the push()/sends()'s are not
> serialized.
> while you may clear the ATM_VF_READY flag, you might not clear it soon
> enough for any particular push() that is already running. so it still
> seems like you are racing close() against push() at this point. the
> window is greatly reduced, but it still exists.
I think it's actually fixed for pppoatm by the bh_lock_sock() and the
sock_owned_by_user() check. As soon as vcc_release() calls lock_sock(),
pppoatm stops accepting packets.
It should be simple enough to do the same in br2684.
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply
* [PATCH] 6lowpan: consider checksum bytes in fragmentation threshold
From: Alan Ott @ 2012-11-30 1:55 UTC (permalink / raw)
To: Alexander Smirnov, Dmitry Eremin-Solenikov, David S. Miller
Cc: linux-zigbee-devel, netdev, linux-kernel, Alan Ott
In-Reply-To: <504EA95C.9010003@signal11.us>
Change the threshold for framentation of a lowpan packet from
using the MTU size to now use the MTU size minus the checksum length,
which is added by the hardware. For IEEE 802.15.4, this effectively
changes it from 127 bytes to 125 bytes.
Signed-off-by: Alan Ott <alan@signal11.us>
---
net/ieee802154/6lowpan.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 6d42c17..f651da6 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -1047,7 +1047,8 @@ static netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
goto error;
}
- if (skb->len <= IEEE802154_MTU) {
+ /* Send directly if less than the MTU minus the 2 checksum bytes. */
+ if (skb->len <= IEEE802154_MTU - IEEE802154_MFR_SIZE) {
err = dev_queue_xmit(skb);
goto out;
}
--
1.7.11.2
^ permalink raw reply related
* Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc
From: Chas Williams (CONTRACTOR) @ 2012-11-30 1:38 UTC (permalink / raw)
To: David Woodhouse
Cc: Krzysztof Mazur, David Laight, davem, netdev, linux-kernel,
nathan
In-Reply-To: <1354227428.21562.230.camel@shinybook.infradead.org>
In message <1354227428.21562.230.camel@shinybook.infradead.org>,David Woodhouse writes:
>At this point, I think we're better off as we are (with Krzysztof's
>patch 1/7 dropped, and leaving vcc->dev->ops->close() being called
>before vcc->push(NULL). We've fairly much solved the issues with that
>arrangement, by checking ATM_VF_READY in the protocols' ->push()
>functions.
it isnt clear to me that fixes the race entirely either.
vcc_destroy_socket() and any of the push()/sends()'s are not serialized.
while you may clear the ATM_VF_READY flag, you might not clear it soon
enough for any particular push() that is already running. so it still
seems like you are racing close() against push() at this point. the
window is greatly reduced, but it still exists.
^ permalink raw reply
* regarding netlink usage
From: i trilok @ 2012-11-30 1:36 UTC (permalink / raw)
To: netdev
Hi,
I was trying to port some module code from 2.6.29 to 2.6.34.10 kernel.
I'm using netlink for communication between kernel module and user
application. In the netlink receive call back function I'm retrieving
netlink message header from (struct nlmsghdr*)skb->data and the same
was working in 2.6.29 kernel but when I ported this to 2.6.34.10 I was
seeing (struct nlmsghdr*)skb->head is holding the netlink message
header instead of skb->data, what could be wrong, is this intended
behavior?.
Thanks in advance.
Thanks,
Ina.
^ permalink raw reply
* Re: [PATCH] br2684: don't send frames on not-ready vcc
From: David Woodhouse @ 2012-11-30 1:34 UTC (permalink / raw)
To: Nathan Williams; +Cc: David Miller, chas, krzysiek, netdev, linux-kernel
In-Reply-To: <1354238285.3128.273.camel@dualcore.traverse>
[-- Attachment #1: Type: text/plain, Size: 716 bytes --]
On Fri, 2012-11-30 at 12:18 +1100, Nathan Williams wrote:
> The customer has confirmed that they haven't seen any panics. I tested
> these patches on OpenWrt with Kernel 3.3.8 and couldn't get a panic:
Thanks.
> I haven't tested these ones:
>
> 230a012 pppoatm: fix missing wakeup in pppoatm_send()
> 1c0c800 atm: Add release_cb() callback to vcc
The race that fixes is fairly unlikely, but if you do see transmit
stalls you can make them apply to your 3.3.8 kernel by applying the same
preliminary patch that is now in the OpenWRT Attitude Adjustment branch:
https://dev.openwrt.org/browser/branches/attitude_adjustment/target/linux/generic/patches-3.3/080-prot-release-cb.patch
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply
* Re: [PATCH] br2684: don't send frames on not-ready vcc
From: Nathan Williams @ 2012-11-30 1:18 UTC (permalink / raw)
To: David Woodhouse; +Cc: David Miller, chas, krzysiek, netdev, linux-kernel
In-Reply-To: <1354122555.21562.92.camel@shinybook.infradead.org>
On Wed, 2012-11-28 at 17:09 +0000, David Woodhouse wrote:
> On Wed, 2012-11-28 at 12:04 -0500, David Miller wrote:
> > Do you want me to pull that tree into net-next or is there a plan to
> > repost the entire series of work for a final submission?
>
> I think it needs a little more testing/consensus first. I'd like an ack
> from Chas on the atm ->release_cb() thing, at least. And I wouldn't mind
> confirmation from Nathan's customer that they're no longer seeing the
> panics.
>
The customer has confirmed that they haven't seen any panics. I tested
these patches on OpenWrt with Kernel 3.3.8 and couldn't get a panic:
c118dc5 solos-pci: Fix leak of skb received for unknown vcc
e539793 br2684: fix module_put() race
3656320 br2684: don't send frames on not-ready vcc
753f920 solos-pci: Wait for pending TX to complete when releasing vcc
91ab2cf pppoatm: do not inline pppoatm_may_send()
85b48fa pppoatm: drop frames to not-ready vcc
3ac1080 pppoatm: take ATM socket lock in pppoatm_send()
e41faed pppoatm: fix module_put() race
3b1a914 pppoatm: allow assign only on a connected socket
ec809bd atm: add owner of push() callback to atmvcc
ae088d6 atm: br2684: Fix excessive queue bloat
I haven't tested these ones:
230a012 pppoatm: fix missing wakeup in pppoatm_send()
1c0c800 atm: Add release_cb() callback to vcc
^ permalink raw reply
* [PATCH 16/17] solos-pci: use GFP_KERNEL where possible, not GFP_ATOMIC
From: David Woodhouse @ 2012-11-30 0:35 UTC (permalink / raw)
To: netdev; +Cc: chas, krzysiek, David Woodhouse
In-Reply-To: <1354235736-26833-1-git-send-email-dwmw2@infradead.org>
From: David Woodhouse <David.Woodhouse@intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
drivers/atm/solos-pci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 7c56286..af19202 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -842,7 +842,7 @@ static int popen(struct atm_vcc *vcc)
return -EINVAL;
}
- skb = alloc_skb(sizeof(*header), GFP_ATOMIC);
+ skb = alloc_skb(sizeof(*header), GFP_KERNEL);
if (!skb) {
if (net_ratelimit())
dev_warn(&card->dev->dev, "Failed to allocate sk_buff in popen()\n");
@@ -882,7 +882,7 @@ static void pclose(struct atm_vcc *vcc)
}
spin_unlock(&card->tx_queue_lock);
- skb = alloc_skb(sizeof(*header), GFP_ATOMIC);
+ skb = alloc_skb(sizeof(*header), GFP_KERNEL);
if (!skb) {
dev_warn(&card->dev->dev, "Failed to allocate sk_buff in pclose()\n");
return;
@@ -1270,7 +1270,7 @@ static int atm_init(struct solos_card *card, struct device *parent)
card->atmdev[i]->phy_data = (void *)(unsigned long)i;
atm_dev_signal_change(card->atmdev[i], ATM_PHY_SIG_FOUND);
- skb = alloc_skb(sizeof(*header), GFP_ATOMIC);
+ skb = alloc_skb(sizeof(*header), GFP_KERNEL);
if (!skb) {
dev_warn(&card->dev->dev, "Failed to allocate sk_buff in atm_init()\n");
continue;
--
1.8.0
^ permalink raw reply related
* [PATCH 06/17] pppoatm: do not inline pppoatm_may_send()
From: David Woodhouse @ 2012-11-30 0:35 UTC (permalink / raw)
To: netdev; +Cc: chas, krzysiek, David Woodhouse
In-Reply-To: <1354235736-26833-1-git-send-email-dwmw2@infradead.org>
From: Krzysztof Mazur <krzysiek@podlesie.net>
The pppoatm_may_send() is quite heavy and it's called three times
in pppoatm_send() and inlining costs more than 200 bytes of code
(more than 10% of total pppoatm driver code size).
add/remove: 1/0 grow/shrink: 0/1 up/down: 132/-367 (-235)
function old new delta
pppoatm_may_send - 132 +132
pppoatm_send 900 533 -367
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
net/atm/pppoatm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index aeb726c..3dce84a 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -214,7 +214,7 @@ error:
ppp_input_error(&pvcc->chan, 0);
}
-static inline int pppoatm_may_send(struct pppoatm_vcc *pvcc, int size)
+static int pppoatm_may_send(struct pppoatm_vcc *pvcc, int size)
{
/*
* It's not clear that we need to bother with using atm_may_send()
--
1.8.0
^ permalink raw reply related
* [PATCH 15/17] solos-pci: clean up pclose() function
From: David Woodhouse @ 2012-11-30 0:35 UTC (permalink / raw)
To: netdev; +Cc: chas, krzysiek, David Woodhouse
In-Reply-To: <1354235736-26833-1-git-send-email-dwmw2@infradead.org>
From: David Woodhouse <David.Woodhouse@intel.com>
- Flush pending TX skbs from the queue rather than waiting for them all to
complete (suggested by Krzysztof Mazur <krzysiek@podlesie.net>).
- Clear ATM_VF_ADDR only when the PKT_PCLOSE packet has been submitted.
- Don't clear ATM_VF_READY at all — vcc_destroy_socket() does that for us.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
drivers/atm/solos-pci.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 6258961..7c56286 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -868,9 +868,20 @@ static int popen(struct atm_vcc *vcc)
static void pclose(struct atm_vcc *vcc)
{
struct solos_card *card = vcc->dev->dev_data;
- struct sk_buff *skb;
+ unsigned char port = SOLOS_CHAN(vcc->dev);
+ struct sk_buff *skb, *tmpskb;
struct pkt_hdr *header;
+ /* Remove any yet-to-be-transmitted packets from the pending queue */
+ spin_lock(&card->tx_queue_lock);
+ skb_queue_walk_safe(&card->tx_queue[port], skb, tmpskb) {
+ if (SKB_CB(skb)->vcc == vcc) {
+ skb_unlink(skb, &card->tx_queue[port]);
+ solos_pop(vcc, skb);
+ }
+ }
+ spin_unlock(&card->tx_queue_lock);
+
skb = alloc_skb(sizeof(*header), GFP_ATOMIC);
if (!skb) {
dev_warn(&card->dev->dev, "Failed to allocate sk_buff in pclose()\n");
@@ -885,20 +896,19 @@ static void pclose(struct atm_vcc *vcc)
init_completion(&SKB_CB(skb)->c);
- fpga_queue(card, SOLOS_CHAN(vcc->dev), skb, NULL);
-
- clear_bit(ATM_VF_ADDR, &vcc->flags);
- clear_bit(ATM_VF_READY, &vcc->flags);
+ fpga_queue(card, port, skb, NULL);
- if (!wait_for_completion_timeout(&SKB_CB(skb)->c,
- msecs_to_jiffies(5000)))
+ if (!wait_for_completion_timeout(&SKB_CB(skb)->c, 5 * HZ))
dev_warn(&card->dev->dev, "Timeout waiting for VCC close on port %d\n",
- SOLOS_CHAN(vcc->dev));
+ port);
/* Hold up vcc_destroy_socket() (our caller) until solos_bh() in the
tasklet has finished processing any incoming packets (and, more to
the point, using the vcc pointer). */
tasklet_unlock_wait(&card->tlet);
+
+ clear_bit(ATM_VF_ADDR, &vcc->flags);
+
return;
}
--
1.8.0
^ permalink raw reply related
* [PATCH 09/17] atm: Add release_cb() callback to vcc
From: David Woodhouse @ 2012-11-30 0:35 UTC (permalink / raw)
To: netdev; +Cc: chas, krzysiek, David Woodhouse
In-Reply-To: <1354235736-26833-1-git-send-email-dwmw2@infradead.org>
From: David Woodhouse <David.Woodhouse@intel.com>
The immediate use case for this is that it will allow us to ensure that a
pppoatm queue is woken after it has to drop a packet due to the sock being
locked.
Note that 'release_cb' is called when the socket is *unlocked*. This is
not to be confused with vcc_release() — which probably ought to be called
vcc_close().
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Acked-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
include/linux/atmdev.h | 1 +
net/atm/common.c | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/include/linux/atmdev.h b/include/linux/atmdev.h
index 72db2af..c1da539 100644
--- a/include/linux/atmdev.h
+++ b/include/linux/atmdev.h
@@ -99,6 +99,7 @@ struct atm_vcc {
struct atm_dev *dev; /* device back pointer */
struct atm_qos qos; /* QOS */
struct atm_sap sap; /* SAP */
+ void (*release_cb)(struct atm_vcc *vcc); /* release_sock callback */
void (*push)(struct atm_vcc *vcc,struct sk_buff *skb);
void (*pop)(struct atm_vcc *vcc,struct sk_buff *skb); /* optional */
int (*push_oam)(struct atm_vcc *vcc,void *cell);
diff --git a/net/atm/common.c b/net/atm/common.c
index 2421664..806fc0a 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -126,10 +126,19 @@ static void vcc_write_space(struct sock *sk)
rcu_read_unlock();
}
+static void vcc_release_cb(struct sock *sk)
+{
+ struct atm_vcc *vcc = atm_sk(sk);
+
+ if (vcc->release_cb)
+ vcc->release_cb(vcc);
+}
+
static struct proto vcc_proto = {
.name = "VCC",
.owner = THIS_MODULE,
.obj_size = sizeof(struct atm_vcc),
+ .release_cb = vcc_release_cb,
};
int vcc_create(struct net *net, struct socket *sock, int protocol, int family)
@@ -158,6 +167,7 @@ int vcc_create(struct net *net, struct socket *sock, int protocol, int family)
vcc->pop = NULL;
vcc->owner = NULL;
vcc->push_oam = NULL;
+ vcc->release_cb = NULL;
vcc->vpi = vcc->vci = 0; /* no VCI/VPI yet */
vcc->atm_options = vcc->aal_options = 0;
sk->sk_destruct = vcc_sock_destruct;
--
1.8.0
^ permalink raw reply related
* [PATCH 12/17] solos-pci: Fix leak of skb received for unknown vcc
From: David Woodhouse @ 2012-11-30 0:35 UTC (permalink / raw)
To: netdev; +Cc: chas, krzysiek, Nathan Williams, David Woodhouse
In-Reply-To: <1354235736-26833-1-git-send-email-dwmw2@infradead.org>
From: Nathan Williams <nathan@traverse.com.au>
... and ensure that the next skb is set up for RX in the DMA case.
Signed-off-by: Nathan Williams <nathan@traverse.com.au>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
drivers/atm/solos-pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 387052d..6258961 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -711,7 +711,8 @@ void solos_bh(unsigned long card_arg)
dev_warn(&card->dev->dev, "Received packet for unknown VPI.VCI %d.%d on port %d\n",
le16_to_cpu(header->vpi), le16_to_cpu(header->vci),
port);
- continue;
+ dev_kfree_skb_any(skb);
+ break;
}
atm_charge(vcc, skb->truesize);
vcc->push(vcc, skb);
--
1.8.0
^ permalink raw reply related
* [PATCH 17/17] solos-pci: remove list_vccs() debugging function
From: David Woodhouse @ 2012-11-30 0:35 UTC (permalink / raw)
To: netdev; +Cc: chas, krzysiek, David Woodhouse
In-Reply-To: <1354235736-26833-1-git-send-email-dwmw2@infradead.org>
From: David Woodhouse <David.Woodhouse@intel.com>
No idea why we've gone so long dumping a list of VCCs with vci==0 on
every ->open() call...
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
drivers/atm/solos-pci.c | 41 -----------------------------------------
1 file changed, 41 deletions(-)
diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index af19202..e59bcfd 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -165,7 +165,6 @@ static void fpga_queue(struct solos_card *card, int port, struct sk_buff *skb,
static uint32_t fpga_tx(struct solos_card *);
static irqreturn_t solos_irq(int irq, void *dev_id);
static struct atm_vcc* find_vcc(struct atm_dev *dev, short vpi, int vci);
-static int list_vccs(int vci);
static int atm_init(struct solos_card *, struct device *);
static void atm_remove(struct solos_card *);
static int send_command(struct solos_card *card, int dev, const char *buf, size_t size);
@@ -792,44 +791,6 @@ static struct atm_vcc *find_vcc(struct atm_dev *dev, short vpi, int vci)
return vcc;
}
-static int list_vccs(int vci)
-{
- struct hlist_head *head;
- struct atm_vcc *vcc;
- struct hlist_node *node;
- struct sock *s;
- int num_found = 0;
- int i;
-
- read_lock(&vcc_sklist_lock);
- if (vci != 0){
- head = &vcc_hash[vci & (VCC_HTABLE_SIZE -1)];
- sk_for_each(s, node, head) {
- num_found ++;
- vcc = atm_sk(s);
- printk(KERN_DEBUG "Device: %d Vpi: %d Vci: %d\n",
- vcc->dev->number,
- vcc->vpi,
- vcc->vci);
- }
- } else {
- for(i = 0; i < VCC_HTABLE_SIZE; i++){
- head = &vcc_hash[i];
- sk_for_each(s, node, head) {
- num_found ++;
- vcc = atm_sk(s);
- printk(KERN_DEBUG "Device: %d Vpi: %d Vci: %d\n",
- vcc->dev->number,
- vcc->vpi,
- vcc->vci);
- }
- }
- }
- read_unlock(&vcc_sklist_lock);
- return num_found;
-}
-
-
static int popen(struct atm_vcc *vcc)
{
struct solos_card *card = vcc->dev->dev_data;
@@ -859,8 +820,6 @@ static int popen(struct atm_vcc *vcc)
set_bit(ATM_VF_ADDR, &vcc->flags);
set_bit(ATM_VF_READY, &vcc->flags);
- list_vccs(0);
-
return 0;
}
--
1.8.0
^ permalink raw reply related
* [PATCH 03/17] pppoatm: fix module_put() race
From: David Woodhouse @ 2012-11-30 0:35 UTC (permalink / raw)
To: netdev; +Cc: chas, krzysiek, David Woodhouse
In-Reply-To: <1354235736-26833-1-git-send-email-dwmw2@infradead.org>
From: Krzysztof Mazur <krzysiek@podlesie.net>
The pppoatm used module_put() during unassignment from vcc with
hope that we have BKL. This assumption is no longer true.
Now owner field in atmvcc is used to move this module_put()
to vcc_destroy_socket().
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
net/atm/pppoatm.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index f27a07a..b23c672 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -60,6 +60,7 @@ struct pppoatm_vcc {
struct atm_vcc *atmvcc; /* VCC descriptor */
void (*old_push)(struct atm_vcc *, struct sk_buff *);
void (*old_pop)(struct atm_vcc *, struct sk_buff *);
+ struct module *old_owner;
/* keep old push/pop for detaching */
enum pppoatm_encaps encaps;
atomic_t inflight;
@@ -155,8 +156,6 @@ static void pppoatm_unassign_vcc(struct atm_vcc *atmvcc)
ppp_unregister_channel(&pvcc->chan);
atmvcc->user_back = NULL;
kfree(pvcc);
- /* Gee, I hope we have the big kernel lock here... */
- module_put(THIS_MODULE);
}
/* Called when an AAL5 PDU comes in */
@@ -165,9 +164,13 @@ static void pppoatm_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
struct pppoatm_vcc *pvcc = atmvcc_to_pvcc(atmvcc);
pr_debug("\n");
if (skb == NULL) { /* VCC was closed */
+ struct module *module;
+
pr_debug("removing ATMPPP VCC %p\n", pvcc);
+ module = pvcc->old_owner;
pppoatm_unassign_vcc(atmvcc);
atmvcc->push(atmvcc, NULL); /* Pass along bad news */
+ module_put(module);
return;
}
atm_return(atmvcc, skb->truesize);
@@ -362,6 +365,7 @@ static int pppoatm_assign_vcc(struct atm_vcc *atmvcc, void __user *arg)
atomic_set(&pvcc->inflight, NONE_INFLIGHT);
pvcc->old_push = atmvcc->push;
pvcc->old_pop = atmvcc->pop;
+ pvcc->old_owner = atmvcc->owner;
pvcc->encaps = (enum pppoatm_encaps) be.encaps;
pvcc->chan.private = pvcc;
pvcc->chan.ops = &pppoatm_ops;
@@ -378,6 +382,7 @@ static int pppoatm_assign_vcc(struct atm_vcc *atmvcc, void __user *arg)
atmvcc->push = pppoatm_push;
atmvcc->pop = pppoatm_pop;
__module_get(THIS_MODULE);
+ atmvcc->owner = THIS_MODULE;
/* re-process everything received between connection setup and
backend setup */
--
1.8.0
^ permalink raw reply related
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