* Re: [PATCH 2/4] macvlan: cleanup rx statistics
From: Arnd Bergmann @ 2009-11-24 9:28 UTC (permalink / raw)
To: virtualization
Cc: Eric Dumazet, Herbert Xu, Anna Fischer, netdev, bridge,
linux-kernel, Mark Smith, Gerhard Stenzel, Eric W. Biederman,
Jens Osterkamp, Patrick Mullaney, Stephen Hemminger,
Edge Virtual Bridging, David Miller
In-Reply-To: <200911240845.14454.arnd@arndb.de>
On Tuesday 24 November 2009 08:45:14 Arnd Bergmann wrote:
> On Tuesday 24 November 2009 08:15:53 Eric Dumazet wrote:
> > Arnd Bergmann a écrit :
> > I find following more readable, it probably generates more branches,
> > but avoid dirtying rx_errors if it is in another cache line.
> >
> > if (likely(success)) {
> > rx_stats->rx_packets++;
> > rx_stats->rx_bytes += length;
> > if (multicast)
> > rx_stats->multicast++;
> > } else {
> > rx_stats->rx_errors++;
> > }
>
> Given that the structure only has four members and alloc_percpu requests
> cache aligned data, it is rather likely to be in the same cache line.
>
> I'll have a look at what gcc generates on x86-64 for both versions
> and use the version you suggested unless it looks significantly more
> expensive.
Ok, that's what I got for trying to be clever. My version did not avoid
any branches, just created more code. I'll fold this update into my
patches then:
---
macvlan: cleanups
Use bool instead of int for flags, don't misoptimize rx counters,
avoid accessing a NULL skb.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 3db96b9..ff5f0b0 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -119,19 +119,23 @@ static int macvlan_addr_busy(const struct macvlan_port *port,
}
static inline void macvlan_count_rx(const struct macvlan_dev *vlan, int length,
- int success, int multicast)
+ bool success, bool multicast)
{
struct macvlan_rx_stats *rx_stats;
rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id());
- rx_stats->rx_packets += success != 0;
- rx_stats->rx_bytes += success ? length : 0;
- rx_stats->multicast += success && multicast;
- rx_stats->rx_errors += !success;
+ if (likely(success)) {
+ rx_stats->rx_packets++;;
+ rx_stats->rx_bytes += length;
+ if (multicast)
+ rx_stats->multicast++;
+ } else {
+ rx_stats->rx_errors++;
+ }
}
static int macvlan_broadcast_one(struct sk_buff *skb, struct net_device *dev,
- const struct ethhdr *eth, int local)
+ const struct ethhdr *eth, bool local)
{
if (!skb)
return NET_RX_DROP;
@@ -173,7 +177,7 @@ static void macvlan_broadcast(struct sk_buff *skb,
err = macvlan_broadcast_one(nskb, vlan->dev, eth,
mode == MACVLAN_MODE_BRIDGE);
macvlan_count_rx(vlan, skb->len + ETH_HLEN,
- likely(err == NET_RX_SUCCESS), 1);
+ err == NET_RX_SUCCESS, 1);
}
}
}
@@ -186,6 +190,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
const struct macvlan_dev *vlan;
const struct macvlan_dev *src;
struct net_device *dev;
+ int len;
port = rcu_dereference(skb->dev->macvlan_port);
if (port == NULL)
@@ -218,8 +223,11 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
kfree_skb(skb);
return NULL;
}
+ len = skb->len + ETH_HLEN;
skb = skb_share_check(skb, GFP_ATOMIC);
- macvlan_count_rx(vlan, skb->len + ETH_HLEN, likely(skb != NULL), 0);
+ macvlan_count_rx(vlan, len, skb != NULL, 0);
+ if (!skb)
+ return NULL;
skb->dev = dev;
skb->pkt_type = PACKET_HOST;
@@ -248,7 +256,7 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
int length = skb->len + ETH_HLEN;
int ret = dev_forward_skb(dest->dev, skb);
macvlan_count_rx(dest, length,
- likely(ret == NET_RX_SUCCESS), 0);
+ ret == NET_RX_SUCCESS, 0);
return NET_XMIT_SUCCESS;
}
^ permalink raw reply related
* [PATCH v2] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Peter P Waskiewicz Jr @ 2009-11-24 9:35 UTC (permalink / raw)
To: linux-kernel, arjan; +Cc: mingo, tglx, yong.zhang0, davem, netdev
This patchset adds a new CPU mask for SMP systems to the irq_desc
struct. It also exposes an API for underlying device drivers to
assist irqbalance in making smarter decisions when balancing, especially
in a NUMA environment. For example, an ethernet driver with MSI-X may
wish to limit the CPUs that an interrupt can be balanced within to
stay on a single NUMA node. Current irqbalance operation can move the
interrupt off the node, resulting in cross-node memory accesses and
locks.
The API is a get/set API within the kernel, along with a /proc entry
for the interrupt.
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
---
include/linux/interrupt.h | 8 ++++++
include/linux/irq.h | 8 ++++++
kernel/irq/manage.c | 32 +++++++++++++++++++++++++
kernel/irq/proc.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 105 insertions(+), 0 deletions(-)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 75f3f00..9fd08aa 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -208,6 +208,8 @@ extern cpumask_var_t irq_default_affinity;
extern int irq_set_affinity(unsigned int irq, const struct cpumask *cpumask);
extern int irq_can_set_affinity(unsigned int irq);
extern int irq_select_affinity(unsigned int irq);
+extern int irq_set_node_affinity(unsigned int irq,
+ const struct cpumask *cpumask);
#else /* CONFIG_SMP */
@@ -223,6 +225,12 @@ static inline int irq_can_set_affinity(unsigned int irq)
static inline int irq_select_affinity(unsigned int irq) { return 0; }
+static inline int irq_set_node_affinity(unsigned int irq,
+ const struct cpumask *m)
+{
+ return -EINVAL;
+}
+
#endif /* CONFIG_SMP && CONFIG_GENERIC_HARDIRQS */
#ifdef CONFIG_GENERIC_HARDIRQS
diff --git a/include/linux/irq.h b/include/linux/irq.h
index ae9653d..819cda0 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -166,6 +166,7 @@ struct irq_2_iommu;
* @lock: locking for SMP
* @affinity: IRQ affinity on SMP
* @node: node index useful for balancing
+ * @node_affinity: irq mask hints for irqbalance
* @pending_mask: pending rebalanced interrupts
* @threads_active: number of irqaction threads currently running
* @wait_for_threads: wait queue for sync_irq to wait for threaded handlers
@@ -196,6 +197,7 @@ struct irq_desc {
#ifdef CONFIG_SMP
cpumask_var_t affinity;
unsigned int node;
+ cpumask_var_t node_affinity;
#ifdef CONFIG_GENERIC_PENDING_IRQ
cpumask_var_t pending_mask;
#endif
@@ -445,9 +447,15 @@ static inline bool alloc_desc_masks(struct irq_desc *desc, int node,
if (!alloc_cpumask_var_node(&desc->affinity, gfp, node))
return false;
+ if (!alloc_cpumask_var_node(&desc->node_affinity, gfp, node)) {
+ free_cpumask_var(desc->affinity);
+ return false;
+ }
+
#ifdef CONFIG_GENERIC_PENDING_IRQ
if (!alloc_cpumask_var_node(&desc->pending_mask, gfp, node)) {
free_cpumask_var(desc->affinity);
+ free_cpumask_var(desc->node_affinity);
return false;
}
#endif
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 7305b29..9e80783 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -138,6 +138,38 @@ int irq_set_affinity(unsigned int irq, const struct cpumask *cpumask)
return 0;
}
+/**
+ * irq_set_node_affinity - Set the CPU mask this interrupt can run on
+ * @irq: Interrupt to modify
+ * @cpumask: CPU mask to assign to the interrupt
+ *
+ */
+int irq_set_node_affinity(unsigned int irq, const struct cpumask *cpumask)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+ unsigned long flags;
+
+ spin_lock_irqsave(&desc->lock, flags);
+ cpumask_copy(desc->node_affinity, cpumask);
+ spin_unlock_irqrestore(&desc->lock, flags);
+
+ return 0;
+}
+EXPORT_SYMBOL(irq_set_node_affinity);
+
+/**
+ * irq_get_node_affinity - Get the CPU mask this interrupt can run on
+ * @irq: Interrupt to get information
+ *
+ */
+struct cpumask *irq_get_node_affinity(unsigned int irq)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ return desc->node_affinity;
+}
+EXPORT_SYMBOL(irq_get_node_affinity);
+
#ifndef CONFIG_AUTO_IRQ_AFFINITY
/*
* Generic version of the affinity autoselector.
diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index 0832145..192e3fb 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -31,6 +31,16 @@ static int irq_affinity_proc_show(struct seq_file *m, void *v)
return 0;
}
+static int irq_node_affinity_proc_show(struct seq_file *m, void *v)
+{
+ struct irq_desc *desc = irq_to_desc((long)m->private);
+ const struct cpumask *mask = desc->node_affinity;
+
+ seq_cpumask(m, mask);
+ seq_putc(m, '\n');
+ return 0;
+}
+
#ifndef is_affinity_mask_valid
#define is_affinity_mask_valid(val) 1
#endif
@@ -78,11 +88,46 @@ free_cpumask:
return err;
}
+static ssize_t irq_node_affinity_proc_write(struct file *file,
+ const char __user *buffer, size_t count, loff_t *pos)
+{
+ unsigned int irq = (int)(long)PDE(file->f_path.dentry->d_inode)->data;
+ cpumask_var_t new_value;
+ int err;
+
+ if (no_irq_affinity || irq_balancing_disabled(irq))
+ return -EIO;
+
+ if (!alloc_cpumask_var(&new_value, GFP_KERNEL))
+ return -ENOMEM;
+
+ err = cpumask_parse_user(buffer, count, new_value);
+ if (err)
+ goto free_cpumask;
+
+ if (!is_affinity_mask_valid(new_value)) {
+ err = -EINVAL;
+ goto free_cpumask;
+ }
+
+ irq_set_node_affinity(irq, new_value);
+ err = count;
+
+free_cpumask:
+ free_cpumask_var(new_value);
+ return err;
+}
+
static int irq_affinity_proc_open(struct inode *inode, struct file *file)
{
return single_open(file, irq_affinity_proc_show, PDE(inode)->data);
}
+static int irq_node_affinity_proc_open(struct inode *inode, struct file *f)
+{
+ return single_open(f, irq_node_affinity_proc_show, PDE(inode)->data);
+}
+
static const struct file_operations irq_affinity_proc_fops = {
.open = irq_affinity_proc_open,
.read = seq_read,
@@ -91,6 +136,14 @@ static const struct file_operations irq_affinity_proc_fops = {
.write = irq_affinity_proc_write,
};
+static const struct file_operations irq_node_affinity_proc_fops = {
+ .open = irq_node_affinity_proc_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+ .write = irq_node_affinity_proc_write,
+};
+
static int default_affinity_show(struct seq_file *m, void *v)
{
seq_cpumask(m, irq_default_affinity);
@@ -230,6 +283,10 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
/* create /proc/irq/<irq>/smp_affinity */
proc_create_data("smp_affinity", 0600, desc->dir,
&irq_affinity_proc_fops, (void *)(long)irq);
+
+ /* create /proc/irq/<irq>/node_affinity */
+ proc_create_data("node_affinity", 0600, desc->dir,
+ &irq_node_affinity_proc_fops, (void *)(long)irq);
#endif
proc_create_data("spurious", 0444, desc->dir,
^ permalink raw reply related
* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Patrick McHardy @ 2009-11-24 9:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Eric Dumazet, linux-kernel, netdev, David Miller,
Stephen Hemminger, Herbert Xu, Patrick Mullaney,
Eric W. Biederman, Edge Virtual Bridging, Anna Fischer, bridge,
virtualization, Jens Osterkamp, Gerhard Stenzel, Mark Smith
In-Reply-To: <1259024166-28158-2-git-send-email-arnd@arndb.de>
Arnd Bergmann wrote:
> +int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
> +{
> + skb_orphan(skb);
> +
> + if (!(dev->flags & IFF_UP))
> + return NET_RX_DROP;
> +
> + if (skb->len > (dev->mtu + dev->hard_header_len))
> + return NET_RX_DROP;
> +
> + skb_dst_drop(skb);
> + skb->tstamp.tv64 = 0;
> + skb->pkt_type = PACKET_HOST;
> + skb->protocol = eth_type_trans(skb, dev);
> + skb->mark = 0;
skb->mark clearing should stay private to veth since its usually
supposed to stay intact. The only exception is packets crossing
namespaces, where they should appear like a freshly received skbs.
> + secpath_reset(skb);
> + nf_reset(skb);
> + return netif_rx(skb);
> +}
> +EXPORT_SYMBOL_GPL(dev_forward_skb);
^ permalink raw reply
* Re: ixgbe question
From: Eric Dumazet @ 2009-11-24 9:55 UTC (permalink / raw)
To: Peter P Waskiewicz Jr
Cc: robert@herjulf.net, Jesper Dangaard Brouer, Linux Netdev List
In-Reply-To: <1259053673.2631.30.camel@ppwaskie-mobl2>
Peter P Waskiewicz Jr a écrit :
> You might have this elsewhere, but it sounds like you're connecting back
> to back with another 82599 NIC. Our optics in that NIC are dual-rate,
> and the software mechanism that tries to "autoneg" link speed gets out
> of sync easily in back-to-back setups.
>
> If it's really annoying, and you're willing to run with a local patch to
> disable the autotry mechanism, try this:
>
> diff --git a/drivers/net/ixgbe/ixgbe_main.c
> b/drivers/net/ixgbe/ixgbe_main.c
> index a5036f7..62c0915 100644
> --- a/drivers/net/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ixgbe/ixgbe_main.c
> @@ -4670,6 +4670,10 @@ static void ixgbe_multispeed_fiber_task(struct
> work_struct *work)
> autoneg = hw->phy.autoneg_advertised;
> if ((!autoneg) && (hw->mac.ops.get_link_capabilities))
> hw->mac.ops.get_link_capabilities(hw, &autoneg,
> &negotiation);
> +
> + /* force 10G only */
> + autoneg = IXGBE_LINK_SPEED_10GB_FULL;
> +
> if (hw->mac.ops.setup_link)
> hw->mac.ops.setup_link(hw, autoneg, negotiation, true);
> adapter->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
Thanks ! This did the trick :)
If I am not mistaken, number of TX queues should be capped by number of possible cpus ?
Its currently a fixed 128 value, allocating 128*128 = 16384 bytes,
and polluting "tc -s -d class show dev fiber0" output.
[PATCH net-next-2.6] ixgbe: Do not allocate too many netdev txqueues
Instead of allocating 128 struct netdev_queue per device, use the minimum
value between 128 and number of possible cpus, to reduce ram usage and
"tc -s -d class show dev ..." output
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index ebcec30..ec2508d 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -5582,7 +5583,10 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
pci_set_master(pdev);
pci_save_state(pdev);
- netdev = alloc_etherdev_mq(sizeof(struct ixgbe_adapter), MAX_TX_QUEUES);
+ netdev = alloc_etherdev_mq(sizeof(struct ixgbe_adapter),
+ min_t(unsigned int,
+ MAX_TX_QUEUES,
+ num_possible_cpus()));
if (!netdev) {
err = -ENOMEM;
goto err_alloc_etherdev;
^ permalink raw reply related
* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Arnd Bergmann @ 2009-11-24 10:02 UTC (permalink / raw)
To: Patrick McHardy
Cc: Eric Dumazet, linux-kernel, netdev, David Miller,
Stephen Hemminger, Herbert Xu, Patrick Mullaney,
Eric W. Biederman, Edge Virtual Bridging, Anna Fischer, bridge,
virtualization, Jens Osterkamp, Gerhard Stenzel, Mark Smith
In-Reply-To: <4B0BAC97.6010000@trash.net>
On Tuesday 24 November 2009 09:51:19 Patrick McHardy wrote:
> > + skb_dst_drop(skb);
> > + skb->tstamp.tv64 = 0;
> > + skb->pkt_type = PACKET_HOST;
> > + skb->protocol = eth_type_trans(skb, dev);
> > + skb->mark = 0;
>
> skb->mark clearing should stay private to veth since its usually
> supposed to stay intact. The only exception is packets crossing
> namespaces, where they should appear like a freshly received skbs.
But isn't that what we want in macvlan as well when we're
forwarding from one downstream interface to another?
I did all my testing with macvlan interfaces in separate namespaces
communicating with each other, so I'd assume that we should always
clear skb->mark and skb->dst in this function. Maybe I should make
the documentation clearer?
---
net: clarify documentation of dev_forward_skb
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1433,6 +1433,10 @@ static inline void net_timestamp(struct sk_buff *skb)
* dev_forward_skb can be used for injecting an skb from the
* start_xmit function of one device into the receive queue
* of another device.
+ *
+ * The receiving device may be in another namespace, so
+ * we have to clear all information in the skb that could
+ * impact namespace isolation.
*/
int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
{
^ permalink raw reply
* Re: ixgbe question
From: Peter P Waskiewicz Jr @ 2009-11-24 10:06 UTC (permalink / raw)
To: Eric Dumazet
Cc: robert@herjulf.net, Jesper Dangaard Brouer, Linux Netdev List
In-Reply-To: <4B0BADA6.7080602@gmail.com>
On Tue, 2009-11-24 at 02:55 -0700, Eric Dumazet wrote:
> Peter P Waskiewicz Jr a écrit :
>
> > You might have this elsewhere, but it sounds like you're connecting back
> > to back with another 82599 NIC. Our optics in that NIC are dual-rate,
> > and the software mechanism that tries to "autoneg" link speed gets out
> > of sync easily in back-to-back setups.
> >
> > If it's really annoying, and you're willing to run with a local patch to
> > disable the autotry mechanism, try this:
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_main.c
> > b/drivers/net/ixgbe/ixgbe_main.c
> > index a5036f7..62c0915 100644
> > --- a/drivers/net/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ixgbe/ixgbe_main.c
> > @@ -4670,6 +4670,10 @@ static void ixgbe_multispeed_fiber_task(struct
> > work_struct *work)
> > autoneg = hw->phy.autoneg_advertised;
> > if ((!autoneg) && (hw->mac.ops.get_link_capabilities))
> > hw->mac.ops.get_link_capabilities(hw, &autoneg,
> > &negotiation);
> > +
> > + /* force 10G only */
> > + autoneg = IXGBE_LINK_SPEED_10GB_FULL;
> > +
> > if (hw->mac.ops.setup_link)
> > hw->mac.ops.setup_link(hw, autoneg, negotiation, true);
> > adapter->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
>
> Thanks ! This did the trick :)
>
> If I am not mistaken, number of TX queues should be capped by number of possible cpus ?
>
> Its currently a fixed 128 value, allocating 128*128 = 16384 bytes,
> and polluting "tc -s -d class show dev fiber0" output.
>
Yes, this is a stupid issue we haven't gotten around to fixing yet.
This looks fine to me. Thanks for putting it together.
> [PATCH net-next-2.6] ixgbe: Do not allocate too many netdev txqueues
>
> Instead of allocating 128 struct netdev_queue per device, use the minimum
> value between 128 and number of possible cpus, to reduce ram usage and
> "tc -s -d class show dev ..." output
>
> diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
> index ebcec30..ec2508d 100644
> --- a/drivers/net/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ixgbe/ixgbe_main.c
> @@ -5582,7 +5583,10 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
> pci_set_master(pdev);
> pci_save_state(pdev);
>
> - netdev = alloc_etherdev_mq(sizeof(struct ixgbe_adapter), MAX_TX_QUEUES);
> + netdev = alloc_etherdev_mq(sizeof(struct ixgbe_adapter),
> + min_t(unsigned int,
> + MAX_TX_QUEUES,
> + num_possible_cpus()));
> if (!netdev) {
> err = -ENOMEM;
> goto err_alloc_etherdev;
^ permalink raw reply
* Re: [PATCH] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Thomas Gleixner @ 2009-11-24 10:07 UTC (permalink / raw)
To: Peter P Waskiewicz Jr
Cc: Peter Zijlstra, Yong Zhang, linux-kernel@vger.kernel.org,
arjan@linux.jf.intel.com, davem@davemloft.net,
netdev@vger.kernel.org, Jesse Barnes
In-Reply-To: <1259053156.2631.21.camel@ppwaskie-mobl2>
On Tue, 24 Nov 2009, Peter P Waskiewicz Jr wrote:
> On Tue, 2009-11-24 at 01:38 -0700, Peter Zijlstra wrote:
> > On Mon, 2009-11-23 at 15:32 -0800, Waskiewicz Jr, Peter P wrote:
> >
> > > Unfortunately, a driver can't. The irq_set_affinity() function isn't
> > > exported. I proposed a patch on netdev to export it, and then to tie down
> > > an interrupt using IRQF_NOBALANCING, so irqbalance won't touch it. That
> > > was rejected, since the driver is enforcing policy of the interrupt
> > > balancing, not irqbalance.
> >
> > Why would a patch touching the irq subsystem go to netdev?
>
> The only change to the IRQ subsystem was:
>
> EXPORT_SYMBOL(irq_set_affinity);
Which is still touching the generic irq subsystem and needs the ack of
the relevant maintainer. If there is a need to expose such an
interface to drivers then the maintainer wants to know exactly why and
needs to be part of the discussion of alternative solutions. Otherwise
you waste time on implementing stuff like the current patch which is
definitely not going anywhere near the irq subsystem.
> > If all you want is to expose policy to userspace then you don't need any
> > of this, simply expose the NICs home node through a sysfs device thingy
> > (I was under the impression its already there somewhere, but I can't
> > ever find anything in /sys).
> >
> > No need what so ever to poke at the IRQ subsystem.
>
> The point is we need something common that the kernel side (whether a
> driver or /proc can modify) that irqbalance can use.
/sys/class/net/ethX/device/numa_node
perhaps ?
> > > Also, if you use the /proc interface to change smp_affinity on an
> > > interrupt without any of these changes, irqbalance will override it on its
> > > next poll interval. This also is not desirable.
> >
> > This all sounds backwards.. we've got a perfectly functional interface
> > for affinity -- which people object to being used for some reason. So
> > you add another interface on top, and that is ok?
> >
>
> But it's not functional. If I set the affinity in smp_affinity, then
> irqbalance will override it 10 seconds later.
And to work around the brain wreckage of irqbalanced you want to
fiddle in the irq code instead of teaching irqbalanced to handle node
affinities ?
The only thing which is worth to investigate is whether the irq core
code should honour the dev->numa_node setting and restrict the
possible irq affinity settings to that node. If a device is tied to a
node it makes a certain amount of sense to do that.
But such a change would not need a new interface in the irq core and
definitely not a new cpumask_t member in the irq_desc structure to
store a node affinity which can be expressed with a simple
integer.
But this needs more thoughts and I want to know more about the
background and the reasoning for such a change.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Patrick McHardy @ 2009-11-24 10:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Eric Dumazet, linux-kernel, netdev, David Miller,
Stephen Hemminger, Herbert Xu, Patrick Mullaney,
Eric W. Biederman, Edge Virtual Bridging, Anna Fischer, bridge,
virtualization, Jens Osterkamp, Gerhard Stenzel, Mark Smith
In-Reply-To: <200911241002.20904.arnd@arndb.de>
Arnd Bergmann wrote:
> On Tuesday 24 November 2009 09:51:19 Patrick McHardy wrote:
>>> + skb_dst_drop(skb);
>>> + skb->tstamp.tv64 = 0;
>>> + skb->pkt_type = PACKET_HOST;
>>> + skb->protocol = eth_type_trans(skb, dev);
>>> + skb->mark = 0;
>> skb->mark clearing should stay private to veth since its usually
>> supposed to stay intact. The only exception is packets crossing
>> namespaces, where they should appear like a freshly received skbs.
>
> But isn't that what we want in macvlan as well when we're
> forwarding from one downstream interface to another?
In the TX direction you can use the mark for TC classification
on the underlying device.
> I did all my testing with macvlan interfaces in separate namespaces
> communicating with each other, so I'd assume that we should always
> clear skb->mark and skb->dst in this function.
Good point, in that case we probably should clear it as well. But
in the non-namespace case the TC classification currently works and
this is consistent with any other virtual device driver, so it
should continue to work.
^ permalink raw reply
* [PATCH] igb: fix misinterpreted return value of pci_enable_msix
From: Stefan Assmann @ 2009-11-24 10:31 UTC (permalink / raw)
To: netdev; +Cc: Andy Gospodarek, alexander.h.duyck, davem
From: Stefan Assmann <sassmann@redhat.com>
In the igb driver a return value of 0 of function pci_enable_msix is
interpreted as an error case. The correct behaviour is to check < 0 for error
values.
Signed-off-by: Stefan Assmann <sassmann@redhat.com>
---
drivers/net/igb/igb_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index bb1a6ee..745481d 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -694,7 +694,7 @@ static void igb_set_interrupt_capability(struct igb_adapter *adapter)
err = pci_enable_msix(adapter->pdev,
adapter->msix_entries,
numvecs);
- if (err == 0)
+ if (err < 0)
goto out;
igb_reset_interrupt_capability(adapter);
--
1.6.5.2
^ permalink raw reply related
* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Arnd Bergmann @ 2009-11-24 10:34 UTC (permalink / raw)
To: virtualization
Cc: Patrick McHardy, Herbert Xu, Eric Dumazet, Anna Fischer, netdev,
bridge, linux-kernel, Mark Smith, Gerhard Stenzel,
Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
Stephen Hemminger, Edge Virtual Bridging, David Miller
In-Reply-To: <4B0BB2A7.5040707@trash.net>
On Tuesday 24 November 2009 10:17:11 Patrick McHardy wrote:
> Arnd Bergmann wrote:
> > On Tuesday 24 November 2009 09:51:19 Patrick McHardy wrote:
> >>> + skb_dst_drop(skb);
> >>> + skb->tstamp.tv64 = 0;
> >>> + skb->pkt_type = PACKET_HOST;
> >>> + skb->protocol = eth_type_trans(skb, dev);
> >>> + skb->mark = 0;
> >> skb->mark clearing should stay private to veth since its usually
> >> supposed to stay intact. The only exception is packets crossing
> >> namespaces, where they should appear like a freshly received skbs.
> >
> > But isn't that what we want in macvlan as well when we're
> > forwarding from one downstream interface to another?
>
> In the TX direction you can use the mark for TC classification
> on the underlying device.
I don't use dev_forward_skb for the case where the data is sent
to the underlying device, so the TC classification should stay
intact.
> > I did all my testing with macvlan interfaces in separate namespaces
> > communicating with each other, so I'd assume that we should always
> > clear skb->mark and skb->dst in this function.
>
> Good point, in that case we probably should clear it as well. But
> in the non-namespace case the TC classification currently works and
> this is consistent with any other virtual device driver, so it
> should continue to work.
Do you think we should be able to use TC to direct traffic between
macvlans on the same underlying device in bridge mode? It does sound
useful, but I'm not sure how to implement that or if you'd expect
it to work with the current code. If we support that, it should probably
also work with namespaces, by consuming the mark in the macvlan
and veth drivers.
Arnd <><
^ permalink raw reply
* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Patrick McHardy @ 2009-11-24 10:40 UTC (permalink / raw)
To: Arnd Bergmann
Cc: virtualization, Herbert Xu, Eric Dumazet, Anna Fischer, netdev,
bridge, linux-kernel, Mark Smith, Gerhard Stenzel,
Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
Stephen Hemminger, Edge Virtual Bridging, David Miller
In-Reply-To: <200911241034.43961.arnd@arndb.de>
Arnd Bergmann wrote:
> On Tuesday 24 November 2009 10:17:11 Patrick McHardy wrote:
>> Arnd Bergmann wrote:
>>> On Tuesday 24 November 2009 09:51:19 Patrick McHardy wrote:
>>>>> + skb_dst_drop(skb);
>>>>> + skb->tstamp.tv64 = 0;
>>>>> + skb->pkt_type = PACKET_HOST;
>>>>> + skb->protocol = eth_type_trans(skb, dev);
>>>>> + skb->mark = 0;
>>>> skb->mark clearing should stay private to veth since its usually
>>>> supposed to stay intact. The only exception is packets crossing
>>>> namespaces, where they should appear like a freshly received skbs.
>>> But isn't that what we want in macvlan as well when we're
>>> forwarding from one downstream interface to another?
>> In the TX direction you can use the mark for TC classification
>> on the underlying device.
>
> I don't use dev_forward_skb for the case where the data is sent
> to the underlying device, so the TC classification should stay
> intact.
Right, I see. This looks fine.
>>> I did all my testing with macvlan interfaces in separate namespaces
>>> communicating with each other, so I'd assume that we should always
>>> clear skb->mark and skb->dst in this function.
>> Good point, in that case we probably should clear it as well. But
>> in the non-namespace case the TC classification currently works and
>> this is consistent with any other virtual device driver, so it
>> should continue to work.
>
> Do you think we should be able to use TC to direct traffic between
> macvlans on the same underlying device in bridge mode? It does sound
> useful, but I'm not sure how to implement that or if you'd expect
> it to work with the current code. If we support that, it should probably
> also work with namespaces, by consuming the mark in the macvlan
> and veth drivers.
I don't think its necessary, we bypass outgoing queuing anyways.
But if you'd want to add it, just keeping the skb->mark clearing
in veth should work from what I can tell.
^ permalink raw reply
* Re: [PATCH] igb: fix misinterpreted return value of pci_enable_msix
From: Florian Westphal @ 2009-11-24 10:40 UTC (permalink / raw)
To: Stefan Assmann; +Cc: netdev, Andy Gospodarek, alexander.h.duyck, davem
In-Reply-To: <4B0BB5FE.2000907@redhat.com>
Stefan Assmann <sassmann@redhat.com> wrote:
> From: Stefan Assmann <sassmann@redhat.com>
>
> In the igb driver a return value of 0 of function pci_enable_msix is
> interpreted as an error case. The correct behaviour is to check < 0 for error
> values.
>
> Signed-off-by: Stefan Assmann <sassmann@redhat.com>
> ---
> drivers/net/igb/igb_main.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
> index bb1a6ee..745481d 100644
> --- a/drivers/net/igb/igb_main.c
> +++ b/drivers/net/igb/igb_main.c
> @@ -694,7 +694,7 @@ static void igb_set_interrupt_capability(struct igb_adapter *adapter)
> err = pci_enable_msix(adapter->pdev,
> adapter->msix_entries,
> numvecs);
> - if (err == 0)
> + if (err < 0)
> goto out;
>
The existing code looks correct to me:
if (err == 0)
goto out;
igb_reset_interrupt_capability(adapter);
/* If we can't do MSI-X, try MSI * */
msi_only:
so the "goto out" appears to be the "success" case.
^ permalink raw reply
* Re: [PATCH 2/4] macvlan: cleanup rx statistics
From: Patrick McHardy @ 2009-11-24 10:41 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Eric Dumazet, linux-kernel, netdev, David Miller,
Stephen Hemminger, Herbert Xu, Patrick Mullaney,
Eric W. Biederman, Edge Virtual Bridging, Anna Fischer, bridge,
virtualization, Jens Osterkamp, Gerhard Stenzel, Mark Smith
In-Reply-To: <1259024166-28158-3-git-send-email-arnd@arndb.de>
Arnd Bergmann wrote:
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index ae2b5c7..a0dea23 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -116,42 +116,53 @@ static int macvlan_addr_busy(const struct macvlan_port *port,
> return 0;
> }
>
> +static inline void macvlan_count_rx(const struct macvlan_dev *vlan, int length,
Please use unsigned int for length values.
Regarding Eric's comments, I also think it would be more readable to use
if (success) {
...
} else {
...
}
> + int success, int multicast)
> +{
> + struct macvlan_rx_stats *rx_stats;
> +
> + rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id());
> + rx_stats->rx_packets += success != 0;
> + rx_stats->rx_bytes += success ? length : 0;
> + rx_stats->multicast += success && multicast;
> + rx_stats->rx_errors += !success;
> +}
> +
> +static int macvlan_broadcast_one(struct sk_buff *skb, struct net_device *dev,
> + const struct ethhdr *eth)
> +{
> + if (!skb)
> + return NET_RX_DROP;
> +
> + skb->dev = dev;
> + if (!compare_ether_addr_64bits(eth->h_dest,
> + dev->broadcast))
This would fit on one line without reducing readability.
> + skb->pkt_type = PACKET_BROADCAST;
> + else
> + skb->pkt_type = PACKET_MULTICAST;
> +
> + return netif_rx(skb);
> +}
^ permalink raw reply
* Re: [PATCH 3/4] macvlan: implement bridge, VEPA and private mode
From: Patrick McHardy @ 2009-11-24 10:42 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Eric Dumazet, linux-kernel, netdev, David Miller,
Stephen Hemminger, Herbert Xu, Patrick Mullaney,
Eric W. Biederman, Edge Virtual Bridging, Anna Fischer, bridge,
virtualization, Jens Osterkamp, Gerhard Stenzel, Mark Smith
In-Reply-To: <1259024166-28158-4-git-send-email-arnd@arndb.de>
Arnd Bergmann wrote:
> This allows each macvlan slave device to be in one
> of three modes, depending on the use case:
>
> MACVLAN_PRIVATE:
> The device never communicates with any other device
> on the same upper_dev. This even includes frames
> coming back from a reflective relay, where supported
> by the adjacent bridge.
>
> MACVLAN_VEPA:
> The new Virtual Ethernet Port Aggregator (VEPA) mode,
> we assume that the adjacent bridge returns all frames
> where both source and destination are local to the
> macvlan port, i.e. the bridge is set up as a reflective
> relay.
> Broadcast frames coming in from the upper_dev get
> flooded to all macvlan interfaces in VEPA mode.
> We never deliver any frames locally.
>
> MACVLAN_BRIDGE:
> We provide the behavior of a simple bridge between
> different macvlan interfaces on the same port. Frames
> from one interface to another one get delivered directly
> and are not sent out externally. Broadcast frames get
> flooded to all other bridge ports and to the external
> interface, but when they come back from a reflective
> relay, we don't deliver them again.
> Since we know all the MAC addresses, the macvlan bridge
> mode does not require learning or STP like the bridge
> module does.
This looks pretty nice. Some stylistic nitpicking below :)
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index a0dea23..b840b3a 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -29,9 +29,16 @@
> #include <linux/if_link.h>
> #include <linux/if_macvlan.h>
> #include <net/rtnetlink.h>
> +#include <net/xfrm.h>
Do we really need this?
> @@ -129,11 +137,14 @@ static inline void macvlan_count_rx(const struct macvlan_dev *vlan, int length,
> }
>
> static int macvlan_broadcast_one(struct sk_buff *skb, struct net_device *dev,
> - const struct ethhdr *eth)
> + const struct ethhdr *eth, int local)
bool local?
> {
> if (!skb)
> return NET_RX_DROP;
>
> + if (local)
> + return dev_forward_skb(dev, skb);
> +
> skb->dev = dev;
> if (!compare_ether_addr_64bits(eth->h_dest,
> dev->broadcast))
> @@ -145,7 +156,9 @@ static int macvlan_broadcast_one(struct sk_buff *skb, struct net_device *dev,
> }
>
> static void macvlan_broadcast(struct sk_buff *skb,
> - const struct macvlan_port *port)
> + const struct macvlan_port *port,
> + struct net_device *src,
> + enum macvlan_mode mode)
> {
> const struct ethhdr *eth = eth_hdr(skb);
> const struct macvlan_dev *vlan;
> @@ -159,8 +172,12 @@ static void macvlan_broadcast(struct sk_buff *skb,
>
> for (i = 0; i < MACVLAN_HASH_SIZE; i++) {
> hlist_for_each_entry_rcu(vlan, n, &port->vlan_hash[i], hlist) {
> + if ((vlan->dev == src) || !(vlan->mode & mode))
Please remove those unnecessary parentheses around the
device comparison.
> @@ -173,6 +190,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
> const struct ethhdr *eth = eth_hdr(skb);
> const struct macvlan_port *port;
> const struct macvlan_dev *vlan;
> + const struct macvlan_dev *src;
> struct net_device *dev;
>
> port = rcu_dereference(skb->dev->macvlan_port);
> @@ -180,7 +198,20 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
> return skb;
>
> if (is_multicast_ether_addr(eth->h_dest)) {
> - macvlan_broadcast(skb, port);
> + src = macvlan_hash_lookup(port, eth->h_source);
> + if (!src)
> + /* frame comes from an external address */
> + macvlan_broadcast(skb, port, NULL, MACVLAN_MODE_PRIVATE
> + | MACVLAN_MODE_VEPA | MACVLAN_MODE_BRIDGE);
The '|' should go on the first line.
> + else if (src->mode == MACVLAN_MODE_VEPA)
> + /* flood to everyone except source */
> + macvlan_broadcast(skb, port, src->dev,
> + MACVLAN_MODE_VEPA | MACVLAN_MODE_BRIDGE);
> + else if (src->mode == MACVLAN_MODE_BRIDGE)
> + /* flood only to VEPA ports, bridge ports
> + already saw the frame */
Multi-line comments should begin every line with '*'.
> + macvlan_broadcast(skb, port, src->dev,
> + MACVLAN_MODE_VEPA);
Please align the mode values (in all cases above) to the arguments
on the line above.
> return skb;
> }
>
> @@ -203,18 +234,46 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
> return NULL;
> }
>
> +static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + const struct macvlan_dev *vlan = netdev_priv(dev);
> + const struct macvlan_port *port = vlan->port;
> + const struct macvlan_dev *dest;
> +
> + if (vlan->mode == MACVLAN_MODE_BRIDGE) {
> + const struct ethhdr *eth = (void *)skb->data;
eth_hdr()?
> +
> + /* send to other bridge ports directly */
> + if (is_multicast_ether_addr(eth->h_dest)) {
> + macvlan_broadcast(skb, port, dev, MACVLAN_MODE_BRIDGE);
> + goto xmit_world;
> + }
> +
> + dest = macvlan_hash_lookup(port, eth->h_dest);
> + if (dest && dest->mode == MACVLAN_MODE_BRIDGE) {
> + int length = skb->len + ETH_HLEN;
unsigned int for length values please.
> + int ret = dev_forward_skb(dest->dev, skb);
> + macvlan_count_rx(dest, length,
> + likely(ret == NET_RX_SUCCESS), 0);
> +
> + return NET_XMIT_SUCCESS;
> + }
> + }
> +
> +xmit_world:
> + skb->dev = vlan->lowerdev;
> + return dev_queue_xmit(skb);
> +}
^ permalink raw reply
* Re: [PATCH] igb: fix misinterpreted return value of pci_enable_msix
From: Stefan Assmann @ 2009-11-24 10:51 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev, Andy Gospodarek, alexander.h.duyck, davem
In-Reply-To: <20091124104049.GB21600@Chamillionaire.breakpoint.cc>
On 24.11.2009 11:40, Florian Westphal wrote:
> Stefan Assmann <sassmann@redhat.com> wrote:
>> From: Stefan Assmann <sassmann@redhat.com>
>>
>> In the igb driver a return value of 0 of function pci_enable_msix is
>> interpreted as an error case. The correct behaviour is to check < 0 for error
>> values.
>>
>> Signed-off-by: Stefan Assmann <sassmann@redhat.com>
>> ---
>> drivers/net/igb/igb_main.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
>> index bb1a6ee..745481d 100644
>> --- a/drivers/net/igb/igb_main.c
>> +++ b/drivers/net/igb/igb_main.c
>> @@ -694,7 +694,7 @@ static void igb_set_interrupt_capability(struct igb_adapter *adapter)
>> err = pci_enable_msix(adapter->pdev,
>> adapter->msix_entries,
>> numvecs);
>> - if (err == 0)
>> + if (err < 0)
>> goto out;
>>
>
> The existing code looks correct to me:
>
> if (err == 0)
> goto out;
>
> igb_reset_interrupt_capability(adapter);
>
> /* If we can't do MSI-X, try MSI * */
> msi_only:
>
>
> so the "goto out" appears to be the "success" case.
Hi Florian,
thanks for pointing this out. I've jumped to conclusions too fast.
There's a problem with igb MSI-X in RHEL5 in the kdump case and this
fixed it for me. But you're right, it's not the proper solution. I'll
report back when I have more information.
Stefan
--
Stefan Assmann | Red Hat GmbH
Software Engineer | Otto-Hahn-Strasse 20, 85609 Dornach
| HR: Amtsgericht Muenchen HRB 153243
| GF: Brendan Lane, Charlie Peters,
sassmann at redhat.com | Michael Cunningham, Charles Cachera
^ permalink raw reply
* Re: [PATCH 4/4] macvlan: export macvlan mode through netlink
From: Patrick McHardy @ 2009-11-24 10:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Eric Dumazet, linux-kernel, netdev, David Miller,
Stephen Hemminger, Herbert Xu, Patrick Mullaney,
Eric W. Biederman, Edge Virtual Bridging, Anna Fischer, bridge,
virtualization, Jens Osterkamp, Gerhard Stenzel, Mark Smith
In-Reply-To: <1259024166-28158-5-git-send-email-arnd@arndb.de>
Arnd Bergmann wrote:
> @@ -600,6 +594,18 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
> if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
> return -EADDRNOTAVAIL;
> }
> +
> + if (data && data[IFLA_MACVLAN_MODE]) {
> + u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> + switch (mode) {
> + case MACVLAN_MODE_PRIVATE:
> + case MACVLAN_MODE_VEPA:
> + case MACVLAN_MODE_BRIDGE:
> + break;
> + default:
> + return -EINVAL;
EINVAL is quite unspecific. In this case I think EOPNOTSUPP would
be fine and provide more information.
> + }
> + }
> return 0;
> }
> @@ -664,6 +670,13 @@ static int macvlan_newlink(struct net *src_net, struct net_device *dev,
> vlan->dev = dev;
> vlan->port = port;
>
> + vlan->mode = MACVLAN_MODE_VEPA;
> + if (data && data[IFLA_MACVLAN_MODE]) {
> + u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> +
> + vlan->mode = mode;
This looks a bit strange, like cut-and-paste without reformatting :)
I'd suggest to simply use "vlan->mode = nla_get_u32(...)".
> + }
> +
> err = register_netdevice(dev);
> if (err < 0)
> return err;
> @@ -685,6 +698,39 @@ static void macvlan_dellink(struct net_device *dev, struct list_head *head)
> macvlan_port_destroy(port->dev);
> }
>
> +static int macvlan_changelink(struct net_device *dev,
> + struct nlattr *tb[], struct nlattr *data[])
> +{
> + struct macvlan_dev *vlan = netdev_priv(dev);
> + if (data && data[IFLA_MACVLAN_MODE]) {
> + u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> + vlan->mode = mode;
Same here.
> + }
> +
> + return 0;
> +}
^ permalink raw reply
* Re: macvlan: fix gso_max_size setting
From: Patrick McHardy @ 2009-11-24 10:57 UTC (permalink / raw)
To: Herbert Xu; +Cc: David Miller, netdev
In-Reply-To: <20091124010817.GB13660@gondor.apana.org.au>
Herbert Xu wrote:
> On Mon, Nov 23, 2009 at 11:33:05PM +0100, Patrick McHardy wrote:
>> Perhaps we should simply fall back to software in that case.
>> Compile tested only.
>
> Sure.
>
> Since ixgbe appears to be the only driver in our entire tree
> that has this limitation, I think it might make more sense to
> move this special-case check into it and get it to do software
> GSO just like tg3 does for a few other cases that the hardware
> can't handle.
Sure, that would also be fine. I'll leave that for the Intel
guys however :)
^ permalink raw reply
* [patch]reset_resume for cdc-ether
From: Oliver Neukum @ 2009-11-24 10:59 UTC (permalink / raw)
To: David Brownell, netdev, USB list, David S. Miller
Hi,
this is quite trivial but might be convenient to many users.
Regards
Oliver
Signed-off-by: Oliver Neukum <oliver@neukum.org>
--
commit 31deb426ccea8faf91f4373c1f12c98bb0d82c73
Author: Oliver Neukum <oliver@neukum.org>
Date: Tue Nov 24 08:30:41 2009 +0100
cdc-ether:imlement reset_resume()
Normal resume can do double duty as reset_resume() for this driver
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index ed55024..11f73dd 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -617,6 +617,7 @@ static struct usb_driver cdc_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .reset_resume = usbnet_resume,
.supports_autosuspend = 1,
};
^ permalink raw reply related
* Re: [PATCH v2] irq: Add node_affinity CPU masks for smarter irqbalance hints
From: Thomas Gleixner @ 2009-11-24 11:07 UTC (permalink / raw)
To: Peter P Waskiewicz Jr
Cc: linux-kernel, arjan, mingo, yong.zhang0, davem, netdev
In-Reply-To: <20091124093518.3909.16435.stgit@ppwaskie-hc2.jf.intel.com>
On Tue, 24 Nov 2009, Peter P Waskiewicz Jr wrote:
> This patchset adds a new CPU mask for SMP systems to the irq_desc
> struct. It also exposes an API for underlying device drivers to
> assist irqbalance in making smarter decisions when balancing, especially
> in a NUMA environment. For example, an ethernet driver with MSI-X may
> wish to limit the CPUs that an interrupt can be balanced within to
> stay on a single NUMA node. Current irqbalance operation can move the
> interrupt off the node, resulting in cross-node memory accesses and
> locks.
>
> The API is a get/set API within the kernel, along with a /proc entry
> for the interrupt.
And what does the kernel do with this information and why are we not
using the existing device/numa_node information ?
> +extern int irq_set_node_affinity(unsigned int irq,
> + const struct cpumask *cpumask);
A node can be described with a single integer, right ?
> +static int irq_node_affinity_proc_show(struct seq_file *m, void *v)
> +{
> + struct irq_desc *desc = irq_to_desc((long)m->private);
> + const struct cpumask *mask = desc->node_affinity;
> +
> + seq_cpumask(m, mask);
> + seq_putc(m, '\n');
> + return 0;
> +}
> +
> #ifndef is_affinity_mask_valid
> #define is_affinity_mask_valid(val) 1
> #endif
> @@ -78,11 +88,46 @@ free_cpumask:
> return err;
> }
>
> +static ssize_t irq_node_affinity_proc_write(struct file *file,
> + const char __user *buffer, size_t count, loff_t *pos)
> +{
> + unsigned int irq = (int)(long)PDE(file->f_path.dentry->d_inode)->data;
> + cpumask_var_t new_value;
> + int err;
> +
> + if (no_irq_affinity || irq_balancing_disabled(irq))
> + return -EIO;
Yikes. Why should user space be allowed to write to that file ? And
the whole business is what for ? Storing that value in the irq_desc
data structure for use space to read out again ?
Cool design. We provide storage space for user space applications in
the kernel now ?
See also my earlier reply in the thread. This patch is just adding
code and memory bloat while not solving anything at all.
Again, this is going nowhere else than into /dev/null.
Thanks,
tglx
^ permalink raw reply
* [ tc ] RTNETLINK answers: Invalid argument
From: thomas yang @ 2009-11-24 11:12 UTC (permalink / raw)
To: netdev
# tc filter add dev eth1 parent 1: protocol ip prio 10 u32 match ip
src 192.168.1.0/24 flowid 1:16 action mirred egress mirror dev wlan0
Action 3 device wlan0 ifindex 4
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
# uname -a
Linux localhost.localdomain 2.6.30.8-64.fc11.i586 #1 SMP Fri Sep 25
04:30:19 EDT 2009 i686 i686 i386 GNU/Linux
OS : Fedora 11
^ permalink raw reply
* Re: large packet loss take2 2.6.31.x
From: Jarek Poplawski @ 2009-11-24 11:19 UTC (permalink / raw)
To: Caleb Cushing
Cc: e1000-devel, netdev, Frans Pop, Jesse Brandeburg, linux-kernel,
Andi Kleen, Jeff Kirsher
In-Reply-To: <81bfc67a0911232217n41b9ac02w3b7770b789e5d209@mail.gmail.com>
On Tue, Nov 24, 2009 at 01:17:09AM -0500, Caleb Cushing wrote:
> > Btw, currently I don't consider this dropping means there has to be
> > a bug. It could be otherwise - a feature... e.g. when a new kernel
> > can transmit faster (then dropping in some other, slower place can
> > happen).
>
> um... where would it be dropping that we wouldn't have a bug? I mean
> sure faster is great... but if it makes my network not work right...
E.g. if it were dropped because of a queue overflow (but it doesn't
seem to be the case, at least at your box) or because of memory
problems while handling a lot of traffic.
>
> I've added all (I think) information you've asked for to the bug
> http://bugzilla.kernel.org/show_bug.cgi?id=13835 except for ethtool
> and netstat on the router side. ethtool complains about not having
> driver or capability (maybe because it's a 2.4 kernel?) and the
> version of netstat doesn't support -s. I disabled everything that I
> can think of that would send/receive packets before doing the test
> client side, except dhcp/dns windows box's were probably sending some
> broadcasts too. but the traffic should be pretty low. I did remember
> to set the txqueuelen didn't seem to make a difference
Alas it's not all information I asked. E.g. "netstat -s before faulty
kernel" and "netstat -s after faulty kernel" seem to be the same file:
netstat_after.slave4.log.gz. Anyway, since there are problems with
getting stats from the router we still can't compare them, or check
for the dropped stats. (Btw, could you check for /proc/net/softnet_stat
yet?)
So, it might be the kernel problem you reported, but there is not
enough data to prove it. Then my proposal is to try to repeat this
problem in more "testing friendly" conditions - preferably against
some other, more up-to-date linux box, if possible?
> only error in dmesg I see is
>
> e1000e 0000:00:19.0: pci_enable_pcie_error_reporting failed 0xfffffffb
I added e1000e maintainers to CC to have a look at this warning.
Jarek P.
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
^ permalink raw reply
* [PATCH net-next-2.6] ixgbe: Fix TX stats accounting
From: Eric Dumazet @ 2009-11-24 11:37 UTC (permalink / raw)
To: Peter P Waskiewicz Jr, Jeff Kirsher
Cc: robert@herjulf.net, Jesper Dangaard Brouer, Linux Netdev List,
David S. Miller
In-Reply-To: <1259057164.2631.36.camel@ppwaskie-mobl2>
Several cpus can update netdev->stats.tx_bytes & netdev->stats.tx_packets
in parallel. In this case, TX stats are under estimated and false sharing
takes place.
After a pktgen session sending exactly 200000000 packets :
# ifconfig fiber0 | grep TX
TX packets:198501982 errors:0 dropped:0 overruns:0 carrier:0
Multi queue devices should instead use txq->tx_bytes & txq->tx_packets
in their xmit() method (appropriate txq lock already held by caller, no
cache line miss), or use appropriate locking.
After patch, same pktgen session gives :
# ifconfig fiber0 | grep TX
TX packets:200000000 errors:0 dropped:0 overruns:0 carrier:0
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
drivers/net/ixgbe/ixgbe_main.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index ebcec30..1cea120 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -425,8 +425,6 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
tx_ring->total_packets += total_packets;
tx_ring->stats.packets += total_packets;
tx_ring->stats.bytes += total_bytes;
- netdev->stats.tx_bytes += total_bytes;
- netdev->stats.tx_packets += total_packets;
return (count < tx_ring->work_limit);
}
@@ -5249,6 +5247,7 @@ static netdev_tx_t ixgbe_xmit_frame(struct sk_buff *skb,
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
struct ixgbe_ring *tx_ring;
+ struct netdev_queue *txq;
unsigned int first;
unsigned int tx_flags = 0;
u8 hdr_len = 0;
@@ -5345,6 +5344,9 @@ static netdev_tx_t ixgbe_xmit_frame(struct sk_buff *skb,
tx_ring->atr_count = 0;
}
}
+ txq = netdev_get_tx_queue(netdev, r_idx);
+ txq->tx_bytes += skb->len;
+ txq->tx_packets++;
ixgbe_tx_queue(adapter, tx_ring, tx_flags, count, skb->len,
hdr_len);
ixgbe_maybe_stop_tx(netdev, tx_ring, DESC_NEEDED);
@@ -5359,19 +5361,6 @@ static netdev_tx_t ixgbe_xmit_frame(struct sk_buff *skb,
}
/**
- * ixgbe_get_stats - Get System Network Statistics
- * @netdev: network interface device structure
- *
- * Returns the address of the device statistics structure.
- * The statistics are actually updated from the timer callback.
- **/
-static struct net_device_stats *ixgbe_get_stats(struct net_device *netdev)
-{
- /* only return the current stats */
- return &netdev->stats;
-}
-
-/**
* ixgbe_set_mac - Change the Ethernet Address of the NIC
* @netdev: network interface device structure
* @p: pointer to an address structure
@@ -5501,7 +5490,6 @@ static const struct net_device_ops ixgbe_netdev_ops = {
.ndo_stop = ixgbe_close,
.ndo_start_xmit = ixgbe_xmit_frame,
.ndo_select_queue = ixgbe_select_queue,
- .ndo_get_stats = ixgbe_get_stats,
.ndo_set_rx_mode = ixgbe_set_rx_mode,
.ndo_set_multicast_list = ixgbe_set_rx_mode,
.ndo_validate_addr = eth_validate_addr,
^ permalink raw reply related
* Re: [PATCH 1/1] Defer skb allocation for both mergeable buffers and big packets in virtio_net
From: Michael S. Tsirkin @ 2009-11-24 11:37 UTC (permalink / raw)
To: Rusty Russell
Cc: Shirley Ma, Eric Dumazet, Avi Kivity, netdev, kvm, linux-kernel,
Hollis Blanchard
In-Reply-To: <200911240854.24054.rusty@rustcorp.com.au>
On Tue, Nov 24, 2009 at 08:54:23AM +1030, Rusty Russell wrote:
> On Tue, 24 Nov 2009 02:37:01 am Shirley Ma wrote:
> > > > + skb = (struct sk_buff *)buf;
> > > This cast is unnecessary, but a comment would be nice:
> >
> > Without this cast there is a compile warning.
>
> Hi Shirley,
>
> Looks like buf is a void *, so no cast should be necessary. But I could
> be reading the patch wrong.
>
> > > However, I question whether making it 16 byte is the right thing: the
> > > ethernet header is 14 bytes long, so don't we want 8 bytes of padding?
> >
> > Because in QEMU it requires 10 bytes header in a separately, so one page
> > is used to share between virtio_net_hdr header which is 10 bytes head
> > and rest of data. So I put 6 bytes offset here between two buffers. I
> > didn't look at the reason why a seperate buf is used for virtio_net_hdr
> > in QEMU.
>
> It's a qemu bug. It insists the header be an element in the scatterlist by
> itself. Unfortunately we have to accommodate it.
We do? Let's just fix this?
All we have to do is replace memcpy with proper iovec walk, correct?
Something like the followng (untested) patch? It's probably not too
late to put this in the next qemu release...
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 2f147e5..06c5148 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -434,26 +434,59 @@ static int iov_fill(struct iovec *iov, int iovcnt, const void *buf, int count)
return offset;
}
+static int iov_skip(struct iovec *iov, int iovcnt, int count)
+{
+ int offset, i;
+
+ offset = i = 0;
+ while (offset < count && i < iovcnt) {
+ int len = MIN(iov[i].iov_len, count - offset);
+ iov[i].iov_base += len;
+ iov[i].iov_len -= len;
+ offset += len;
+ i++;
+ }
+
+ return offset;
+}
+
+static int iov_copy(struct iovec *to, struct iovec *from, int iovcnt, int count)
+{
+ int offset, i;
+
+ offset = i = 0;
+ while (offset < count && i < iovcnt) {
+ int len = MIN(from[i].iov_len, count - offset);
+ to[i].iov_base = from[i].iov_base;
+ to[i].iov_len = from[i].iov_len;
+ offset += len;
+ i++;
+ }
+
+ return i;
+}
+
static int receive_header(VirtIONet *n, struct iovec *iov, int iovcnt,
const void *buf, size_t size, size_t hdr_len)
{
- struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)iov[0].iov_base;
+ struct virtio_net_hdr hdr = {};
int offset = 0;
- hdr->flags = 0;
- hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+ hdr.flags = 0;
+ hdr.gso_type = VIRTIO_NET_HDR_GSO_NONE;
if (n->has_vnet_hdr) {
- memcpy(hdr, buf, sizeof(*hdr));
- offset = sizeof(*hdr);
- work_around_broken_dhclient(hdr, buf + offset, size - offset);
+ memcpy(&hdr, buf, sizeof hdr);
+ offset = sizeof hdr;
+ work_around_broken_dhclient(&hdr, buf + offset, size - offset);
}
+ iov_fill(iov, iovcnt, &hdr, sizeof hdr);
+
/* We only ever receive a struct virtio_net_hdr from the tapfd,
* but we may be passing along a larger header to the guest.
*/
- iov[0].iov_base += hdr_len;
- iov[0].iov_len -= hdr_len;
+ iov_skip(iov, iovcnt, hdr_len);
return offset;
}
@@ -514,7 +547,8 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
static ssize_t virtio_net_receive(VLANClientState *vc, const uint8_t *buf, size_t size)
{
VirtIONet *n = vc->opaque;
- struct virtio_net_hdr_mrg_rxbuf *mhdr = NULL;
+ struct iovec mhdr[VIRTQUEUE_MAX_SIZE];
+ int mhdrcnt = 0;
size_t hdr_len, offset, i;
if (!virtio_net_can_receive(n->vc))
@@ -552,16 +586,13 @@ static ssize_t virtio_net_receive(VLANClientState *vc, const uint8_t *buf, size_
exit(1);
}
- if (!n->mergeable_rx_bufs && elem.in_sg[0].iov_len != hdr_len) {
- fprintf(stderr, "virtio-net header not in first element\n");
- exit(1);
- }
-
memcpy(&sg, &elem.in_sg[0], sizeof(sg[0]) * elem.in_num);
if (i == 0) {
- if (n->mergeable_rx_bufs)
- mhdr = (struct virtio_net_hdr_mrg_rxbuf *)sg[0].iov_base;
+ if (n->mergeable_rx_bufs) {
+ mhdrcnt = iov_copy(mhdr, sg, elem.in_num,
+ sizeof(struct virtio_net_hdr_mrg_rxbuf));
+ }
offset += receive_header(n, sg, elem.in_num,
buf + offset, size - offset, hdr_len);
@@ -579,8 +610,12 @@ static ssize_t virtio_net_receive(VLANClientState *vc, const uint8_t *buf, size_
offset += len;
}
- if (mhdr)
- mhdr->num_buffers = i;
+ if (mhdrcnt) {
+ uint16_t num = i;
+ iov_skip(mhdr, mhdrcnt,
+ offsetof(struct virtio_net_hdr_mrg_rxbuf, num_buffers));
+ iov_fill(mhdr, mhdrcnt, &num, sizeof num);
+ }
virtqueue_flush(n->rx_vq, i);
virtio_notify(&n->vdev, n->rx_vq);
@@ -627,20 +662,19 @@ static void virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq)
sizeof(struct virtio_net_hdr_mrg_rxbuf) :
sizeof(struct virtio_net_hdr);
- if (out_num < 1 || out_sg->iov_len != hdr_len) {
- fprintf(stderr, "virtio-net header not in first element\n");
+ if (out_num < 1) {
+ fprintf(stderr, "virtio-net: no output element\n");
exit(1);
}
/* ignore the header if GSO is not supported */
if (!n->has_vnet_hdr) {
- out_num--;
- out_sg++;
+ iov_skip(out_sg, out_num, hdr_len);
len += hdr_len;
} else if (n->mergeable_rx_bufs) {
/* tapfd expects a struct virtio_net_hdr */
hdr_len -= sizeof(struct virtio_net_hdr);
- out_sg->iov_len -= hdr_len;
+ iov_skip(out_sg, out_num, hdr_len);
len += hdr_len;
}
^ permalink raw reply related
* Re: [PATCH 3/4] macvlan: implement bridge, VEPA and private mode
From: Arnd Bergmann @ 2009-11-24 12:45 UTC (permalink / raw)
To: Patrick McHardy
Cc: Eric Dumazet, linux-kernel, netdev, David Miller,
Stephen Hemminger, Herbert Xu, Patrick Mullaney,
Eric W. Biederman, Edge Virtual Bridging, Anna Fischer, bridge,
virtualization, Jens Osterkamp, Gerhard Stenzel, Mark Smith
In-Reply-To: <4B0BB89F.7030605@trash.net>
On Tuesday 24 November 2009, Patrick McHardy wrote:
> Arnd Bergmann wrote:
> > @@ -29,9 +29,16 @@
> > #include <linux/if_link.h>
> > #include <linux/if_macvlan.h>
> > #include <net/rtnetlink.h>
> > +#include <net/xfrm.h>
>
> Do we really need this?
Not any more. I originally did secpath_reset here, but that is now moved
to dev_forward_skb() in net/core/dev.c so I'll remove the include here.
> > @@ -129,11 +137,14 @@ static inline void macvlan_count_rx(const struct macvlan_dev *vlan, int length,
> > }
> >
> > static int macvlan_broadcast_one(struct sk_buff *skb, struct net_device *dev,
> > - const struct ethhdr *eth)
> > + const struct ethhdr *eth, int local)
>
> bool local?
Already changed as a reply to Eric's comments
> > @@ -159,8 +172,12 @@ static void macvlan_broadcast(struct sk_buff *skb,
> >
> > for (i = 0; i < MACVLAN_HASH_SIZE; i++) {
> > hlist_for_each_entry_rcu(vlan, n, &port->vlan_hash[i], hlist) {
> > + if ((vlan->dev == src) || !(vlan->mode & mode))
>
> Please remove those unnecessary parentheses around the
> device comparison.
Ok. I usually prefer to have some extra parentheses in places that not
everyone finds obvious but these are pretty clear.
>
> The '|' should go on the first line.
>
> Multi-line comments should begin every line with '*'.
>
> Please align the mode values (in all cases above) to the arguments
> on the line above.
ok
> > +static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
> > +{
> > + const struct macvlan_dev *vlan = netdev_priv(dev);
> > + const struct macvlan_port *port = vlan->port;
> > + const struct macvlan_dev *dest;
> > +
> > + if (vlan->mode == MACVLAN_MODE_BRIDGE) {
> > + const struct ethhdr *eth = (void *)skb->data;
>
> eth_hdr()?
This is done before calling eth_type_trans and skb->mac_header
appears to be uninitialized then, which gave me an instant kernel
panic here.
> > + dest = macvlan_hash_lookup(port, eth->h_dest);
> > + if (dest && dest->mode == MACVLAN_MODE_BRIDGE) {
> > + int length = skb->len + ETH_HLEN;
>
> unsigned int for length values please.
ok
Thanks for the detailed review,
Arnd <><
^ permalink raw reply
* Re: [PATCH 4/4] macvlan: export macvlan mode through netlink
From: Arnd Bergmann @ 2009-11-24 12:57 UTC (permalink / raw)
To: virtualization
Cc: Patrick McHardy, Herbert Xu, Eric Dumazet, Anna Fischer, netdev,
bridge, linux-kernel, Mark Smith, Gerhard Stenzel,
Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
Stephen Hemminger, Edge Virtual Bridging, David Miller
In-Reply-To: <4B0BBB2E.8020502@trash.net>
On Tuesday 24 November 2009, Patrick McHardy wrote:
> Arnd Bergmann wrote:
> > @@ -600,6 +594,18 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
> > if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
> > return -EADDRNOTAVAIL;
> > }
> > +
> > + if (data && data[IFLA_MACVLAN_MODE]) {
> > + u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> > + switch (mode) {
> > + case MACVLAN_MODE_PRIVATE:
> > + case MACVLAN_MODE_VEPA:
> > + case MACVLAN_MODE_BRIDGE:
> > + break;
> > + default:
> > + return -EINVAL;
>
> EINVAL is quite unspecific. In this case I think EOPNOTSUPP would
> be fine and provide more information.
ok
> > + }
> > + }
> > return 0;
> > }
> > @@ -664,6 +670,13 @@ static int macvlan_newlink(struct net *src_net, struct net_device *dev,
> > vlan->dev = dev;
> > vlan->port = port;
> >
> > + vlan->mode = MACVLAN_MODE_VEPA;
> > + if (data && data[IFLA_MACVLAN_MODE]) {
> > + u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> > +
> > + vlan->mode = mode;
>
> This looks a bit strange, like cut-and-paste without reformatting :)
> I'd suggest to simply use "vlan->mode = nla_get_u32(...)".
Yep.
Thanks for the review. Combined changes below.
Arnd <><
---
drivers/net/macvlan.c | 47 +++++++++++++++++++++++------------------------
1 files changed, 23 insertions(+), 24 deletions(-)
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -118,15 +118,16 @@ static int macvlan_addr_busy(const struct macvlan_port *port,
return 0;
}
-static inline void macvlan_count_rx(const struct macvlan_dev *vlan, int length,
- bool success, bool multicast)
+static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
+ unsigned int len, bool success,
+ bool multicast)
{
struct macvlan_rx_stats *rx_stats;
rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id());
if (likely(success)) {
rx_stats->rx_packets++;;
- rx_stats->rx_bytes += length;
+ rx_stats->rx_bytes += len;
if (multicast)
rx_stats->multicast++;
} else {
@@ -170,7 +171,7 @@ static void macvlan_broadcast(struct sk_buff *skb,
for (i = 0; i < MACVLAN_HASH_SIZE; i++) {
hlist_for_each_entry_rcu(vlan, n, &port->vlan_hash[i], hlist) {
- if ((vlan->dev == src) || !(vlan->mode & mode))
+ if (vlan->dev == src || !(vlan->mode & mode))
continue;
nskb = skb_clone(skb, GFP_ATOMIC);
@@ -190,7 +191,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
const struct macvlan_dev *vlan;
const struct macvlan_dev *src;
struct net_device *dev;
- int len;
+ unsigned int len;
port = rcu_dereference(skb->dev->macvlan_port);
if (port == NULL)
@@ -200,17 +201,22 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
src = macvlan_hash_lookup(port, eth->h_source);
if (!src)
/* frame comes from an external address */
- macvlan_broadcast(skb, port, NULL, MACVLAN_MODE_PRIVATE
- | MACVLAN_MODE_VEPA | MACVLAN_MODE_BRIDGE);
+ macvlan_broadcast(skb, port, NULL,
+ MACVLAN_MODE_PRIVATE |
+ MACVLAN_MODE_VEPA |
+ MACVLAN_MODE_BRIDGE);
else if (src->mode == MACVLAN_MODE_VEPA)
/* flood to everyone except source */
macvlan_broadcast(skb, port, src->dev,
- MACVLAN_MODE_VEPA | MACVLAN_MODE_BRIDGE);
+ MACVLAN_MODE_VEPA |
+ MACVLAN_MODE_BRIDGE);
else if (src->mode == MACVLAN_MODE_BRIDGE)
- /* flood only to VEPA ports, bridge ports
- already saw the frame */
+ /*
+ * flood only to VEPA ports, bridge ports
+ * already saw the frame on the way out.
+ */
macvlan_broadcast(skb, port, src->dev,
- MACVLAN_MODE_VEPA);
+ MACVLAN_MODE_VEPA);
return skb;
}
@@ -253,7 +259,7 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
dest = macvlan_hash_lookup(port, eth->h_dest);
if (dest && dest->mode == MACVLAN_MODE_BRIDGE) {
- int length = skb->len + ETH_HLEN;
+ unsigned int length = skb->len + ETH_HLEN;
int ret = dev_forward_skb(dest->dev, skb);
macvlan_count_rx(dest, length,
ret == NET_RX_SUCCESS, 0);
@@ -604,8 +610,7 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
}
if (data && data[IFLA_MACVLAN_MODE]) {
- u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
- switch (mode) {
+ switch (nla_get_u32(data[IFLA_MACVLAN_MODE])) {
case MACVLAN_MODE_PRIVATE:
case MACVLAN_MODE_VEPA:
case MACVLAN_MODE_BRIDGE:
@@ -679,11 +684,8 @@ static int macvlan_newlink(struct net *src_net, struct net_device *dev,
vlan->port = port;
vlan->mode = MACVLAN_MODE_VEPA;
- if (data && data[IFLA_MACVLAN_MODE]) {
- u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
-
- vlan->mode = mode;
- }
+ if (data && data[IFLA_MACVLAN_MODE])
+ vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
err = register_netdevice(dev);
if (err < 0)
@@ -710,11 +712,8 @@ static int macvlan_changelink(struct net_device *dev,
struct nlattr *tb[], struct nlattr *data[])
{
struct macvlan_dev *vlan = netdev_priv(dev);
- if (data && data[IFLA_MACVLAN_MODE]) {
- u32 mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
- vlan->mode = mode;
- }
-
+ if (data && data[IFLA_MACVLAN_MODE])
+ vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
return 0;
}
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox