* [PATCH 1/2] forcedeth: make module parameters readable in /sys/module
From: David Decotigny @ 2011-05-18 21:09 UTC (permalink / raw)
To: David S. Miller, Joe Perches, Szymon Janc, netdev, linux-kernel
Cc: kernel-net-upstream, David Decotigny
This change allows to publish the values of the module parameters in
/sys/module.
Signed-off-by: David Decotigny <decot@google.com>
---
drivers/net/forcedeth.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 112dc0b..9566567 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -5990,21 +5990,21 @@ static void __exit exit_nic(void)
pci_unregister_driver(&driver);
}
-module_param(max_interrupt_work, int, 0);
+module_param(max_interrupt_work, int, S_IRUGO);
MODULE_PARM_DESC(max_interrupt_work, "forcedeth maximum events handled per interrupt");
-module_param(optimization_mode, int, 0);
+module_param(optimization_mode, int, S_IRUGO);
MODULE_PARM_DESC(optimization_mode, "In throughput mode (0), every tx & rx packet will generate an interrupt. In CPU mode (1), interrupts are controlled by a timer. In dynamic mode (2), the mode toggles between throughput and CPU mode based on network load.");
-module_param(poll_interval, int, 0);
+module_param(poll_interval, int, S_IRUGO);
MODULE_PARM_DESC(poll_interval, "Interval determines how frequent timer interrupt is generated by [(time_in_micro_secs * 100) / (2^10)]. Min is 0 and Max is 65535.");
-module_param(msi, int, 0);
+module_param(msi, int, S_IRUGO);
MODULE_PARM_DESC(msi, "MSI interrupts are enabled by setting to 1 and disabled by setting to 0.");
-module_param(msix, int, 0);
+module_param(msix, int, S_IRUGO);
MODULE_PARM_DESC(msix, "MSIX interrupts are enabled by setting to 1 and disabled by setting to 0.");
-module_param(dma_64bit, int, 0);
+module_param(dma_64bit, int, S_IRUGO);
MODULE_PARM_DESC(dma_64bit, "High DMA is enabled by setting to 1 and disabled by setting to 0.");
-module_param(phy_cross, int, 0);
+module_param(phy_cross, int, S_IRUGO);
MODULE_PARM_DESC(phy_cross, "Phy crossover detection for Realtek 8201 phy is enabled by setting to 1 and disabled by setting to 0.");
-module_param(phy_power_down, int, 0);
+module_param(phy_power_down, int, S_IRUGO);
MODULE_PARM_DESC(phy_power_down, "Power down phy and disable link when interface is down (1), or leave phy powered up (0).");
MODULE_AUTHOR("Manfred Spraul <manfred@colorfullife.com>");
--
1.7.3.1
^ permalink raw reply related
* [PATCH] forcedeth: Add messages to indicate using MSI or MSI-X
From: David Decotigny @ 2011-05-18 21:10 UTC (permalink / raw)
To: David S. Miller, Joe Perches, Szymon Janc, netdev, linux-kernel
Cc: kernel-net-upstream, Mike Ditto, David Decotigny
In-Reply-To: <1305753049-15025-1-git-send-email-decot@google.com>
From: Mike Ditto <mditto@google.com>
This adds a few debug messages to indicate whether PCIe interrupts are
signaled with MSI or MSI-X.
Signed-off-by: David Decotigny <decot@google.com>
---
drivers/net/forcedeth.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index c9bdee6..e5c5849 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -3745,6 +3745,7 @@ static int nv_request_irq(struct net_device *dev, int intr_test)
writel(0, base + NvRegMSIXMap0);
writel(0, base + NvRegMSIXMap1);
}
+ netdev_info(dev, "forcedeth: MSI-X enabled\n");
}
}
if (ret != 0 && np->msi_flags & NV_MSI_CAPABLE) {
@@ -3766,6 +3767,7 @@ static int nv_request_irq(struct net_device *dev, int intr_test)
writel(0, base + NvRegMSIMap1);
/* enable msi vector 0 */
writel(NVREG_MSI_VECTOR_0_ENABLED, base + NvRegMSIIrqMask);
+ netdev_info(dev, "forcedeth: MSI enabled\n");
}
}
if (ret != 0) {
--
1.7.3.1
^ permalink raw reply related
* [PATCH] forcedeth: Fix a race during rmmod of forcedeth
From: David Decotigny @ 2011-05-18 21:10 UTC (permalink / raw)
To: David S. Miller, Joe Perches, Szymon Janc, netdev, linux-kernel
Cc: kernel-net-upstream, Salman Qazi, David Decotigny
In-Reply-To: <1305753049-15025-1-git-send-email-decot@google.com>
From: Salman Qazi <sqazi@google.com>
The race was between del_timer_sync and nv_do_stats_poll called through
nv_get_ethtool_stats. To prevent this, we have to introduce mutual
exclusion between nv_get_ethtool_stats and del_timer_sync. Notice
that we don't put the mutual exclusion in nv_do_stats_poll. That's
because doing so would result in a deadlock, since it is a timer
callback and hence already waited for by timer deletion.
Signed-off-by: David Decotigny <decot@google.com>
---
drivers/net/forcedeth.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index e5c5849..3163a2b 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -3921,6 +3921,10 @@ static void nv_poll_controller(struct net_device *dev)
}
#endif
+/* No locking is needed as long as this is in the timer
+ * callback. However, any other callers must call this
+ * function with np->lock held.
+ */
static void nv_do_stats_poll(unsigned long data)
{
struct net_device *dev = (struct net_device *) data;
@@ -4553,12 +4557,17 @@ static int nv_get_sset_count(struct net_device *dev, int sset)
static void nv_get_ethtool_stats(struct net_device *dev, struct ethtool_stats *estats, u64 *buffer)
{
+ unsigned long flags;
struct fe_priv *np = netdev_priv(dev);
+ spin_lock_irqsave(&np->lock, flags);
+
/* update stats */
nv_do_stats_poll((unsigned long)dev);
memcpy(buffer, &np->estats, nv_get_sset_count(dev, ETH_SS_STATS)*sizeof(u64));
+
+ spin_unlock_irqrestore(&np->lock, flags);
}
static int nv_link_test(struct net_device *dev)
@@ -5176,13 +5185,13 @@ static int nv_close(struct net_device *dev)
spin_lock_irq(&np->lock);
np->in_shutdown = 1;
+ del_timer_sync(&np->stats_poll);
spin_unlock_irq(&np->lock);
nv_napi_disable(dev);
synchronize_irq(np->pci_dev->irq);
del_timer_sync(&np->oom_kick);
del_timer_sync(&np->nic_poll);
- del_timer_sync(&np->stats_poll);
netif_stop_queue(dev);
spin_lock_irq(&np->lock);
--
1.7.3.1
^ permalink raw reply related
* [PATCH] forcedeth: Acknowledge only interrupts that are being processed
From: David Decotigny @ 2011-05-18 21:10 UTC (permalink / raw)
To: David S. Miller, Joe Perches, Szymon Janc, netdev, linux-kernel
Cc: kernel-net-upstream, Mike Ditto, David Decotigny
From: Mike Ditto <mditto@google.com>
This is to avoid a race, accidentally acknowledging an interrupt that
we didn't notice and won't immediately process. This is based solely
on code inspection; it is not known if there was an actual bug here.
Signed-off-by: David Decotigny <decot@google.com>
---
drivers/net/forcedeth.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 2c176ff..c9bdee6 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -3403,7 +3403,8 @@ static irqreturn_t nv_nic_irq_tx(int foo, void *data)
for (i = 0;; i++) {
events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_TX_ALL;
- writel(NVREG_IRQ_TX_ALL, base + NvRegMSIXIrqStatus);
+ writel(events, base + NvRegMSIXIrqStatus);
+ netdev_dbg(dev, "%s: tx irq: %08x\n", dev->name, events);
if (!(events & np->irqmask))
break;
@@ -3514,7 +3515,8 @@ static irqreturn_t nv_nic_irq_rx(int foo, void *data)
for (i = 0;; i++) {
events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_RX_ALL;
- writel(NVREG_IRQ_RX_ALL, base + NvRegMSIXIrqStatus);
+ writel(events, base + NvRegMSIXIrqStatus);
+ netdev_dbg(dev, "%s: rx irq: %08x\n", dev->name, events);
if (!(events & np->irqmask))
break;
@@ -3558,7 +3560,8 @@ static irqreturn_t nv_nic_irq_other(int foo, void *data)
for (i = 0;; i++) {
events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_OTHER;
- writel(NVREG_IRQ_OTHER, base + NvRegMSIXIrqStatus);
+ writel(events, base + NvRegMSIXIrqStatus);
+ netdev_dbg(dev, "%s: irq: %08x\n", dev->name, events);
if (!(events & np->irqmask))
break;
@@ -3622,10 +3625,10 @@ static irqreturn_t nv_nic_irq_test(int foo, void *data)
if (!(np->msi_flags & NV_MSI_X_ENABLED)) {
events = readl(base + NvRegIrqStatus) & NVREG_IRQSTAT_MASK;
- writel(NVREG_IRQ_TIMER, base + NvRegIrqStatus);
+ writel(events & NVREG_IRQ_TIMER, base + NvRegIrqStatus);
} else {
events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQSTAT_MASK;
- writel(NVREG_IRQ_TIMER, base + NvRegMSIXIrqStatus);
+ writel(events & NVREG_IRQ_TIMER, base + NvRegMSIXIrqStatus);
}
pci_push(base);
if (!(events & NVREG_IRQ_TIMER))
--
1.7.3.1
^ permalink raw reply related
* [PATCH RFC] vhost: fix enable notify: write out last avail value we saw
From: Michael S. Tsirkin @ 2011-05-18 21:13 UTC (permalink / raw)
To: virtualization
Cc: netdev, linux-kernel, rusty, habanero, Shirley Ma, Krishna Kumar2,
kvm, steved, Tom Lendacky, borntraeger, avi
With RX ring and mergeable buffers, vhost-net sometimes
enables notifications when ring is not empty - just
doesn't have enough entries for the incoming packet.
To get event when entries are added in this case,
we should always write out the last index value
we saw into event index field, which is ahead
of the index we have consumed.
Otherwise we get a kick, see that there are not enough
entries in the ring, reenable notifications
but since last used index was not updated
we don't get any more events.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
This bugfix is on top of my patchset, I'm working on addressing
Rusty's comments for that now.
Will push to my git tree event idx branch for everyone's testing pleasure
shortly.
drivers/vhost/vhost.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 2aea4cb..1267a3d 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1438,7 +1438,7 @@ bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
return false;
}
} else {
- r = put_user(vq->last_avail_idx, vhost_avail_event(vq));
+ r = put_user(vq->avail_idx, vhost_avail_event(vq));
if (r) {
vq_err(vq, "Failed to update avail event index at %p: %d\n",
vhost_avail_event(vq), r);
--
1.7.5.53.gc233e
^ permalink raw reply related
* Re: [PATCH] forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts
From: David Miller @ 2011-05-18 21:14 UTC (permalink / raw)
To: decot; +Cc: joe, szymon, netdev, linux-kernel, kernel-net-upstream, snanda
In-Reply-To: <1305752945-14843-2-git-send-email-decot@google.com>
When submitting multiple patches in a patch set, NUMBER THEM.
Otherwise there is no unambiguous way to figure out what order
I should apply these patches.
^ permalink raw reply
* Re: [PATCH 2/2] forcedeth: allow to silence tx_timeout debug messages
From: David Miller @ 2011-05-18 21:16 UTC (permalink / raw)
To: decot; +Cc: joe, szymon, netdev, linux-kernel, kernel-net-upstream, snanda
In-Reply-To: <1305753000-14933-2-git-send-email-decot@google.com>
From: David Decotigny <decot@google.com>
Date: Wed, 18 May 2011 14:10:00 -0700
> From: Sameer Nanda <snanda@google.com>
>
> This change allows to silence most debug messages in case of TX
> timeout. These messages don't provide a signare/noise ratio high
> enough for production systems and, with ~30kB logged each time, they
> tend to add to a cascade effect if the system is already under stress
> (memory pressure, disk, etc.).
>
> By default, the debug messages are not displayed but this can be
> overriden by setting the debug_tx_timeout module parameter.
>
>
> Signed-off-by: David Decotigny <decot@google.com>
I would rather you make the messages less verbose, instead of
having it say absolutely nothing when this happens as it is
a serious problem.
You can add a knob which when enabled gives the old verbosity
back for diagnostic purposes.
^ permalink raw reply
* Re: [PATCH] forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts
From: David Decotigny @ 2011-05-18 21:20 UTC (permalink / raw)
To: David Miller
Cc: Joe Perches, szymon, netdev, linux-kernel, kernel-net-upstream,
snanda
In-Reply-To: <20110518.171416.1780927258138770742.davem@davemloft.net>
Hi David,
They should be independent and applicable in any order, except for the
two 'PATCH x/2' which have to go together.
FYI, here is the order in which I'm applying them:
1/ forcedeth: Improve stats counters
2/ forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts
3/ [PATCH 1/2] forcedeth: make module parameters readable in /sys/module
4/ [PATCH 2/2] forcedeth: allow to silence tx_timeout debug messages
5/ forcedeth: Acknowledge only interrupts that are being processed
6/ forcedeth: Add messages to indicate using MSI or MSI-X
7/ forcedeth: Fix a race during rmmod of forcedeth
Sorry for that.
Regards,
--
David Decotigny
On Wed, May 18, 2011 at 2:14 PM, David Miller <davem@davemloft.net> wrote:
>
> When submitting multiple patches in a patch set, NUMBER THEM.
>
> Otherwise there is no unambiguous way to figure out what order
> I should apply these patches.
>
^ permalink raw reply
* Re: [PATCH] forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts
From: David Miller @ 2011-05-18 21:26 UTC (permalink / raw)
To: decot; +Cc: joe, szymon, netdev, linux-kernel, kernel-net-upstream, snanda
In-Reply-To: <BANLkTimNvuB5HtOKfAc7rf5wF2QeC3xdXw@mail.gmail.com>
From: David Decotigny <decot@google.com>
Date: Wed, 18 May 2011 14:20:21 -0700
> Hi David,
>
> They should be independent and applicable in any order, except for the
> two 'PATCH x/2' which have to go together.
That doesn't matter.
When you are sending a set of patches, always number them.
Please resubmit your changes with my feedback incorporated, I'm
not applying what you sent until everything is fixed up.
Thanks.
^ permalink raw reply
* Re: [patch 1/1] qeth: use ndo_set_features callback for initial setup and recovery
From: David Miller @ 2011-05-18 21:27 UTC (permalink / raw)
To: mirqus; +Cc: frank.blaschka, netdev, linux-s390
In-Reply-To: <BANLkTik3A8zdDODnSntYZs8sfBvdtLGrOw@mail.gmail.com>
From: Michał Mirosław <mirqus@gmail.com>
Date: Wed, 18 May 2011 15:43:11 +0200
> 2011/5/18 <frank.blaschka@de.ibm.com>:
>> From: Frank Blaschka <frank.blaschka@de.ibm.com>
>>
>> This patch uses the ndo_set_features callback during normal device
>> startup or recovery to turn on hardware RX checksum. Patch was done
>> with much help from Michal Miroslaw, thx!!!
>>
>> Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
>
> [regarding usage of ndo_set_features and friends]
> Reviewed-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] bnx2x: add support for retrieving dcb peer configuration
From: David Miller @ 2011-05-18 21:29 UTC (permalink / raw)
To: shmulikr; +Cc: eilong, netdev
In-Reply-To: <1305723331.14458.13.camel@lb-tlvb-shmulik.il.broadcom.com>
From: "Shmulik Ravid" <shmulikr@broadcom.com>
Date: Wed, 18 May 2011 15:55:31 +0300
> This patch adds support to the bnx2x for retrieving dcb peer (remote)
> configuration from the embedded DCBX stack.
>
> Signed-off-by: Shmulik Ravid <shmulikr@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next-2.6] be2net: Enable SR-IOV for Lancer
From: David Miller @ 2011-05-18 21:31 UTC (permalink / raw)
To: padmanabh.ratnakar; +Cc: netdev, mammatha.edhala
In-Reply-To: <55d7e67f-84d8-4b3a-9d39-a68fab9a2a54@exht1.ad.emulex.com>
From: Padmanabh Ratnakar <padmanabh.ratnakar@Emulex.Com>
Date: Wed, 18 May 2011 18:56:22 +0530
> From: Mammatha Edhala <mammatha.edhala@emulex.com>
>
> Enable SR-IOV for Lancer
>
> Signed-off-by: Mammatha Edhala <mammatha.edhala@emulex.com>
> Signed-off-by: Padmanabh Ratnakar <padmanabh.ratnakar@emulex.com>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2] forcedeth: allow to silence tx_timeout debug messages
From: David Decotigny @ 2011-05-18 21:43 UTC (permalink / raw)
To: David Miller
Cc: Joe Perches, szymon, netdev, linux-kernel, kernel-net-upstream,
Sameer Nanda
In-Reply-To: <20110518.171603.2048423328863525352.davem@davemloft.net>
David,
No problem, I will send the patch series correctly numbered. Sorry for that.
Regarding your comment about this debug info change:
On Wed, May 18, 2011 at 2:16 PM, David Miller <davem@davemloft.net> wrote:
> You can add a knob which when enabled gives the old verbosity
> back for diagnostic purposes.
That was the intent of this patch: it adds a debug_tx_timeout module
parameter to act as the knob. I can rephrase the description of the
patch, it didn't make this so clear.
Or are you suggesting I should implement this with another kind of knob?
Thanks!
^ permalink raw reply
* [PATCH RFC] virtio_net: fix patch: virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-05-18 22:01 UTC (permalink / raw)
To: rusty, habanero, Shirley Ma, Krishna Kumar2, kvm, steved,
Tom Lendacky <tahm@
Cc: Rusty Russell, Michael S. Tsirkin, virtualization, netdev,
linux-kernel
The patch virtio_net: limit xmit polling
got the logic reversed: it polled while we had
capacity not while ring was empty.
Fix it up and clean up a bit by using a for loop.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
OK, turns out that patch was borken. Here's
a fix that survived stress test on my box.
Pushed on my branch, I'll send a rebased series
with Rusty's comments addressed ASAP.
drivers/net/virtio_net.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 9982bd7..c8cd22d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -514,12 +514,14 @@ static bool free_old_xmit_skbs(struct virtnet_info *vi, int capacity)
struct sk_buff *skb;
unsigned int len;
bool c;
+ int n;
+
/* We try to free up at least 2 skbs per one sent, so that we'll get
* all of the memory back if they are used fast enough. */
- int n = 2;
-
- while ((c = virtqueue_get_capacity(vi->svq) >= capacity) && --n > 0 &&
- (skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
+ for (n = 0;
+ ((c = virtqueue_get_capacity(vi->svq)) < capacity || n < 2) &&
+ ((skb = virtqueue_get_buf(vi->svq, &len)));
+ ++n) {
pr_debug("Sent skb %p\n", skb);
vi->dev->stats.tx_bytes += skb->len;
vi->dev->stats.tx_packets++;
--
1.7.5.53.gc233e
^ permalink raw reply related
* Re: [PATCH 2/2] forcedeth: allow to silence tx_timeout debug messages
From: David Miller @ 2011-05-18 22:01 UTC (permalink / raw)
To: decot; +Cc: joe, szymon, netdev, linux-kernel, kernel-net-upstream, snanda
In-Reply-To: <BANLkTinG4K-bWS1qqBr_G9mK5OHDYknszg@mail.gmail.com>
From: David Decotigny <decot@google.com>
Date: Wed, 18 May 2011 14:43:58 -0700
> On Wed, May 18, 2011 at 2:16 PM, David Miller <davem@davemloft.net> wrote:
>> You can add a knob which when enabled gives the old verbosity
>> back for diagnostic purposes.
>
> That was the intent of this patch: it adds a debug_tx_timeout module
> parameter to act as the knob. I can rephrase the description of the
> patch, it didn't make this so clear.
> Or are you suggesting I should implement this with another kind of knob?
I'm saying implement things differently.
Reduce the verbosity of the TX timeout message but still print some
amount of information, but provide a new knob that turns on the
existing verbose messages.
^ permalink raw reply
* Re: [PATCH 1/2] forcedeth: make module parameters readable in /sys/module
From: Stephen Hemminger @ 2011-05-18 22:03 UTC (permalink / raw)
To: David Decotigny
Cc: David S. Miller, Joe Perches, Szymon Janc, netdev, linux-kernel,
kernel-net-upstream
In-Reply-To: <1305753000-14933-1-git-send-email-decot@google.com>
On Wed, 18 May 2011 14:09:59 -0700
David Decotigny <decot@google.com> wrote:
> This change allows to publish the values of the module parameters in
> /sys/module.
>
>
Although this makes more info for developer, it also means more
stuff in sysfs taking more memory and not providing any real value
that can't be found by looking at the /etc/modprobe.d for any parameters
user might have set.
^ permalink raw reply
* Re: [RFC V4 PATCH] rtnetlink: Compute and store minimum ifinfo dump size
From: David Miller @ 2011-05-18 22:09 UTC (permalink / raw)
To: gregory.v.rose; +Cc: netdev, bhutchings, eric.dumazet
In-Reply-To: <20110518155410.28491.62723.stgit@gitlad.jf.intel.com>
From: Greg Rose <gregory.v.rose@intel.com>
Date: Wed, 18 May 2011 08:54:11 -0700
> The message size allocated for rtnl info dumps was limited to a single
> page. This is not enough for additional interface info available with
> devices that support SR-IOV. Calculate the amount of data required so
> the dump can allocate enough data to satisfy the request.
>
> V2 added the calcit function to the rtnl_register calls so that
> dump functions could get the minimum dump allocation size if they
> needed to.
>
> V3 leverages the fact that the netdev register function ends up
> calling if_nlmsg_size. We collect the minimum dump allocation size
> there and keep it in a module static variable so that the calcit
> function doesn't have to search for the device on every info dump.
>
> V4 fixes the title.
>
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
This only works if someone does a plain GETLINK request on the largest
space requiring device before the dump request happens.
Otherwise the value used by the calcit function won't be properly
prepared yet.
^ permalink raw reply
* Re: [PATCH net-next] bridge: add notification over netlink when STP changes state
From: David Miller @ 2011-05-18 22:10 UTC (permalink / raw)
To: shemminger; +Cc: bridge, netdev
In-Reply-To: <20110518081718.0b1fc390@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 18 May 2011 08:17:18 -0700
> The first netlink code in the bridge module was to notify
> user space implementations of Spanning Tree Protocol about
> new ports. It did not handle the case of kernel mode STP
> changing states which could be useful for monitoring.
>
> This patch causes RTM_NEWLINK message to occur on kernel
> transitions. It does not send message if request was from user
> space STP, since that would cause reflection and break existing
> API.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
I agree with Ben that hiding events in any way is going to
be a severe limitation, long term.
I do not want to apply this and keep going down this road.
Sorry.
^ permalink raw reply
* Re: [PATCH v3] ipconfig wait for carrier
From: David Miller @ 2011-05-18 22:14 UTC (permalink / raw)
To: micha; +Cc: netdev
In-Reply-To: <4DD36E54.9040706@neli.hopto.org>
From: Micha Nelissen <micha@neli.hopto.org>
Date: Wed, 18 May 2011 08:59:32 +0200
> Op 2011-05-18 8:37, David Miller schreef:
>> From: Micha Nelissen<micha@neli.hopto.org>
>> Date: Wed, 18 May 2011 08:32:35 +0200
>>
>>> I'm confused. Against which tree/commit do you want it then?
>>
>> Linus's current tree would be fine as would:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6.git
>
> Ok I see, thanks. The patch should apply just fine to your tree, there
> is only a spelling change since 2.6.38 which does not conflict.
Please fix ic_is_init_dev() to return a proper boolean "false" instead
of "0" when IFF_LOOPBACK is set.
^ permalink raw reply
* Re: [PATCH 2/2] bna: Add Generic Netlink Interface
From: Debashis Dutt @ 2011-05-18 22:28 UTC (permalink / raw)
To: David Lamparter
Cc: Rasesh Mody, davem, netdev, adapter_linux_open_src_team,
Debashis Dutt
In-Reply-To: <20110518120005.GC3762520@jupiter.n2.diac24.net>
On Wed, May 18, 2011 at 5:00 AM, David Lamparter <equinox@diac24.net> wrote:
> On Tue, May 17, 2011 at 09:57:01PM -0700, Rasesh Mody wrote:
>> This patch adds the generic netlink communication interface to BNA driver. The
>> in-kernel generic netlink infrastructure can be used to collect hardware
>> specific control information and control attributes. The driver makes use of
>> the "doit" handler provided by the generic netlink layer to accomplish this.
> [...]
>> +struct bnad_genl_ioc_info {
>> + int status;
>> + u16 bnad_num;
>> + u16 rsvd;
>> + char serialnum[64];
>> + char hwpath[BFA_STRING_32];
>> + char adapter_hwpath[BFA_STRING_32];
>> + char guid[BFA_ADAPTER_SYM_NAME_LEN*2];
>> + char name[BFA_ADAPTER_SYM_NAME_LEN];
>> + char port_name[BFA_ADAPTER_SYM_NAME_LEN];
>> + char eth_name[BFA_ADAPTER_SYM_NAME_LEN];
>> + u64 rsvd1[4];
>> + mac_t mac;
>> + mac_t factory_mac; /* Factory mac address */
>> + mac_t current_mac; /* Currently assigned mac address */
>> + enum bfa_ioc_type ioc_type;
>> + u16 pvid; /* Port vlan id */
>> + u16 rsvd2;
>> + u32 host;
>> + u32 bandwidth; /* For PF support */
>> + u32 rsvd3;
>> +};
>> +
>> +struct bnad_genl_ioc_attr {
>> + int status;
>> + u16 bnad_num;
>> + u16 rsvd;
>> + struct bfa_ioc_attr ioc_attr;
>> +};
>
> These things all look like they're better put into sysfs. Why would you
> create a genl protocol just to query some presumably static attributes?
>
>
> -David
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Hi David,
There could be different ways of doing it, but the reason we chose to go
the generic netlink route are as follows:
1) The recommended format of sysfs (as per sysfs.txt in kernel documentation)
is one single line of ASCII text per file.
As a result this:
a) if there are a lot of attributes to be collected,
the driver will end up in creating that many sysfs files.
b) Reading / writing each attribute will result in a set of
[open()/read()/write()/close()] calls.
This is not very scalable, particularly if we want to expand on this
interface for better management functionality in the future.
Instead, generic netlink provides a much better way of multiplexing data
over a single socket.
2) Asynchronous event notifications can be much easily handled using the generic
netlink interface.
Thanks
--Debashis
^ permalink raw reply
* Re: [PATCH] Check the value of doi before referencing it
From: Paul Moore @ 2011-05-18 22:32 UTC (permalink / raw)
To: huzaifas; +Cc: David Miller, netdev, kaber, yoshfuji, jmorris, pekkas, kuznet
In-Reply-To: <20110518.010453.1295824792800350412.davem@davemloft.net>
On Wednesday, May 18, 2011 1:04:53 AM David Miller wrote:
> From: Huzaifa Sidhpurwala <huzaifas@redhat.com>
> Date: Wed, 18 May 2011 09:59:40 +0530
>
> > Value of doi is not checked before referencing it.
> > Though this does not cause any null pointer dereference since
> > all the callers of cipso_v4_doi_add check the value of doi
> > before calling the function, but it would be a good programming
> > practice to do so anyways :)
> >
> > Signed-off-by: Huzaifa Sidhpurwala <huzaifas@redhat.com>
>
> I don't think we should fix bugs that do not exist.
I agree with David.
If there were a large number of callers or cipso_v4_doi_add() was a more
general function there might be some merit in performing more sanity checks on
the values passed to the function. However, as it stands, cipso_v4_doi_add()
is a fairly specialized function which is called by a small number of
functions all of which are internal to NetLabel.
--
paul moore
linux @ hp
^ permalink raw reply
* RE: [RFC V4 PATCH] rtnetlink: Compute and store minimum ifinfo dump size
From: Rose, Gregory V @ 2011-05-18 22:35 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, bhutchings@solarflare.com,
eric.dumazet@gmail.com
In-Reply-To: <20110518.180913.273729651812642114.davem@davemloft.net>
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, May 18, 2011 3:09 PM
> To: Rose, Gregory V
> Cc: netdev@vger.kernel.org; bhutchings@solarflare.com;
> eric.dumazet@gmail.com
> Subject: Re: [RFC V4 PATCH] rtnetlink: Compute and store minimum ifinfo
> dump size
>
> From: Greg Rose <gregory.v.rose@intel.com>
> Date: Wed, 18 May 2011 08:54:11 -0700
>
> > The message size allocated for rtnl info dumps was limited to a single
> > page. This is not enough for additional interface info available with
> > devices that support SR-IOV. Calculate the amount of data required so
> > the dump can allocate enough data to satisfy the request.
> >
> > V2 added the calcit function to the rtnl_register calls so that
> > dump functions could get the minimum dump allocation size if they
> > needed to.
> >
> > V3 leverages the fact that the netdev register function ends up
> > calling if_nlmsg_size. We collect the minimum dump allocation size
> > there and keep it in a module static variable so that the calcit
> > function doesn't have to search for the device on every info dump.
> >
> > V4 fixes the title.
> >
> > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
>
> This only works if someone does a plain GETLINK request on the largest
> space requiring device before the dump request happens.
It looked to me like rtmsg_ifinfo is called when each device is registered through register_netdevice() and rtmsg_ifinfo in turn calls the if_nlmsg_size function returning the ifinfo dump size for each device. In my testing it looked like the proper maximum size was being set after I loaded the drivers for the SR-IOV capable devices.
Perhaps I missed something? Is register_netdevice not always called before a device is used?
- Greg
^ permalink raw reply
* Re: [Patch net-next-2.6] rfkill: remove CONFIG_RFKILL_INPUT
From: Johannes Berg @ 2011-05-18 22:47 UTC (permalink / raw)
To: Américo Wang
Cc: Linux Kernel Network Developers,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
Andrew Morton
In-Reply-To: <BANLkTin9EbYhOHiU-EwGDTsXegks_hAT_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, 2011-05-16 at 18:44 +0800, Américo Wang wrote:
> This is scheduled to be removed in 2.6.33, now we are at 2.6.39-rc2.
> So I think it is safe to remove it completely.
We still don't have urfkill widely deployed, so NAK. 2.6.33 was wishful
thinking.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC V4 PATCH] rtnetlink: Compute and store minimum ifinfo dump size
From: David Miller @ 2011-05-18 22:47 UTC (permalink / raw)
To: gregory.v.rose; +Cc: netdev, bhutchings, eric.dumazet
In-Reply-To: <43F901BD926A4E43B106BF17856F0755018E321C21@orsmsx508.amr.corp.intel.com>
From: "Rose, Gregory V" <gregory.v.rose@intel.com>
Date: Wed, 18 May 2011 15:35:58 -0700
> It looked to me like rtmsg_ifinfo is called when each device is
> registered through register_netdevice() and rtmsg_ifinfo in turn
> calls the if_nlmsg_size function returning the ifinfo dump size for
> each device. In my testing it looked like the proper maximum size
> was being set after I loaded the drivers for the SR-IOV capable
> devices.
Aha, indeed you're right. I missed this part. Thanks for explaining.
So as far as I can tell it should work, I'll let others review it to
see if they are ok with this approach.
Thanks.
^ permalink raw reply
* RE: [RFC V4 PATCH] rtnetlink: Compute and store minimum ifinfo dump size
From: Rose, Gregory V @ 2011-05-18 22:49 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, bhutchings@solarflare.com,
eric.dumazet@gmail.com
In-Reply-To: <20110518.184741.476096648010572360.davem@davemloft.net>
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, May 18, 2011 3:48 PM
> To: Rose, Gregory V
> Cc: netdev@vger.kernel.org; bhutchings@solarflare.com;
> eric.dumazet@gmail.com
> Subject: Re: [RFC V4 PATCH] rtnetlink: Compute and store minimum ifinfo
> dump size
>
> From: "Rose, Gregory V" <gregory.v.rose@intel.com>
> Date: Wed, 18 May 2011 15:35:58 -0700
>
> > It looked to me like rtmsg_ifinfo is called when each device is
> > registered through register_netdevice() and rtmsg_ifinfo in turn
> > calls the if_nlmsg_size function returning the ifinfo dump size for
> > each device. In my testing it looked like the proper maximum size
> > was being set after I loaded the drivers for the SR-IOV capable
> > devices.
>
> Aha, indeed you're right. I missed this part. Thanks for explaining.
>
> So as far as I can tell it should work, I'll let others review it to
> see if they are ok with this approach.
Ok, sure. We'll see if Eric catches anything, he's got an eagle eye.
;^)
- Greg
^ 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