* Re: PCI: make pci_restore_state return void
From: Mauro Carvalho Chehab @ 2010-12-03 13:08 UTC (permalink / raw)
To: Jon Mason
Cc: Jesse Barnes, linux-pci, Jonathan Corbet, linux-media,
Andrew Gallatin, Brice Goglin, netdev,
Solarflare linux maintainers, Steve Hodgson, Ben Hutchings,
Stephen Hemminger, Ivo van Doorn, Gertjan van Wingerde,
linux-wireless, Brian King, Anil Ravindranath, linux-scsi,
Jaya Kumar, boyod.yang
In-Reply-To: <1291160606-31494-1-git-send-email-jon.mason@exar.com>
Em 30-11-2010 21:43, Jon Mason escreveu:
> pci_restore_state only ever returns 0, thus there is no benefit in
> having it return any value. Also, a large majority of the callers do
> not check the return code of pci_restore_state. Make the
> pci_restore_state a void return and avoid the overhead.
>
> Signed-off-by: Jon Mason <jon.mason@exar.com>
> ---
> drivers/media/video/cafe_ccic.c | 4 +---
Seems ok to me.
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> drivers/net/myri10ge/myri10ge.c | 4 +---
> drivers/net/sfc/falcon.c | 25 +++++--------------------
> drivers/net/skge.c | 4 +---
> drivers/net/sky2.c | 5 +----
> drivers/net/wireless/rt2x00/rt2x00pci.c | 4 ++--
> drivers/pci/pci-driver.c | 3 ++-
> drivers/pci/pci.c | 7 ++-----
> drivers/scsi/ipr.c | 8 +-------
> drivers/scsi/pmcraid.c | 7 +------
> drivers/staging/sm7xx/smtcfb.c | 2 +-
> include/linux/pci.h | 8 +++-----
> sound/pci/cs5535audio/cs5535audio_pm.c | 7 +------
> 13 files changed, 22 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/media/video/cafe_ccic.c b/drivers/media/video/cafe_ccic.c
> index 2934770..3e653f3 100644
> --- a/drivers/media/video/cafe_ccic.c
> +++ b/drivers/media/video/cafe_ccic.c
> @@ -2186,9 +2186,7 @@ static int cafe_pci_resume(struct pci_dev *pdev)
> struct cafe_camera *cam = to_cam(v4l2_dev);
> int ret = 0;
>
> - ret = pci_restore_state(pdev);
> - if (ret)
> - return ret;
> + pci_restore_state(pdev);
> ret = pci_enable_device(pdev);
>
> if (ret) {
> diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
> index 8524cc4..d3c4a37 100644
> --- a/drivers/net/myri10ge/myri10ge.c
> +++ b/drivers/net/myri10ge/myri10ge.c
> @@ -3403,9 +3403,7 @@ static int myri10ge_resume(struct pci_dev *pdev)
> return -EIO;
> }
>
> - status = pci_restore_state(pdev);
> - if (status)
> - return status;
> + pci_restore_state(pdev);
>
> status = pci_enable_device(pdev);
> if (status) {
> diff --git a/drivers/net/sfc/falcon.c b/drivers/net/sfc/falcon.c
> index 267019b..1763b9a 100644
> --- a/drivers/net/sfc/falcon.c
> +++ b/drivers/net/sfc/falcon.c
> @@ -1066,22 +1066,9 @@ static int falcon_reset_hw(struct efx_nic *efx, enum reset_type method)
>
> /* Restore PCI configuration if needed */
> if (method == RESET_TYPE_WORLD) {
> - if (efx_nic_is_dual_func(efx)) {
> - rc = pci_restore_state(nic_data->pci_dev2);
> - if (rc) {
> - netif_err(efx, drv, efx->net_dev,
> - "failed to restore PCI config for "
> - "the secondary function\n");
> - goto fail3;
> - }
> - }
> - rc = pci_restore_state(efx->pci_dev);
> - if (rc) {
> - netif_err(efx, drv, efx->net_dev,
> - "failed to restore PCI config for the "
> - "primary function\n");
> - goto fail4;
> - }
> + if (efx_nic_is_dual_func(efx))
> + pci_restore_state(nic_data->pci_dev2);
> + pci_restore_state(efx->pci_dev);
> netif_dbg(efx, drv, efx->net_dev,
> "successfully restored PCI config\n");
> }
> @@ -1092,7 +1079,7 @@ static int falcon_reset_hw(struct efx_nic *efx, enum reset_type method)
> rc = -ETIMEDOUT;
> netif_err(efx, hw, efx->net_dev,
> "timed out waiting for hardware reset\n");
> - goto fail5;
> + goto fail3;
> }
> netif_dbg(efx, hw, efx->net_dev, "hardware reset complete\n");
>
> @@ -1100,11 +1087,9 @@ static int falcon_reset_hw(struct efx_nic *efx, enum reset_type method)
>
> /* pci_save_state() and pci_restore_state() MUST be called in pairs */
> fail2:
> -fail3:
> pci_restore_state(efx->pci_dev);
> fail1:
> -fail4:
> -fail5:
> +fail3:
> return rc;
> }
>
> diff --git a/drivers/net/skge.c b/drivers/net/skge.c
> index 220e039..61553af 100644
> --- a/drivers/net/skge.c
> +++ b/drivers/net/skge.c
> @@ -4087,9 +4087,7 @@ static int skge_resume(struct pci_dev *pdev)
> if (err)
> goto out;
>
> - err = pci_restore_state(pdev);
> - if (err)
> - goto out;
> + pci_restore_state(pdev);
>
> err = skge_reset(hw);
> if (err)
> diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
> index d657708..be3aee7 100644
> --- a/drivers/net/sky2.c
> +++ b/drivers/net/sky2.c
> @@ -4969,10 +4969,7 @@ static int sky2_resume(struct pci_dev *pdev)
> if (err)
> goto out;
>
> - err = pci_restore_state(pdev);
> - if (err)
> - goto out;
> -
> + pci_restore_state(pdev);
> pci_enable_wake(pdev, PCI_D0, 0);
>
> /* Re-enable all clocks */
> diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.c b/drivers/net/wireless/rt2x00/rt2x00pci.c
> index 868ca19..5e3c46f 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00pci.c
> @@ -356,12 +356,12 @@ int rt2x00pci_resume(struct pci_dev *pci_dev)
> struct rt2x00_dev *rt2x00dev = hw->priv;
>
> if (pci_set_power_state(pci_dev, PCI_D0) ||
> - pci_enable_device(pci_dev) ||
> - pci_restore_state(pci_dev)) {
> + pci_enable_device(pci_dev)) {
> ERROR(rt2x00dev, "Failed to resume device.\n");
> return -EIO;
> }
>
> + pci_restore_state(pci_dev);
> return rt2x00lib_resume(rt2x00dev);
> }
> EXPORT_SYMBOL_GPL(rt2x00pci_resume);
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 8a6f797..80e551e 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -449,7 +449,8 @@ static int pci_restore_standard_config(struct pci_dev *pci_dev)
> return error;
> }
>
> - return pci_restore_state(pci_dev);
> + pci_restore_state(pci_dev);
> + return 0;
> }
>
> static void pci_pm_default_resume_early(struct pci_dev *pci_dev)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e98c810..c711d1b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -937,14 +937,13 @@ pci_save_state(struct pci_dev *dev)
> * pci_restore_state - Restore the saved state of a PCI device
> * @dev: - PCI device that we're dealing with
> */
> -int
> -pci_restore_state(struct pci_dev *dev)
> +void pci_restore_state(struct pci_dev *dev)
> {
> int i;
> u32 val;
>
> if (!dev->state_saved)
> - return 0;
> + return;
>
> /* PCI Express register must be restored first */
> pci_restore_pcie_state(dev);
> @@ -968,8 +967,6 @@ pci_restore_state(struct pci_dev *dev)
> pci_restore_iov_state(dev);
>
> dev->state_saved = false;
> -
> - return 0;
> }
>
> static int do_pci_enable_device(struct pci_dev *dev, int bars)
> diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> index fa60d7d..1d7dbe6 100644
> --- a/drivers/scsi/ipr.c
> +++ b/drivers/scsi/ipr.c
> @@ -7485,16 +7485,10 @@ static int ipr_reset_restore_cfg_space(struct ipr_cmnd *ipr_cmd)
> {
> struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg;
> volatile u32 int_reg;
> - int rc;
>
> ENTER;
> ioa_cfg->pdev->state_saved = true;
> - rc = pci_restore_state(ioa_cfg->pdev);
> -
> - if (rc != PCIBIOS_SUCCESSFUL) {
> - ipr_cmd->s.ioasa.hdr.ioasc = cpu_to_be32(IPR_IOASC_PCI_ACCESS_ERROR);
> - return IPR_RC_JOB_CONTINUE;
> - }
> + pci_restore_state(ioa_cfg->pdev);
>
> if (ipr_set_pcix_cmd_reg(ioa_cfg)) {
> ipr_cmd->s.ioasa.hdr.ioasc = cpu_to_be32(IPR_IOASC_PCI_ACCESS_ERROR);
> diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
> index cf89091..091baf2 100644
> --- a/drivers/scsi/pmcraid.c
> +++ b/drivers/scsi/pmcraid.c
> @@ -2227,12 +2227,7 @@ static void pmcraid_ioa_reset(struct pmcraid_cmd *cmd)
> /* Once either bist or pci reset is done, restore PCI config
> * space. If this fails, proceed with hard reset again
> */
> - if (pci_restore_state(pinstance->pdev)) {
> - pmcraid_info("config-space error resetting again\n");
> - pinstance->ioa_state = IOA_STATE_IN_RESET_ALERT;
> - pmcraid_reset_alert(cmd);
> - break;
> - }
> + pci_restore_state(pinstance->pdev);
>
> /* fail all pending commands */
> pmcraid_fail_outstanding_cmds(pinstance);
> diff --git a/drivers/staging/sm7xx/smtcfb.c b/drivers/staging/sm7xx/smtcfb.c
> index 24f47d6..7162dee 100644
> --- a/drivers/staging/sm7xx/smtcfb.c
> +++ b/drivers/staging/sm7xx/smtcfb.c
> @@ -1071,7 +1071,7 @@ static int __maybe_unused smtcfb_resume(struct pci_dev *pdev)
> /* when resuming, restore pci data and fb cursor */
> if (pdev->dev.power.power_state.event != PM_EVENT_FREEZE) {
> retv = pci_set_power_state(pdev, PCI_D0);
> - retv = pci_restore_state(pdev);
> + pci_restore_state(pdev);
> if (pci_enable_device(pdev))
> return -1;
> pci_set_master(pdev);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 7454408..63cbadc 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -806,7 +806,7 @@ size_t pci_get_rom_size(struct pci_dev *pdev, void __iomem *rom, size_t size);
>
> /* Power management related routines */
> int pci_save_state(struct pci_dev *dev);
> -int pci_restore_state(struct pci_dev *dev);
> +void pci_restore_state(struct pci_dev *dev);
> int __pci_complete_power_transition(struct pci_dev *dev, pci_power_t state);
> int pci_set_power_state(struct pci_dev *dev, pci_power_t state);
> pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state);
> @@ -1168,10 +1168,8 @@ static inline int pci_save_state(struct pci_dev *dev)
> return 0;
> }
>
> -static inline int pci_restore_state(struct pci_dev *dev)
> -{
> - return 0;
> -}
> +static inline void pci_restore_state(struct pci_dev *dev)
> +{ }
>
> static inline int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
> {
> diff --git a/sound/pci/cs5535audio/cs5535audio_pm.c b/sound/pci/cs5535audio/cs5535audio_pm.c
> index a3301cc..185b000 100644
> --- a/sound/pci/cs5535audio/cs5535audio_pm.c
> +++ b/sound/pci/cs5535audio/cs5535audio_pm.c
> @@ -90,12 +90,7 @@ int snd_cs5535audio_resume(struct pci_dev *pci)
> int i;
>
> pci_set_power_state(pci, PCI_D0);
> - if (pci_restore_state(pci) < 0) {
> - printk(KERN_ERR "cs5535audio: pci_restore_state failed, "
> - "disabling device\n");
> - snd_card_disconnect(card);
> - return -EIO;
> - }
> + pci_restore_state(pci);
> if (pci_enable_device(pci) < 0) {
> printk(KERN_ERR "cs5535audio: pci_enable_device failed, "
> "disabling device\n");
^ permalink raw reply
* Re: unable to handle kernel NULL pointer dereference in skb_dequeue
From: Eric Dumazet @ 2010-12-03 13:09 UTC (permalink / raw)
To: Toshio; +Cc: linux-kernel, gvs, Rami Rosen, netdev
In-Reply-To: <0fe401cb92e7$85ba2260$912e6720$@si>
Le vendredi 03 décembre 2010 à 13:42 +0100, Toshio a écrit :
> I have also hit on Bug 20292 (https://bugzilla.kernel.org/show_bug.cgi?id=20292) in final 2.6.36. After investigating changes made between 2.6.35.4, which worked, and 2.6.36 which started oopsing, I think the problem was in double freeing of skb caused by change of return value for __pppoe_xmit in case of errors.
>
> As it turned out this might be the cause of random BUG reports throught the kernel, whenever something touched skb. Most common BUG with my use case happened at skb_dequeue:
>
CC netdev and Rami Rosen
> 00000060 <skb_dequeue>:
> 60: 53 push %ebx
> 61: 89 c2 mov %eax,%edx
> 63: 9c pushf
> 64: 59 pop %ecx
> 65: fa cli
> 66: 8b 00 mov (%eax),%eax
> 68: 39 c2 cmp %eax,%edx
> 6a: 74 24 je 90 <skb_dequeue+0x30>
> 6c: 85 c0 test %eax,%eax
> 6e: 74 1a je 8a <skb_dequeue+0x2a>
> 70: ff 4a 08 decl 0x8(%edx)
> 73: 8b 18 mov (%eax),%ebx
> 75: c7 00 00 00 00 00 movl $0x0,(%eax)
> 7b: 8b 50 04 mov 0x4(%eax),%edx
> 7e: c7 40 04 00 00 00 00 movl $0x0,0x4(%eax)
> 85: 89 53 04 mov %edx,0x4(%ebx)
> 88:* 89 1a mov %ebx,(%edx)
> 8a: 51 push %ecx
> 8b: 9d popf
> 8c: 5b pop %ebx
> 8d: c3 ret
> 8e: 66 90 xchg %ax,%ax
> 90: b8 00 00 00 00 mov $0x0,%eax
> 95: eb f3 jmp 8a <skb_dequeue+0x2a>
> 97: 89 f6 mov %esi,%esi
> 99: 8d bc 27 00 00 00 00 lea 0x0(%edi,%eiz,1),%edi
>
>
> This location corresponds to line "next = next->next" from inlined __skb_dequeue(). The only reason BUG could happen here is something overwriting or otherwise corrupting skb list.
>
> Patch that works for me is below. Now I only hope I haven't (re)introduced a memory leak...
>
> I am not subscribed to LKML, so please reply-to-all if you need to contact me.
>
> -----------------------------------------------------------------------------
> --- linux-2.6.36/drivers/net/pppoe.c 2010-10-20 22:30:22.000000000 +0200
> +++ linux-2.6.36.toshio/drivers/net/pppoe.c 2010-12-03 13:11:56.000000000 +0100
> @@ -924,8 +924,10 @@
> /* Copy the data if there is no space for the header or if it's
> * read-only.
> */
> - if (skb_cow_head(skb, sizeof(*ph) + dev->hard_header_len))
> + if (skb_cow_head(skb, sizeof(*ph) + dev->hard_header_len)) {
> + kfree_skb(skb);
> goto abort;
> + }
>
> __skb_push(skb, sizeof(*ph));
> skb_reset_network_header(skb);
> @@ -947,7 +949,6 @@
> return 1;
>
> abort:
> - kfree_skb(skb);
> return 0;
> }
>
Problem comes from commit 55c95e738da85 (fix return value of
__pppoe_xmit() method)
I am not sure patch is OK
^ permalink raw reply
* Re: TSO/GRO/LRO/somethingO breaks LVS on 2.6.36
From: Ben Hutchings @ 2010-12-03 13:27 UTC (permalink / raw)
To: Simon Horman; +Cc: Eric Dumazet, Simon Kirby, netdev
In-Reply-To: <20101203123617.GA6993@verge.net.au>
On Fri, 2010-12-03 at 21:36 +0900, Simon Horman wrote:
> On Fri, Dec 03, 2010 at 12:29:03PM +0100, Eric Dumazet wrote:
[...]
> > I believe Simon Horman has some patches for GRO and LVS.
> >
> > Please send the results of "ethtool -k eth0" on all your nics / vlans ?
> >
> > For TSO, I am not sure why and where it could matter...
>
> There is a patch to teach LVS how to cope with GRO in nf-next-2.6
> and I expect it to be included in 2.6.38. The patch is "ipvs: allow
> transmit of GRO aggregated skbs" and perhaps it should be considered
> for 2.6.37 and stable. In general the work around is to disable GRO.
>
> The patch does not resolve the incompatibility of LVS with LRO.
> The work around there is to disable LRO. I'm not entirely sure
> how to teach LVS to disable LRO automatically, or if its desirable.
[...]
I think it is desirable, and we already do it for bridging and
forwarding - we call dev_disable_lro() whenever a device is added to a
bridge or has forwarding enabled. For IPVS it looks like you would have
to resolve the server IP address to a device first. Though if the
administrator assigns the server address to a different interface later
then the failure mode will be very hard to understand.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [net-next-2.6 PATCH] ixgbe: add support for 82599 FCoE SKU
From: Jeff Kirsher @ 2010-12-03 13:32 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Don Skidmore, Jeff Kirsher
From: Don Skidmore <donald.c.skidmore@intel.com>
Add both NIC and backplane support for FCoE enabled devices IDs.
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_82599.c | 2 ++
drivers/net/ixgbe/ixgbe_main.c | 4 ++++
drivers/net/ixgbe/ixgbe_type.h | 2 ++
3 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_82599.c b/drivers/net/ixgbe/ixgbe_82599.c
index 743bdec..6827ddd 100644
--- a/drivers/net/ixgbe/ixgbe_82599.c
+++ b/drivers/net/ixgbe/ixgbe_82599.c
@@ -341,11 +341,13 @@ static enum ixgbe_media_type ixgbe_get_media_type_82599(struct ixgbe_hw *hw)
case IXGBE_DEV_ID_82599_KX4_MEZZ:
case IXGBE_DEV_ID_82599_COMBO_BACKPLANE:
case IXGBE_DEV_ID_82599_KR:
+ case IXGBE_DEV_ID_82599_BACKPLANE_FCOE:
case IXGBE_DEV_ID_82599_XAUI_LOM:
/* Default device ID is mezzanine card KX/KX4 */
media_type = ixgbe_media_type_backplane;
break;
case IXGBE_DEV_ID_82599_SFP:
+ case IXGBE_DEV_ID_82599_SFP_FCOE:
case IXGBE_DEV_ID_82599_SFP_EM:
media_type = ixgbe_media_type_fiber;
break;
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index c876a69..c3338b5 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -109,6 +109,10 @@ static DEFINE_PCI_DEVICE_TABLE(ixgbe_pci_tbl) = {
board_82599 },
{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_CX4),
board_82599 },
+ {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_BACKPLANE_FCOE),
+ board_82599 },
+ {PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_SFP_FCOE),
+ board_82599 },
{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_T3_LOM),
board_82599 },
{PCI_VDEVICE(INTEL, IXGBE_DEV_ID_82599_COMBO_BACKPLANE),
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index bbcfafc..44bcabb 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -54,6 +54,8 @@
#define IXGBE_DEV_ID_82599_T3_LOM 0x151C
#define IXGBE_DEV_ID_82599_CX4 0x10F9
#define IXGBE_DEV_ID_82599_SFP 0x10FB
+#define IXGBE_DEV_ID_82599_BACKPLANE_FCOE 0x152a
+#define IXGBE_DEV_ID_82599_SFP_FCOE 0x1529
#define IXGBE_SUBDEV_ID_82599_SFP 0x11A9
#define IXGBE_DEV_ID_82599_SFP_EM 0x1507
#define IXGBE_DEV_ID_82599_XAUI_LOM 0x10FC
^ permalink raw reply related
* [net-next-2.6 PATCH] ixgbe: add support for new format of PBA numbers
From: Jeff Kirsher @ 2010-12-03 13:32 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Don Skidmore, Jeff Kirsher
From: Don Skidmore <donald.c.skidmore@intel.com>
The new PBA format is stored as a string. This patch allows the
driver to support both the old and new format.
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_common.c | 94 +++++++++++++++++++++++++++++++++++---
drivers/net/ixgbe/ixgbe_common.h | 3 +
drivers/net/ixgbe/ixgbe_main.c | 18 ++++---
drivers/net/ixgbe/ixgbe_type.h | 6 ++
4 files changed, 105 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_common.c b/drivers/net/ixgbe/ixgbe_common.c
index 5605257..cc11e42 100644
--- a/drivers/net/ixgbe/ixgbe_common.c
+++ b/drivers/net/ixgbe/ixgbe_common.c
@@ -196,30 +196,110 @@ s32 ixgbe_clear_hw_cntrs_generic(struct ixgbe_hw *hw)
}
/**
- * ixgbe_read_pba_num_generic - Reads part number from EEPROM
+ * ixgbe_read_pba_string_generic - Reads part number string from EEPROM
* @hw: pointer to hardware structure
- * @pba_num: stores the part number from the EEPROM
+ * @pba_num: stores the part number string from the EEPROM
+ * @pba_num_size: part number string buffer length
*
- * Reads the part number from the EEPROM.
+ * Reads the part number string from the EEPROM.
**/
-s32 ixgbe_read_pba_num_generic(struct ixgbe_hw *hw, u32 *pba_num)
+s32 ixgbe_read_pba_string_generic(struct ixgbe_hw *hw, u8 *pba_num,
+ u32 pba_num_size)
{
s32 ret_val;
u16 data;
+ u16 pba_ptr;
+ u16 offset;
+ u16 length;
+
+ if (pba_num == NULL) {
+ hw_dbg(hw, "PBA string buffer was null\n");
+ return IXGBE_ERR_INVALID_ARGUMENT;
+ }
ret_val = hw->eeprom.ops.read(hw, IXGBE_PBANUM0_PTR, &data);
if (ret_val) {
hw_dbg(hw, "NVM Read Error\n");
return ret_val;
}
- *pba_num = (u32)(data << 16);
- ret_val = hw->eeprom.ops.read(hw, IXGBE_PBANUM1_PTR, &data);
+ ret_val = hw->eeprom.ops.read(hw, IXGBE_PBANUM1_PTR, &pba_ptr);
if (ret_val) {
hw_dbg(hw, "NVM Read Error\n");
return ret_val;
}
- *pba_num |= data;
+
+ /*
+ * if data is not ptr guard the PBA must be in legacy format which
+ * means pba_ptr is actually our second data word for the PBA number
+ * and we can decode it into an ascii string
+ */
+ if (data != IXGBE_PBANUM_PTR_GUARD) {
+ hw_dbg(hw, "NVM PBA number is not stored as string\n");
+
+ /* we will need 11 characters to store the PBA */
+ if (pba_num_size < 11) {
+ hw_dbg(hw, "PBA string buffer too small\n");
+ return IXGBE_ERR_NO_SPACE;
+ }
+
+ /* extract hex string from data and pba_ptr */
+ pba_num[0] = (data >> 12) & 0xF;
+ pba_num[1] = (data >> 8) & 0xF;
+ pba_num[2] = (data >> 4) & 0xF;
+ pba_num[3] = data & 0xF;
+ pba_num[4] = (pba_ptr >> 12) & 0xF;
+ pba_num[5] = (pba_ptr >> 8) & 0xF;
+ pba_num[6] = '-';
+ pba_num[7] = 0;
+ pba_num[8] = (pba_ptr >> 4) & 0xF;
+ pba_num[9] = pba_ptr & 0xF;
+
+ /* put a null character on the end of our string */
+ pba_num[10] = '\0';
+
+ /* switch all the data but the '-' to hex char */
+ for (offset = 0; offset < 10; offset++) {
+ if (pba_num[offset] < 0xA)
+ pba_num[offset] += '0';
+ else if (pba_num[offset] < 0x10)
+ pba_num[offset] += 'A' - 0xA;
+ }
+
+ return 0;
+ }
+
+ ret_val = hw->eeprom.ops.read(hw, pba_ptr, &length);
+ if (ret_val) {
+ hw_dbg(hw, "NVM Read Error\n");
+ return ret_val;
+ }
+
+ if (length == 0xFFFF || length == 0) {
+ hw_dbg(hw, "NVM PBA number section invalid length\n");
+ return IXGBE_ERR_PBA_SECTION;
+ }
+
+ /* check if pba_num buffer is big enough */
+ if (pba_num_size < (((u32)length * 2) - 1)) {
+ hw_dbg(hw, "PBA string buffer too small\n");
+ return IXGBE_ERR_NO_SPACE;
+ }
+
+ /* trim pba length from start of string */
+ pba_ptr++;
+ length--;
+
+ for (offset = 0; offset < length; offset++) {
+ ret_val = hw->eeprom.ops.read(hw, pba_ptr + offset, &data);
+ if (ret_val) {
+ hw_dbg(hw, "NVM Read Error\n");
+ return ret_val;
+ }
+ pba_num[offset * 2] = (u8)(data >> 8);
+ pba_num[(offset * 2) + 1] = (u8)(data & 0xFF);
+ }
+ pba_num[offset * 2] = '\0';
return 0;
}
diff --git a/drivers/net/ixgbe/ixgbe_common.h b/drivers/net/ixgbe/ixgbe_common.h
index 341ca51..e1f980a 100644
--- a/drivers/net/ixgbe/ixgbe_common.h
+++ b/drivers/net/ixgbe/ixgbe_common.h
@@ -35,7 +35,8 @@ s32 ixgbe_init_ops_generic(struct ixgbe_hw *hw);
s32 ixgbe_init_hw_generic(struct ixgbe_hw *hw);
s32 ixgbe_start_hw_generic(struct ixgbe_hw *hw);
s32 ixgbe_clear_hw_cntrs_generic(struct ixgbe_hw *hw);
-s32 ixgbe_read_pba_num_generic(struct ixgbe_hw *hw, u32 *pba_num);
+s32 ixgbe_read_pba_string_generic(struct ixgbe_hw *hw, u8 *pba_num,
+ u32 pba_num_size);
s32 ixgbe_get_mac_addr_generic(struct ixgbe_hw *hw, u8 *mac_addr);
s32 ixgbe_get_bus_info_generic(struct ixgbe_hw *hw);
void ixgbe_set_lan_id_multi_port_pcie(struct ixgbe_hw *hw);
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index c3338b5..c5998ca 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -6955,11 +6955,12 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
const struct ixgbe_info *ii = ixgbe_info_tbl[ent->driver_data];
static int cards_found;
int i, err, pci_using_dac;
+ u8 part_str[IXGBE_PBANUM_LENGTH];
unsigned int indices = num_possible_cpus();
#ifdef IXGBE_FCOE
u16 device_caps;
#endif
- u32 part_num, eec;
+ u32 eec;
/* Catch broken hardware that put the wrong VF device ID in
* the PCIe SR-IOV capability.
@@ -7265,16 +7266,17 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
hw->bus.width == ixgbe_bus_width_pcie_x1 ? "Width x1" :
"Unknown"),
netdev->dev_addr);
- ixgbe_read_pba_num_generic(hw, &part_num);
+
+ err = ixgbe_read_pba_string_generic(hw, part_str, IXGBE_PBANUM_LENGTH);
+ if (err)
+ strcpy(part_str, "Unknown");
if (ixgbe_is_sfp(hw) && hw->phy.sfp_type != ixgbe_sfp_type_not_present)
- e_dev_info("MAC: %d, PHY: %d, SFP+: %d, "
- "PBA No: %06x-%03x\n",
+ e_dev_info("MAC: %d, PHY: %d, SFP+: %d, PBA No: %s\n",
hw->mac.type, hw->phy.type, hw->phy.sfp_type,
- (part_num >> 8), (part_num & 0xff));
+ part_str);
else
- e_dev_info("MAC: %d, PHY: %d, PBA No: %06x-%03x\n",
- hw->mac.type, hw->phy.type,
- (part_num >> 8), (part_num & 0xff));
+ e_dev_info("MAC: %d, PHY: %d, PBA No: %s\n",
+ hw->mac.type, hw->phy.type, part_str);
if (hw->bus.width <= ixgbe_bus_width_pcie_x4) {
e_dev_warn("PCI-Express bandwidth available for this card is "
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index 44bcabb..0f80893 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -1510,7 +1510,11 @@
#define IXGBE_EEPROM_WORD_SIZE_SHIFT 6
#define IXGBE_EEPROM_OPCODE_BITS 8
+/* Part Number String Length */
+#define IXGBE_PBANUM_LENGTH 11
+
/* Checksum and EEPROM pointers */
+#define IXGBE_PBANUM_PTR_GUARD 0xFAFA
#define IXGBE_EEPROM_CHECKSUM 0x3F
#define IXGBE_EEPROM_SUM 0xBABA
#define IXGBE_PCIE_ANALOG_PTR 0x03
@@ -2640,6 +2644,8 @@ struct ixgbe_info {
#define IXGBE_ERR_OVERTEMP -26
#define IXGBE_ERR_RAR_INDEX -27
#define IXGBE_ERR_SFP_SETUP_NOT_COMPLETE -30
+#define IXGBE_ERR_PBA_SECTION -31
+#define IXGBE_ERR_INVALID_ARGUMENT -32
#define IXGBE_NOT_IMPLEMENTED 0x7FFFFFFF
#endif /* _IXGBE_TYPE_H_ */
^ permalink raw reply related
* [net-next-2.6 PATCH] ixgbe: fix link behavior for SFP+ when driver is brought down
From: Jeff Kirsher @ 2010-12-03 13:31 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Don Skidmore, Jeff Kirsher
From: Don Skidmore <donald.c.skidmore@intel.com>
We have had several requests to have ifconfig down command disable
the SFP+ laser and thus make link go down. Likewise on ifconfig up
the laser would be enabled and link would come up. This patch enables
that behavior.
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_82599.c | 12 +++++++++---
drivers/net/ixgbe/ixgbe_main.c | 26 ++++++++++++++++++--------
2 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_82599.c b/drivers/net/ixgbe/ixgbe_82599.c
index 26b8ceb..743bdec 100644
--- a/drivers/net/ixgbe/ixgbe_82599.c
+++ b/drivers/net/ixgbe/ixgbe_82599.c
@@ -65,9 +65,9 @@ static s32 ixgbe_verify_fw_version_82599(struct ixgbe_hw *hw);
static void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
{
struct ixgbe_mac_info *mac = &hw->mac;
- if (hw->phy.multispeed_fiber) {
- /* Set up dual speed SFP+ support */
- mac->ops.setup_link = &ixgbe_setup_mac_link_multispeed_fiber;
+
+ /* enable the laser control functions for SFP+ fiber */
+ if (mac->ops.get_media_type(hw) == ixgbe_media_type_fiber) {
mac->ops.disable_tx_laser =
&ixgbe_disable_tx_laser_multispeed_fiber;
mac->ops.enable_tx_laser =
@@ -77,6 +77,12 @@ static void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
mac->ops.disable_tx_laser = NULL;
mac->ops.enable_tx_laser = NULL;
mac->ops.flap_tx_laser = NULL;
+ }
+
+ if (hw->phy.multispeed_fiber) {
+ /* Set up dual speed SFP+ support */
+ mac->ops.setup_link = &ixgbe_setup_mac_link_multispeed_fiber;
+ } else {
if ((mac->ops.get_media_type(hw) ==
ixgbe_media_type_backplane) &&
(hw->phy.smart_speed == ixgbe_smart_speed_auto ||
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index bd64a4d..fb81418 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -3793,8 +3793,11 @@ static int ixgbe_up_complete(struct ixgbe_adapter *adapter)
else
ixgbe_configure_msi_and_legacy(adapter);
- /* enable the optics */
- if (hw->phy.multispeed_fiber && hw->mac.ops.enable_tx_laser)
+ /* enable the optics for both mult-speed fiber and 82599 SFP+ fiber */
+ if (hw->mac.ops.enable_tx_laser &&
+ ((hw->phy.multispeed_fiber) ||
+ ((hw->phy.type == ixgbe_media_type_fiber) &&
+ (hw->mac.type == ixgbe_mac_82599EB))))
hw->mac.ops.enable_tx_laser(hw);
clear_bit(__IXGBE_DOWN, &adapter->state);
@@ -4106,15 +4109,19 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
break;
}
- /* power down the optics */
- if (hw->phy.multispeed_fiber && hw->mac.ops.disable_tx_laser)
- hw->mac.ops.disable_tx_laser(hw);
-
/* clear n-tuple filters that are cached */
ethtool_ntuple_flush(netdev);
if (!pci_channel_offline(adapter->pdev))
ixgbe_reset(adapter);
+
+ /* power down the optics for multispeed fiber and 82599 SFP+ fiber */
+ if (hw->mac.ops.disable_tx_laser &&
+ ((hw->phy.multispeed_fiber) ||
+ ((hw->phy.type == ixgbe_media_type_fiber) &&
+ (hw->mac.type == ixgbe_mac_82599EB))))
+ hw->mac.ops.disable_tx_laser(hw);
+
ixgbe_clean_all_tx_rings(adapter);
ixgbe_clean_all_rx_rings(adapter);
@@ -7200,8 +7207,11 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
goto err_eeprom;
}
- /* power down the optics */
- if (hw->phy.multispeed_fiber && hw->mac.ops.disable_tx_laser)
+ /* power down the optics for multispeed fiber and 82599 SFP+ fiber */
+ if (hw->mac.ops.disable_tx_laser &&
+ ((hw->phy.multispeed_fiber) ||
+ ((hw->phy.type == ixgbe_media_type_fiber) &&
+ (hw->mac.type == ixgbe_mac_82599EB))))
hw->mac.ops.disable_tx_laser(hw);
init_timer(&adapter->watchdog_timer);
^ permalink raw reply related
* Re: Bonding, GRO and tcp_reordering
From: Simon Horman @ 2010-12-03 13:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Ben Hutchings, netdev
In-Reply-To: <20101201043445.GC3485@verge.net.au>
On Wed, Dec 01, 2010 at 01:34:45PM +0900, Simon Horman wrote:
> On Tue, Nov 30, 2010 at 05:04:33PM +0100, Eric Dumazet wrote:
> > Le mardi 30 novembre 2010 à 15:42 +0000, Ben Hutchings a écrit :
> > > On Tue, 2010-11-30 at 22:55 +0900, Simon Horman wrote:
To clarify my statement in a previous email that GSO had no effect: I
re-ran the tests and I still haven't observed any affect of GSO on my
results. However, I did notice that in order for GRO on the server to have
effect I also need TSO enabled on the client. I thought that I had
previously checked that but I was mistaken.
Enabling TSO on the client while leaving GSO disabled on the server
resulted in increased CPU utilisation on the client, from ~15% to ~20%.
> > > > The only other parameter that seemed to have significant effect was to
> > > > increase the mtu. In the case of MTU=9000, GRO seemed to have a negative
> > > > impact on throughput, though a significant positive effect on CPU
> > > > utilisation.
> > > [...]
> > >
> > > Increasing MTU also increases the interval between packets on a TCP flow
> > > using maximum segment size so that it is more likely to exceed the
> > > difference in delay.
> > >
> >
> > GRO really is operational _if_ we receive in same NAPI run several
> > packets for the same flow.
> >
> > As soon as we exit NAPI mode, GRO packets are flushed.
> >
> > Big MTU --> bigger delays between packets, so big chance that GRO cannot
> > trigger at all, since NAPI runs for one packet only.
> >
> > One possibility with big MTU is to tweak "ethtool -c eth0" params
> > rx-usecs: 20
> > rx-frames: 5
> > rx-usecs-irq: 0
> > rx-frames-irq: 5
> > so that "rx-usecs" is bigger than the delay between two MTU full sized
> > packets.
> >
> > Gigabit speed means 1 nano second per bit, and MTU=9000 means 72 us
> > delay between packets.
> >
> > So try :
> >
> > ethtool -C eth0 rx-usecs 100
> >
> > to get chance that several packets are delivered at once by NIC.
> >
> > Unfortunately, this also add some latency, so it helps bulk transferts,
> > and slowdown interactive traffic
>
> Thanks Eric,
>
> I was tweaking those values recently for some latency tuning
> but I didn't think of them in relation to last night's tests.
>
> In terms of my measurements, its just benchmarking at this stage.
> So a trade-off between throughput and latency is acceptable, so long
> as I remember to measure what it is.
Thanks, rx-usecs was set to 3 and changing it to 15 on the server
did seem increase throughput with 1500 byte packets. Although
CPU utilisation increased too, disproportionally so on the client.
MTU=1500, client,server:tcp_reordering=3, client:GSO=off,
client:TSO=on, server:GRO=off, server:rx-usecs=3(default)
# netperf -c -4 -t TCP_STREAM -H 172.17.60.216
TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 172.17.60.216 (172.17.60.216) port 0 AF_INET
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % U us/KB us/KB
87380 16384 16384 10.00 1591.34 16.35 5.80 1.683 2.390
MTU=1500, client,server:tcp_reordering=3(default), client:GSO=off,
client:TSO=on, server:GRO=off server:rx-usecs=15
# netperf -c -4 -t TCP_STREAM -H 172.17.60.216
TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 172.17.60.216 (172.17.60.216) port 0 AF_INET
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % U us/KB us/KB
87380 16384 16384 10.00 1774.38 23.75 7.58 2.193 2.801
I also saw an improvement with GRO enabled on the server and TSO enabled on
the client. Although in this case I found rx-usecs=45 to be the best
value.
MTU=1500, client,server:tcp_reordering=3(default), client:GSO=off,
client:TSO=on, server:GRO=on server:rx-usecs=3(default)
# netperf -c -4 -t TCP_STREAM -H 172.17.60.216
TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 172.17.60.216 (172.17.60.216) port 0 AF_INET
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % U us/KB us/KB
87380 16384 16384 10.00 2553.27 13.31 3.35 0.854 0.860
MTU=1500, client,server:tcp_reordering=3(default), client:GSO=off,
client:TSO=on, server:GRO=on server:rx-usecs=45
# netperf -c -4 -t TCP_STREAM -H 172.17.60.216
TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 172.17.60.216 (172.17.60.216) port 0 AF_INET
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % U us/KB us/KB
87380 16384 16384 10.00 2727.53 29.45 9.48 1.769 2.278
I did not observe any improvement in throughput when increasing rx-usecs
from 3 when using mtu=9000 although there was a slight increase in CPU
utilisation (maybe, there is quite a lot of noise in the results).
^ permalink raw reply
* [net-next-2.6 PATCH] ixgbe: add WOL support for SFP+ subdevice
From: Jeff Kirsher @ 2010-12-03 13:32 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Don Skidmore, Jeff Kirsher
From: Don Skidmore <donald.c.skidmore@intel.com>
This patch will add wake on LAN support to the dev/sub_dev 10FB 11A9. This
will also include ixgbe ethtool support for this device.
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_ethtool.c | 11 +++++++++++
drivers/net/ixgbe/ixgbe_main.c | 15 ++++++++++-----
drivers/net/ixgbe/ixgbe_type.h | 1 +
3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethtool.c b/drivers/net/ixgbe/ixgbe_ethtool.c
index f9b5839..ef3f910 100644
--- a/drivers/net/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ixgbe/ixgbe_ethtool.c
@@ -1879,7 +1879,16 @@ static int ixgbe_wol_exclusion(struct ixgbe_adapter *adapter,
struct ixgbe_hw *hw = &adapter->hw;
int retval = 1;
+ /* WOL not supported except for the following */
switch(hw->device_id) {
+ case IXGBE_DEV_ID_82599_SFP:
+ /* Only this subdevice supports WOL */
+ if (hw->subsystem_device_id != IXGBE_SUBDEV_ID_82599_SFP) {
+ wol->supported = 0;
+ break;
+ }
+ retval = 0;
+ break;
case IXGBE_DEV_ID_82599_COMBO_BACKPLANE:
/* All except this subdevice support WOL */
if (hw->subsystem_device_id ==
@@ -1887,6 +1896,8 @@ static int ixgbe_wol_exclusion(struct ixgbe_adapter *adapter,
wol->supported = 0;
break;
}
+ retval = 0;
+ break;
case IXGBE_DEV_ID_82599_KX4:
retval = 0;
break;
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index fb81418..c876a69 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -7226,13 +7226,18 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
goto err_sw_init;
switch (pdev->device) {
+ case IXGBE_DEV_ID_82599_SFP:
+ /* Only this subdevice supports WOL */
+ if (pdev->subsystem_device == IXGBE_SUBDEV_ID_82599_SFP)
+ adapter->wol = (IXGBE_WUFC_MAG | IXGBE_WUFC_EX |
+ IXGBE_WUFC_MC | IXGBE_WUFC_BC);
+ break;
case IXGBE_DEV_ID_82599_COMBO_BACKPLANE:
/* All except this subdevice support WOL */
- if (pdev->subsystem_device ==
- IXGBE_SUBDEV_ID_82599_KX4_KR_MEZZ) {
- adapter->wol = 0;
- break;
- }
+ if (pdev->subsystem_device != IXGBE_SUBDEV_ID_82599_KX4_KR_MEZZ)
+ adapter->wol = (IXGBE_WUFC_MAG | IXGBE_WUFC_EX |
+ IXGBE_WUFC_MC | IXGBE_WUFC_BC);
+ break;
case IXGBE_DEV_ID_82599_KX4:
adapter->wol = (IXGBE_WUFC_MAG | IXGBE_WUFC_EX |
IXGBE_WUFC_MC | IXGBE_WUFC_BC);
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index 20e1f03..bbcfafc 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -54,6 +54,7 @@
#define IXGBE_DEV_ID_82599_T3_LOM 0x151C
#define IXGBE_DEV_ID_82599_CX4 0x10F9
#define IXGBE_DEV_ID_82599_SFP 0x10FB
+#define IXGBE_SUBDEV_ID_82599_SFP 0x11A9
#define IXGBE_DEV_ID_82599_SFP_EM 0x1507
#define IXGBE_DEV_ID_82599_XAUI_LOM 0x10FC
#define IXGBE_DEV_ID_82599_COMBO_BACKPLANE 0x10F8
^ permalink raw reply related
* ctnetlink loop
From: Holger Eitzenberger @ 2010-12-03 13:39 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, LKML
[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]
Hi,
I see a problem with how ctnetlink GET requests are being
processed in the kernel (2.6.32.24) under high load.
Initially I saw this problem on a large performance testing
system when getting HTTP proxy performance numbers, but lately
there have been two reports on large customers boxes (both
many-core with 10G NICs).
The sympton is Netlink looping around nfnetlink_rcv_msg(),
which is just because netlink_unicast() came back with -EAGAIN
when trying to write the newly created Netlink skb to the SK
receive buffer in ctnetlink_get_conntrack(). In this case a
(possibly) infinit loop is entered. Mostly infinit in fact in
case the userland party trying to receive those messages may
be stuck in the sendmsg() call, being unable to read anything
if being single threaded.
I tried to reproduce several times, a few times the loop
disappeared and the box proceeded normally after some time.
I have no explanation for this.
The attached patch tries to solve it by simple not trying
again to netlink_unicast() the reply skb and just fail with
-ENOBUFS. The reasoning is that at the point a Netlink overrun
is observed it seems counter intuitive to insist on sending
one more Netlink message.
I checked for possible side effects to other Netlink requests,
please check.
The patch applies to net-next-2.6.
Feedback appreciated.
/holger
[-- Attachment #2: nfnl-fix.diff --]
[-- Type: text/x-diff, Size: 1696 bytes --]
nfnetlink: avoid unbound loop on busy Netlink socket
I see a problem with how ctnetlink GET requests are being
processed in the kernel (2.6.32.24) under high load.
The sympton is Netlink looping around nfnetlink_rcv_msg(), which
is just because netlink_unicast() came back with EAGAIN when
trying to write the newly created Netlink skb to the SK receive
buffer in ctnetlink_get_conntrack(). In this case a (possibly)
infinit loop is entered. Mostly infinit I think in case the
userland party trying to receive those messages may be stuck in
the sendmsg() call, being unable to read anything if being single
threaded.
I tried to reproduce several times, a few times the loop
disappeared and the box proceeded normally after some minutes.
I have no explanation for this.
The attached patch tries to solve it by simple not trying again
to netlink_unicast() the reply skb and just fail with -ENOBUFS.
The reasoning is that at the point a Netlink overrun is detected
it seems counter intuitive to insist on sending one more Netlink
message.
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
Index: net-next-2.6/net/netfilter/nfnetlink.c
===================================================================
--- net-next-2.6.orig/net/netfilter/nfnetlink.c 2010-12-03 14:33:32.000000000 +0100
+++ net-next-2.6/net/netfilter/nfnetlink.c 2010-12-03 14:34:21.000000000 +0100
@@ -138,7 +138,6 @@
return 0;
type = nlh->nlmsg_type;
-replay:
ss = nfnetlink_get_subsys(type);
if (!ss) {
#ifdef CONFIG_MODULES
@@ -169,7 +168,7 @@
err = nc->call(net->nfnl, skb, nlh, (const struct nlattr **)cda);
if (err == -EAGAIN)
- goto replay;
+ err = -ENOBUFS;
return err;
}
}
^ permalink raw reply
* Re: TSO/GRO/LRO/somethingO breaks LVS on 2.6.36
From: Simon Horman @ 2010-12-03 13:39 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Eric Dumazet, Simon Kirby, netdev
In-Reply-To: <1291382839.8025.40.camel@localhost>
On Fri, Dec 03, 2010 at 01:27:19PM +0000, Ben Hutchings wrote:
> On Fri, 2010-12-03 at 21:36 +0900, Simon Horman wrote:
> > On Fri, Dec 03, 2010 at 12:29:03PM +0100, Eric Dumazet wrote:
> [...]
> > > I believe Simon Horman has some patches for GRO and LVS.
> > >
> > > Please send the results of "ethtool -k eth0" on all your nics / vlans ?
> > >
> > > For TSO, I am not sure why and where it could matter...
> >
> > There is a patch to teach LVS how to cope with GRO in nf-next-2.6
> > and I expect it to be included in 2.6.38. The patch is "ipvs: allow
> > transmit of GRO aggregated skbs" and perhaps it should be considered
> > for 2.6.37 and stable. In general the work around is to disable GRO.
> >
> > The patch does not resolve the incompatibility of LVS with LRO.
> > The work around there is to disable LRO. I'm not entirely sure
> > how to teach LVS to disable LRO automatically, or if its desirable.
> [...]
>
> I think it is desirable, and we already do it for bridging and
> forwarding - we call dev_disable_lro() whenever a device is added to a
> bridge or has forwarding enabled. For IPVS it looks like you would have
> to resolve the server IP address to a device first. Though if the
> administrator assigns the server address to a different interface later
> then the failure mode will be very hard to understand.
Thanks, I'll try and make it so.
^ permalink raw reply
* Re: ctnetlink loop
From: Holger Eitzenberger @ 2010-12-03 13:58 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, LKML
In-Reply-To: <20101203133903.GG13225@mail.eitzenberger.org>
[changed netfilter-devel address]
I is even stuck for O_NONBLOCK sockets, which may be due to nfnl_mutex
being held in the receive path.
/holger
^ permalink raw reply
* Re: unable to handle kernel NULL pointer dereference in skb_dequeue
From: Andrej Ota @ 2010-12-03 14:37 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, gvs, Rami Rosen, netdev
In-Reply-To: <1291381791.2897.250.camel@edumazet-laptop>
>> Patch that works for me is below. Now I only hope I haven't
>> (re)introduced a memory leak...
> Problem comes from commit 55c95e738da85 (fix return value of
> __pppoe_xmit() method)
>
> I am not sure patch is OK
Me neither. That's why I wrote "works for me". All I dare say is that it
works better than current code and is probably no worse than it was before
above mentioned commit. Apart from that, there is no point in having return
value for __pppoe_xmit if return value isn't needed.
Easiest way of triggering this BUG is by terminating PPPoE on the server
side, which then hits "if (!dev) { goto abort; }". This in turn calls
"kfree_skb(skb); return 0;" which returns to pppoe_rcv_core which then
goto-s to "abort_put" which again calls "kfree_skb(skb)". Voila the bug.
I don't know how to trigger "if (skb_cow_head(skb, ..." to see if I have
just caused another BUG. However, if I read file comments at the top, I see
a comment from 19/07/01 stating that I have to delete original skb if code
succeeds and never delete it on failure. About the skb copy mentioned in
the same comment, I don't know. 2001 was many commits ago.
Andrej Ota.
^ permalink raw reply
* Re: unable to handle kernel NULL pointer dereference in skb_dequeue
From: Eric Dumazet @ 2010-12-03 14:46 UTC (permalink / raw)
To: Andrej Ota; +Cc: linux-kernel, gvs, Rami Rosen, netdev
In-Reply-To: <aa43945b38cee751a8231084866b074d@192.168.253.23>
Le vendredi 03 décembre 2010 à 15:37 +0100, Andrej Ota a écrit :
> >> Patch that works for me is below. Now I only hope I haven't
> >> (re)introduced a memory leak...
>
> > Problem comes from commit 55c95e738da85 (fix return value of
> > __pppoe_xmit() method)
> >
> > I am not sure patch is OK
>
>
> Me neither. That's why I wrote "works for me". All I dare say is that it
> works better than current code and is probably no worse than it was before
> above mentioned commit. Apart from that, there is no point in having return
> value for __pppoe_xmit if return value isn't needed.
>
> Easiest way of triggering this BUG is by terminating PPPoE on the server
> side, which then hits "if (!dev) { goto abort; }". This in turn calls
> "kfree_skb(skb); return 0;" which returns to pppoe_rcv_core which then
> goto-s to "abort_put" which again calls "kfree_skb(skb)". Voila the bug.
>
> I don't know how to trigger "if (skb_cow_head(skb, ..." to see if I have
> just caused another BUG. However, if I read file comments at the top, I see
> a comment from 19/07/01 stating that I have to delete original skb if code
> succeeds and never delete it on failure. About the skb copy mentioned in
> the same comment, I don't know. 2001 was many commits ago.
Well, all I wanted to say was that _I_ was not sure, but probably other
network guys have a better diagnostic.
Rami, could you re-explain the rationale of your patch ?
Thanks
^ permalink raw reply
* DONATION!!!
From: W.I.N.S @ 2010-12-03 15:08 UTC (permalink / raw)
Your email was chosen in the EUROPEAN CHARITY DONATION promo award
that was participated December 2nd, and you are to claim the amount
of 1,000,000 USD Please contact Thomas Herman with the following detail Below:
EMAIL: thomasherman7@onfruit.cn
NAME IN FULL:
ADDRESS:
AGE:
NATIONALITY:
OCCUPATION:
PHONE:
SEX:
Joyce Patrick,
Promo Announcer.
^ permalink raw reply
* Re: [PATCH] net: am79c961a: Omit private ndo_get_stats function
From: Russell King - ARM Linux @ 2010-12-03 15:40 UTC (permalink / raw)
To: Tobias Klauser; +Cc: David S. Miller, netdev
In-Reply-To: <1291310405-5759-1-git-send-email-tklauser@distanz.ch>
On Thu, Dec 02, 2010 at 06:20:05PM +0100, Tobias Klauser wrote:
> am79c961_getstats() just returns dev->stats so we can leave it out
> alltogether and let dev_get_stats() do the job.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> drivers/net/arm/am79c961a.c | 9 ---------
> 1 files changed, 0 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/arm/am79c961a.c b/drivers/net/arm/am79c961a.c
> index 62f2110..0c9217f 100644
> --- a/drivers/net/arm/am79c961a.c
> +++ b/drivers/net/arm/am79c961a.c
> @@ -340,14 +340,6 @@ am79c961_close(struct net_device *dev)
> return 0;
> }
>
> -/*
> - * Get the current statistics.
> - */
> -static struct net_device_stats *am79c961_getstats (struct net_device *dev)
> -{
> - return &dev->stats;
> -}
> -
> static void am79c961_mc_hash(char *addr, unsigned short *hash)
> {
> if (addr[0] & 0x01) {
> @@ -665,7 +657,6 @@ static const struct net_device_ops am79c961_netdev_ops = {
> .ndo_open = am79c961_open,
> .ndo_stop = am79c961_close,
> .ndo_start_xmit = am79c961_sendpacket,
> - .ndo_get_stats = am79c961_getstats,
> .ndo_set_multicast_list = am79c961_setmulticastlist,
> .ndo_tx_timeout = am79c961_timeout,
> .ndo_validate_addr = eth_validate_addr,
> --
> 1.7.0.4
>
^ permalink raw reply
* Re: RFS configuration questions
From: Shawn Bohrer @ 2010-12-03 16:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, therbert
In-Reply-To: <1291326041.2854.2.camel@edumazet-laptop>
On Thu, Dec 02, 2010 at 10:40:41PM +0100, Eric Dumazet wrote:
> Le jeudi 02 décembre 2010 à 15:16 -0600, Shawn Bohrer a écrit :
> > I've been playing around with RPS/RFS on my multiqueue 10g Chelsio NIC
> > and I've got some questions about configuring RFS.
> >
> > I've enabled RPS with:
> >
> > for x in $(seq 0 7); do
> > echo FFFFFFFF,FFFFFFFF > /sys/class/net/vlan816/queues/rx-${x}/rps_cpus
> > done
> >
> > This appears to work when I watch 'mpstat -P ALL 1' as I can see the
> > softirq load is now getting distributed across all of the CPUs instead
> > of just the four (the card is a two port card and assigns four queues
> > per port) original hw receive queues which I have bound to CPUs
> > 0-3.
> >
> > To enable RFS I've run:
> >
> > echo 16384 > /proc/sys/net/core/rps_sock_flow_entries
> >
> > Is there any explanation of what this sysctl actually does? Is this
> > the max number of sockets/flows that the kernel can steer? Is this a
> > system wide max, a per interface max, or a per receive queue max?
> >
>
> Yes, some doc is missing...
>
> Its a system wide and shared limit.
So the sum of /sys/class/net/*/queues/rx-*/rps_flow_cnt should be less
than or equal to rps_sock_flow_entries?
> > Next I ran:
> >
> > for x in $(seq 0 7); do
> > echo 16384 > /sys/class/net/vlan816/queues/rx-${x}/rps_flow_cnt
> > done
> >
> > Is this correct? Is these the max number of sockets/flows that can be
> > steered per receive queue? Does the sum of these values need to add
> > up to rps_sock_flow_entries (I also tried 2048)? Is this all that is
> > needed to enable RFS?
> >
>
> Yes thats all.
Same as above... I should be using 2048 if I have 8 queues and have
set rps_sock_flow_entries to 16384? Out of curiosity what happens
when you open more sockets than you have rps_flow_cnt?
> > With these settings I can watch 'mpstat -P ALL 1' and it doesn't
> > appear RFS has changed the softirq load. To get a better idea if it
> > was working I used taskset to bind my receiving processes to a set of
> > cores, yet mpstat still shows the softirq load getting distributed
> > across all cores, not just the ones where my receiving processes are
> > bound. Is there a better way to determine if RFS is actually working?
> > Have I configured RFS incorrectly?
>
> It seems fine to me, but what kind of workload do you have, and what
> version of kernel do you run ?
I just did some more testing on 2.6.36.1. Using netperf UDP_STREAM
and TCP_STREAM I was able to see that the softirq load would run on
the CPU where netperf was bound so it appears that RFS is working.
However if I run one of my applications which is a single process
listening to ~30 multicast addresses the softirq load does not run on
the CPU where the application is bound. Does RFS not support
receiving multicast?
--
Shawn
^ permalink raw reply
* Re: [PATCH v2] ethtool : Allow ethtool to set interface in loopback mode.
From: David Lamparter @ 2010-12-03 16:03 UTC (permalink / raw)
To: Mahesh Bandewar; +Cc: Ben Hutchings, linux-netdev, Tom Herbert, David Miller
In-Reply-To: <AANLkTi=GygGKnuSZmWecmGejDjzLx2uaHuk+DroVfHDy@mail.gmail.com>
On Thu, Dec 02, 2010 at 02:35:51PM -0800, Mahesh Bandewar wrote:
> This patch adds -L command-line option to switch loopback mode on/off
> and -l option to display current loopback mode on a specified interface.
[...]
> +.A1 on off
> +Switches loopback mode
> +.B on
> +or
> +.B off
What about devices that support different kinds of loopback, like MAC
loopback & PHY loopback? (No idea if we have kernel support for this
either...)
-David
^ permalink raw reply
* Re: RFS configuration questions
From: Ben Hutchings @ 2010-12-03 16:30 UTC (permalink / raw)
To: Shawn Bohrer; +Cc: Eric Dumazet, netdev, therbert
In-Reply-To: <20101203160035.GA2698@BohrerMBP.rgmadvisors.com>
On Fri, 2010-12-03 at 10:00 -0600, Shawn Bohrer wrote:
> On Thu, Dec 02, 2010 at 10:40:41PM +0100, Eric Dumazet wrote:
> > Le jeudi 02 décembre 2010 à 15:16 -0600, Shawn Bohrer a écrit :
> > > I've been playing around with RPS/RFS on my multiqueue 10g Chelsio NIC
> > > and I've got some questions about configuring RFS.
> > >
> > > I've enabled RPS with:
> > >
> > > for x in $(seq 0 7); do
> > > echo FFFFFFFF,FFFFFFFF > /sys/class/net/vlan816/queues/rx-${x}/rps_cpus
> > > done
> > >
> > > This appears to work when I watch 'mpstat -P ALL 1' as I can see the
> > > softirq load is now getting distributed across all of the CPUs instead
> > > of just the four (the card is a two port card and assigns four queues
> > > per port) original hw receive queues which I have bound to CPUs
> > > 0-3.
> > >
> > > To enable RFS I've run:
> > >
> > > echo 16384 > /proc/sys/net/core/rps_sock_flow_entries
> > >
> > > Is there any explanation of what this sysctl actually does? Is this
> > > the max number of sockets/flows that the kernel can steer? Is this a
> > > system wide max, a per interface max, or a per receive queue max?
> > >
> >
> > Yes, some doc is missing...
> >
> > Its a system wide and shared limit.
>
> So the sum of /sys/class/net/*/queues/rx-*/rps_flow_cnt should be less
> than or equal to rps_sock_flow_entries?
No, that is not required.
> > > Next I ran:
> > >
> > > for x in $(seq 0 7); do
> > > echo 16384 > /sys/class/net/vlan816/queues/rx-${x}/rps_flow_cnt
> > > done
> > >
> > > Is this correct? Is these the max number of sockets/flows that can be
> > > steered per receive queue? Does the sum of these values need to add
> > > up to rps_sock_flow_entries (I also tried 2048)? Is this all that is
> > > needed to enable RFS?
> > >
> >
> > Yes thats all.
>
> Same as above... I should be using 2048 if I have 8 queues and have
> set rps_sock_flow_entries to 16384? Out of curiosity what happens
> when you open more sockets than you have rps_flow_cnt?
[...]
There will be a hash collision - multiple flows will hash to the same
value and must therefore be handled on the same processor. (In fact
this is likely to happen long before you fill the table completely.)
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH v2] ethtool : Allow ethtool to set interface in loopback mode.
From: Ben Hutchings @ 2010-12-03 16:33 UTC (permalink / raw)
To: David Lamparter; +Cc: Mahesh Bandewar, linux-netdev, Tom Herbert, David Miller
In-Reply-To: <20101203160333.GA235120@jupiter.n2.diac24.net>
On Fri, 2010-12-03 at 17:03 +0100, David Lamparter wrote:
> On Thu, Dec 02, 2010 at 02:35:51PM -0800, Mahesh Bandewar wrote:
> > This patch adds -L command-line option to switch loopback mode on/off
> > and -l option to display current loopback mode on a specified interface.
> [...]
> > +.A1 on off
> > +Switches loopback mode
> > +.B on
> > +or
> > +.B off
>
> What about devices that support different kinds of loopback, like MAC
> loopback & PHY loopback? (No idea if we have kernel support for this
> either...)
That distinction is useful for diagnostic purposes, but drivers can
already cover those different loopback modes in self-test. The
motivation for this feature is testing data path behaviour, and Mahesh
has specified that loopback should be enabled as near as possible to the
host.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: RFS configuration questions
From: Eric Dumazet @ 2010-12-03 16:34 UTC (permalink / raw)
To: Shawn Bohrer; +Cc: netdev, therbert
In-Reply-To: <20101203160035.GA2698@BohrerMBP.rgmadvisors.com>
Le vendredi 03 décembre 2010 à 10:00 -0600, Shawn Bohrer a écrit :
> On Thu, Dec 02, 2010 at 10:40:41PM +0100, Eric Dumazet wrote:
> > Le jeudi 02 décembre 2010 à 15:16 -0600, Shawn Bohrer a écrit :
> > > I've been playing around with RPS/RFS on my multiqueue 10g Chelsio NIC
> > > and I've got some questions about configuring RFS.
> > >
> > > I've enabled RPS with:
> > >
> > > for x in $(seq 0 7); do
> > > echo FFFFFFFF,FFFFFFFF > /sys/class/net/vlan816/queues/rx-${x}/rps_cpus
> > > done
> > >
> > > This appears to work when I watch 'mpstat -P ALL 1' as I can see the
> > > softirq load is now getting distributed across all of the CPUs instead
> > > of just the four (the card is a two port card and assigns four queues
> > > per port) original hw receive queues which I have bound to CPUs
> > > 0-3.
> > >
> > > To enable RFS I've run:
> > >
> > > echo 16384 > /proc/sys/net/core/rps_sock_flow_entries
> > >
> > > Is there any explanation of what this sysctl actually does? Is this
> > > the max number of sockets/flows that the kernel can steer? Is this a
> > > system wide max, a per interface max, or a per receive queue max?
> > >
> >
> > Yes, some doc is missing...
> >
> > Its a system wide and shared limit.
>
> So the sum of /sys/class/net/*/queues/rx-*/rps_flow_cnt should be less
> than or equal to rps_sock_flow_entries?
>
I always used same count, but you probably can use lower values
for /sys/class/net/*/queues/rx-*/rps_flow_cnt if you dont have much
memory.
> > > Next I ran:
> > >
> > > for x in $(seq 0 7); do
> > > echo 16384 > /sys/class/net/vlan816/queues/rx-${x}/rps_flow_cnt
> > > done
> > >
> > > Is this correct? Is these the max number of sockets/flows that can be
> > > steered per receive queue? Does the sum of these values need to add
> > > up to rps_sock_flow_entries (I also tried 2048)? Is this all that is
> > > needed to enable RFS?
> > >
> >
> > Yes thats all.
>
> Same as above... I should be using 2048 if I have 8 queues and have
> set rps_sock_flow_entries to 16384? Out of curiosity what happens
> when you open more sockets than you have rps_flow_cnt?
Its a hash table, so collisions can happen. Nothing bad happens.
>
> > > With these settings I can watch 'mpstat -P ALL 1' and it doesn't
> > > appear RFS has changed the softirq load. To get a better idea if it
> > > was working I used taskset to bind my receiving processes to a set of
> > > cores, yet mpstat still shows the softirq load getting distributed
> > > across all cores, not just the ones where my receiving processes are
> > > bound. Is there a better way to determine if RFS is actually working?
> > > Have I configured RFS incorrectly?
> >
> > It seems fine to me, but what kind of workload do you have, and what
> > version of kernel do you run ?
>
> I just did some more testing on 2.6.36.1. Using netperf UDP_STREAM
> and TCP_STREAM I was able to see that the softirq load would run on
> the CPU where netperf was bound so it appears that RFS is working.
Be careful about softirq times, they most of the time are wrong.
You can do "cat /proc/net/softnet_stat" and check last column to see if
packets are really distributed to "other cpus"
>
> However if I run one of my applications which is a single process
> listening to ~30 multicast addresses the softirq load does not run on
> the CPU where the application is bound. Does RFS not support
> receiving multicast?
No because :
net/ipv4/udp.c
static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
int rc;
if (inet_sk(sk)->inet_daddr)
sock_rps_save_rxhash(sk, skb->rxhash);
Rationale is : RFS is implemented for _connected_ sockets only (TCP or
connected UDP)
(You can check commit changelog :
http://git2.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commit;h=fec5e652e58fa6017b2c9e06466cb2a6538de5b4
This is because rxhash value is different for each src address.
If we allowed your process to call sock_rps_save_rxhash(sk,
skb->rxhash); with many different rxhash, it would blow the hash table.
You might try to remove the "if (inet_sk(sk)->inet_daddr)" test for
your benches, or add a new logic (socket flag maybe ?) to really trigger
RFS on your UDP sockets.
^ permalink raw reply
* BUG: secpath not clearing between namespaces
From: Andrew Dickinson @ 2010-12-03 16:47 UTC (permalink / raw)
To: netdev
Hi,
I think I've found a bug in the xfrm stack relating to namespaces. I
can reproduce this consistently (see below).
I have the 3 namespaces which I refer to as "CORE", "VPN1" and "VPN2".
VPN1 and VPN2 each have 1 veth interfaces, CORE has 2 veth interfaces.
I'm using a pair of bridge (not in a namespace) to plumb the
namespaces together:
VPN1 and CORE are on 10.0.1.12/30 with each of their veth pairs in a
non-namespaced bridge.
VPN2 and CORE are on 10.0.1.16/30 with each of their veth pairs in a
different non-namespaced bridge.
VPN1 has an ipsec tunnel to 10.254.0.0/23
VPN2 has an ipsec tunnel to 10.254.2.0/23
What does work: From CORE, I can happily ping hosts on the far side of
either tunnel (as expected). Likewise, from VPN1 I can ping hosts
down its tunnel and the same for VPN2. (again: as expected).
What DOESN'T work: from VPN1 I can't ping a host on the far side of
VPN2 and visa-versa.
What I see is that when the traffic arrives at VPN2 it tries to ARP
for it's next-hop (which _should_ be encrypted and set across the
tunnel). It appears that it's chosing to NOT encrypt the traffic and
perform normal IPv4 forwarding. I suspect that this is because when
the frame was being processed in VPN1 it hit an SPD policy setting the
secpath action to "none":
The last few lines in VPN1's "setkey.conf" file:
spdadd 0.0.0.0/0 0.0.0.0/0 any -P in prio 10 none;
spdadd 0.0.0.0/0 0.0.0.0/0 any -P out prio 10 none;
As such, when the packet arrives on VPN2, VPN2 is seeing that the sp
is already set and not trying to re-compute the policy.
I can further confirm this by setting an SPD in CORE which does ONLY
the "none/none" policy and the traffic will instantly break; clearing
the SPD and SAD in CORE restores service. See here:
root@CORE:~# cat /etc/racoon/setkey.conf
flush;
spdflush;
spdadd 0.0.0.0/0 0.0.0.0/0 any -P in prio 10 none;
spdadd 0.0.0.0/0 0.0.0.0/0 any -P out prio 10 none;
root@CORE:~# ping -c1 10.254.2.5
PING 10.254.2.5 (10.254.2.5) 56(84) bytes of data.
>From 10.1.2.18: icmp_seq=1 Redirect Host(New nexthop: 169.254.254.5)
>From 10.1.2.18 icmp_seq=1 Destination Host Unreachable
--- 10.254.2.5 ping statistics ---
1 packets transmitted, 0 received, +1 errors, 100% packet loss, time 0ms
root@CORE:~# setkey -F; setkey -PF
root@CORE:~# ping -c1 10.254.2.5
PING 10.254.2.5 (10.254.2.5) 56(84) bytes of data.
64 bytes from 10.254.2.5: icmp_seq=1 ttl=62 time=186 ms
--- 10.254.2.5 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 186.187/186.187/186.187/0.000 ms
It seems like the xfrm code needs to validate that the sp that's set
belongs to the current namespace that's processing it???
-A
^ permalink raw reply
* Re: [PATCH v2] ethtool : Allow ethtool to set interface in loopback mode.
From: David Lamparter @ 2010-12-03 16:48 UTC (permalink / raw)
To: Ben Hutchings
Cc: David Lamparter, Mahesh Bandewar, linux-netdev, Tom Herbert,
David Miller
In-Reply-To: <1291394014.13384.7.camel@bwh-desktop>
On Fri, Dec 03, 2010 at 04:33:34PM +0000, Ben Hutchings wrote:
> On Fri, 2010-12-03 at 17:03 +0100, David Lamparter wrote:
> > On Thu, Dec 02, 2010 at 02:35:51PM -0800, Mahesh Bandewar wrote:
> > > This patch adds -L command-line option to switch loopback mode on/off
> > > and -l option to display current loopback mode on a specified interface.
> > [...]
> > > +.A1 on off
> > > +Switches loopback mode
> > > +.B on
> > > +or
> > > +.B off
> >
> > What about devices that support different kinds of loopback, like MAC
> > loopback & PHY loopback? (No idea if we have kernel support for this
> > either...)
>
> That distinction is useful for diagnostic purposes, but drivers can
> already cover those different loopback modes in self-test. The
> motivation for this feature is testing data path behaviour, and Mahesh
> has specified that loopback should be enabled as near as possible to the
> host.
Ah, good enough. Thanks.
-David
^ permalink raw reply
* Re: [PATCH net-next-2.6 01/17] sfc: Reduce log level for MCDI error response in efx_mcdi_rpc()
From: David Miller @ 2010-12-03 17:08 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1291333569.3259.24.camel@bwh-desktop>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 02 Dec 2010 23:46:09 +0000
> Some errors are expected, e.g. when sending new commands to an MC
> running old firmware. Only the caller of efx_mcdi_rpc() can decide
> what is a real error. Therefore log the error responses with
> netif_dbg().
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6 02/17] sfc: Fix condition for no-op in set_phy_flash_cfg()
From: David Miller @ 2010-12-03 17:08 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1291333574.3259.25.camel@bwh-desktop>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 02 Dec 2010 23:46:14 +0000
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6 03/17] sfc: Distinguish critical and non-critical over-temperature conditions
From: David Miller @ 2010-12-03 17:08 UTC (permalink / raw)
To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1291333584.3259.26.camel@bwh-desktop>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 02 Dec 2010 23:46:24 +0000
> Set both the 'maximum' and critical temperature limits for LM87
> hardware monitors on Falcon boards. Do not shut down a port until the
> critical temperature is reached, but warn as soon as the 'maximum'
> temperature is reached.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ 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