* [PATCH 2/7] tg3: Fix NVRAM selftest
From: Matt Carlson @ 2011-03-10 2:58 UTC (permalink / raw)
To: davem; +Cc: netdev, mcarlson, stable
The tg3 NVRAM selftest actually fails when validating the checksum of
the legacy NVRAM format. However, the test still reported success
because the last update of the return code was a success from the NVRAM
reads. This patch fixes the code so that the error return code defaults
to a failure status. Then the patch fixes the reason why the checsum
validation failed.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 6fd5cf0..8f71608 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -10487,14 +10487,16 @@ static int tg3_test_nvram(struct tg3 *tp)
goto out;
}
+ err = -EIO;
+
/* Bootstrap checksum at offset 0x10 */
csum = calc_crc((unsigned char *) buf, 0x10);
- if (csum != be32_to_cpu(buf[0x10/4]))
+ if (csum != le32_to_cpu(buf[0x10/4]))
goto out;
/* Manufacturing block starts at offset 0x74, checksum at 0xfc */
csum = calc_crc((unsigned char *) &buf[0x74/4], 0x88);
- if (csum != be32_to_cpu(buf[0xfc/4]))
+ if (csum != le32_to_cpu(buf[0xfc/4]))
goto out;
err = 0;
--
1.7.3.4
^ permalink raw reply related
* [PATCH 1/7] tg3: Add missed 5719 workaround change
From: Matt Carlson @ 2011-03-10 2:58 UTC (permalink / raw)
To: davem; +Cc: netdev, mcarlson, stable
Commit 2866d956fe0ad8fc8d8a7c54104ccc879b49406d, entitled
"tg3: Expand 5719 workaround" extended a 5719 A0 workaround to all
revisions of the chip. There was a change that should have been a
part of that patch that was missed. This patch adds the missing
piece.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 6be4185..6fd5cf0 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -8103,7 +8103,7 @@ static int tg3_reset_hw(struct tg3 *tp, int reset_phy)
/* Program the jumbo buffer descriptor ring control
* blocks on those devices that have them.
*/
- if (tp->pci_chip_rev_id == CHIPREV_ID_5719_A0 ||
+ if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5719 ||
((tp->tg3_flags & TG3_FLAG_JUMBO_CAPABLE) &&
!(tp->tg3_flags2 & TG3_FLG2_5780_CLASS))) {
/* Setup replenish threshold. */
--
1.7.3.4
^ permalink raw reply related
* [PATCH net-next 0/7] tg3: Cleanups
From: Matt Carlson @ 2011-03-10 2:58 UTC (permalink / raw)
To: davem; +Cc: netdev, mcarlson, stable
This patchset adds a 5719 workaround, VPD RODATA checksum verification,
and some code cleanups
^ permalink raw reply
* [PATCH 3/7] tg3: Add code to verify RODATA checksum of VPD
From: Matt Carlson @ 2011-03-10 2:58 UTC (permalink / raw)
To: davem; +Cc: netdev, mcarlson, stable
This patch adds code to verify the checksum stored in the "RV" info
keyword of the RODATA VPD section.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/pci.h | 1 +
2 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 8f71608..ffb0979 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -10499,6 +10499,41 @@ static int tg3_test_nvram(struct tg3 *tp)
if (csum != le32_to_cpu(buf[0xfc/4]))
goto out;
+ for (i = 0; i < TG3_NVM_VPD_LEN; i += 4) {
+ /* The data is in little-endian format in NVRAM.
+ * Use the big-endian read routines to preserve
+ * the byte order as it exists in NVRAM.
+ */
+ if (tg3_nvram_read_be32(tp, TG3_NVM_VPD_OFF + i, &buf[i/4]))
+ goto out;
+ }
+
+ i = pci_vpd_find_tag((u8 *)buf, 0, TG3_NVM_VPD_LEN,
+ PCI_VPD_LRDT_RO_DATA);
+ if (i > 0) {
+ j = pci_vpd_lrdt_size(&((u8 *)buf)[i]);
+ if (j < 0)
+ goto out;
+
+ if (i + PCI_VPD_LRDT_TAG_SIZE + j > TG3_NVM_VPD_LEN)
+ goto out;
+
+ i += PCI_VPD_LRDT_TAG_SIZE;
+ j = pci_vpd_find_info_keyword((u8 *)buf, i, j,
+ PCI_VPD_RO_KEYWORD_CHKSUM);
+ if (j > 0) {
+ u8 csum8 = 0;
+
+ j += PCI_VPD_INFO_FLD_HDR_SIZE;
+
+ for (i = 0; i <= j; i++)
+ csum8 += ((u8 *)buf)[i];
+
+ if (csum8)
+ goto out;
+ }
+ }
+
err = 0;
out:
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 559d028..ff5bccb 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1479,6 +1479,7 @@ void pci_request_acs(void);
#define PCI_VPD_RO_KEYWORD_PARTNO "PN"
#define PCI_VPD_RO_KEYWORD_MFR_ID "MN"
#define PCI_VPD_RO_KEYWORD_VENDOR0 "V0"
+#define PCI_VPD_RO_KEYWORD_CHKSUM "RV"
/**
* pci_vpd_lrdt_size - Extracts the Large Resource Data Type length
--
1.7.3.4
^ permalink raw reply related
* [PATCH 6/7] tg3: Move tg3_init_link_config to tg3_phy_probe
From: Matt Carlson @ 2011-03-10 2:58 UTC (permalink / raw)
To: davem; +Cc: netdev, mcarlson, stable
This patch moves the function that initializes the link configuration
closer to the place where the rest of the phy code is initialized.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 69 +++++++++++++++++++++++++++--------------------------
1 files changed, 35 insertions(+), 34 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 159eb23..2c67cc9 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -12557,12 +12557,45 @@ static u32 __devinit tg3_read_otp_phycfg(struct tg3 *tp)
return ((thalf_otp & 0x0000ffff) << 16) | (bhalf_otp >> 16);
}
+static void __devinit tg3_phy_init_link_config(struct tg3 *tp)
+{
+ u32 adv = ADVERTISED_Autoneg |
+ ADVERTISED_Pause;
+
+ if (!(tp->phy_flags & TG3_PHYFLG_10_100_ONLY))
+ adv |= ADVERTISED_1000baseT_Half |
+ ADVERTISED_1000baseT_Full;
+
+ if (!(tp->phy_flags & TG3_PHYFLG_ANY_SERDES))
+ adv |= ADVERTISED_100baseT_Half |
+ ADVERTISED_100baseT_Full |
+ ADVERTISED_10baseT_Half |
+ ADVERTISED_10baseT_Full |
+ ADVERTISED_TP;
+ else
+ adv |= ADVERTISED_FIBRE;
+
+ tp->link_config.advertising = adv;
+ tp->link_config.speed = SPEED_INVALID;
+ tp->link_config.duplex = DUPLEX_INVALID;
+ tp->link_config.autoneg = AUTONEG_ENABLE;
+ tp->link_config.active_speed = SPEED_INVALID;
+ tp->link_config.active_duplex = DUPLEX_INVALID;
+ tp->link_config.orig_speed = SPEED_INVALID;
+ tp->link_config.orig_duplex = DUPLEX_INVALID;
+ tp->link_config.orig_autoneg = AUTONEG_INVALID;
+}
+
static int __devinit tg3_phy_probe(struct tg3 *tp)
{
u32 hw_phy_id_1, hw_phy_id_2;
u32 hw_phy_id, hw_phy_id_masked;
int err;
+ /* flow control autonegotiation is default behavior */
+ tp->tg3_flags |= TG3_FLAG_PAUSE_AUTONEG;
+ tp->link_config.flowctrl = FLOW_CTRL_TX | FLOW_CTRL_RX;
+
if (tp->tg3_flags3 & TG3_FLG3_USE_PHYLIB)
return tg3_phy_init(tp);
@@ -12624,6 +12657,8 @@ static int __devinit tg3_phy_probe(struct tg3 *tp)
tp->pci_chip_rev_id != CHIPREV_ID_57765_A0)))
tp->phy_flags |= TG3_PHYFLG_EEE_CAP;
+ tg3_phy_init_link_config(tp);
+
if (!(tp->phy_flags & TG3_PHYFLG_ANY_SERDES) &&
!(tp->tg3_flags3 & TG3_FLG3_ENABLE_APE) &&
!(tp->tg3_flags & TG3_FLAG_ENABLE_ASF)) {
@@ -12679,17 +12714,6 @@ skip_phy_reset:
err = tg3_init_5401phy_dsp(tp);
}
- if (tp->phy_flags & TG3_PHYFLG_ANY_SERDES)
- tp->link_config.advertising =
- (ADVERTISED_1000baseT_Half |
- ADVERTISED_1000baseT_Full |
- ADVERTISED_Autoneg |
- ADVERTISED_FIBRE);
- if (tp->phy_flags & TG3_PHYFLG_10_100_ONLY)
- tp->link_config.advertising &=
- ~(ADVERTISED_1000baseT_Half |
- ADVERTISED_1000baseT_Full);
-
return err;
}
@@ -14422,23 +14446,6 @@ out_nofree:
return ret;
}
-static void __devinit tg3_init_link_config(struct tg3 *tp)
-{
- tp->link_config.advertising =
- (ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
- ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full |
- ADVERTISED_1000baseT_Half | ADVERTISED_1000baseT_Full |
- ADVERTISED_Autoneg | ADVERTISED_MII);
- tp->link_config.speed = SPEED_INVALID;
- tp->link_config.duplex = DUPLEX_INVALID;
- tp->link_config.autoneg = AUTONEG_ENABLE;
- tp->link_config.active_speed = SPEED_INVALID;
- tp->link_config.active_duplex = DUPLEX_INVALID;
- tp->link_config.orig_speed = SPEED_INVALID;
- tp->link_config.orig_duplex = DUPLEX_INVALID;
- tp->link_config.orig_autoneg = AUTONEG_INVALID;
-}
-
static void __devinit tg3_init_bufmgr_config(struct tg3 *tp)
{
if (tp->tg3_flags3 & TG3_FLG3_5717_PLUS) {
@@ -14742,8 +14749,6 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
goto err_out_free_dev;
}
- tg3_init_link_config(tp);
-
tp->rx_pending = TG3_DEF_RX_RING_PENDING;
tp->rx_jumbo_pending = TG3_DEF_RX_JUMBO_RING_PENDING;
@@ -14891,10 +14896,6 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
goto err_out_apeunmap;
}
- /* flow control autonegotiation is default behavior */
- tp->tg3_flags |= TG3_FLAG_PAUSE_AUTONEG;
- tp->link_config.flowctrl = FLOW_CTRL_TX | FLOW_CTRL_RX;
-
intmbx = MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW;
rcvmbx = MAILBOX_RCVRET_CON_IDX_0 + TG3_64BIT_REG_LOW;
sndmbx = MAILBOX_SNDHOST_PROD_IDX_0 + TG3_64BIT_REG_LOW;
--
1.7.3.4
^ permalink raw reply related
* [PATCH 4/7] tg3: cleanup pci device table vars
From: Matt Carlson @ 2011-03-10 2:58 UTC (permalink / raw)
To: davem; +Cc: netdev, mcarlson, stable
Commit 895950c2a6565d9eefda4a38b00fa28537e39fcb, entitled
"tg3: Use DEFINE_PCI_DEVICE_TABLE" moved two pci device tables into the
global address space, but didn't declare them static and didn't prefix
them with "tg3_". This patch fixes those problems.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index ffb0979..73eacbd 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -13115,7 +13115,7 @@ static inline u32 tg3_rx_ret_ring_size(struct tg3 *tp)
return 512;
}
-DEFINE_PCI_DEVICE_TABLE(write_reorder_chipsets) = {
+static DEFINE_PCI_DEVICE_TABLE(tg3_write_reorder_chipsets) = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_FE_GATE_700C) },
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8131_BRIDGE) },
{ PCI_DEVICE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8385_0) },
@@ -13469,7 +13469,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
* every mailbox register write to force the writes to be
* posted to the chip in order.
*/
- if (pci_dev_present(write_reorder_chipsets) &&
+ if (pci_dev_present(tg3_write_reorder_chipsets) &&
!(tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS))
tp->tg3_flags |= TG3_FLAG_MBOX_WRITE_REORDER;
@@ -14225,7 +14225,7 @@ static int __devinit tg3_do_test_dma(struct tg3 *tp, u32 *buf, dma_addr_t buf_dm
#define TEST_BUFFER_SIZE 0x2000
-DEFINE_PCI_DEVICE_TABLE(dma_wait_state_chipsets) = {
+static DEFINE_PCI_DEVICE_TABLE(tg3_dma_wait_state_chipsets) = {
{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_UNI_N_PCI15) },
{ },
};
@@ -14404,7 +14404,7 @@ static int __devinit tg3_test_dma(struct tg3 *tp)
* now look for chipsets that are known to expose the
* DMA bug without failing the test.
*/
- if (pci_dev_present(dma_wait_state_chipsets)) {
+ if (pci_dev_present(tg3_dma_wait_state_chipsets)) {
tp->dma_rwctrl &= ~DMA_RWCTRL_WRITE_BNDRY_MASK;
tp->dma_rwctrl |= DMA_RWCTRL_WRITE_BNDRY_16;
} else {
--
1.7.3.4
^ permalink raw reply related
* [PATCH 7/7] tg3: Remove 5750 PCI code
From: Matt Carlson @ 2011-03-10 2:58 UTC (permalink / raw)
To: davem; +Cc: netdev, mcarlson, stable
The 5750 ASIC rev was never released as a PCI device. It only exists as
a PCIe device. This patch removes the code that supports the former
configuration.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 15 +++++----------
1 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 2c67cc9..ebec888 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -8193,10 +8193,8 @@ static int tg3_reset_hw(struct tg3 *tp, int reset_phy)
RDMAC_MODE_MBUF_RBD_CRPT_ENAB |
RDMAC_MODE_MBUF_SBD_CRPT_ENAB;
- /* If statement applies to 5705 and 5750 PCI devices only */
- if ((GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705 &&
- tp->pci_chip_rev_id != CHIPREV_ID_5705_A0) ||
- (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5750)) {
+ if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705 &&
+ tp->pci_chip_rev_id != CHIPREV_ID_5705_A0) {
if (tp->tg3_flags2 & TG3_FLG2_TSO_CAPABLE &&
GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705) {
rdmac_mode |= RDMAC_MODE_FIFO_SIZE_128;
@@ -8369,17 +8367,14 @@ static int tg3_reset_hw(struct tg3 *tp, int reset_phy)
WDMAC_MODE_FIFOURUN_ENAB | WDMAC_MODE_FIFOOREAD_ENAB |
WDMAC_MODE_LNGREAD_ENAB);
- /* If statement applies to 5705 and 5750 PCI devices only */
- if ((GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705 &&
- tp->pci_chip_rev_id != CHIPREV_ID_5705_A0) ||
- GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5750) {
+ if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705 &&
+ tp->pci_chip_rev_id != CHIPREV_ID_5705_A0) {
if ((tp->tg3_flags2 & TG3_FLG2_TSO_CAPABLE) &&
(tp->pci_chip_rev_id == CHIPREV_ID_5705_A1 ||
tp->pci_chip_rev_id == CHIPREV_ID_5705_A2)) {
/* nothing */
} else if (!(tr32(TG3PCI_PCISTATE) & PCISTATE_BUS_SPEED_HIGH) &&
- !(tp->tg3_flags2 & TG3_FLG2_IS_5788) &&
- !(tp->tg3_flags2 & TG3_FLG2_PCI_EXPRESS)) {
+ !(tp->tg3_flags2 & TG3_FLG2_IS_5788)) {
val |= WDMAC_MODE_RX_ACCEL;
}
}
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH 03/18] ipvs: zero percpu stats
From: David Miller @ 2011-03-10 2:53 UTC (permalink / raw)
To: horms; +Cc: ja, eric.dumazet, netdev, netfilter-devel, netfilter, lvs-devel,
hans
In-Reply-To: <20110310013442.GD3028@verge.net.au>
From: Simon Horman <horms@verge.net.au>
Date: Thu, 10 Mar 2011 10:34:42 +0900
> On Sun, Mar 06, 2011 at 02:18:35PM +0200, Julian Anastasov wrote:
>
> [ snip ]
>
>> Zero the new percpu stats because we copy from there.
>> Use the stats spin lock to synchronize the percpu zeroing with
>> the percpu reading, both in user context and not in a hot path.
>>
>> Signed-off-by: Julian Anastasov <ja@ssi.bg>
>
> Eric, do you have any thoughts on this?
> It seems clean to me.
Eric is away until this weekend, so don't be alarmed by a
late response :-)
^ permalink raw reply
* Re: Network performance with small packets - continued
From: Rick Jones @ 2011-03-10 2:30 UTC (permalink / raw)
To: Shirley Ma
Cc: Michael S. Tsirkin, Tom Lendacky, Rusty Russell, Krishna Kumar2,
David Miller, kvm, netdev, steved
In-Reply-To: <1299718745.25664.200.camel@localhost.localdomain>
On Wed, 2011-03-09 at 16:59 -0800, Shirley Ma wrote:
> In theory, for lots of TCP_RR streams, the guest should be able to keep
> sending xmit skbs to send vq, so vhost should be able to disable
> notification most of the time, then number of guest exits should be
> significantly reduced? Why we saw lots of guest exits here still? Is it
> worth to try 256 (send queue size) TCP_RRs?
If these are single-transaction-at-a-time TCP_RRs rather than "burst
mode" then the number may be something other than send queue size to
keep it constantly active given the RTTs. In the "bare iron" world at
least, that is one of the reasons I added the "burst mode" to the _RR
test - because it could take a Very Large Number of concurrent netperfs
to take a link to saturation, at which point it might have been just as
much a context switching benchmark as anything else :)
happy benchmarking,
rick jones
^ permalink raw reply
* Re: [net-next-2.6 PATCH] if_link: Add PORT_REQUEST_MAX
From: roprabhu @ 2011-03-10 2:16 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110309.124916.183062823.davem@davemloft.net>
On 3/9/11 12:49 PM, "David Miller" <davem@davemloft.net> wrote:
> From: Roopa Prabhu <roprabhu@cisco.com>
> Date: Wed, 09 Mar 2011 12:34:31 -0800
>
>> From: Roopa Prabhu <roprabhu@cisco.com>
>>
>> This patch adds __PORT_REQUEST_MAX to port request enumeration. And defines
>> PORT_REQUEST_MAX.
>>
>> Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
>> Signed-off-by: David Wang <dwang2@cisco.com>
>> Signed-off-by: Christian Benvenuti <benve@cisco.com>
>
> Why?
>
> If some new request types get added, this max value will increase and
> we don't want that to happen for things exposed to userspace.
>
> Userspace should really not depend upon how many requests there are.
O ok. Did not know the reason why its was not there. Point taken. Pls
ignore. Thanks.
^ permalink raw reply
* Re: [PATCH] via-rhine: Work around invalid MAC address error
From: Alex G. @ 2011-03-10 2:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev, rl, florian
In-Reply-To: <20110309.134436.28819615.davem@davemloft.net>
On 03/09/2011 11:44 PM, David Miller wrote:
> Sorry.
>
Don't sweat it.
> I'm going to ignore you're postings for a day or two,
That means I can bash you and you won't notice too soon.
> I've already
> invested an enormous amount of time and energy into getting you to
> submit patches which are properly formed and actually get accepted by
> the tools that we all use.
You have done nothing more than point me to obsolete documentation, and
bash me after following it. If the the less than five minutes of your
life that you have spent giving me short, uninformative, or outright
wrong information are "an enormous amount of time and energy", please
tell me how much you get paid an hour, so that I can pay you for those
minutes (preferably via PayPal). Please be advised, that I will ask my
bank for a chargeback should it come to this.
> Nobody else has these kinds of problems
> and can submit properly formed patches.
>
I assume this tool is" git". If so, it is called "git" for a reason.
I simply wanted to fix a problem I found. I don't know what tools you
are using, how they work, etc. Please don't expect me to guess. If you
want _everybody_ to get it right the first time, then have the decency
to update the fucking documentation, before insolently directing people
to it.
> I would strongly suggest that you try to email the patch to yourself,
> save the email into a file, and feed it to "git am" on a fresh tree,
> making sure that when then looking at what the resulting commit looks
> like.
Now this is the first useful bit of information you have bothered giving
me this whole time.
Alex
^ permalink raw reply
* Re: Stale entries in RT_TABLE_LOCAL
From: Julian Anastasov @ 2011-03-10 1:50 UTC (permalink / raw)
To: David Miller; +Cc: alexandre.sidorenko, netdev
In-Reply-To: <alpine.LFD.2.00.1103100139510.2372@ja.ssi.bg>
Hello,
On Thu, 10 Mar 2011, Julian Anastasov wrote:
> On Wed, 9 Mar 2011, David Miller wrote:
>
>> From: Alex Sidorenko <alexandre.sidorenko@hp.com>
>> Date: Wed, 23 Feb 2011 12:43:23 -0500
>>
>>> I am not sure what is the best way to fix this, I can think of several
>>> approaches:
>>>
>>> (a) change the sources so that it would be impossible to add the same IP
>>> multiple times, even with different masks. I cannot think of any
>>> situation where adding the same IP (but with different mask) to the
>>> same
>>> interface could be useful. But maybe I am wrong?
>>
>> I'm leaning towards this solution if it's viable. But I'm not so sure that
>> nobody uses this feature, maybe Julian knows?
>>
>> Julian, the issue is that if you add the same IP address multiple times
>> using
>> different subnet masks, we allow it.
>>
>> But removal doesn't work correctly, we clear the IFA list on the device but
>> we
>> leave stale entries in the local routing table.
>>
>> The test case is:
>>
>> ip addr add 192.168.142.109/23 dev dummy0
>> ip addr add 192.168.142.109/22 dev dummy0
>
> Here I have just one local route.
Aha, it seems the problem happens when the
both lines are executed while there is another address on
device and the last added address becomes secondary
for the 1st, eg:
IP1: 192.168.140.31/22, primary
IP2: 192.168.142.109/23, primary
IP3: 192.168.142.109/22, secondary for primary IP1
It is the route for IP3 that is leaked, with prefsrc=IP1.
We create local route for secondary IPs with prefsrc=ItsPrimaryIP.
Both local routes for 109 differ in prefsrc (fa_info). But on
deletion only one route is deleted due to last_ip check - the first
because on deletion prefsrc is not matched, fib_table_delete
does not work in symmetric way. So, the local route created
for IP3 remains no matter the deletion order.
If we decide to create one unique local route for this case,
there is a risk device unregistration to remove
it (fib_sync_down_dev with force > 0). I have to think more
on this issue...
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH 03/18] ipvs: zero percpu stats
From: Simon Horman @ 2011-03-10 1:34 UTC (permalink / raw)
To: Julian Anastasov
Cc: Eric Dumazet, netdev, netfilter-devel, netfilter, lvs-devel,
Hans Schillstrom
In-Reply-To: <alpine.LFD.2.00.1103061218470.1879@ja.ssi.bg>
On Sun, Mar 06, 2011 at 02:18:35PM +0200, Julian Anastasov wrote:
[ snip ]
> Zero the new percpu stats because we copy from there.
> Use the stats spin lock to synchronize the percpu zeroing with
> the percpu reading, both in user context and not in a hot path.
>
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
Eric, do you have any thoughts on this?
It seems clean to me.
> ---
>
> diff -urp lvs-test-2.6-8a80c79/linux/net/netfilter/ipvs/ip_vs_ctl.c linux/net/netfilter/ipvs/ip_vs_ctl.c
> --- lvs-test-2.6-8a80c79/linux/net/netfilter/ipvs/ip_vs_ctl.c 2011-03-06 13:39:59.000000000 +0200
> +++ linux/net/netfilter/ipvs/ip_vs_ctl.c 2011-03-06 13:44:56.108275455 +0200
> @@ -713,8 +713,25 @@ static void ip_vs_trash_cleanup(struct n
> static void
> ip_vs_zero_stats(struct ip_vs_stats *stats)
> {
> + struct ip_vs_cpu_stats *cpustats = stats->cpustats;
> + int i;
> +
> spin_lock_bh(&stats->lock);
>
> + for_each_possible_cpu(i) {
> + struct ip_vs_cpu_stats *u = per_cpu_ptr(cpustats, i);
> + unsigned int start;
> +
> + /* Do not pretend to be writer, it is enough to
> + * sync with writers that modify the u64 counters
> + * because under stats->lock there are no other readers
> + */
> + do {
> + start = u64_stats_fetch_begin(&u->syncp);
> + memset(&u->ustats, 0, sizeof(u->ustats));
> + } while (u64_stats_fetch_retry(&u->syncp, start));
> + }
> +
> memset(&stats->ustats, 0, sizeof(stats->ustats));
> ip_vs_zero_estimator(stats);
>
> @@ -2015,16 +2032,19 @@ static int ip_vs_stats_percpu_show(struc
> seq_printf(seq,
> "CPU Conns Packets Packets Bytes Bytes\n");
>
> + /* Use spin lock early to synchronize with percpu zeroing */
> + spin_lock_bh(&tot_stats->lock);
> +
> for_each_possible_cpu(i) {
> struct ip_vs_cpu_stats *u = per_cpu_ptr(cpustats, i);
> unsigned int start;
> __u64 inbytes, outbytes;
>
> do {
> - start = u64_stats_fetch_begin_bh(&u->syncp);
> + start = u64_stats_fetch_begin(&u->syncp);
> inbytes = u->ustats.inbytes;
> outbytes = u->ustats.outbytes;
> - } while (u64_stats_fetch_retry_bh(&u->syncp, start));
> + } while (u64_stats_fetch_retry(&u->syncp, start));
>
> seq_printf(seq, "%3X %8X %8X %8X %16LX %16LX\n",
> i, u->ustats.conns, u->ustats.inpkts,
> @@ -2032,7 +2052,6 @@ static int ip_vs_stats_percpu_show(struc
> (__u64)outbytes);
> }
>
> - spin_lock_bh(&tot_stats->lock);
> seq_printf(seq, " ~ %8X %8X %8X %16LX %16LX\n\n",
> tot_stats->ustats.conns, tot_stats->ustats.inpkts,
> tot_stats->ustats.outpkts,
>
^ permalink raw reply
* Re: Network performance with small packets - continued
From: Shirley Ma @ 2011-03-10 0:59 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Tom Lendacky, Rusty Russell, Krishna Kumar2, David Miller, kvm,
netdev, steved
In-Reply-To: <20110309215537.GA11516@redhat.com>
On Wed, 2011-03-09 at 23:56 +0200, Michael S. Tsirkin wrote:
> > Txn Rate: 153,696.59 Txn/Sec, Pkt Rate: 305,358 Pkgs/Sec
> > Exits: 62,603.37 Exits/Sec
> > TxCPU: 3.73% RxCPU: 98.52%
> > Virtio1-input Interrupts/Sec (CPU0/CPU1): 11,564/0
> > Virtio1-output Interrupts/Sec (CPU0/CPU1): 0/0
> >
> > About a 77% increase over baseline and about 74% of baremetal.
>
> Hmm we get about 20 packets per interrupt on average.
> That's pretty decent. The problem is with exits.
> Let's try something adaptive in the host?
I did some hack before, for 32-64 multiple stream TCP_RR cases, either
queue multiple skbs per kick or delay vhost exit from handle_tx, both
improved TCP_RR aggregation performance, but single TCP_RR latency
increased.
Here, the test is about 100 TCP_RR streams from a bare metal client to
KVM guest, the kick_notify from guest RX path should be small (every 1/2
ring size, it does a kick and even under that kick, vhost might already
disable the notification).
The kick_notify from guest TX path seems the main reason causes the
guest huge of exits, (it does a kick for every send skb, under that kick
vhost might mostly likely exit from empty ring not reaching
VHOST_NET_WEIGH. The indirect buffer is used, so I wonder how many
packets per handle_tx processed here?
In theory, for lots of TCP_RR streams, the guest should be able to keep
sending xmit skbs to send vq, so vhost should be able to disable
notification most of the time, then number of guest exits should be
significantly reduced? Why we saw lots of guest exits here still? Is it
worth to try 256 (send queue size) TCP_RRs?
Tom's kick_notify data from Rusty's patch would be helpful to understand
what's going here.
Thanks
Shirley
^ permalink raw reply
* RE: [PATCH V4] Export ACPI _DSM provided firmware instance number and string name to sysfs
From: Jordan_Hargrave @ 2011-03-10 0:35 UTC (permalink / raw)
To: greg, Narendra_K
Cc: a.beregalov, linux-next, jbarnes, linux-pci, linux-hotplug,
netdev, mjg, Matt_Domsch, Charles_Rose, Shyam_Iyer, sfr
In-Reply-To: <20110307195616.GA14888@kroah.com>
That happens though for CONFIG_XXXX, though usually the include file is further down in the #ifdef region.
--jordan hargrave
Dell Enterprise Linux Engineering
-----Original Message-----
From: Greg KH [mailto:greg@kroah.com]
Sent: Monday, March 07, 2011 1:56 PM
To: K, Narendra
Cc: a.beregalov@gmail.com; linux-next@vger.kernel.org; jbarnes@virtuousgeek.org; linux-pci@vger.kernel.org; linux-hotplug@vger.kernel.org; netdev@vger.kernel.org; mjg@redhat.com; Domsch, Matt; Rose, Charles; Hargrave, Jordan; Iyer, Shyam; sfr@canb.auug.org.au
Subject: Re: [PATCH V4] Export ACPI _DSM provided firmware instance number and string name to sysfs
On Mon, Mar 07, 2011 at 11:44:52AM -0800, Narendra_K@Dell.com wrote:
> --- a/drivers/pci/pci-label.c
> +++ b/drivers/pci/pci-label.c
> @@ -29,7 +29,9 @@
> #include <linux/nls.h>
> #include <linux/acpi.h>
> #include <linux/pci-acpi.h>
> +#ifdef CONFIG_ACPI
> #include <acpi/acpi_drivers.h>
> +#endif
You should never need a #ifdef in a .c file for an include file. If so,
something is really wrong.
thanks,
greg k-h
^ permalink raw reply
* [PATCH] tcp: mark tcp_congestion_ops read_mostly
From: Stephen Hemminger @ 2011-03-10 0:02 UTC (permalink / raw)
To: David Miller, Eric Dumazet; +Cc: netdev
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
net/ipv4/tcp_bic.c | 2 +-
net/ipv4/tcp_cubic.c | 2 +-
net/ipv4/tcp_highspeed.c | 2 +-
net/ipv4/tcp_htcp.c | 2 +-
net/ipv4/tcp_hybla.c | 2 +-
net/ipv4/tcp_illinois.c | 2 +-
net/ipv4/tcp_lp.c | 2 +-
net/ipv4/tcp_scalable.c | 2 +-
net/ipv4/tcp_vegas.c | 2 +-
net/ipv4/tcp_veno.c | 2 +-
net/ipv4/tcp_westwood.c | 2 +-
net/ipv4/tcp_yeah.c | 2 +-
12 files changed, 12 insertions(+), 12 deletions(-)
--- a/net/ipv4/tcp_bic.c 2011-03-09 14:21:25.015099639 -0800
+++ b/net/ipv4/tcp_bic.c 2011-03-09 14:21:46.291314688 -0800
@@ -209,7 +209,7 @@ static void bictcp_acked(struct sock *sk
}
-static struct tcp_congestion_ops bictcp = {
+static struct tcp_congestion_ops bictcp __read_mostly = {
.init = bictcp_init,
.ssthresh = bictcp_recalc_ssthresh,
.cong_avoid = bictcp_cong_avoid,
--- a/net/ipv4/tcp_cubic.c 2011-03-09 14:20:30.818552460 -0800
+++ b/net/ipv4/tcp_cubic.c 2011-03-09 14:23:24.524309323 -0800
@@ -405,7 +405,7 @@ static void bictcp_acked(struct sock *sk
hystart_update(sk, delay);
}
-static struct tcp_congestion_ops cubictcp = {
+static struct tcp_congestion_ops cubictcp __read_mostly = {
.init = bictcp_init,
.ssthresh = bictcp_recalc_ssthresh,
.cong_avoid = bictcp_cong_avoid,
--- a/net/ipv4/tcp_highspeed.c 2011-03-09 14:21:25.055100042 -0800
+++ b/net/ipv4/tcp_highspeed.c 2011-03-09 14:22:00.187455217 -0800
@@ -158,7 +158,7 @@ static u32 hstcp_ssthresh(struct sock *s
}
-static struct tcp_congestion_ops tcp_highspeed = {
+static struct tcp_congestion_ops tcp_highspeed __read_mostly = {
.init = hstcp_init,
.ssthresh = hstcp_ssthresh,
.cong_avoid = hstcp_cong_avoid,
--- a/net/ipv4/tcp_htcp.c 2011-03-09 14:21:25.075100244 -0800
+++ b/net/ipv4/tcp_htcp.c 2011-03-09 14:22:07.451528700 -0800
@@ -284,7 +284,7 @@ static void htcp_state(struct sock *sk,
}
}
-static struct tcp_congestion_ops htcp = {
+static struct tcp_congestion_ops htcp __read_mostly = {
.init = htcp_init,
.ssthresh = htcp_recalc_ssthresh,
.cong_avoid = htcp_cong_avoid,
--- a/net/ipv4/tcp_hybla.c 2011-03-09 14:21:25.095100447 -0800
+++ b/net/ipv4/tcp_hybla.c 2011-03-09 14:22:20.603661788 -0800
@@ -162,7 +162,7 @@ static void hybla_cong_avoid(struct sock
tp->snd_cwnd = min_t(u32, tp->snd_cwnd, tp->snd_cwnd_clamp);
}
-static struct tcp_congestion_ops tcp_hybla = {
+static struct tcp_congestion_ops tcp_hybla __read_mostly = {
.init = hybla_init,
.ssthresh = tcp_reno_ssthresh,
.min_cwnd = tcp_reno_min_cwnd,
--- a/net/ipv4/tcp_illinois.c 2011-03-09 14:21:25.115100649 -0800
+++ b/net/ipv4/tcp_illinois.c 2011-03-09 14:22:24.823704499 -0800
@@ -322,7 +322,7 @@ static void tcp_illinois_info(struct soc
}
}
-static struct tcp_congestion_ops tcp_illinois = {
+static struct tcp_congestion_ops tcp_illinois __read_mostly = {
.flags = TCP_CONG_RTT_STAMP,
.init = tcp_illinois_init,
.ssthresh = tcp_illinois_ssthresh,
--- a/net/ipv4/tcp_lp.c 2011-03-09 14:21:25.155101052 -0800
+++ b/net/ipv4/tcp_lp.c 2011-03-09 14:22:32.879786056 -0800
@@ -313,7 +313,7 @@ static void tcp_lp_pkts_acked(struct soc
lp->last_drop = tcp_time_stamp;
}
-static struct tcp_congestion_ops tcp_lp = {
+static struct tcp_congestion_ops tcp_lp __read_mostly = {
.flags = TCP_CONG_RTT_STAMP,
.init = tcp_lp_init,
.ssthresh = tcp_reno_ssthresh,
--- a/net/ipv4/tcp_scalable.c 2011-03-09 14:21:25.175101255 -0800
+++ b/net/ipv4/tcp_scalable.c 2011-03-09 14:22:37.179829592 -0800
@@ -35,7 +35,7 @@ static u32 tcp_scalable_ssthresh(struct
}
-static struct tcp_congestion_ops tcp_scalable = {
+static struct tcp_congestion_ops tcp_scalable __read_mostly = {
.ssthresh = tcp_scalable_ssthresh,
.cong_avoid = tcp_scalable_cong_avoid,
.min_cwnd = tcp_reno_min_cwnd,
--- a/net/ipv4/tcp_vegas.c 2011-03-09 14:21:25.195101458 -0800
+++ b/net/ipv4/tcp_vegas.c 2011-03-09 14:22:40.999868286 -0800
@@ -304,7 +304,7 @@ void tcp_vegas_get_info(struct sock *sk,
}
EXPORT_SYMBOL_GPL(tcp_vegas_get_info);
-static struct tcp_congestion_ops tcp_vegas = {
+static struct tcp_congestion_ops tcp_vegas __read_mostly = {
.flags = TCP_CONG_RTT_STAMP,
.init = tcp_vegas_init,
.ssthresh = tcp_reno_ssthresh,
--- a/net/ipv4/tcp_veno.c 2011-03-09 14:21:25.215101658 -0800
+++ b/net/ipv4/tcp_veno.c 2011-03-09 14:22:45.163910441 -0800
@@ -201,7 +201,7 @@ static u32 tcp_veno_ssthresh(struct sock
return max(tp->snd_cwnd >> 1U, 2U);
}
-static struct tcp_congestion_ops tcp_veno = {
+static struct tcp_congestion_ops tcp_veno __read_mostly = {
.flags = TCP_CONG_RTT_STAMP,
.init = tcp_veno_init,
.ssthresh = tcp_veno_ssthresh,
--- a/net/ipv4/tcp_westwood.c 2011-03-09 14:21:25.235101861 -0800
+++ b/net/ipv4/tcp_westwood.c 2011-03-09 14:22:48.999949301 -0800
@@ -272,7 +272,7 @@ static void tcp_westwood_info(struct soc
}
-static struct tcp_congestion_ops tcp_westwood = {
+static struct tcp_congestion_ops tcp_westwood __read_mostly = {
.init = tcp_westwood_init,
.ssthresh = tcp_reno_ssthresh,
.cong_avoid = tcp_reno_cong_avoid,
--- a/net/ipv4/tcp_yeah.c 2011-03-09 14:21:25.255102064 -0800
+++ b/net/ipv4/tcp_yeah.c 2011-03-09 14:22:54.108001044 -0800
@@ -225,7 +225,7 @@ static u32 tcp_yeah_ssthresh(struct sock
return tp->snd_cwnd - reduction;
}
-static struct tcp_congestion_ops tcp_yeah = {
+static struct tcp_congestion_ops tcp_yeah __read_mostly = {
.flags = TCP_CONG_RTT_STAMP,
.init = tcp_yeah_init,
.ssthresh = tcp_yeah_ssthresh,
^ permalink raw reply
* Re: Stale entries in RT_TABLE_LOCAL
From: Julian Anastasov @ 2011-03-10 0:04 UTC (permalink / raw)
To: David Miller; +Cc: alexandre.sidorenko, netdev
In-Reply-To: <20110309.135308.102544831.davem@davemloft.net>
Hello,
On Wed, 9 Mar 2011, David Miller wrote:
> From: Alex Sidorenko <alexandre.sidorenko@hp.com>
> Date: Wed, 23 Feb 2011 12:43:23 -0500
>
>> I am not sure what is the best way to fix this, I can think of several
>> approaches:
>>
>> (a) change the sources so that it would be impossible to add the same IP
>> multiple times, even with different masks. I cannot think of any
>> situation where adding the same IP (but with different mask) to the same
>> interface could be useful. But maybe I am wrong?
>
> I'm leaning towards this solution if it's viable. But I'm not so sure that
> nobody uses this feature, maybe Julian knows?
>
> Julian, the issue is that if you add the same IP address multiple times using
> different subnet masks, we allow it.
>
> But removal doesn't work correctly, we clear the IFA list on the device but we
> leave stale entries in the local routing table.
>
> The test case is:
>
> ip addr add 192.168.142.109/23 dev dummy0
> ip addr add 192.168.142.109/22 dev dummy0
Here I have just one local route.
> ip addr del 192.168.142.109/22 dev dummy0
> ip addr del 192.168.142.109/23 dev dummy0
Hm, my 2.6.34 kernel has no such problem. I have
to do some tests with recent tree tomorrow. But fib_magic uses
NLM_F_CREATE | NLM_F_APPEND but even that can not overcome
the check for equal alias, so I don't think it is possible
two equal FIB aliases to be added. I expect only to see
2 local routes if the IP addresses are added to different
devices. May be for some reason we can not remove the single
local route when last IP is deleted.
Alex, what is the kernel version and can you test
it on different kernels? Also, do you really see 2 equal
local routes after the two adds?
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [Bugme-new] [Bug 29252] New: IPv6 doesn't work in a kvm guest.
From: David Miller @ 2011-03-09 23:58 UTC (permalink / raw)
To: akpm; +Cc: netdev, bugzilla-daemon, bugme-daemon, slash, ernstp
In-Reply-To: <20110217142517.b9919481.akpm@linux-foundation.org>
Ok, the following should address both bugs, #29252 and #30462, please
give it some testing.
--------------------
ipv6: Don't create clones of nonexthop routes forever.
Addresses https://bugzilla.kernel.org/show_bug.cgi?id=29252
Addresses https://bugzilla.kernel.org/show_bug.cgi?id=30462
In commit d80bc0fd262ef840ed4e82593ad6416fa1ba3fc4 ("ipv6: Always
clone offlink routes.") we forced the kernel to always clone offlink
routes.
The reason we do that is to make sure we never bind an inetpeer to a
prefixed route.
The logic turned on here has existed in the tree for many years,
but was always off due to a protecting CPP define. So perhaps
it's no surprise that there is a logic bug here.
The issue is that there is nothing that stops the cloning process.
Every route call will find the same route in the same condition,
reclone it, and insert.
This causes two problems:
1) If the in6_rt_ins() call succeeds, we get tons of crap in the
routing tree.
2) If the int6_rt_ins() call fails, we get no networking to that
destination.
We've had reports of both cases.
End the loop and the failures by keying on the RTF_CACHE flag which
will only be set in the clone routes.
Reported-by: slash@ac.auone-net.jp
Reported-by: Ernst Sjöstrand <ernstp@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 904312e..8963635 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -739,8 +739,10 @@ restart:
if (!rt->rt6i_nexthop && !(rt->rt6i_flags & RTF_NONEXTHOP))
nrt = rt6_alloc_cow(rt, &fl->fl6_dst, &fl->fl6_src);
- else
+ else if (!(rt->rt6i_flags & RTF_CACHE))
nrt = rt6_alloc_clone(rt, &fl->fl6_dst);
+ else
+ goto out2;
dst_release(&rt->dst);
rt = nrt ? : net->ipv6.ip6_null_entry;
^ permalink raw reply related
* Re: [PATCH] ipv4: Cache source address in nexthop entries.
From: David Miller @ 2011-03-09 23:40 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <alpine.LFD.2.00.1103100014430.2160@ja.ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Thu, 10 Mar 2011 01:32:18 +0200 (EET)
> On Wed, 9 Mar 2011, David Miller wrote:
>
>> So device address operations increment a generation ID and
>> FIB_RES_PREFSRC() checks that ID against one stored in the nexthop.
>
> Yes, may be it is better this generation ID to be global
> value for netns, because the global addresses should cause
> refresh for all nexthops on all devices. The ID should be
> matched with corresponding value in NH. Another option is
> to use per-device gen_id instead of netns gen_id but in this
> case scope-global address event should increment gen_id for all
> devices while the device_UP event will increment only
> gen_id for the concerned DEV.
>
> I.e. there are 2 variants:
I think the per-namespace ID makes the most sense.
If the whole idea of the genid is to reduce complication, trying to place
games on a per-device basis wrt. the scope cases seems to work against that
goal :-)
After I try to attack the ipv6 routing regression I added in net-2.6, I'll
take a stab at this.
Thanks!
^ permalink raw reply
* Re: [PATCH] ipv4: Cache source address in nexthop entries.
From: Julian Anastasov @ 2011-03-09 23:32 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110309.130817.115943793.davem@davemloft.net>
Hello,
On Wed, 9 Mar 2011, David Miller wrote:
> From: Julian Anastasov <ja@ssi.bg>
> Date: Wed, 9 Mar 2011 02:49:49 +0200 (EET)
>
>> But we still need to propagate the address events
>> to nexthops on all devices. Even if it is slow, I see it in
>> this way (not tested):
>
> Ok, I think it is even more involved than what you propose.
>
> We have to potentially re-run inet_select_addr() on all nexthops, even
> those pointing to devices other than the one being modified, because
> when an interface loses it's last IP address we have to look for a
> source address to use on other active interfaces.
Yes. The common case is addresses to be scope
global. That means they can influence any nexthop on
any device. Only when address has scope >= LINK we can
use version that walks single table row for the same device.
In the default case we need to walk all table rows.
Additionally, it is possible same address to exist
on different devices, possibly with different scope. That is
why reacting only on "last_ip" indication when deleting IP
is not enough. May be while we delete scope-link address on the
concerned device, there can be same address as scope-link
on another device but its scope is not enough to keep
it as nh_saddr, so the right thing is to get different
IP as saddr. That is why in my version of fib_update_nh_saddrs
I don't care about "last_ip" indication. I think, my
version of fib_update_nh_saddrs is ok for IP add/del but
as I said, device UP events should recalc nh_saddr too.
In my version the logic is:
- add IP:
- check all NHs on all devices
- goal: if the added IP-scope is suitable for NH
it can replace other less-priority IPs, where the
priority depends on factors such as device, placement in
device list and scope
- if NH has same nh_saddr then nothing to do
- IP scope and DEV should be allowed for the nh_cfg_scope
to change the nh_saddr
- del IP:
- check all NHs on all devices
- goal: if the removed IP-scope was nh_saddr somewhere
then we should recalc NH, even when IP is not the
last address with same value
- if NH has different nh_saddr then nothing to do
- IP scope and DEV should be allowed for the nh_cfg_scope
to change the nh_saddr because may be this IP-scope-DEV
was used as nh_saddr
> Probably it makes sense to recompute this at use time instead of
> walking all of this stuff at every event.
Yes, it looks less dangerous.
> So device address operations increment a generation ID and
> FIB_RES_PREFSRC() checks that ID against one stored in the nexthop.
Yes, may be it is better this generation ID to be global
value for netns, because the global addresses should cause
refresh for all nexthops on all devices. The ID should be
matched with corresponding value in NH. Another option is
to use per-device gen_id instead of netns gen_id but in this
case scope-global address event should increment gen_id for all
devices while the device_UP event will increment only
gen_id for the concerned DEV.
I.e. there are 2 variants:
1. global netns gen_id:
- add/del IP, device UP => netns->gen_id ++
2. dev->gen_id:
- add/del IP with scope < LINK => increment
dev->gen_id for all devices
- add/del IP with scope >= LINK => increment
dev->gen_id only for concerned device
- device UP: increment dev->gen_id only for
concerned device
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: Network performance with small packets - continued
From: Tom Lendacky @ 2011-03-09 23:25 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Shirley Ma, Rusty Russell, Krishna Kumar2, David Miller, kvm,
netdev, steved
In-Reply-To: <20110309215537.GA11516@redhat.com>
On Wednesday, March 09, 2011 03:56:15 pm Michael S. Tsirkin wrote:
> On Wed, Mar 09, 2011 at 02:11:07PM -0600, Tom Lendacky wrote:
> > Here are the results again with the addition of the interrupt rate that
> > occurred on the guest virtio_net device:
> >
> > Here is the KVM baseline (average of six runs):
> > Txn Rate: 87,070.34 Txn/Sec, Pkt Rate: 172,992 Pkts/Sec
> > Exits: 148,444.58 Exits/Sec
> > TxCPU: 2.40% RxCPU: 99.35%
> > Virtio1-input Interrupts/Sec (CPU0/CPU1): 5,154/5,222
> > Virtio1-output Interrupts/Sec (CPU0/CPU1): 0/0
> >
> > About 42% of baremetal.
> >
> > Delayed freeing of TX buffers (average of six runs):
> > Txn Rate: 90,886.19 Txn/Sec, Pkt Rate: 180,571 Pkts/Sec
> > Exits: 142,681.67 Exits/Sec
> > TxCPU: 2.78% RxCPU: 99.36%
> > Virtio1-input Interrupts/Sec (CPU0/CPU1): 4,796/4,908
> > Virtio1-output Interrupts/Sec (CPU0/CPU1): 0/0
> >
> > About a 4% increase over baseline and about 44% of baremetal.
>
> Looks like delayed freeing is a good idea generally.
> Is this my patch? Yours?
These results are for my patch, I haven't had a chance to run your patch yet.
>
> > Delaying kick_notify (kick every 5 packets -average of six runs):
> > Txn Rate: 107,106.36 Txn/Sec, Pkt Rate: 212,796 Pkts/Sec
> > Exits: 102,587.28 Exits/Sec
> > TxCPU: 3.03% RxCPU: 99.33%
> > Virtio1-input Interrupts/Sec (CPU0/CPU1): 4,200/4,293
> > Virtio1-output Interrupts/Sec (CPU0/CPU1): 0/0
> >
> > About a 23% increase over baseline and about 52% of baremetal.
>
> > Delaying kick_notify and pinning virtio1-input to CPU0 (average of six
runs):
> What exactly moves the interrupt handler between CPUs?
> irqbalancer? Does it matter which CPU you pin it to?
> If yes, do you have any idea why?
Looking at the guest, irqbalance isn't running and the smp_affinity for the
irq is set to 3 (both CPUs). It could be that irqbalance would help in this
situation since it would probably change the smp_affinity mask to a single CPU
and remove the irq lock contention (I think the last used index patch would be
best though since it will avoid the extra irq injections). I'll kick off a
run with irqbalance running.
As for which CPU the interrupt gets pinned to, that doesn't matter - see
below.
>
> Also, what happens without delaying kick_notify
> but with pinning?
Here are the results of a single "baseline" run with the IRQ pinned to CPU0:
Txn Rate: 108,212.12 Txn/Sec, Pkt Rate: 214,994 Pkts/Sec
Exits: 119,310.21 Exits/Sec
TxCPU: 9.63% RxCPU: 99.47%
Virtio1-input Interrupts/Sec (CPU0/CPU1):
Virtio1-output Interrupts/Sec (CPU0/CPU1):
and CPU1:
Txn Rate: 108,053.02 Txn/Sec, Pkt Rate: 214,678 Pkts/Sec
Exits: 119,320.12 Exits/Sec
TxCPU: 9.64% RxCPU: 99.42%
Virtio1-input Interrupts/Sec (CPU0/CPU1): 13,608/0
Virtio1-output Interrupts/Sec (CPU0/CPU1): 0/13,830
About a 24% increase over baseline.
>
> > Txn Rate: 153,696.59 Txn/Sec, Pkt Rate: 305,358 Pkts/Sec
> > Exits: 62,603.37 Exits/Sec
> > TxCPU: 3.73% RxCPU: 98.52%
> > Virtio1-input Interrupts/Sec (CPU0/CPU1): 11,564/0
> > Virtio1-output Interrupts/Sec (CPU0/CPU1): 0/0
> >
> > About a 77% increase over baseline and about 74% of baremetal.
>
> Hmm we get about 20 packets per interrupt on average.
> That's pretty decent. The problem is with exits.
> Let's try something adaptive in the host?
^ permalink raw reply
* Re: Network performance with small packets - continued
From: Tom Lendacky @ 2011-03-09 22:57 UTC (permalink / raw)
To: Shirley Ma
Cc: Michael S. Tsirkin, Rusty Russell, Krishna Kumar2, David Miller,
kvm, netdev, steved
In-Reply-To: <1299710712.25664.175.camel@localhost.localdomain>
On Wednesday, March 09, 2011 04:45:12 pm Shirley Ma wrote:
> Hello Tom,
>
> Do you also have Rusty's virtio stat patch results for both send queue
> and recv queue to share here?
Let me see what I can do about getting the data extracted, averaged and in a
form that I can put in an email.
>
> Thanks
> Shirley
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: James Morris @ 2011-03-09 22:53 UTC (permalink / raw)
To: David Miller
Cc: segoon, mjt, linux-kernel, arnd, mirqus, netdev, bhutchings,
kuznet, pekkas, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo,
jesse, kees.cook, eugene, dan.j.rosenberg
In-Reply-To: <20110309.140928.70185544.davem@davemloft.net>
On Wed, 9 Mar 2011, David Miller wrote:
> I was hoping someone other than me would take this upstream, feel free
> to submit it directly to Linus with my ack:
>
> Acked-by: David S. Miller <davem@davemloft.net>
I can submit it via my tree.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: Network performance with small packets - continued
From: Tom Lendacky @ 2011-03-09 22:51 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Shirley Ma, Rusty Russell, Krishna Kumar2, David Miller, kvm,
netdev, steved
In-Reply-To: <201103091009.27932.tahm@linux.vnet.ibm.com>
On Wednesday, March 09, 2011 10:09:26 am Tom Lendacky wrote:
> On Wednesday, March 09, 2011 01:15:58 am Michael S. Tsirkin wrote:
> > On Mon, Mar 07, 2011 at 04:31:41PM -0600, Tom Lendacky wrote:
> > > We've been doing some more experimenting with the small packet network
> > > performance problem in KVM. I have a different setup than what Steve
> > > D. was using so I re-baselined things on the kvm.git kernel on both
> > > the host and guest with a 10GbE adapter. I also made use of the
> > > virtio-stats patch.
> > >
> > > The virtual machine has 2 vCPUs, 8GB of memory and two virtio network
> > > adapters (the first connected to a 1GbE adapter and a LAN, the second
> > > connected to a 10GbE adapter that is direct connected to another system
> > > with the same 10GbE adapter) running the kvm.git kernel. The test was
> > > a TCP_RR test with 100 connections from a baremetal client to the KVM
> > > guest using a 256 byte message size in both directions.
> > >
> > > I used the uperf tool to do this after verifying the results against
> > > netperf. Uperf allows the specification of the number of connections as
> > > a parameter in an XML file as opposed to launching, in this case, 100
> > > separate instances of netperf.
> > >
> > > Here is the baseline for baremetal using 2 physical CPUs:
> > > Txn Rate: 206,389.59 Txn/Sec, Pkt Rate: 410,048 Pkts/Sec
> > > TxCPU: 7.88% RxCPU: 99.41%
> > >
> > > To be sure to get consistent results with KVM I disabled the
> > > hyperthreads, pinned the qemu-kvm process, vCPUs, vhost thread and
> > > ethernet adapter interrupts (this resulted in runs that differed by
> > > only about 2% from lowest to highest). The fact that pinning is
> > > required to get consistent results is a different problem that we'll
> > > have to look into later...
> > >
> > > Here is the KVM baseline (average of six runs):
> > > Txn Rate: 87,070.34 Txn/Sec, Pkt Rate: 172,992 Pkts/Sec
> > > Exits: 148,444.58 Exits/Sec
> > > TxCPU: 2.40% RxCPU: 99.35%
> > >
> > > About 42% of baremetal.
> >
> > Can you add interrupt stats as well please?
>
> Yes I can. Just the guest interrupts for the virtio device?
>
> > > empty. So I coded a quick patch to delay freeing of the used Tx
> > > buffers until more than half the ring was used (I did not test this
> > > under a stream condition so I don't know if this would have a negative
> > > impact). Here are the results
> > >
> > > from delaying the freeing of used Tx buffers (average of six runs):
> > > Txn Rate: 90,886.19 Txn/Sec, Pkt Rate: 180,571 Pkts/Sec
> > > Exits: 142,681.67 Exits/Sec
> > > TxCPU: 2.78% RxCPU: 99.36%
> > >
> > > About a 4% increase over baseline and about 44% of baremetal.
> >
> > Hmm, I am not sure what you mean by delaying freeing.
>
> In the start_xmit function of virtio_net.c the first thing done is to free
> any used entries from the ring. I patched the code to track the number of
> used tx ring entries and only free the used entries when they are greater
> than half the capacity of the ring (similar to the way the rx ring is
> re-filled).
>
> > I think we do have a problem that free_old_xmit_skbs
> > tries to flush out the ring aggressively:
> > it always polls until the ring is empty,
> > so there could be bursts of activity where
> > we spend a lot of time flushing the old entries
> > before e.g. sending an ack, resulting in
> > latency bursts.
> >
> > Generally we'll need some smarter logic,
> > but with indirect at the moment we can just poll
> > a single packet after we post a new one, and be done with it.
> > Is your patch something like the patch below?
> > Could you try mine as well please?
>
> Yes, I'll try the patch and post the results.
>
> > > This spread out the kick_notify but still resulted in alot of them. I
> > > decided to build on the delayed Tx buffer freeing and code up an
> > > "ethtool" like coalescing patch in order to delay the kick_notify until
> > > there were at least 5 packets on the ring or 2000 usecs, whichever
> > > occurred first. Here are the
> > >
> > > results of delaying the kick_notify (average of six runs):
> > > Txn Rate: 107,106.36 Txn/Sec, Pkt Rate: 212,796 Pkts/Sec
> > > Exits: 102,587.28 Exits/Sec
> > > TxCPU: 3.03% RxCPU: 99.33%
> > >
> > > About a 23% increase over baseline and about 52% of baremetal.
> > >
> > > Running the perf command against the guest I noticed almost 19% of the
> > > time being spent in _raw_spin_lock. Enabling lockstat in the guest
> > > showed alot of contention in the "irq_desc_lock_class". Pinning the
> > > virtio1-input interrupt to a single cpu in the guest and re-running the
> > > last test resulted in
> > >
> > > tremendous gains (average of six runs):
> > > Txn Rate: 153,696.59 Txn/Sec, Pkt Rate: 305,358 Pkgs/Sec
> > > Exits: 62,603.37 Exits/Sec
> > > TxCPU: 3.73% RxCPU: 98.52%
> > >
> > > About a 77% increase over baseline and about 74% of baremetal.
> > >
> > > Vhost is receiving a lot of notifications for packets that are to be
> > > transmitted (over 60% of the packets generate a kick_notify). Also, it
> > > looks like vhost is sending a lot of notifications for packets it has
> > > received before the guest can get scheduled to disable notifications
> > > and begin processing the packets
> >
> > Hmm, is this really what happens to you? The effect would be that guest
> > gets an interrupt while notifications are disabled in guest, right? Could
> > you add a counter and check this please?
>
> The disabling of the interrupt/notifications is done by the guest. So the
> guest has to get scheduled and handle the notification before it disables
> them. The vhost_signal routine will keep injecting an interrupt until this
> happens causing the contention in the guest. I'll try the patches you
> specify below and post the results. They look like they should take care
> of this issue.
>
> > Another possible thing to try would be these old patches to publish used
> >
> > index from guest to make sure this double interrupt does not happen:
> > [PATCHv2] virtio: put last seen used index into ring itself
> > [PATCHv2] vhost-net: utilize PUBLISH_USED_IDX feature
I was able to apply these patches with a little work, but unfortunately the
guest oops during boot up in virtqueue_add_buf_gfp. It happens in the
virtio_blk driver. Any chance you can re-work these patches against the
kvm.git tree?
> >
> > > resulting in some lock contention in the guest (and
> > > high interrupt rates).
> > >
> > > Some thoughts for the transmit path... can vhost be enhanced to do
> > > some adaptive polling so that the number of kick_notify events are
> > > reduced and replaced by kick_no_notify events?
> >
> > Worth a try.
> >
> > > Comparing the transmit path to the receive path, the guest disables
> > > notifications after the first kick and vhost re-enables notifications
> > > after completing processing of the tx ring.
> >
> > Is this really what happens? I though the host disables notifications
> > after the first kick.
>
> Yup, sorry for the confusion. The kick is done by the guest and then vhost
> disables notifications. Maybe a similar approach to the above patches of
> checking the used index in the virtio_net driver could also help here?
>
> > > Can a similar thing be done for the
> > >
> > > receive path? Once vhost sends the first notification for a received
> > > packet it can disable notifications and let the guest re-enable
> > > notifications when it has finished processing the receive ring. Also,
> > > can the virtio-net driver do some adaptive polling (or does napi take
> > > care of that for the guest)?
> >
> > Worth a try. I don't think napi does anything like this.
> >
> > > Running the same workload on the same configuration with a different
> > > hypervisor results in performance that is almost equivalent to
> > > baremetal without doing any pinning.
> > >
> > > Thanks,
> > > Tom Lendacky
> >
> > There's no need to flush out all used buffers
> > before we post more for transmit: with indirect,
> > just a single one is enough. Without indirect we'll
> > need more possibly, but just for testing this should
> > be enough.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > ---
> >
> > Note: untested.
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 82dba5a..ebe3337 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -514,11 +514,11 @@ static unsigned int free_old_xmit_skbs(struct
> > virtnet_info *vi) struct sk_buff *skb;
> >
> > unsigned int len, tot_sgs = 0;
> >
> > - while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
> > + if ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
> >
> > pr_debug("Sent skb %p\n", skb);
> > vi->dev->stats.tx_bytes += skb->len;
> > vi->dev->stats.tx_packets++;
> >
> > - tot_sgs += skb_vnet_hdr(skb)->num_sg;
> > + tot_sgs = 2+MAX_SKB_FRAGS;
> >
> > dev_kfree_skb_any(skb);
> >
> > }
> > return tot_sgs;
> >
> > @@ -576,9 +576,6 @@ static netdev_tx_t start_xmit(struct sk_buff *skb,
> > struct net_device *dev) struct virtnet_info *vi = netdev_priv(dev);
> >
> > int capacity;
> >
> > - /* Free up any pending old buffers before queueing new ones. */
> > - free_old_xmit_skbs(vi);
> > -
> >
> > /* Try to transmit */
> > capacity = xmit_skb(vi, skb);
> >
> > @@ -605,6 +602,10 @@ static netdev_tx_t start_xmit(struct sk_buff *skb,
> > struct net_device *dev) skb_orphan(skb);
> >
> > nf_reset(skb);
> >
> > + /* Free up any old buffers so we can queue new ones. */
> > + if (capacity < 2+MAX_SKB_FRAGS)
> > + capacity += free_old_xmit_skbs(vi);
> > +
> >
> > /* Apparently nice girls don't return TX_BUSY; stop the queue
> >
> > * before it gets out of hand. Naturally, this wastes entries. */
> >
> > if (capacity < 2+MAX_SKB_FRAGS) {
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Network performance with small packets - continued
From: Shirley Ma @ 2011-03-09 22:45 UTC (permalink / raw)
To: Tom Lendacky
Cc: Michael S. Tsirkin, Rusty Russell, Krishna Kumar2, David Miller,
kvm, netdev, steved
In-Reply-To: <201103091411.09062.tahm@linux.vnet.ibm.com>
Hello Tom,
Do you also have Rusty's virtio stat patch results for both send queue
and recv queue to share here?
Thanks
Shirley
^ 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