* [PATCH 06/14] netpoll: take rcu_read_lock_bh() in netpoll_send_skb_on_dev()
From: Cong Wang @ 2012-08-09 15:00 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David S. Miller
In-Reply-To: <1344524426-11588-1-git-send-email-amwang@redhat.com>
This patch fixes several problems in the call path of
netpoll_send_skb_on_dev():
1. Disable IRQ's before calling netpoll_send_skb_on_dev().
2. All the callees of netpoll_send_skb_on_dev() should use
rcu_dereference_bh() to dereference ->npinfo.
3. Rename arp_reply() to netpoll_arp_reply(), the former is too generic.
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
include/linux/netpoll.h | 3 +++
net/core/netpoll.c | 31 +++++++++++++++++--------------
2 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 2d178ba..61aee86 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -57,7 +57,10 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
struct net_device *dev);
static inline void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
{
+ unsigned long flags;
+ local_irq_save(flags);
netpoll_send_skb_on_dev(np, skb, np->dev);
+ local_irq_restore(flags);
}
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index d055bb0..174346a 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -54,7 +54,7 @@ static atomic_t trapped;
MAX_UDP_CHUNK)
static void zap_completion_queue(void);
-static void arp_reply(struct sk_buff *skb);
+static void netpoll_arp_reply(struct sk_buff *skb, struct netpoll_info *npinfo);
static unsigned int carrier_timeout = 4;
module_param(carrier_timeout, uint, 0644);
@@ -170,7 +170,8 @@ static void poll_napi(struct net_device *dev)
list_for_each_entry(napi, &dev->napi_list, dev_list) {
if (napi->poll_owner != smp_processor_id() &&
spin_trylock(&napi->poll_lock)) {
- budget = poll_one_napi(dev->npinfo, napi, budget);
+ budget = poll_one_napi(rcu_dereference_bh(dev->npinfo),
+ napi, budget);
spin_unlock(&napi->poll_lock);
if (!budget)
@@ -185,13 +186,14 @@ static void service_arp_queue(struct netpoll_info *npi)
struct sk_buff *skb;
while ((skb = skb_dequeue(&npi->arp_tx)))
- arp_reply(skb);
+ netpoll_arp_reply(skb, npi);
}
}
static void netpoll_poll_dev(struct net_device *dev)
{
const struct net_device_ops *ops;
+ struct netpoll_info *ni = rcu_dereference_bh(dev->npinfo);
if (!dev || !netif_running(dev))
return;
@@ -206,17 +208,18 @@ static void netpoll_poll_dev(struct net_device *dev)
poll_napi(dev);
if (dev->flags & IFF_SLAVE) {
- if (dev->npinfo) {
+ if (ni) {
struct net_device *bond_dev = dev->master;
struct sk_buff *skb;
- while ((skb = skb_dequeue(&dev->npinfo->arp_tx))) {
+ struct netpoll_info *bond_ni = rcu_dereference_bh(bond_dev->npinfo);
+ while ((skb = skb_dequeue(&ni->arp_tx))) {
skb->dev = bond_dev;
- skb_queue_tail(&bond_dev->npinfo->arp_tx, skb);
+ skb_queue_tail(&bond_ni->arp_tx, skb);
}
}
}
- service_arp_queue(dev->npinfo);
+ service_arp_queue(ni);
zap_completion_queue();
}
@@ -302,6 +305,7 @@ static int netpoll_owner_active(struct net_device *dev)
return 0;
}
+/* call with IRQ disabled */
void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
struct net_device *dev)
{
@@ -309,8 +313,11 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
unsigned long tries;
const struct net_device_ops *ops = dev->netdev_ops;
/* It is up to the caller to keep npinfo alive. */
- struct netpoll_info *npinfo = np->dev->npinfo;
+ struct netpoll_info *npinfo;
+
+ WARN_ON_ONCE(!irqs_disabled());
+ npinfo = rcu_dereference_bh(np->dev->npinfo);
if (!npinfo || !netif_running(dev) || !netif_device_present(dev)) {
__kfree_skb(skb);
return;
@@ -319,11 +326,9 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
/* don't get messages out of order, and no recursion */
if (skb_queue_len(&npinfo->txq) == 0 && !netpoll_owner_active(dev)) {
struct netdev_queue *txq;
- unsigned long flags;
txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
- local_irq_save(flags);
/* try until next clock tick */
for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
tries > 0; --tries) {
@@ -347,10 +352,9 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
}
WARN_ONCE(!irqs_disabled(),
- "netpoll_send_skb(): %s enabled interrupts in poll (%pF)\n",
+ "netpoll_send_skb_on_dev(): %s enabled interrupts in poll (%pF)\n",
dev->name, ops->ndo_start_xmit);
- local_irq_restore(flags);
}
if (status != NETDEV_TX_OK) {
@@ -423,9 +427,8 @@ void netpoll_send_udp(struct netpoll *np, const char *msg, int len)
}
EXPORT_SYMBOL(netpoll_send_udp);
-static void arp_reply(struct sk_buff *skb)
+static void netpoll_arp_reply(struct sk_buff *skb, struct netpoll_info *npinfo)
{
- struct netpoll_info *npinfo = skb->dev->npinfo;
struct arphdr *arp;
unsigned char *arp_ptr;
int size, type = ARPOP_REPLY, ptype = ETH_P_ARP;
--
1.7.7.6
^ permalink raw reply related
* [PATCH 07/14] bridge: add some comments for NETDEV_RELEASE
From: Cong Wang @ 2012-08-09 15:00 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David Miller, Stephen Hemminger
In-Reply-To: <1344524426-11588-1-git-send-email-amwang@redhat.com>
Add comments on why we don't notify NETDEV_RELEASE.
Cc: David Miller <davem@davemloft.net>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/bridge/br_if.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 171fd6b..bf47d4f 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -427,6 +427,9 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
if (!p || p->br != br)
return -EINVAL;
+ /* We don't notify NETDEV_RELEASE event, as this will
+ * stop netconsole on the bridge.
+ */
del_nbp(p);
spin_lock_bh(&br->lock);
--
1.7.7.6
^ permalink raw reply related
* [PATCH 02/14] netpoll: make __netpoll_cleanup non-block
From: Cong Wang @ 2012-08-09 15:00 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David S. Miller
In-Reply-To: <1344524426-11588-1-git-send-email-amwang@redhat.com>
Like the previous patch, slave_disable_netpoll() and __netpoll_cleanup()
may be called with read_lock() held too, so we should make them
non-block, by moving the cleanup and kfree() to call_rcu_bh() callbacks.
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
drivers/net/bonding/bond_main.c | 4 +--
include/linux/netpoll.h | 3 ++
net/8021q/vlan_dev.c | 6 +----
net/bridge/br_device.c | 6 +----
net/core/netpoll.c | 42 +++++++++++++++++++++++++++++---------
5 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8697136..e428916 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1257,9 +1257,7 @@ static inline void slave_disable_netpoll(struct slave *slave)
return;
slave->np = NULL;
- synchronize_rcu_bh();
- __netpoll_cleanup(np);
- kfree(np);
+ __netpoll_free_rcu(np);
}
static inline bool slave_dev_support_netpoll(struct net_device *slave_dev)
{
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index bf2d51e..907812e 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -23,6 +23,7 @@ struct netpoll {
u8 remote_mac[ETH_ALEN];
struct list_head rx; /* rx_np list element */
+ struct rcu_head rcu;
};
struct netpoll_info {
@@ -38,6 +39,7 @@ struct netpoll_info {
struct delayed_work tx_work;
struct netpoll *netpoll;
+ struct rcu_head rcu;
};
void netpoll_send_udp(struct netpoll *np, const char *msg, int len);
@@ -48,6 +50,7 @@ int netpoll_setup(struct netpoll *np);
int netpoll_trap(void);
void netpoll_set_trap(int trap);
void __netpoll_cleanup(struct netpoll *np);
+void __netpoll_free_rcu(struct netpoll *np);
void netpoll_cleanup(struct netpoll *np);
int __netpoll_rx(struct sk_buff *skb);
void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index ee4ae09..b65623f 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -704,11 +704,7 @@ static void vlan_dev_netpoll_cleanup(struct net_device *dev)
info->netpoll = NULL;
- /* Wait for transmitting packets to finish before freeing. */
- synchronize_rcu_bh();
-
- __netpoll_cleanup(netpoll);
- kfree(netpoll);
+ __netpoll_free_rcu(netpoll);
}
#endif /* CONFIG_NET_POLL_CONTROLLER */
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index ed0e0f9..f41ba40 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -267,11 +267,7 @@ void br_netpoll_disable(struct net_bridge_port *p)
p->np = NULL;
- /* Wait for transmitting packets to finish before freeing. */
- synchronize_rcu_bh();
-
- __netpoll_cleanup(np);
- kfree(np);
+ __netpoll_free_rcu(np);
}
#endif
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 37cc854..dc17f1d 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -878,6 +878,24 @@ static int __init netpoll_init(void)
}
core_initcall(netpoll_init);
+static void rcu_cleanup_netpoll_info(struct rcu_head *rcu_head)
+{
+ struct netpoll_info *npinfo =
+ container_of(rcu_head, struct netpoll_info, rcu);
+
+ skb_queue_purge(&npinfo->arp_tx);
+ skb_queue_purge(&npinfo->txq);
+
+ /* we can't call cancel_delayed_work_sync here, as we are in softirq */
+ cancel_delayed_work(&npinfo->tx_work);
+
+ /* clean after last, unfinished work */
+ __skb_queue_purge(&npinfo->txq);
+ /* now cancel it again */
+ cancel_delayed_work(&npinfo->tx_work);
+ kfree(npinfo);
+}
+
void __netpoll_cleanup(struct netpoll *np)
{
struct netpoll_info *npinfo;
@@ -903,20 +921,24 @@ void __netpoll_cleanup(struct netpoll *np)
ops->ndo_netpoll_cleanup(np->dev);
RCU_INIT_POINTER(np->dev->npinfo, NULL);
+ call_rcu_bh(&npinfo->rcu, rcu_cleanup_netpoll_info);
+ }
+}
+EXPORT_SYMBOL_GPL(__netpoll_cleanup);
- /* avoid racing with NAPI reading npinfo */
- synchronize_rcu_bh();
+static void rcu_cleanup_netpoll(struct rcu_head *rcu_head)
+{
+ struct netpoll *np = container_of(rcu_head, struct netpoll, rcu);
- skb_queue_purge(&npinfo->arp_tx);
- skb_queue_purge(&npinfo->txq);
- cancel_delayed_work_sync(&npinfo->tx_work);
+ __netpoll_cleanup(np);
+ kfree(np);
+}
- /* clean after last, unfinished work */
- __skb_queue_purge(&npinfo->txq);
- kfree(npinfo);
- }
+void __netpoll_free_rcu(struct netpoll *np)
+{
+ call_rcu_bh(&np->rcu, rcu_cleanup_netpoll);
}
-EXPORT_SYMBOL_GPL(__netpoll_cleanup);
+EXPORT_SYMBOL_GPL(__netpoll_free_rcu);
void netpoll_cleanup(struct netpoll *np)
{
--
1.7.7.6
^ permalink raw reply related
* [PATCH 08/14] bridge: use list_for_each_entry() in netpoll functions
From: Cong Wang @ 2012-08-09 15:00 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David Miller, Stephen Hemminger
In-Reply-To: <1344524426-11588-1-git-send-email-amwang@redhat.com>
We don't delete 'p' from the list in the loop,
so we can just use list_for_each_entry().
Cc: David Miller <davem@davemloft.net>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/bridge/br_device.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index f41ba40..32211fa 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -206,21 +206,20 @@ static void br_poll_controller(struct net_device *br_dev)
static void br_netpoll_cleanup(struct net_device *dev)
{
struct net_bridge *br = netdev_priv(dev);
- struct net_bridge_port *p, *n;
+ struct net_bridge_port *p;
- list_for_each_entry_safe(p, n, &br->port_list, list) {
+ list_for_each_entry(p, &br->port_list, list)
br_netpoll_disable(p);
- }
}
static int br_netpoll_setup(struct net_device *dev, struct netpoll_info *ni,
gfp_t gfp)
{
struct net_bridge *br = netdev_priv(dev);
- struct net_bridge_port *p, *n;
+ struct net_bridge_port *p;
int err = 0;
- list_for_each_entry_safe(p, n, &br->port_list, list) {
+ list_for_each_entry(p, &br->port_list, list) {
if (!p->dev)
continue;
err = br_netpoll_enable(p, gfp);
--
1.7.7.6
^ permalink raw reply related
* [PATCH 09/14] netpoll: check netpoll tx status on the right device
From: Cong Wang @ 2012-08-09 15:00 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David Miller
In-Reply-To: <1344524426-11588-1-git-send-email-amwang@redhat.com>
Although this doesn't matter actually, because netpoll_tx_running()
doesn't use the parameter, the code will be more readable.
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
drivers/net/bonding/bond_main.c | 2 +-
include/linux/if_team.h | 2 +-
net/bridge/br_forward.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e428916..d688a8a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -398,7 +398,7 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
sizeof(qdisc_skb_cb(skb)->slave_dev_queue_mapping));
skb->queue_mapping = qdisc_skb_cb(skb)->slave_dev_queue_mapping;
- if (unlikely(netpoll_tx_running(slave_dev)))
+ if (unlikely(netpoll_tx_running(bond->dev)))
bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
else
dev_queue_xmit(skb);
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index 33fcc20..a55df56 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -107,7 +107,7 @@ static inline int team_dev_queue_xmit(struct team *team, struct team_port *port,
skb_set_queue_mapping(skb, qdisc_skb_cb(skb)->slave_dev_queue_mapping);
skb->dev = port->dev;
- if (unlikely(netpoll_tx_running(port->dev))) {
+ if (unlikely(netpoll_tx_running(team->dev))) {
team_netpoll_send_skb(port, skb);
return 0;
}
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index e9466d4..02015a5 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -65,7 +65,7 @@ static void __br_deliver(const struct net_bridge_port *to, struct sk_buff *skb)
{
skb->dev = to->dev;
- if (unlikely(netpoll_tx_running(to->dev))) {
+ if (unlikely(netpoll_tx_running(to->br->dev))) {
if (packet_length(skb) > skb->dev->mtu && !skb_is_gso(skb))
kfree_skb(skb);
else {
--
1.7.7.6
^ permalink raw reply related
* [PATCH 10/14] netpoll: convert several functions to bool
From: Cong Wang @ 2012-08-09 15:00 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David Miller
In-Reply-To: <1344524426-11588-1-git-send-email-amwang@redhat.com>
These functions are just boolean, let them return
bool instead of int.
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
include/linux/netpoll.h | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 61aee86..66d5379 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -66,7 +66,7 @@ static inline void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
#ifdef CONFIG_NETPOLL
-static inline int netpoll_rx_on(struct sk_buff *skb)
+static inline bool netpoll_rx_on(struct sk_buff *skb)
{
struct netpoll_info *npinfo = rcu_dereference_bh(skb->dev->npinfo);
@@ -125,7 +125,7 @@ static inline void netpoll_poll_unlock(void *have)
}
}
-static inline int netpoll_tx_running(struct net_device *dev)
+static inline bool netpoll_tx_running(struct net_device *dev)
{
return irqs_disabled();
}
@@ -133,11 +133,11 @@ static inline int netpoll_tx_running(struct net_device *dev)
#else
static inline bool netpoll_rx(struct sk_buff *skb)
{
- return 0;
+ return false;
}
-static inline int netpoll_rx_on(struct sk_buff *skb)
+static inline bool netpoll_rx_on(struct sk_buff *skb)
{
- return 0;
+ return false;
}
static inline int netpoll_receive_skb(struct sk_buff *skb)
{
@@ -153,9 +153,9 @@ static inline void netpoll_poll_unlock(void *have)
static inline void netpoll_netdev_init(struct net_device *dev)
{
}
-static inline int netpoll_tx_running(struct net_device *dev)
+static inline bool netpoll_tx_running(struct net_device *dev)
{
- return 0;
+ return false;
}
#endif
--
1.7.7.6
^ permalink raw reply related
* [PATCH 11/14] vlan: clean up vlan_dev_hard_start_xmit()
From: Cong Wang @ 2012-08-09 15:00 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Benjamin LaHaise, Patrick McHardy, David Miller
In-Reply-To: <1344524426-11588-1-git-send-email-amwang@redhat.com>
Clean up vlan_dev_hard_start_xmit() function.
Cc: Benjamin LaHaise <bcrl@kvack.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/8021q/vlan_dev.c | 27 ++++++++++++++++++++-------
1 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index b65623f..dedc56c 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -137,9 +137,21 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
return rc;
}
+static inline netdev_tx_t vlan_netpoll_send_skb(struct vlan_dev_priv *info, struct sk_buff *skb)
+{
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ if (info->netpoll)
+ netpoll_send_skb(info->netpoll, skb);
+#else
+ BUG();
+#endif
+ return NETDEV_TX_OK;
+}
+
static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
struct net_device *dev)
{
+ struct vlan_dev_priv *info = vlan_dev_priv(dev);
struct vlan_ethhdr *veth = (struct vlan_ethhdr *)(skb->data);
unsigned int len;
int ret;
@@ -150,29 +162,30 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
* OTHER THINGS LIKE FDDI/TokenRing/802.3 SNAPs...
*/
if (veth->h_vlan_proto != htons(ETH_P_8021Q) ||
- vlan_dev_priv(dev)->flags & VLAN_FLAG_REORDER_HDR) {
+ info->flags & VLAN_FLAG_REORDER_HDR) {
u16 vlan_tci;
- vlan_tci = vlan_dev_priv(dev)->vlan_id;
+ vlan_tci = info->vlan_id;
vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb);
skb = __vlan_hwaccel_put_tag(skb, vlan_tci);
}
- skb->dev = vlan_dev_priv(dev)->real_dev;
+ skb->dev = info->real_dev;
len = skb->len;
- if (netpoll_tx_running(dev))
- return skb->dev->netdev_ops->ndo_start_xmit(skb, skb->dev);
+ if (unlikely(netpoll_tx_running(dev)))
+ return vlan_netpoll_send_skb(info, skb);
+
ret = dev_queue_xmit(skb);
if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN)) {
struct vlan_pcpu_stats *stats;
- stats = this_cpu_ptr(vlan_dev_priv(dev)->vlan_pcpu_stats);
+ stats = this_cpu_ptr(info->vlan_pcpu_stats);
u64_stats_update_begin(&stats->syncp);
stats->tx_packets++;
stats->tx_bytes += len;
u64_stats_update_end(&stats->syncp);
} else {
- this_cpu_inc(vlan_dev_priv(dev)->vlan_pcpu_stats->tx_dropped);
+ this_cpu_inc(info->vlan_pcpu_stats->tx_dropped);
}
return ret;
--
1.7.7.6
^ permalink raw reply related
* [PATCH 13/14] netpoll: re-enable irq in poll_napi()
From: Cong Wang @ 2012-08-09 15:00 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David Miller
In-Reply-To: <1344524426-11588-1-git-send-email-amwang@redhat.com>
napi->poll() needs IRQ enabled, so we have to re-enable IRQ before
calling it.
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/core/netpoll.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index e4ba3e7..346b1eb 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -168,16 +168,24 @@ static void poll_napi(struct net_device *dev)
struct napi_struct *napi;
int budget = 16;
+ WARN_ON_ONCE(!irqs_disabled());
+
list_for_each_entry(napi, &dev->napi_list, dev_list) {
+ local_irq_enable();
if (napi->poll_owner != smp_processor_id() &&
spin_trylock(&napi->poll_lock)) {
+ rcu_read_lock_bh();
budget = poll_one_napi(rcu_dereference_bh(dev->npinfo),
napi, budget);
+ rcu_read_unlock_bh();
spin_unlock(&napi->poll_lock);
- if (!budget)
+ if (!budget) {
+ local_irq_disable();
break;
+ }
}
+ local_irq_disable();
}
}
--
1.7.7.6
^ permalink raw reply related
* [PATCH 12/14] netpoll: handle vlan tags in netpoll tx and rx path
From: Cong Wang @ 2012-08-09 15:00 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Benjamin LaHaise, Patrick McHardy, David Miller
In-Reply-To: <1344524426-11588-1-git-send-email-amwang@redhat.com>
Without this patch, I can't get netconsole logs remotely over
vlan. The reason is probably we don't handle vlan tags in either
netpoll tx or rx path.
I am not sure if I use these vlan functions correctly, at
least this patch works.
Cc: Benjamin LaHaise <bcrl@kvack.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/core/netpoll.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 174346a..e4ba3e7 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -26,6 +26,7 @@
#include <linux/workqueue.h>
#include <linux/slab.h>
#include <linux/export.h>
+#include <linux/if_vlan.h>
#include <net/tcp.h>
#include <net/udp.h>
#include <asm/unaligned.h>
@@ -334,6 +335,14 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
tries > 0; --tries) {
if (__netif_tx_trylock(txq)) {
if (!netif_xmit_stopped(txq)) {
+ if (vlan_tx_tag_present(skb) &&
+ !(netif_skb_features(skb) & NETIF_F_HW_VLAN_TX)) {
+ skb = __vlan_put_tag(skb, vlan_tx_tag_get(skb));
+ if (unlikely(!skb))
+ break;
+ skb->vlan_tci = 0;
+ }
+
status = ops->ndo_start_xmit(skb, dev);
if (status == NETDEV_TX_OK)
txq_trans_update(txq);
@@ -567,6 +576,12 @@ int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
return 1;
}
+ if (skb->protocol == cpu_to_be16(ETH_P_8021Q)) {
+ skb = vlan_untag(skb);
+ if (unlikely(!skb))
+ goto out;
+ }
+
proto = ntohs(eth_hdr(skb)->h_proto);
if (proto != ETH_P_IP)
goto out;
--
1.7.7.6
^ permalink raw reply related
* [PATCH 14/14] netconsole.txt: revision of examples for the receiver of kernel messages
From: Cong Wang @ 2012-08-09 15:00 UTC (permalink / raw)
To: netdev; +Cc: Dirk Gouders, Cong Wang
In-Reply-To: <1344524426-11588-1-git-send-email-amwang@redhat.com>
From: Dirk Gouders <gouders@et.bocholt.fh-gelsenkirchen.de>
There are at least 4 implementations of netcat with the BSD-based
being the only one that has to be used without the -p switch to
specify the listening port.
Jan Engelhardt suggested to add an example for socat(1).
Signed-off-by: Dirk Gouders <gouders@et.bocholt.fh-gelsenkirchen.de>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
Documentation/networking/netconsole.txt | 19 +++++++++++++++++--
1 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/netconsole.txt b/Documentation/networking/netconsole.txt
index 8d02207..2e9e0ae2 100644
--- a/Documentation/networking/netconsole.txt
+++ b/Documentation/networking/netconsole.txt
@@ -51,8 +51,23 @@ Built-in netconsole starts immediately after the TCP stack is
initialized and attempts to bring up the supplied dev at the supplied
address.
-The remote host can run either 'netcat -u -l -p <port>',
-'nc -l -u <port>' or syslogd.
+The remote host has several options to receive the kernel messages,
+for example:
+
+1) syslogd
+
+2) netcat
+
+ On distributions using a BSD-based netcat version (e.g. Fedora,
+ openSUSE and Ubuntu) the listening port must be specified without
+ the -p switch:
+
+ 'nc -u -l -p <port>' / 'nc -u -l <port>' or
+ 'netcat -u -l -p <port>' / 'netcat -u -l <port>'
+
+3) socat
+
+ 'socat udp-recv:<port> -'
Dynamic reconfiguration:
========================
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH] ipv4: tcp: security_sk_alloc() needed for unicast_sock
From: Paul Moore @ 2012-08-09 15:07 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Casey Schaufler, Eric Paris, John Stultz,
Serge E. Hallyn, lkml, James Morris, selinux, john.johansen, LSM,
netdev
In-Reply-To: <1344523833.28967.996.camel@edumazet-glaptop>
On Thursday, August 09, 2012 04:50:33 PM Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> commit be9f4a44e7d41cee (ipv4: tcp: remove per net tcp_sock) added a
> selinux regression, reported and bisected by John Stultz
>
> selinux_ip_postroute_compat() expect to find a valid sk->sk_security
> pointer, but this field is NULL for unicast_sock
>
> Fix this by adding a new 'kernel' parameter to security_sk_alloc(),
> set to true if socket might already have a valid sk->sk_security
> pointer. ip_send_unicast_reply() uses a percpu fake socket, so the first
> call to security_sk_alloc() will populate sk->sk_security pointer,
> subsequent ones will reuse existing context.
>
> Reported-by: John Stultz <johnstul@us.ibm.com>
> Bisected-by: John Stultz <johnstul@us.ibm.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Eric Paris <eparis@parisplace.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
...
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 76dde25..b233d6e 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -1524,6 +1524,8 @@ void ip_send_unicast_reply(struct net *net, struct
> sk_buff *skb, __be32 daddr, sk->sk_priority = skb->priority;
> sk->sk_protocol = ip_hdr(skb)->protocol;
> sk->sk_bound_dev_if = arg->bound_dev_if;
> + if (security_sk_alloc(sk, PF_INET, GFP_ATOMIC, true))
> + goto out;
> sock_net_set(sk, net);
> __skb_queue_head_init(&sk->sk_write_queue);
> sk->sk_sndbuf = sysctl_wmem_default;
Is is possible to do the call to security_sk_alloc() in the ip_init() function
or does the per-cpu nature of the socket make this a pain?
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [RFC net-next 0/4] gianfar: Use separate NAPI for Tx confirmation processing
From: Claudiu Manoil @ 2012-08-09 15:07 UTC (permalink / raw)
To: Tomas Hruby, Eric Dumazet, Paul Gortmaker; +Cc: netdev, David S. Miller
In-Reply-To: <CAOy7UX2AW2w8TE0=7HGqRDJnX3Te5XZmW8UfC8F6HZg1RdWCYw@mail.gmail.com>
On 8/9/2012 2:06 AM, Tomas Hruby wrote:
> On Wed, Aug 8, 2012 at 9:44 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Wed, 2012-08-08 at 12:24 -0400, Paul Gortmaker wrote:
>>> [[RFC net-next 0/4] gianfar: Use separate NAPI for Tx confirmation processing] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:
>>>
>>>> Hi all,
>>>> This set of patches basically splits the existing napi poll routine into
>>>> two separate napi functions, one for Rx processing (triggered by frame
>>>> receive interrupts only) and one for the Tx confirmation path processing
>>>> (triggerred by Tx confirmation interrupts only). The polling algorithm
>>>> behind remains much the same.
>>>>
>>>> Important throughput improvements have been noted on low power boards with
>>>> this set of changes.
>>>> For instance, for the following netperf test:
>>>> netperf -l 20 -cC -H 192.168.10.1 -t TCP_STREAM -- -m 1500
>>>> yields a throughput gain from oscilating ~500-~700 Mbps to steady ~940 Mbps,
>>>> (if the Rx/Tx paths are processed on different cores), w/ no increase in CPU%,
>>>> on a p1020rdb - 2 core machine featuring etsec2.0 (Multi-Queue Multi-Group
>>>> driver mode).
>>>
>>> It would be interesting to know more about what was causing that large
>>> an oscillation -- presumably you will have it reappear once one core
>>> becomes 100% utilized. Also, any thoughts on how the change will change
>>> performance on an older low power single core gianfar system (e.g. 83xx)?
>>
>> I also was wondering if this low performance could be caused by BQL
>>
>> Since TCP stack is driven by incoming ACKS, a NAPI run could have to
>> handle 10 TCP acks in a row, and resulting xmits could hit BQL and
>> transit on qdisc (Because NAPI handler wont handle TX completions in the
>> middle of RX handler)
>
> Does disabling BQL help? Is the BQL limit stable? To what value is it
> set? I would be very much interested in more data if the issue is BQL
> related.
>
> .
>
I agree that more tests should be run to investigate why gianfar under-
performs on the low power p1020rdb platform, and BQL seems to be
a good starting point (thanks for the hint). What I can say now is that
the issue is not apparent on p2020rdb, for instance, which is a more
powerful platform: the CPUs - 1200 MHz instead of 800 MHz; twice the
size of L2 cache (512 KB), greater bus (CCB) frequency ... On this
board (p2020rdb) the netperf test reaches 940Mbps both w/ and w/o these
patches.
For a single core system I'm not expecting any performance degradation,
simply because I don't see why the proposed napi poll implementation
would be slower than the existing one. I'll do some measurements on a
p1010rdb too (single core, CPU:800 MHz) and get back to you with the
results.
Thanks.
Claudiu
^ permalink raw reply
* Re: [net-next RFC V5 3/5] virtio: intorduce an API to set affinity for a virtqueue
From: Paolo Bonzini @ 2012-08-09 15:13 UTC (permalink / raw)
To: Jason Wang
Cc: krkumar2, habanero, mashirle, kvm, mst, netdev, linux-kernel,
virtualization, edumazet, tahm, jwhan, davem, sri
In-Reply-To: <1341484194-8108-4-git-send-email-jasowang@redhat.com>
Il 05/07/2012 12:29, Jason Wang ha scritto:
> Sometimes, virtio device need to configure irq affiniry hint to maximize the
> performance. Instead of just exposing the irq of a virtqueue, this patch
> introduce an API to set the affinity for a virtqueue.
>
> The api is best-effort, the affinity hint may not be set as expected due to
> platform support, irq sharing or irq type. Currently, only pci method were
> implemented and we set the affinity according to:
>
> - if device uses INTX, we just ignore the request
> - if device has per vq vector, we force the affinity hint
> - if the virtqueues share MSI, make the affinity OR over all affinities
> requested
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
It looks like both I and Jason will need these patches during the 3.7
merge window, and from different trees (net-next vs. scsi). How do we
synchronize?
Paolo
^ permalink raw reply
* Re: [net-next RFC V5 3/5] virtio: intorduce an API to set affinity for a virtqueue
From: Paolo Bonzini @ 2012-08-09 15:14 UTC (permalink / raw)
Cc: krkumar2, habanero, kvm, Michael S. Tsirkin, netdev, mashirle,
linux-kernel, virtualization, edumazet, tahm, jwhan, davem, sri
In-Reply-To: <5016296A.5070809@redhat.com>
Il 30/07/2012 08:27, Paolo Bonzini ha scritto:
>>>> >> > Did you set the affinity manually in your experiments, or perhaps there
>>>> >> > is a difference between scsi and networking... (interrupt mitigation?)
>> >
>> > You need to run irqbalancer in guest to make it actually work. Do you?
> Yes, of course, now on to debugging that one. I just wanted to ask
> before the weekend if I was missing something as obvious as that.
It was indeed an irqbalance bug, it is fixed now upstream.
Paolo
^ permalink raw reply
* Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
From: Ian Molton @ 2012-08-09 15:21 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, thomas.petazzoni, andrew, netdev,
devicetree-discuss, ben.dooks, dale, linuxppc-dev, David Miller
In-Reply-To: <201208091143.32972.arnd@arndb.de>
On 09/08/12 12:43, Arnd Bergmann wrote:
> On 08/08/12 14:19, Ian Molton wrote:
> > On 08/08/12 13:39, Arnd Bergmann wrote:
> >> On Wednesday 08 August 2012, Ian Molton wrote:
> >>> This method would require a small amount of rework in the driver to
> >>> set up <n> ports, rather than just one.
> >> This looks quite nice, but it is still very much incompatible with the
> >> existing binding. Obviously we can abandon an existing binding and
> >> introduce a second one for the same hardware, but that should not
> >> be taken lightly.
> > Fair, however the existing users aren't anywhere near as
> > numerous as the new ones.
>
> Depends on how you count the numbers. I see at least three machines
> supported in the kernel with the old binding and none with the new one
> so far ;-)
I'm curious as to how any of those actually work, given the
apparent total lack of a mv64360-mdio device binding...
> > As you can see, instead of putting port1 at +0x1700 or so,
> > marvell have overlapped the register files - in fact, doubly
> > so, since port1 + 0x1080 is right in the middle of
> > (port0 + 0x1000) -> (port0 + 0x16ff), so one cant simply map two
> > sets of regs like 0x0000->0x03ff and 0x1000->0x16ff for port one
> > either.
>
> This could theoretically be dealt with by having 5 register ranges
I make that three...
> per device, but that would cause extra overhead and also be
> incompatible with the existing binding.
Indeed.
> I think showing one
> parent device with children at address 0, 1 and 2 is ok.
Is it acceptable for the child devices to directly access the
parents register space? because there would be no other
way for that to work.
> The driver
> already knows all those offsets and they are always the same
> for all variants of mv643xx, right?
Yes, but its not clean. And no amount of refactoring is
really going to make a nice driver that also fits the ancient
(and badly thought out) OF bindings.
If we have to break things, we can at least go for a nice
clean design, surely?
The ports arent really child devices of the MAC. The MAC
just has 3 ports.
Luckily, it looks like the existing users don't actually use
the device tree to set up the driver at all, preferring to
translate their D-T bindings to calls to
platform_device_register() so all we'd need to do to
support them is completely ignore them.
We're going to have to maintain a legacy
platform_device -> DT bindings hack somewhere anyway,
at least until the remaining other users of the driver
convert to D-T.
-Ian
^ permalink raw reply
* Re: [PATCH] net: add new QCA alx ethernet driver
From: Joe Perches @ 2012-08-09 15:22 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ren, Cloud, David Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, qca-linux-team, nic-devel,
Huang, Xiong, hao-ran.liu@canonical.com, Rodriguez, Luis
In-Reply-To: <20120809145454.GA21195@home.goodmis.org>
On Thu, 2012-08-09 at 10:54 -0400, Steven Rostedt wrote:
> On Thu, Aug 09, 2012 at 06:49:25AM +0000, Ren, Cloud wrote:
> > Luis has ever asked joe for adding as a Signed-off-by.
[]
> I'm curious, did Luis ask on list or off?
Off vger lists with some qca list or exploder added.
> The Signed-off tag has some legal bindings (all other tags are for
> credit/info only). It is that person saying that the changes they made
> to a patch are theirs and are giving the right to distribute it.
I believe I posted all patches I signed-off to vger lists.
> The sign-off is a simple line at the end of the explanation for the
> patch, which certifies that you wrote it or otherwise have the right to
> pass it on as an open-source patch.
>
> Joe, If you actually modified part of the code in the patch, you do have
> the right to add a signed-off-by tag. If you just made suggestions or
> reviewed, then a 'Suggested-by' or 'Reviewed-by' tag would be more
> appropriate.
Yes, I sent signed-off patches to their original RFC submission
(back last year?). To me, this single large patch is like a
snapshot of a git tree and not all contributors to that git
tree should be noted as signers of the entire thing.
^ permalink raw reply
* Re: [PATCH] net: add new QCA alx ethernet driver
From: Steven Rostedt @ 2012-08-09 15:28 UTC (permalink / raw)
To: Joe Perches
Cc: Ren, Cloud, David Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, qca-linux-team, nic-devel,
Huang, Xiong, hao-ran.liu@canonical.com, Rodriguez, Luis
In-Reply-To: <1344525758.3165.8.camel@joe2Laptop>
On Thu, 2012-08-09 at 08:22 -0700, Joe Perches wrote:
> Yes, I sent signed-off patches to their original RFC submission
> (back last year?). To me, this single large patch is like a
> snapshot of a git tree and not all contributors to that git
> tree should be noted as signers of the entire thing.
Ouch, that looks like we are losing all history.
One large patch should not be submitted. If it was developed in steps,
then it is best to show those steps when they are incorporated into the
Linux repository.
Sure, the patches may be reworked and folded together where appropriate,
but when I read "this single large patch is like a snapshot of a git
tree", that right there is a red flag.
-- Steve
^ permalink raw reply
* Re: [net-next RFC V5 3/5] virtio: intorduce an API to set affinity for a virtqueue
From: Avi Kivity @ 2012-08-09 15:35 UTC (permalink / raw)
To: Paolo Bonzini
Cc: krkumar2, habanero, kvm, mst, netdev, mashirle, linux-kernel,
virtualization, edumazet, tahm, jwhan, davem, sri
In-Reply-To: <5023D3A1.8040102@redhat.com>
On 08/09/2012 06:13 PM, Paolo Bonzini wrote:
> Il 05/07/2012 12:29, Jason Wang ha scritto:
>> Sometimes, virtio device need to configure irq affiniry hint to maximize the
>> performance. Instead of just exposing the irq of a virtqueue, this patch
>> introduce an API to set the affinity for a virtqueue.
>>
>> The api is best-effort, the affinity hint may not be set as expected due to
>> platform support, irq sharing or irq type. Currently, only pci method were
>> implemented and we set the affinity according to:
>>
>> - if device uses INTX, we just ignore the request
>> - if device has per vq vector, we force the affinity hint
>> - if the virtqueues share MSI, make the affinity OR over all affinities
>> requested
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>
> It looks like both I and Jason will need these patches during the 3.7
> merge window, and from different trees (net-next vs. scsi). How do we
> synchronize?
Get one of them to promise not to rebase, merge it, and base your
patches on top of the merge.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* Re: [PATCH] ipv4: tcp: security_sk_alloc() needed for unicast_sock
From: Eric Dumazet @ 2012-08-09 15:36 UTC (permalink / raw)
To: Paul Moore
Cc: David Miller, Casey Schaufler, Eric Paris, John Stultz,
Serge E. Hallyn, lkml, James Morris, selinux, john.johansen, LSM,
netdev
In-Reply-To: <5799181.tjlnF0gIh2@sifl>
On Thu, 2012-08-09 at 11:07 -0400, Paul Moore wrote:
> Is is possible to do the call to security_sk_alloc() in the ip_init() function
> or does the per-cpu nature of the socket make this a pain?
>
Its a pain, if we want NUMA affinity.
Here, each cpu should get memory from its closest node.
^ permalink raw reply
* Re: [PATCH] ipv4: tcp: security_sk_alloc() needed for unicast_sock
From: Paul Moore @ 2012-08-09 15:59 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Casey Schaufler, Eric Paris, John Stultz,
Serge E. Hallyn, lkml, James Morris, selinux, john.johansen, LSM,
netdev
In-Reply-To: <1344526608.28967.1092.camel@edumazet-glaptop>
On Thu, Aug 9, 2012 at 11:36 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2012-08-09 at 11:07 -0400, Paul Moore wrote:
>
>> Is is possible to do the call to security_sk_alloc() in the ip_init() function
>> or does the per-cpu nature of the socket make this a pain?
>>
>
> Its a pain, if we want NUMA affinity.
>
> Here, each cpu should get memory from its closest node.
Okay, makes sense.
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [RFC net-next 2/4] gianfar: Clear ievent from interrupt handler for [RT]x int
From: Claudiu Manoil @ 2012-08-09 16:04 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: netdev, David S. Miller
In-Reply-To: <20120808161156.GB11043@windriver.com>
On 8/8/2012 7:11 PM, Paul Gortmaker wrote:
> [[RFC net-next 2/4] gianfar: Clear ievent from interrupt handler for [RT]x int] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:
>
>> It's the interrupt handler's job to clear ievent for the Tx/Rx paths, as soon
>> as the corresponding interrupt sources have been masked.
>
> What wasn't clear to me was whether we'd ever have an instance of
> gfar_poll run without RTX_MASK being cleared (in less normal conditions,
> like netconsole, KGDBoE etc), since the gfar_schedule_cleanup is only
> called from rx/tx IRQ threads, and neither of those are used by
> gfar_poll, it seems.
Hi Paul,
As I see it, netconsole has the ndo_poll_controller hook, which points
to gfar_netpoll() -> gfar_interrupt() -> gfar_receive|transmit() ->
gfar_schedule_cleanup(), so it passes through schedule_cleanup.
My understanding is that gfar_poll() is scheduled for execution
only by "__napi_schedule(&gfargrp->napi);" from the Rx/Tx interrupt
handler (gfar_receive/transmit()->gfar_schedule_cleanup()), and that
it will be executed sometimes after the interrupt handler returns,
which means RTX_MASK has been cleared (and the interrupt sources are
already masked).
I think that we might have an issue if we don't clear IEVENT right away
in the interrupt handler, as this register might cause additional hw
interrupts to the PIC upon returning from the interrupt handler.
^ permalink raw reply
* Re: [PATCH] ipv4: tcp: security_sk_alloc() needed for unicast_sock
From: Eric Paris @ 2012-08-09 16:05 UTC (permalink / raw)
To: Eric Dumazet
Cc: Paul Moore, David Miller, Casey Schaufler, John Stultz,
Serge E. Hallyn, lkml, James Morris, selinux, john.johansen, LSM,
netdev
In-Reply-To: <1344526608.28967.1092.camel@edumazet-glaptop>
On Thu, Aug 9, 2012 at 11:36 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2012-08-09 at 11:07 -0400, Paul Moore wrote:
>
>> Is is possible to do the call to security_sk_alloc() in the ip_init() function
>> or does the per-cpu nature of the socket make this a pain?
>>
>
> Its a pain, if we want NUMA affinity.
>
> Here, each cpu should get memory from its closest node.
I really really don't like it. I won't say NAK, but it is the first
and only place in the kernel where I believe we allocate an object and
don't allocate the security blob until some random later point in
time. If it is such a performance issue to have the security blob in
the same numa node, isn't adding a number of branches and putting this
function call on every output at least as bad? Aren't we discouraged
from GFP_ATOMIC? In __init we can use GFP_KERNEL.
This still doesn't fix these sockets entirely. We now have the
security blob allocated, but it was never set to something useful.
Paul, are you looking into this? This is a bandaide, not a fix....
-Eric
^ permalink raw reply
* Re: [PATCH] ipv4: tcp: security_sk_alloc() needed for unicast_sock
From: Paul Moore @ 2012-08-09 16:09 UTC (permalink / raw)
To: Eric Paris
Cc: Eric Dumazet, David Miller, Casey Schaufler, John Stultz,
Serge E. Hallyn, lkml, James Morris, selinux, john.johansen, LSM,
netdev
In-Reply-To: <CACLa4ptTfMzvhYk7_DaUJd-9u406FXf2CUHjn1mQrPDa4fFW4w@mail.gmail.com>
On Thu, Aug 9, 2012 at 12:05 PM, Eric Paris <eparis@parisplace.org> wrote:
> Paul, are you looking into this? This is a bandaide, not a fix....
Yep, I mentioned this a few times in the other thread. The problem is
there is not going to be an easy fix for the labeling so I'd rather we
see this patch, or something like it, go in now to resolve the kernel
panic, and fix the labeling later.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [RFC net-next 3/4] gianfar: Separate out the Rx and Tx coalescing functions
From: Claudiu Manoil @ 2012-08-09 16:24 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: netdev, David S. Miller
In-Reply-To: <20120808154454.GA11043@windriver.com>
On 8/8/2012 6:44 PM, Paul Gortmaker wrote:
> [[RFC net-next 3/4] gianfar: Separate out the Rx and Tx coalescing functions] On 08/08/2012 (Wed 15:26) Claudiu Manoil wrote:
>
>> Split the coalescing programming support by Rx and Tx h/w queues, in order to
>> introduce a separate NAPI for the Tx confirmation path (next patch). This way,
>> the Rx processing path will handle the coalescing settings for the Rx queues
>> only, resp. the Tx confirmation processing path will handle the Tx queues.
>>
>> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
>> ---
>> drivers/net/ethernet/freescale/gianfar.c | 36 +++++++++++++++++++++++------
>> 1 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
>> index ddd350a..919acb3 100644
>> --- a/drivers/net/ethernet/freescale/gianfar.c
>> +++ b/drivers/net/ethernet/freescale/gianfar.c
>> @@ -1794,8 +1794,8 @@ void gfar_start(struct net_device *dev)
>> dev->trans_start = jiffies; /* prevent tx timeout */
>> }
>>
>> -void gfar_configure_coalescing(struct gfar_private *priv,
>> - unsigned long tx_mask, unsigned long rx_mask)
>> +static inline void gfar_configure_tx_coalescing(struct gfar_private *priv,
>
> I believe the preference is to not specify inline when all the chunks in
> play are present in the one C file -- i.e. let gcc figure it out. Same
> for the Rx instance below.
>
> P.
> --
I agree with you.
thanks
^ permalink raw reply
* [PATCH net-next] af_packet: relax BUG statement in tpacket_destruct_skb
From: Daniel Borkmann @ 2012-08-09 16:39 UTC (permalink / raw)
To: davem; +Cc: netdev
Here's a quote of the comment about the BUG macro from asm-generic/bug.h:
Don't use BUG() or BUG_ON() unless there's really no way out; one
example might be detecting data structure corruption in the middle
of an operation that can't be backed out of. If the (sub)system
can somehow continue operating, perhaps with reduced functionality,
it's probably not BUG-worthy.
If you're tempted to BUG(), think again: is completely giving up
really the *only* solution? There are usually better options, where
users don't need to reboot ASAP and can mostly shut down cleanly.
In our case, the status flag of a ring buffer slot is managed from both sides,
the kernel space and the user space. This means that even though the kernel
side might work as expected, the user space screws up and changes this flag
right between the send(2) is triggered when the flag is changed to
TP_STATUS_SENDING and a given skb is destructed after some time. Then, this
will hit the BUG macro. Instead, we relax this condition with a WARN_ON_ONCE
macro, so that the user is aware of this situation. I've tested it and the
system still behaves /stable/, so in accordance with the above comment, we
should rather relax this behavior with a warning.
Signed-off-by: Daniel Borkmann <daniel.borkmann@tik.ee.ethz.ch>
---
net/packet/af_packet.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index ceaca7c..4def36f 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1936,7 +1936,7 @@ static void tpacket_destruct_skb(struct sk_buff *skb)
if (likely(po->tx_ring.pg_vec)) {
ph = skb_shinfo(skb)->destructor_arg;
- BUG_ON(__packet_get_status(po, ph) != TP_STATUS_SENDING);
+ WARN_ON_ONCE(__packet_get_status(po, ph) != TP_STATUS_SENDING);
BUG_ON(atomic_read(&po->tx_ring.pending) == 0);
atomic_dec(&po->tx_ring.pending);
__packet_set_status(po, ph, TP_STATUS_AVAILABLE);
^ 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