Netdev List
 help / color / mirror / Atom feed
* [PATCH 1/2] bnx2x: Use pci_dev pm_cap
From: Jon Mason @ 2013-09-11 18:22 UTC (permalink / raw)
  To: netdev; +Cc: Eilon Greenstein

Use the already existing pm_cap variable in struct pci_dev for
determining the power management offset.  This saves the driver from
having to keep track of an extra variable.

Signed-off-by: Jon Mason <jdmason@kudzu.us>
Cc: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x.h         |    1 -
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c     |    8 ++++----
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c |    4 ++--
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c    |   10 +++++-----
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index 0c33802..70b6a05 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -1542,7 +1542,6 @@ struct bnx2x {
 	 */
 	bool			fcoe_init;
 
-	int			pm_cap;
 	int			mrrs;
 
 	struct delayed_work	sp_task;
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 2361bf2..44a1261 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -3000,16 +3000,16 @@ int bnx2x_set_power_state(struct bnx2x *bp, pci_power_t state)
 	u16 pmcsr;
 
 	/* If there is no power capability, silently succeed */
-	if (!bp->pm_cap) {
+	if (!bp->pdev->pm_cap) {
 		BNX2X_DEV_INFO("No power capability. Breaking.\n");
 		return 0;
 	}
 
-	pci_read_config_word(bp->pdev, bp->pm_cap + PCI_PM_CTRL, &pmcsr);
+	pci_read_config_word(bp->pdev, bp->pdev->pm_cap + PCI_PM_CTRL, &pmcsr);
 
 	switch (state) {
 	case PCI_D0:
-		pci_write_config_word(bp->pdev, bp->pm_cap + PCI_PM_CTRL,
+		pci_write_config_word(bp->pdev, bp->pdev->pm_cap + PCI_PM_CTRL,
 				      ((pmcsr & ~PCI_PM_CTRL_STATE_MASK) |
 				       PCI_PM_CTRL_PME_STATUS));
 
@@ -3033,7 +3033,7 @@ int bnx2x_set_power_state(struct bnx2x *bp, pci_power_t state)
 		if (bp->wol)
 			pmcsr |= PCI_PM_CTRL_PME_ENABLE;
 
-		pci_write_config_word(bp->pdev, bp->pm_cap + PCI_PM_CTRL,
+		pci_write_config_word(bp->pdev, bp->pdev->pm_cap + PCI_PM_CTRL,
 				      pmcsr);
 
 		/* No more memory access after this point until
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index 2612e3c..324de5f 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -1387,9 +1387,9 @@ static bool bnx2x_is_nvm_accessible(struct bnx2x *bp)
 	u16 pm = 0;
 	struct net_device *dev = pci_get_drvdata(bp->pdev);
 
-	if (bp->pm_cap)
+	if (bp->pdev->pm_cap)
 		rc = pci_read_config_word(bp->pdev,
-					  bp->pm_cap + PCI_PM_CTRL, &pm);
+					  bp->pdev->pm_cap + PCI_PM_CTRL, &pm);
 
 	if ((rc && !netif_running(dev)) ||
 	    (!rc && ((pm & PCI_PM_CTRL_STATE_MASK) != (__force u16)PCI_D0)))
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 634a793..41d6f2d 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -8651,6 +8651,7 @@ u32 bnx2x_send_unload_req(struct bnx2x *bp, int unload_mode)
 	else if (bp->wol) {
 		u32 emac_base = port ? GRCBASE_EMAC1 : GRCBASE_EMAC0;
 		u8 *mac_addr = bp->dev->dev_addr;
+		struct pci_dev *pdev = bp->pdev;
 		u32 val;
 		u16 pmc;
 
@@ -8667,9 +8668,9 @@ u32 bnx2x_send_unload_req(struct bnx2x *bp, int unload_mode)
 		EMAC_WR(bp, EMAC_REG_EMAC_MAC_MATCH + entry + 4, val);
 
 		/* Enable the PME and clear the status */
-		pci_read_config_word(bp->pdev, bp->pm_cap + PCI_PM_CTRL, &pmc);
+		pci_read_config_word(pdev, pdev->pm_cap + PCI_PM_CTRL, &pmc);
 		pmc |= PCI_PM_CTRL_PME_ENABLE | PCI_PM_CTRL_PME_STATUS;
-		pci_write_config_word(bp->pdev, bp->pm_cap + PCI_PM_CTRL, pmc);
+		pci_write_config_word(pdev, pdev->pm_cap + PCI_PM_CTRL, pmc);
 
 		reset_code = DRV_MSG_CODE_UNLOAD_REQ_WOL_EN;
 
@@ -10398,7 +10399,7 @@ static void bnx2x_get_common_hwinfo(struct bnx2x *bp)
 		break;
 	}
 
-	pci_read_config_word(bp->pdev, bp->pm_cap + PCI_PM_PMC, &pmc);
+	pci_read_config_word(bp->pdev, bp->pdev->pm_cap + PCI_PM_PMC, &pmc);
 	bp->flags |= (pmc & PCI_PM_CAP_PME_D3cold) ? 0 : NO_WOL_FLAG;
 
 	BNX2X_DEV_INFO("%sWoL capable\n",
@@ -12140,8 +12141,7 @@ static int bnx2x_init_dev(struct bnx2x *bp, struct pci_dev *pdev,
 	}
 
 	if (IS_PF(bp)) {
-		bp->pm_cap = pdev->pm_cap;
-		if (bp->pm_cap == 0) {
+		if (!pdev->pm_cap) {
 			dev_err(&bp->pdev->dev,
 				"Cannot find power management capability, aborting\n");
 			rc = -EIO;
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH 20/52] net: fealnx: remove unnecessary pci_set_drvdata()
From: Sergei Shtylyov @ 2013-09-11 18:19 UTC (permalink / raw)
  To: Jingoo Han; +Cc: 'David S. Miller', netdev
In-Reply-To: <005e01ceaec2$23e32420$6ba96c60$%han@samsung.com>

Hello.

On 09/11/2013 11:40 AM, Jingoo Han wrote:

> The driver core clears the driver data to NULL after device_release
> or on probe failure. Thus, it is not needed to manually clear the
> device driver data to NULL.

> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>   drivers/net/ethernet/fealnx.c |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

> diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c
> index c706b7a..99194d1 100644
> --- a/drivers/net/ethernet/fealnx.c
> +++ b/drivers/net/ethernet/fealnx.c
> @@ -699,9 +699,9 @@ static void fealnx_remove_one(struct pci_dev *pdev)
>   		pci_iounmap(pdev, np->mem);
>   		free_netdev(dev);
>   		pci_release_regions(pdev);
> -		pci_set_drvdata(pdev, NULL);
> -	} else
> +	} else {
>   		printk(KERN_ERR "fealnx: remove for unknown device\n");
> +	}

    No "drove-by" coding style fixes, please.

WBR, Sergei

^ permalink raw reply

* Re: [PATCH 29/52] net: neterion: remove unnecessary pci_set_drvdata()
From: Jon Mason @ 2013-09-11 18:05 UTC (permalink / raw)
  To: Jingoo Han; +Cc: David S. Miller, netdev
In-Reply-To: <006701ceaec3$dce53e30$96afba90$%han@samsung.com>

On Wed, Sep 11, 2013 at 12:52 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> The driver core clears the driver data to NULL after device_release
> or on probe failure. Thus, it is not needed to manually clear the
> device driver data to NULL.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>

Looks sane to me.
Acked-by: Jon Mason <jdmason@kudzu.us>

> ---
>  drivers/net/ethernet/neterion/s2io.c           |    2 --
>  drivers/net/ethernet/neterion/vxge/vxge-main.c |    2 --
>  2 files changed, 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
> index 51b0094..9eeddbd 100644
> --- a/drivers/net/ethernet/neterion/s2io.c
> +++ b/drivers/net/ethernet/neterion/s2io.c
> @@ -8185,7 +8185,6 @@ mem_alloc_failed:
>         free_shared_mem(sp);
>         pci_disable_device(pdev);
>         pci_release_regions(pdev);
> -       pci_set_drvdata(pdev, NULL);
>         free_netdev(dev);
>
>         return ret;
> @@ -8221,7 +8220,6 @@ static void s2io_rem_nic(struct pci_dev *pdev)
>         iounmap(sp->bar0);
>         iounmap(sp->bar1);
>         pci_release_regions(pdev);
> -       pci_set_drvdata(pdev, NULL);
>         free_netdev(dev);
>         pci_disable_device(pdev);
>  }
> diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c b/drivers/net/ethernet/neterion/vxge/vxge-main.c
> index 5a20eaf..8614eeb 100644
> --- a/drivers/net/ethernet/neterion/vxge/vxge-main.c
> +++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c
> @@ -4739,7 +4739,6 @@ _exit6:
>  _exit5:
>         vxge_device_unregister(hldev);
>  _exit4:
> -       pci_set_drvdata(pdev, NULL);
>         vxge_hw_device_terminate(hldev);
>         pci_disable_sriov(pdev);
>  _exit3:
> @@ -4782,7 +4781,6 @@ static void vxge_remove(struct pci_dev *pdev)
>                 vxge_free_mac_add_list(&vdev->vpaths[i]);
>
>         vxge_device_unregister(hldev);
> -       pci_set_drvdata(pdev, NULL);
>         /* Do not call pci_disable_sriov here, as it will break child devices */
>         vxge_hw_device_terminate(hldev);
>         iounmap(vdev->bar0);
> --
> 1.7.10.4
>
>

^ permalink raw reply

* Re: [PATCH nf] netfilter: use RCU safe kfree for conntrack extensions
From: Eric Dumazet @ 2013-09-11 17:54 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: Phil Oester, netfilter-devel, netdev, Pablo Neira Ayuso,
	Patrick McHardy, Jozsef Kadlecsik
In-Reply-To: <20130911174248.GA8318@unicorn.suse.cz>

On Wed, 2013-09-11 at 19:42 +0200, Michal Kubecek wrote:

> Yes, the patch submitted here is against current nf branch. For
> SLES 11 SP1 (with 2.6.32 kernel), I'm going to use call_rcu() the way
> original commit 68b80f11 does. 

Well, please just submit a patch for current tree, using kfree_rcu()

When doing backport to stable branches, needed adaptation shall be done.

^ permalink raw reply

* Re: [PATCH nf] netfilter: use RCU safe kfree for conntrack extensions
From: Pablo Neira Ayuso @ 2013-09-11 17:50 UTC (permalink / raw)
  To: Phil Oester
  Cc: Michal Kubecek, netfilter-devel, netdev, Patrick McHardy,
	Jozsef Kadlecsik
In-Reply-To: <20130911170946.GA2926@linuxace.com>

On Wed, Sep 11, 2013 at 10:09:47AM -0700, Phil Oester wrote:
> On Wed, Sep 11, 2013 at 05:28:05PM +0200, Michal Kubecek wrote:
> > > Looking at
> > > your proposed fix, the NAT extension data should have been cleaned
> > > from the bysource list in nf_nat_cleanup_conntrack (via __nf_ct_ext_destroy)
> > > before reaching the kfree.  Would you agree?
> > 
> > It is cleaned from the list but as it is an RCU list, other readers can
> > still be holding pointers to it. We have to wait for the RCU grace
> > period before we can reuse it.
> 
> Agreed - looks like your fix should work.  However, two nits:
> 
> 1) normally RCU functions have _rcu suffixes.  So nf_ct_ext_free should
> become nf_ct_ext_free_rcu.

That postfix is there if the function requires to be called holding
rcu read lock, not this case. I'll take this patch.

> 2) kfree_rcu was not added to the kernel until 3.0.  All of the bug
> reports I've been looking into (including the original in netfilter bugzilla
> at http://bugzilla.netfilter.org/show_bug.cgi?id=714) have been reported in
> 2.6.32 or earlier kernels.  So a different fix would need to be backported for
> -stable.  For that, we would probably export __nf_ct_ext_free_rcu from
> nf_conntrack_extend.c and change the kfree call in nf_ct_ext_free_rcu to
> call_rcu(&ct->ext->rcu, __nf_ct_ext_free_rcu). Of course the alternative
> is just to use this fix for both old and new kernels for simplicity.

Either way, we need a specific backport for 2.6.x indeed.

Thanks for tracking up this issue.

^ permalink raw reply

* Re: [PATCH nf] netfilter: use RCU safe kfree for conntrack extensions
From: Michal Kubecek @ 2013-09-11 17:42 UTC (permalink / raw)
  To: Phil Oester
  Cc: netfilter-devel, netdev, Pablo Neira Ayuso, Patrick McHardy,
	Jozsef Kadlecsik
In-Reply-To: <20130911170946.GA2926@linuxace.com>

On Wed, Sep 11, 2013 at 10:09:47AM -0700, Phil Oester wrote:
> 
> 1) normally RCU functions have _rcu suffixes.  So nf_ct_ext_free should
> become nf_ct_ext_free_rcu.

Right. I'll post updated version with the rename tomorrow.

> 2) kfree_rcu was not added to the kernel until 3.0.  All of the bug
> reports I've been looking into (including the original in netfilter bugzilla
> at http://bugzilla.netfilter.org/show_bug.cgi?id=714) have been reported in
> 2.6.32 or earlier kernels.  So a different fix would need to be backported for
> -stable.  For that, we would probably export __nf_ct_ext_free_rcu from
> nf_conntrack_extend.c and change the kfree call in nf_ct_ext_free_rcu to
> call_rcu(&ct->ext->rcu, __nf_ct_ext_free_rcu).

Yes, the patch submitted here is against current nf branch. For
SLES 11 SP1 (with 2.6.32 kernel), I'm going to use call_rcu() the way
original commit 68b80f11 does. IIRC the only pre-3.0 stable branch still
maintained is 2.6.32, all others are 3.0 or newer so they have
kfree_rcu() and also use it in __nf_ct_ext_add() since commit 1f8d36a1. 

                                                        Michal Kubecek


^ permalink raw reply

* Re: [PATCH 14/52] net: cxgb4vf: remove unnecessary pci_set_drvdata()
From: Casey Leedom @ 2013-09-11 17:24 UTC (permalink / raw)
  To: Jingoo Han; +Cc: 'David S. Miller', netdev
In-Reply-To: <005801ceaec1$6b8d3320$42a79960$%han@samsung.com>

   I agree that the redundant pci_set_drvdata(pdev, NULL) in 
cxgb4vf_pci_probe() under the err_release_regions: label is unneeded, 
but don't we need to NULL out the PCI Driver Data under the 
err_free_adapter: label and also in cxgb4vf_pci_remove()?  Or is that 
handled automatically in the PCI infrastructure code which calls the 
Device Probe and Remove routines?  Mostly I was just being an 
obsessively clean housewife assuming that we'd want to clean up these 
references ...

Casey

On 09/11/13 00:34, Jingoo Han wrote:
> The driver core clears the driver data to NULL after device_release
> or on probe failure. Thus, it is not needed to manually clear the
> device driver data to NULL.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>   drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c |    3 ---
>   1 file changed, 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
> index 40c22e7..43bb012 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
> @@ -2782,11 +2782,9 @@ err_unmap_bar:
>   
>   err_free_adapter:
>   	kfree(adapter);
> -	pci_set_drvdata(pdev, NULL);
>   
>   err_release_regions:
>   	pci_release_regions(pdev);
> -	pci_set_drvdata(pdev, NULL);
>   	pci_clear_master(pdev);
>   
>   err_disable_device:
> @@ -2851,7 +2849,6 @@ static void cxgb4vf_pci_remove(struct pci_dev *pdev)
>   		}
>   		iounmap(adapter->regs);
>   		kfree(adapter);
> -		pci_set_drvdata(pdev, NULL);
>   	}
>   
>   	/*

^ permalink raw reply

* Re: [PATCH nf] netfilter: use RCU safe kfree for conntrack extensions
From: Phil Oester @ 2013-09-11 17:09 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: netfilter-devel, netdev, Pablo Neira Ayuso, Patrick McHardy,
	Jozsef Kadlecsik
In-Reply-To: <20130911152804.GA5397@unicorn.suse.cz>

On Wed, Sep 11, 2013 at 05:28:05PM +0200, Michal Kubecek wrote:
> > Looking at
> > your proposed fix, the NAT extension data should have been cleaned
> > from the bysource list in nf_nat_cleanup_conntrack (via __nf_ct_ext_destroy)
> > before reaching the kfree.  Would you agree?
> 
> It is cleaned from the list but as it is an RCU list, other readers can
> still be holding pointers to it. We have to wait for the RCU grace
> period before we can reuse it.

Agreed - looks like your fix should work.  However, two nits:

1) normally RCU functions have _rcu suffixes.  So nf_ct_ext_free should
become nf_ct_ext_free_rcu.

2) kfree_rcu was not added to the kernel until 3.0.  All of the bug
reports I've been looking into (including the original in netfilter bugzilla
at http://bugzilla.netfilter.org/show_bug.cgi?id=714) have been reported in
2.6.32 or earlier kernels.  So a different fix would need to be backported for
-stable.  For that, we would probably export __nf_ct_ext_free_rcu from
nf_conntrack_extend.c and change the kfree call in nf_ct_ext_free_rcu to
call_rcu(&ct->ext->rcu, __nf_ct_ext_free_rcu). Of course the alternative
is just to use this fix for both old and new kernels for simplicity.

> No, it is a bugreport from our customer. And even that customer
> encountered it only once so far. Which is not very surprising as to
> reproduce it, you have to be (un)lucky twice: first to have someone
> overwrite the area soon enough and second to have someone access the
> area after it is overwritten.

Yes, hitting this seems dependent upon phase of the moon.

Phil

^ permalink raw reply

* [v3.11][Regression][Resend] skge: add dma_mapping check
From: Joseph Salisbury @ 2013-09-11 16:42 UTC (permalink / raw)
  To: stephen; +Cc: davem@davemloft.net, mlindner, netdev@vger.kernel.org,
	linux-kernel
In-Reply-To: <521E1552.4010100@canonical.com>

> Hi Stephen,
>
> A bug was opened against the Ubuntu kernel[0].  A bug has also been
> opened in bugzilla[1].  After a kernel bisect, it was found that
> reverting the following commit resolved this bug:
>
> commit 136d8f377e1575463b47840bc5f1b22d94bf8f63
> Author: stephen hemminger <stephen@networkplumber.org>
> Date:   Sun Aug 4 17:22:34 2013 -0700
>
>     skge: add dma_mapping check
>
>
> The regression was introduced as of v3.11-rc6.
>
> I see that you are the author of this patch, so I wanted to run this by
> you.  I was thinking of requesting a revert for v3.11, but I wanted to
> get your feedback first.
>
>
> Thanks,
>
> Joe
>
> [0] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1216745
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=60784

^ permalink raw reply

* Re: 3.10.0 network trace
From: Yuchung Cheng @ 2013-09-11 16:32 UTC (permalink / raw)
  To: Josh Boyer; +Cc: Michael Sterrett, netdev, Neal Cardwell, Nandita Dukkipati
In-Reply-To: <CA+5PVA5bDAz5a-TrM4ALA9vJz2JbE14VT7xM5=WqaRg=Ro0_xQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

On Wed, Sep 11, 2013 at 5:49 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> On Sun, Jul 21, 2013 at 9:36 PM, Michael Sterrett <michael@sterretts.net> wrote:
>> Since upgraded to 3.10.1 which exhibits the same issue (probably not
>> surprising).
>>
>> sysctl net.ipv4.tcp_frto=0 net.ipv4.tcp_early_retrans=3 seems to take
>> care of it.
>
> We're still getting reports of this with 3.10.10 in Fedora [1].  At
> least one of the reporters has said those sysctl settings didn't help.
>
> Is there anything else that can be gathered to help track this down?
since frto=0 didn't help, I suspect it's either a bug introduced in
the tail loss probe patch 9b717a8d (tcp: TLP loss detection) or other
changes related loss recovery.

could you try disable TLP by
sysctl net.ipv4.tcp_frto=0 net.ipv4.tcp_early_retrans=1

if it still does not work. disable any form of early retransmit by
sysctl net.ipv4.tcp_frto=0 net.ipv4.tcp_early_retrans=0

if you can test a custom build kernel, please try this debugging patch.

>
> josh
>
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=989251

[-- Attachment #2: 0001-tcp-debug-fastretrans-warning.patch --]
[-- Type: application/octet-stream, Size: 1525 bytes --]

From 62561c91d4655539781113e0fa04ee681b22a2a4 Mon Sep 17 00:00:00 2001
From: Yuchung Cheng <ycheng@google.com>
Date: Wed, 31 Jul 2013 09:22:35 -0700
Subject: [PATCH] tcp: debug fastretrans warning

printk a ton of states to debug fastretrans warning

Signed-off-by: Yuchung Cheng <ycheng@google.com>
---
 net/ipv4/tcp_input.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9c62257..c18dab1 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2773,7 +2773,20 @@ static void tcp_fastretrans_alert(struct sock *sk, int pkts_acked,
 	/* D. Check state exit conditions. State can be terminated
 	 *    when high_seq is ACKed. */
 	if (icsk->icsk_ca_state == TCP_CA_Open) {
-		WARN_ON(tp->retrans_out != 0);
+		if (WARN_ON(tp->retrans_out != 0)) {
+			printk(KERN_DEBUG "%pI4:%u F0x%x S%u s%d IF%u+%u-%u-%u"
+			       "f%u ur%u rr%u rt%u um%u hs%u nxt%u\n",
+			       &inet_sk(sk)->inet_daddr,
+			       ntohs(inet_sk(sk)->inet_dport),
+			       flag, sk->sk_state, tp->rx_opt.sack_ok,
+			       tp->packets_out, tp->retrans_out,
+			       tp->sacked_out, tp->lost_out,
+			       tp->frto, tp->undo_retrans,
+			       tp->reordering, icsk->icsk_retransmits,
+			       tp->undo_marker ? tp->undo_marker-tp->snd_una:0,
+			       tp->high_seq - tp->snd_una,
+			       tp->snd_nxt - tp->snd_una);
+		}
 		tp->retrans_stamp = 0;
 	} else if (!before(tp->snd_una, tp->high_seq)) {
 		switch (icsk->icsk_ca_state) {
-- 
1.8.3


^ permalink raw reply related

* Re: [PATCH net 2/4] bridge: Handle priority-tagged frames properly
From: Vlad Yasevich @ 2013-09-11 16:32 UTC (permalink / raw)
  To: Toshiaki Makita; +Cc: David S. Miller, netdev
In-Reply-To: <1378882832.3495.12.camel@ubuntu-vm-makita>

On 09/11/2013 03:00 AM, Toshiaki Makita wrote:
> On Tue, 2013-09-10 at 10:03 -0400, Vlad Yasevich wrote:
>> On 09/10/2013 06:34 AM, Toshiaki Makita wrote:
>>> IEEE 802.1Q says that when we receive priority-tagged (VID 0) frames
>>> use the PVID for the port as its VID.
>>> (See IEEE 802.1Q-2005 6.7.1 and Table 9-2)
>>>
>>> Apply the PVID to not only untagged frames but also priority-tagged frames.
>>>
>>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>>> ---
>>>    net/bridge/br_vlan.c | 27 ++++++++++++++++++++-------
>>>    1 file changed, 20 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
>>> index 21b6d21..5a9c44a 100644
>>> --- a/net/bridge/br_vlan.c
>>> +++ b/net/bridge/br_vlan.c
>>> @@ -189,6 +189,8 @@ out:
>>>    bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
>>>    			struct sk_buff *skb, u16 *vid)
>>>    {
>>> +	int err;
>>> +
>>>    	/* If VLAN filtering is disabled on the bridge, all packets are
>>>    	 * permitted.
>>>    	 */
>>> @@ -201,20 +203,31 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
>>>    	if (!v)
>>>    		return false;
>>>
>>> -	if (br_vlan_get_tag(skb, vid)) {
>>> +	err = br_vlan_get_tag(skb, vid);
>>> +	if (!*vid) {
>>>    		u16 pvid = br_get_pvid(v);
>>>
>>> -		/* Frame did not have a tag.  See if pvid is set
>>> -		 * on this port.  That tells us which vlan untagged
>>> -		 * traffic belongs to.
>>> +		/* Frame had a tag with VID 0 or did not have a tag.
>>> +		 * See if pvid is set on this port.  That tells us which
>>> +		 * vlan untagged or priority-tagged traffic belongs to.
>>>    		 */
>>>    		if (pvid == VLAN_N_VID)
>>>    			return false;
>>>
>>> -		/* PVID is set on this port.  Any untagged ingress
>>> -		 * frame is considered to belong to this vlan.
>>> +		/* PVID is set on this port.  Any untagged or priority-tagged
>>> +		 * ingress frame is considered to belong to this vlan.
>>>    		 */
>>> -		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), pvid);
>>> +		if (likely(err))
>>> +			/* Untagged Frame. */
>>> +			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), pvid);
>>> +		else
>>> +			/* Priority-tagged Frame.
>>> +			 * At this point, We know that skb->vlan_tci had
>>> +			 * VLAN_TAG_PRESENT bit and its VID field was 0x000.
>>> +			 * We update only VID field and preserve PCP field.
>>> +			 */
>>> +			skb->vlan_tci |= pvid;
>>> +
>>
>> In the case of a priority tagged frame, we should unroll the
>> modification above and restore the VID field to 0.  Otherwise, you
>> may end up either stripping the vlan header completely or forwarding
>> with pvid of the ingress port.
>
> Thank you for reviewing.
>
> It is my intended behavior that an incoming priority-tagged frame is
> forwarded as a frame untagged or tagged with pvid.
>
> IEEE 802.1Q-2011:
>
>    section 8.1.7 Conversion of frame formats
>
>    NOTE - As all incoming frames, including priority-tagged frames, are
>    classified as belonging to a VLAN, the transmitting Port transmits
>    VLAN-tagged frames or untagged frames. Hence, a station sending a
>    priority-tagged frame via a Bridge will receive a response that is
>    either VLAN-tagged or untagged, as described in 8.5.
>
>    3. Definitions
>
>    3.132 Priority-tagged frame: A tagged frame whose tag header carries
>    priority information but carries no VLAN identification information.
>
>    3.203 VLAN-tagged frame: A VLAN-tagged frame is a tagged frame whose
>    tag header carries *both* VLAN identification and priority
>    information.
>
> Toshiaki Makita
>

Hmm..  The problem is that if a system attached to a port configures a
vlan interface with vid 0 and some priority mappings, then that
interface will not be able to properly receive traffic, as the bridge 
now will never transmit priority tagged frames.

-vlad

>>
>> -vlad
>>>    		return true;
>>>    	}
>>>
>>>
>
>

^ permalink raw reply

* Re: [PATCH] ppc: bpf_jit: support MOD operation
From: Vladimir Murzin @ 2013-09-11 16:15 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Benjamin Herrenschmidt, linuxppc-dev, paulus, davem, Matt Evans,
	netdev
In-Reply-To: <5226DB64.3020207@redhat.com>

On Wed, Sep 04, 2013 at 09:04:04AM +0200, Daniel Borkmann wrote:
> On 09/03/2013 10:52 PM, Daniel Borkmann wrote:
> > On 09/03/2013 09:58 PM, Vladimir Murzin wrote:
> [...]
> >>> Do you have a test case/suite by any chance ?
> >>>
> >>> Ben.
> >>>
> >>
> >> Hi Ben!
> >>
> >> Thanks for your feedback.
> >>
> >> This patch is only compile tested. I have no real hardware, but I'll
> >> probably bring up qemu ppc64 till end of the week...
> >> Meanwhile, I've made simple how-to for testing. You can use it if you wish.
> >> It is mainly based on the [1] and rechecked on x86-64.

Finally I've managed to bring up qemu ppc64 and done simple testing. As a
result I could see difference in opcodes for divide instruction - I've just
sent the patch for that.

WRT mod instruction result is:

For BPF program

(000) ldh      [12]
(001) jeq      #0x800           jt 2	jf 10
(002) ldh      [16]
(003) sub      #20
(004) mod      #5
(005) jeq      #0x0             jt 10	jf 6
(006) ldb      [20]
(007) and      #0x20
(008) jeq      #0x20            jt 9	jf 10
(009) ret      #65535
(010) ret      #0

The following code is generated (with patch divw to divwu applied)

244 bytes emitted from JIT compiler (pass:3, flen:11)
d0000000015c0018 + <x>:
   0:	mflr    r0
   4:	std     r0,16(r1)
   8:	std     r14,-144(r1)
   c:	std     r15,-136(r1)
  10:	stdu    r1,-288(r1)
  14:	lwz     r7,108(r3)
  18:	lwz     r15,104(r3)
  1c:	subf    r15,r7,r15
  20:	ld      r14,216(r3)
  24:	lis     r7,-16384
  28:	rldicr  r7,r7,32,31
  2c:	oris    r7,r7,9
  30:	ori     r7,r7,43428
  34:	mtlr    r7
  38:	li      r6,12
  3c:	blrl
  40:	blt-    0x00000000000000dc
  44:	nop
  48:	cmplwi  r4,2048
  4c:	bne-    0x00000000000000d8
  50:	nop
  54:	lis     r7,-16384
  58:	rldicr  r7,r7,32,31
  5c:	oris    r7,r7,9
  60:	ori     r7,r7,43428
  64:	mtlr    r7
  68:	li      r6,16
  6c:	blrl
  70:	blt-    0x00000000000000dc
  74:	nop
  78:	addi    r4,r4,-20
  7c:	li      r8,5
  80:	divwu   r7,r4,r8
  84:	mullw   r7,r8,r7
  88:	subf    r4,r7,r4
  8c:	cmplwi  r4,0
  90:	beq-    0x00000000000000d8
  94:	nop
  98:	lis     r7,-16384
  9c:	rldicr  r7,r7,32,31
  a0:	oris    r7,r7,9
  a4:	ori     r7,r7,43456
  a8:	mtlr    r7
  ac:	li      r6,20
  b0:	blrl
  b4:	blt-    0x00000000000000dc
  b8:	nop
  bc:	andi.   r4,r4,32
  c0:	cmplwi  r4,32
  c4:	bne-    0x00000000000000d8
  c8:	nop
  cc:	li      r3,-1
  d0:	addis   r3,r3,1
  d4:	b       0x00000000000000dc
  d8:	li      r3,0
  dc:	addi    r1,r1,288
  e0:	ld      r0,16(r1)
  e4:	mtlr    r0
  e8:	ld      r14,-144(r1)
  ec:	ld      r15,-136(r1)
  f0:	blr

Raw codes are

flen=11 proglen=244 pass=3 image=d0000000015c0018
JIT code: 00000000: 7c 08 02 a6 f8 01 00 10 f9 c1 ff 70 f9 e1 ff 78
JIT code: 00000010: f8 21 fe e1 80 e3 00 6c 81 e3 00 68 7d e7 78 50
JIT code: 00000020: e9 c3 00 d8 3c e0 c0 00 78 e7 07 c6 64 e7 00 09
JIT code: 00000030: 60 e7 a9 a4 7c e8 03 a6 38 c0 00 0c 4e 80 00 21
JIT code: 00000040: 41 80 00 9c 60 00 00 00 28 04 08 00 40 82 00 8c
JIT code: 00000050: 60 00 00 00 3c e0 c0 00 78 e7 07 c6 64 e7 00 09
JIT code: 00000060: 60 e7 a9 a4 7c e8 03 a6 38 c0 00 10 4e 80 00 21
JIT code: 00000070: 41 80 00 6c 60 00 00 00 38 84 ff ec 39 00 00 05
JIT code: 00000080: 7c e4 43 96 7c e8 39 d6 7c 87 20 50 28 04 00 00
JIT code: 00000090: 41 82 00 48 60 00 00 00 3c e0 c0 00 78 e7 07 c6
JIT code: 000000a0: 64 e7 00 09 60 e7 a9 c0 7c e8 03 a6 38 c0 00 14
JIT code: 000000b0: 4e 80 00 21 41 80 00 28 60 00 00 00 70 84 00 20
JIT code: 000000c0: 28 04 00 20 40 82 00 14 60 00 00 00 38 60 ff ff
JIT code: 000000d0: 3c 63 00 01 48 00 00 08 38 60 00 00 38 21 01 20
JIT code: 000000e0: e8 01 00 10 7c 08 03 a6 e9 c1 ff 70 e9 e1 ff 78
JIT code: 000000f0: 4e 80 00 20

Ben,

How do you feel about it?

> >
> > Please also cc netdev on BPF related changes.
> >
> > Actually, your test plan can be further simplified ...
> >
> > For retrieving and disassembling the JIT image, we have bpf_jit_disasm [1].
> >
> >   1) echo 2 > /proc/sys/net/core/bpf_jit_enable
> >   2) ... attach filter ...
> >   3) bpf_jit_disasm -o
> >
> > For generating a simple stupid test filter, you can use bpfc [2] (also
> > see its man page). E.g. ...
> >
> >    # cat blub
> >    ldi #10
> >    mod #8
> >    ret a
> >    # bpfc blub
> >    { 0x0, 0, 0, 0x0000000a },
> >    { 0x94, 0, 0, 0x00000008 },
> >    { 0x16, 0, 0, 0x00000000 },
> 
> Plus something like ...
> 
> ldxi #0
> mod x
> ret a
> 

Thanks Daniel!

Unfortunately, I couldn't trigger JIT compiler with the pair bpfc/netsniff-ng
(even for x86-64). I guess I missed something. I'd be very grateful if you
point at my mistakes.

> For longer-term testing, also trinity has BPF support. ;)
> 

Wow! Could do give some hint how to run this for BPF only?

> > And load this array e.g. either into a small C program that attaches this
> > as BPF filter, or simply do bpfc blub > blub2 and run netsniff-ng -f blub2\
> > -s -i eth0, that should also do it.
> >
> > Then, when attached, the kernel should truncate incoming frames for pf_packet
> > into max length of 2, just as an example.
> >
> >    [1] kernel tree, tools/net/bpf_jit_disasm.c
> >    [2] git clone git://github.com/borkmann/netsniff-ng.git

Thanks
Vladimir

^ permalink raw reply

* hi
From: ZHAO Gang @ 2013-09-11 16:06 UTC (permalink / raw)
  To: netdev

subscribe netdev

^ permalink raw reply

* RE: usbnet transmit path problems
From: David Laight @ 2013-09-11 16:05 UTC (permalink / raw)
  To: Ming Lei; +Cc: Oliver Neukum, Network Development, linux-usb
In-Reply-To: <CACVXFVN6ZHLesrsMVNMWrikRs1mMbk=aZD9qZybNn1gB7aFTZQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

> On Wed, Sep 11, 2013 at 8:56 PM, David Laight <David.Laight-JxhZ9S5GRejQT0dZR+AlfA@public.gmane.org> wrote:
> >> > > 2) If 'length % dev->maxpacket == 0' for a multi-fragment packet then
> >> > >    the extra byte isn't added correctly (the code probably falls off
> >> > >    the end of the scatter-gather list).
> >> >
> >> > Indeed. Ming Lei, should usbnet handle this in the sg case or better
> >> > leave it to the subdriver you introduced this for?
> >
> > Is the ZLP issue a problem with the host or with the target?
> 
> Sorry, what do you mean the ZLP issue here? I understand Oliver
> thinks one commit from me may break ZLP handling, are you discussing
> this problem? If not, could you explain it in a bit detail?

I was thinking of the general ZLP problem.
 
> > If it is a host problem then the necessity comes from the host,
> > but the fix needs to be target dependant.
> > If it is a common target problem then generic code can apply
> > a common fix.
> 
> All usbnet device should have sent one ZLP in case the size of
> bulk out transfer can be divided by max packet size, but the one
> byte transfer might be introduced for avoiding some target problem
> (can't deal with zlp well), as said by David, see below discussion:
> 
>    http://marc.info/?l=linux-usb&m=127067487604112&w=2

AFAICT the code avoids sending a zero length packet (that would
terminate a USB bulk transfer packet) by increasing the length
of the bulk packet by (at least) one byte.

> > AFICT there are at least 3 fixes:
> > 1) Extend the ethernet frame by one byte and hope the receiving
> >    system doesn't object to the padding.
> >    This is probably the only option if tx_fixup() doesn't
> >    add a header.
> > 2) Put the ethernet frame length in the header and have the
> >    target discard the added pad byte (ax88179_178a.c).
> > 3) Add a second zero-length frame in the same USB data block
> >    (ax88172a.c).
> 
> Why do we need the above 3 fixes? The patch in my last email can
> fix the problem which is introduced recently, can't it?

I meant there are 3 ways of avoiding the ZLP, each driver will
pick one of them.

I've just looked at all the drivers in net/usb.
It doesn't look like they all handle fragmented skb, shared skb,
or ZLP properly.

A lot of common code could be removed if usbnet knew the size of the
header and allocated it before calling tx_fixup().

None of this is helping me sort out why netperf udp rr tests with
burst 19 are losing all the packets at once :-(

	David



--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [net  8/8] i40e: include i40e in kernel proper
From: Jeff Kirsher @ 2013-09-11 15:37 UTC (permalink / raw)
  To: Waskiewicz Jr, Peter P
  Cc: Joe Perches, davem@davemloft.net, Brandeburg, Jesse,
	netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com,
	Nelson, Shannon, e1000-devel@lists.sourceforge.net
In-Reply-To: <1378913071.3863.27.camel@ppwaskie-mobl2>

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

On Wed, 2013-09-11 at 08:24 -0700, Waskiewicz Jr, Peter P wrote:
> On Wed, 2013-09-11 at 05:20 -0700, Joe Perches wrote:
> > On Wed, 2013-09-11 at 02:50 -0700, Jeff Kirsher wrote:
> > > New driver build option is CONFIG_I40E
> > 
> > > diff --git a/Documentation/networking/00-INDEX b/Documentation/networking/00-INDEX
> > []
> > > +i40e.txt
> > > +	- README for the Intel Ethernet Controller XL710 Driver (i40e).
> > 
> > Just curious but why the XL710 / i40e name mismatch?
> 
> ixgbe is a good example.  ixgbe is the driver name, but it supports the
> 82598, 82599 / X520, and X540 chips.
> 

Another good example is e100, e1000, e1000e, and igb. :-)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [net  8/8] i40e: include i40e in kernel proper
From: Joe Perches @ 2013-09-11 15:31 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem, Jesse Brandeburg, netdev, gospo, sassmann, Shannon Nelson,
	PJ Waskiewicz, e1000-devel
In-Reply-To: <1378909937.2026.8.camel@jtkirshe-mobl>

On Wed, 2013-09-11 at 07:32 -0700, Jeff Kirsher wrote:
> On Wed, 2013-09-11 at 05:20 -0700, Joe Perches wrote:
> > On Wed, 2013-09-11 at 02:50 -0700, Jeff Kirsher wrote:
> > > New driver build option is CONFIG_I40E
> > 
> > > diff --git a/Documentation/networking/00-INDEX b/Documentation/networking/00-INDEX
> > []
> > > +i40e.txt
> > > +	- README for the Intel Ethernet Controller XL710 Driver (i40e).
> > 
> > Just curious but why the XL710 / i40e name mismatch?
> 
> i40e stands for Intel 40 GbE Ethernet which is more generic than naming
> the driver the same as the first part (XL710).  That way when future
> silicon is made, we are not stuck with a driver named after previous
> silicon.

>From the intro and most of the rest of the patches:

----------------------------
This series implements the new i40e driver for Intel's upcoming
Intel(R) Ethernet Controller XL710 Family of devices.
----------------------------

If the xl710 is a specific instance of the i40e family,
I think this is incorrect.

^ permalink raw reply

* Re: [PATCH net v2] net: sctp: fix ipv6 ipsec encryption bug in sctp_v6_xmit
From: Vlad Yasevich @ 2013-09-11 15:30 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: davem, netdev, linux-sctp, adobriyan, Steffen Klassert,
	Hannes Frederic Sowa
In-Reply-To: <1378911516-15942-1-git-send-email-dborkman@redhat.com>

On 09/11/2013 10:58 AM, Daniel Borkmann wrote:
> Alan Chester reported an issue with IPv6 on SCTP that IPsec traffic is not
> being encrypted, whereas on IPv4 it is. Setting up an AH + ESP transport
> does not seem to have the desired effect:
>
> SCTP + IPv4:
>
>    22:14:20.809645 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto AH (51), length 116)
>      192.168.0.2 > 192.168.0.5: AH(spi=0x00000042,sumlen=16,seq=0x1): ESP(spi=0x00000044,seq=0x1), length 72
>    22:14:20.813270 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto AH (51), length 340)
>      192.168.0.5 > 192.168.0.2: AH(spi=0x00000043,sumlen=16,seq=0x1):
>
> SCTP + IPv6:
>
>    22:31:19.215029 IP6 (class 0x02, hlim 64, next-header SCTP (132) payload length: 364)
>      fe80::222:15ff:fe87:7fc.3333 > fe80::92e6:baff:fe0d:5a54.36767: sctp
>      1) [INIT ACK] [init tag: 747759530] [rwnd: 62464] [OS: 10] [MIS: 10]
>
> Moreover, Alan says:
>
>    This problem was seen with both Racoon and Racoon2. Other people have seen
>    this with OpenSwan. When IPsec is configured to encrypt all upper layer
>    protocols the SCTP connection does not initialize. After using Wireshark to
>    follow packets, this is because the SCTP packet leaves Box A unencrypted and
>    Box B believes all upper layer protocols are to be encrypted so it drops
>    this packet, causing the SCTP connection to fail to initialize. When IPsec
>    is configured to encrypt just SCTP, the SCTP packets are observed unencrypted.
>
> In fact, using `socat sctp6-listen:3333 -` on one end and transferring "plaintext"
> string on the other end, results in cleartext on the wire where SCTP eventually
> does not report any errors, thus in the latter case that Alan reports, the
> non-paranoid user might think he's communicating over an encrypted transport on
> SCTP although he's not (tcpdump ... -X):
>
>    ...
>    0x0030: 5d70 8e1a 0003 001a 177d eb6c 0000 0000  ]p.......}.l....
>    0x0040: 0000 0000 706c 6169 6e74 6578 740a 0000  ....plaintext...
>
> Only in /proc/net/xfrm_stat we can see XfrmInTmplMismatch increasing on the
> receiver side. Initial follow-up analysis from Alan's bug report was done by
> Alexey Dobriyan. Also thanks to Vlad Yasevich for feedback on this.
>
> SCTP has its own implementation of sctp_v6_xmit() not calling inet6_csk_xmit().
> This has the implication that it probably never really got updated along with
> changes in inet6_csk_xmit() and therefore does not seem to invoke xfrm handlers.
>
> SCTP's IPv4 xmit however, properly calls ip_queue_xmit() to do the work. Since
> a call to inet6_csk_xmit() would solve this problem, but result in unecessary
> route lookups, let us just use the cached flowi6 instead that we got through
> sctp_v6_get_dst(). Since all SCTP packets are being sent through sctp_packet_transmit(),
> we do the route lookup / flow caching in sctp_transport_route(), hold it in
> tp->dst and skb_dst_set() right after that. If we would alter fl6->daddr in
> sctp_v6_xmit() to np->opt->srcrt, we possibly could run into the same effect
> of not having xfrm layer pick it up, hence, use fl6_update_dst() in sctp_v6_get_dst()
> instead to get the correct source routed dst entry, which we assign to the skb.
>
> Also source address routing example from 625034113 ("sctp: fix sctp to work with
> ipv6 source address routing") still works with this patch! Nevertheless, in RFC5095
> it is actually 'recommended' to not use that anyway due to traffic amplification [1].
> So it seems we're not supposed to do that anyway in sctp_v6_xmit(). Moreover, if
> we overwrite the flow destination here, the lower IPv6 layer will be unable to
> put the correct destination address into IP header, as routing header is added in
> ipv6_push_nfrag_opts() but then probably with wrong final destination. Things aside,
> result of this patch is that we do not have any XfrmInTmplMismatch increase plus on
> the wire with this patch it now looks like:
>
> SCTP + IPv6:
>
>    08:17:47.074080 IP6 2620:52:0:102f:7a2b:cbff:fe27:1b0a > 2620:52:0:102f:213:72ff:fe32:7eba:
>      AH(spi=0x00005fb4,seq=0x1): ESP(spi=0x00005fb5,seq=0x1), length 72
>    08:17:47.074264 IP6 2620:52:0:102f:213:72ff:fe32:7eba > 2620:52:0:102f:7a2b:cbff:fe27:1b0a:
>      AH(spi=0x00003d54,seq=0x1): ESP(spi=0x00003d55,seq=0x1), length 296
>
> This fixes Kernel Bugzilla 24412. This security issue seems to be present since
> 2.6.18 kernels. Lets just hope some big passive adversary in the wild didn't have
> its fun with that. lksctp-tools IPv6 regression test suite passes as well with
> this patch.
>
>   [1] http://www.secdev.org/conf/IPv6_RH_security-csw07.pdf
>
> Reported-by: Alan Chester <alan.chester@tekelec.com>
> Reported-by: Alexey Dobriyan <adobriyan@gmail.com>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Steffen Klassert <steffen.klassert@secunet.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>

Acked-by: Vlad Yasevich <vyasevich@gmail.com>

-vlad

> ---
>   v1->v2:
>    - use cached flow
>    - improved commit msg
>
>   net/sctp/ipv6.c | 42 +++++++++++++-----------------------------
>   1 file changed, 13 insertions(+), 29 deletions(-)
>
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index da613ce..4f52e2c 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -204,44 +204,23 @@ out:
>   		in6_dev_put(idev);
>   }
>
> -/* Based on tcp_v6_xmit() in tcp_ipv6.c. */
>   static int sctp_v6_xmit(struct sk_buff *skb, struct sctp_transport *transport)
>   {
>   	struct sock *sk = skb->sk;
>   	struct ipv6_pinfo *np = inet6_sk(sk);
> -	struct flowi6 fl6;
> -
> -	memset(&fl6, 0, sizeof(fl6));
> -
> -	fl6.flowi6_proto = sk->sk_protocol;
> -
> -	/* Fill in the dest address from the route entry passed with the skb
> -	 * and the source address from the transport.
> -	 */
> -	fl6.daddr = transport->ipaddr.v6.sin6_addr;
> -	fl6.saddr = transport->saddr.v6.sin6_addr;
> -
> -	fl6.flowlabel = np->flow_label;
> -	IP6_ECN_flow_xmit(sk, fl6.flowlabel);
> -	if (ipv6_addr_type(&fl6.saddr) & IPV6_ADDR_LINKLOCAL)
> -		fl6.flowi6_oif = transport->saddr.v6.sin6_scope_id;
> -	else
> -		fl6.flowi6_oif = sk->sk_bound_dev_if;
> -
> -	if (np->opt && np->opt->srcrt) {
> -		struct rt0_hdr *rt0 = (struct rt0_hdr *) np->opt->srcrt;
> -		fl6.daddr = *rt0->addr;
> -	}
> +	struct flowi6 *fl6 = &transport->fl.u.ip6;
>
>   	pr_debug("%s: skb:%p, len:%d, src:%pI6 dst:%pI6\n", __func__, skb,
> -		 skb->len, &fl6.saddr, &fl6.daddr);
> +		 skb->len, &fl6->saddr, &fl6->daddr);
>
> -	SCTP_INC_STATS(sock_net(sk), SCTP_MIB_OUTSCTPPACKS);
> +	IP6_ECN_flow_xmit(sk, fl6->flowlabel);
>
>   	if (!(transport->param_flags & SPP_PMTUD_ENABLE))
>   		skb->local_df = 1;
>
> -	return ip6_xmit(sk, skb, &fl6, np->opt, np->tclass);
> +	SCTP_INC_STATS(sock_net(sk), SCTP_MIB_OUTSCTPPACKS);
> +
> +	return ip6_xmit(sk, skb, fl6, np->opt, np->tclass);
>   }
>
>   /* Returns the dst cache entry for the given source and destination ip
> @@ -254,10 +233,12 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
>   	struct dst_entry *dst = NULL;
>   	struct flowi6 *fl6 = &fl->u.ip6;
>   	struct sctp_bind_addr *bp;
> +	struct ipv6_pinfo *np = inet6_sk(sk);
>   	struct sctp_sockaddr_entry *laddr;
>   	union sctp_addr *baddr = NULL;
>   	union sctp_addr *daddr = &t->ipaddr;
>   	union sctp_addr dst_saddr;
> +	struct in6_addr *final_p, final;
>   	__u8 matchlen = 0;
>   	__u8 bmatchlen;
>   	sctp_scope_t scope;
> @@ -281,7 +262,8 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
>   		pr_debug("src=%pI6 - ", &fl6->saddr);
>   	}
>
> -	dst = ip6_dst_lookup_flow(sk, fl6, NULL, false);
> +	final_p = fl6_update_dst(fl6, np->opt, &final);
> +	dst = ip6_dst_lookup_flow(sk, fl6, final_p, false);
>   	if (!asoc || saddr)
>   		goto out;
>
> @@ -333,10 +315,12 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
>   		}
>   	}
>   	rcu_read_unlock();
> +
>   	if (baddr) {
>   		fl6->saddr = baddr->v6.sin6_addr;
>   		fl6->fl6_sport = baddr->v6.sin6_port;
> -		dst = ip6_dst_lookup_flow(sk, fl6, NULL, false);
> +		final_p = fl6_update_dst(fl6, np->opt, &final);
> +		dst = ip6_dst_lookup_flow(sk, fl6, final_p, false);
>   	}
>
>   out:
>

^ permalink raw reply

* Re: [PATCH nf] netfilter: use RCU safe kfree for conntrack extensions
From: Michal Kubecek @ 2013-09-11 15:28 UTC (permalink / raw)
  To: Phil Oester
  Cc: netfilter-devel, netdev, Pablo Neira Ayuso, Patrick McHardy,
	Jozsef Kadlecsik, coreteam
In-Reply-To: <20130911145715.GA2882@linuxace.com>

On Wed, Sep 11, 2013 at 07:57:15AM -0700, Phil Oester wrote:
> On Wed, Sep 11, 2013 at 10:17:27AM +0200, Michal Kubecek wrote:
> > Commit 68b80f11 (netfilter: nf_nat: fix RCU races) introduced
> > RCU protection for freeing extension data when reallocation
> > moves them to a new location. We need the same protection when
> > freeing them in nf_ct_ext_free() in order to prevent a
> > use-after-free by other threads referencing a NAT extension data
> > via bysource list.
> 
> Hi Michal - 
> 
> coincidentally I've been looking into this area this week due to another
> bug report (https://bugzilla.kernel.org/show_bug.cgi?id=60853).

Looking at the initial command, I would say this bug report is actually
of the same origin as ours.

> Looking at
> your proposed fix, the NAT extension data should have been cleaned
> from the bysource list in nf_nat_cleanup_conntrack (via __nf_ct_ext_destroy)
> before reaching the kfree.  Would you agree?

It is cleaned from the list but as it is an RCU list, other readers can
still be holding pointers to it. We have to wait for the RCU grace
period before we can reuse it.

> The reporter of #60853 suggested adding a synchronize_rcu to the end of the
> nf_nat_cleanup_conntrack function, which seems sane.

That was also my first idea. However, nf_nat_cleanup_conntrack() is
called from __nf_ct_ext_destroy() inside a rcu_read_lock() /
rcu_read_unlock() block. Even if this block is for a different RCU list,
we still cannot call synchronize_rcu() while inside it.

We could call synchronize_rcu() in __nf_ct_ext_destroy() after
rcu_read_unlock() but this would IMHO add an unnecessary delay so it is
more efficient and more appropriate to wait before the actual kfree()
which is the operation that needs to wait for RCU grace period.

> I have been trying to reproduce the crash to test that theory.
> Are you able to reproduce an OOPS in your testing?  Or is there a bug
> report you are working from?

No, it is a bugreport from our customer. And even that customer
encountered it only once so far. Which is not very surprising as to
reproduce it, you have to be (un)lucky twice: first to have someone
overwrite the area soon enough and second to have someone access the
area after it is overwritten. This is IMHO the reason why the reporter
cleared the block with memset() for testing purposes.

                                                     Michal Kubecek


^ permalink raw reply

* Re: [net  8/8] i40e: include i40e in kernel proper
From: Jeff Kirsher @ 2013-09-11 15:25 UTC (permalink / raw)
  To: Joe Perches
  Cc: davem, Jesse Brandeburg, netdev, gospo, sassmann, Shannon Nelson,
	PJ Waskiewicz, e1000-devel
In-Reply-To: <1378910832.4714.2.camel@joe-AO722>

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

On Wed, 2013-09-11 at 07:47 -0700, Joe Perches wrote:
> On Wed, 2013-09-11 at 07:32 -0700, Jeff Kirsher wrote:
> > On Wed, 2013-09-11 at 05:20 -0700, Joe Perches wrote:
> > > On Wed, 2013-09-11 at 02:50 -0700, Jeff Kirsher wrote:
> > > > New driver build option is CONFIG_I40E
> > > 
> > > > diff --git a/Documentation/networking/00-INDEX b/Documentation/networking/00-INDEX
> > > []
> > > > +i40e.txt
> > > > +	- README for the Intel Ethernet Controller XL710 Driver (i40e).
> > > 
> > > Just curious but why the XL710 / i40e name mismatch?
> > 
> > i40e stands for Intel 40 GbE Ethernet which is more generic than naming
> > the driver the same as the first part (XL710).
> 
> Too bad if also means that when 100 comes out
> this driver won't be expected to work.

It will work for our 40GbE parts definitely, as far as future technology
and silicon, I cannot say.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [net  8/8] i40e: include i40e in kernel proper
From: Waskiewicz Jr, Peter P @ 2013-09-11 15:24 UTC (permalink / raw)
  To: Joe Perches
  Cc: Kirsher, Jeffrey T, davem@davemloft.net, Brandeburg, Jesse,
	netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com,
	Nelson, Shannon, e1000-devel@lists.sourceforge.net
In-Reply-To: <1378902002.606.42.camel@joe-AO722>

On Wed, 2013-09-11 at 05:20 -0700, Joe Perches wrote:
> On Wed, 2013-09-11 at 02:50 -0700, Jeff Kirsher wrote:
> > New driver build option is CONFIG_I40E
> 
> > diff --git a/Documentation/networking/00-INDEX b/Documentation/networking/00-INDEX
> []
> > +i40e.txt
> > +	- README for the Intel Ethernet Controller XL710 Driver (i40e).
> 
> Just curious but why the XL710 / i40e name mismatch?

ixgbe is a good example.  ixgbe is the driver name, but it supports the
82598, 82599 / X520, and X540 chips.

Cheers,
-PJ

^ permalink raw reply

* Re: usbnet transmit path problems
From: Ming Lei @ 2013-09-11 15:11 UTC (permalink / raw)
  To: David Laight; +Cc: Oliver Neukum, Network Development, linux-usb
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7326@saturn3.aculab.com>

On Wed, Sep 11, 2013 at 8:56 PM, David Laight <David.Laight@aculab.com> wrote:
>> > > 2) If 'length % dev->maxpacket == 0' for a multi-fragment packet then
>> > >    the extra byte isn't added correctly (the code probably falls off
>> > >    the end of the scatter-gather list).
>> >
>> > Indeed. Ming Lei, should usbnet handle this in the sg case or better
>> > leave it to the subdriver you introduced this for?
>
> Is the ZLP issue a problem with the host or with the target?

Sorry, what do you mean the ZLP issue here? I understand Oliver
thinks one commit from me may break ZLP handling, are you discussing
this problem? If not, could you explain it in a bit detail?

> If it is a host problem then the necessity comes from the host,
> but the fix needs to be target dependant.
> If it is a common target problem then generic code can apply
> a common fix.

All usbnet device should have sent one ZLP in case the size of
bulk out transfer can be divided by max packet size, but the one
byte transfer might be introduced for avoiding some target problem
(can't deal with zlp well), as said by David, see below discussion:

   http://marc.info/?l=linux-usb&m=127067487604112&w=2

>
> AFICT there are at least 3 fixes:
> 1) Extend the ethernet frame by one byte and hope the receiving
>    system doesn't object to the padding.
>    This is probably the only option if tx_fixup() doesn't
>    add a header.
> 2) Put the ethernet frame length in the header and have the
>    target discard the added pad byte (ax88179_178a.c).
> 3) Add a second zero-length frame in the same USB data block
>    (ax88172a.c).

Why do we need the above 3 fixes? The patch in my last email can
fix the problem which is introduced recently, can't it?

>
> Only the third requires that tx_fixup() append to the packet.
> For the other two actual pad can be added by usbnet.
>
>> IMO, it should be handled by usbnet, could you comment on below patch?
>
> Seems excessive to kmalloc() one byte!

It isn't strange, many usb drivers have to do that(maybe kmalloc
two or three, or four bytes) since the buffer is involved into DMA.

> If you can't assume that the 'dev' structure itself can be dma'd from
> allocate the extra byte in the sg list.

It is better to always obey rule of DMA-API, so don't do that since
one extra kmalloc() per device is needed, even though we can allocate
only one global buffer for this purpose.


Thanks,
--
Ming Lei

^ permalink raw reply

* [PATCH net v2] net: sctp: fix ipv6 ipsec encryption bug in sctp_v6_xmit
From: Daniel Borkmann @ 2013-09-11 14:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-sctp, adobriyan, Steffen Klassert,
	Hannes Frederic Sowa

Alan Chester reported an issue with IPv6 on SCTP that IPsec traffic is not
being encrypted, whereas on IPv4 it is. Setting up an AH + ESP transport
does not seem to have the desired effect:

SCTP + IPv4:

  22:14:20.809645 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto AH (51), length 116)
    192.168.0.2 > 192.168.0.5: AH(spi=0x00000042,sumlen=16,seq=0x1): ESP(spi=0x00000044,seq=0x1), length 72
  22:14:20.813270 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto AH (51), length 340)
    192.168.0.5 > 192.168.0.2: AH(spi=0x00000043,sumlen=16,seq=0x1):

SCTP + IPv6:

  22:31:19.215029 IP6 (class 0x02, hlim 64, next-header SCTP (132) payload length: 364)
    fe80::222:15ff:fe87:7fc.3333 > fe80::92e6:baff:fe0d:5a54.36767: sctp
    1) [INIT ACK] [init tag: 747759530] [rwnd: 62464] [OS: 10] [MIS: 10]

Moreover, Alan says:

  This problem was seen with both Racoon and Racoon2. Other people have seen
  this with OpenSwan. When IPsec is configured to encrypt all upper layer
  protocols the SCTP connection does not initialize. After using Wireshark to
  follow packets, this is because the SCTP packet leaves Box A unencrypted and
  Box B believes all upper layer protocols are to be encrypted so it drops
  this packet, causing the SCTP connection to fail to initialize. When IPsec
  is configured to encrypt just SCTP, the SCTP packets are observed unencrypted.

In fact, using `socat sctp6-listen:3333 -` on one end and transferring "plaintext"
string on the other end, results in cleartext on the wire where SCTP eventually
does not report any errors, thus in the latter case that Alan reports, the
non-paranoid user might think he's communicating over an encrypted transport on
SCTP although he's not (tcpdump ... -X):

  ...
  0x0030: 5d70 8e1a 0003 001a 177d eb6c 0000 0000  ]p.......}.l....
  0x0040: 0000 0000 706c 6169 6e74 6578 740a 0000  ....plaintext...

Only in /proc/net/xfrm_stat we can see XfrmInTmplMismatch increasing on the
receiver side. Initial follow-up analysis from Alan's bug report was done by
Alexey Dobriyan. Also thanks to Vlad Yasevich for feedback on this.

SCTP has its own implementation of sctp_v6_xmit() not calling inet6_csk_xmit().
This has the implication that it probably never really got updated along with
changes in inet6_csk_xmit() and therefore does not seem to invoke xfrm handlers.

SCTP's IPv4 xmit however, properly calls ip_queue_xmit() to do the work. Since
a call to inet6_csk_xmit() would solve this problem, but result in unecessary
route lookups, let us just use the cached flowi6 instead that we got through
sctp_v6_get_dst(). Since all SCTP packets are being sent through sctp_packet_transmit(),
we do the route lookup / flow caching in sctp_transport_route(), hold it in
tp->dst and skb_dst_set() right after that. If we would alter fl6->daddr in
sctp_v6_xmit() to np->opt->srcrt, we possibly could run into the same effect
of not having xfrm layer pick it up, hence, use fl6_update_dst() in sctp_v6_get_dst()
instead to get the correct source routed dst entry, which we assign to the skb.

Also source address routing example from 625034113 ("sctp: fix sctp to work with
ipv6 source address routing") still works with this patch! Nevertheless, in RFC5095
it is actually 'recommended' to not use that anyway due to traffic amplification [1].
So it seems we're not supposed to do that anyway in sctp_v6_xmit(). Moreover, if
we overwrite the flow destination here, the lower IPv6 layer will be unable to
put the correct destination address into IP header, as routing header is added in
ipv6_push_nfrag_opts() but then probably with wrong final destination. Things aside,
result of this patch is that we do not have any XfrmInTmplMismatch increase plus on
the wire with this patch it now looks like:

SCTP + IPv6:

  08:17:47.074080 IP6 2620:52:0:102f:7a2b:cbff:fe27:1b0a > 2620:52:0:102f:213:72ff:fe32:7eba:
    AH(spi=0x00005fb4,seq=0x1): ESP(spi=0x00005fb5,seq=0x1), length 72
  08:17:47.074264 IP6 2620:52:0:102f:213:72ff:fe32:7eba > 2620:52:0:102f:7a2b:cbff:fe27:1b0a:
    AH(spi=0x00003d54,seq=0x1): ESP(spi=0x00003d55,seq=0x1), length 296

This fixes Kernel Bugzilla 24412. This security issue seems to be present since
2.6.18 kernels. Lets just hope some big passive adversary in the wild didn't have
its fun with that. lksctp-tools IPv6 regression test suite passes as well with
this patch.

 [1] http://www.secdev.org/conf/IPv6_RH_security-csw07.pdf

Reported-by: Alan Chester <alan.chester@tekelec.com>
Reported-by: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 v1->v2:
  - use cached flow
  - improved commit msg

 net/sctp/ipv6.c | 42 +++++++++++++-----------------------------
 1 file changed, 13 insertions(+), 29 deletions(-)

diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index da613ce..4f52e2c 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -204,44 +204,23 @@ out:
 		in6_dev_put(idev);
 }
 
-/* Based on tcp_v6_xmit() in tcp_ipv6.c. */
 static int sctp_v6_xmit(struct sk_buff *skb, struct sctp_transport *transport)
 {
 	struct sock *sk = skb->sk;
 	struct ipv6_pinfo *np = inet6_sk(sk);
-	struct flowi6 fl6;
-
-	memset(&fl6, 0, sizeof(fl6));
-
-	fl6.flowi6_proto = sk->sk_protocol;
-
-	/* Fill in the dest address from the route entry passed with the skb
-	 * and the source address from the transport.
-	 */
-	fl6.daddr = transport->ipaddr.v6.sin6_addr;
-	fl6.saddr = transport->saddr.v6.sin6_addr;
-
-	fl6.flowlabel = np->flow_label;
-	IP6_ECN_flow_xmit(sk, fl6.flowlabel);
-	if (ipv6_addr_type(&fl6.saddr) & IPV6_ADDR_LINKLOCAL)
-		fl6.flowi6_oif = transport->saddr.v6.sin6_scope_id;
-	else
-		fl6.flowi6_oif = sk->sk_bound_dev_if;
-
-	if (np->opt && np->opt->srcrt) {
-		struct rt0_hdr *rt0 = (struct rt0_hdr *) np->opt->srcrt;
-		fl6.daddr = *rt0->addr;
-	}
+	struct flowi6 *fl6 = &transport->fl.u.ip6;
 
 	pr_debug("%s: skb:%p, len:%d, src:%pI6 dst:%pI6\n", __func__, skb,
-		 skb->len, &fl6.saddr, &fl6.daddr);
+		 skb->len, &fl6->saddr, &fl6->daddr);
 
-	SCTP_INC_STATS(sock_net(sk), SCTP_MIB_OUTSCTPPACKS);
+	IP6_ECN_flow_xmit(sk, fl6->flowlabel);
 
 	if (!(transport->param_flags & SPP_PMTUD_ENABLE))
 		skb->local_df = 1;
 
-	return ip6_xmit(sk, skb, &fl6, np->opt, np->tclass);
+	SCTP_INC_STATS(sock_net(sk), SCTP_MIB_OUTSCTPPACKS);
+
+	return ip6_xmit(sk, skb, fl6, np->opt, np->tclass);
 }
 
 /* Returns the dst cache entry for the given source and destination ip
@@ -254,10 +233,12 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 	struct dst_entry *dst = NULL;
 	struct flowi6 *fl6 = &fl->u.ip6;
 	struct sctp_bind_addr *bp;
+	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct sctp_sockaddr_entry *laddr;
 	union sctp_addr *baddr = NULL;
 	union sctp_addr *daddr = &t->ipaddr;
 	union sctp_addr dst_saddr;
+	struct in6_addr *final_p, final;
 	__u8 matchlen = 0;
 	__u8 bmatchlen;
 	sctp_scope_t scope;
@@ -281,7 +262,8 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 		pr_debug("src=%pI6 - ", &fl6->saddr);
 	}
 
-	dst = ip6_dst_lookup_flow(sk, fl6, NULL, false);
+	final_p = fl6_update_dst(fl6, np->opt, &final);
+	dst = ip6_dst_lookup_flow(sk, fl6, final_p, false);
 	if (!asoc || saddr)
 		goto out;
 
@@ -333,10 +315,12 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 		}
 	}
 	rcu_read_unlock();
+
 	if (baddr) {
 		fl6->saddr = baddr->v6.sin6_addr;
 		fl6->fl6_sport = baddr->v6.sin6_port;
-		dst = ip6_dst_lookup_flow(sk, fl6, NULL, false);
+		final_p = fl6_update_dst(fl6, np->opt, &final);
+		dst = ip6_dst_lookup_flow(sk, fl6, final_p, false);
 	}
 
 out:
-- 
1.7.11.7

^ permalink raw reply related

* Re: [PATCH nf] netfilter: use RCU safe kfree for conntrack extensions
From: Phil Oester @ 2013-09-11 14:57 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: netfilter-devel, netdev, Pablo Neira Ayuso, Patrick McHardy,
	Jozsef Kadlecsik, coreteam
In-Reply-To: <20130911090900.0129AE8AFF@unicorn.suse.cz>

On Wed, Sep 11, 2013 at 10:17:27AM +0200, Michal Kubecek wrote:
> Commit 68b80f11 (netfilter: nf_nat: fix RCU races) introduced
> RCU protection for freeing extension data when reallocation
> moves them to a new location. We need the same protection when
> freeing them in nf_ct_ext_free() in order to prevent a
> use-after-free by other threads referencing a NAT extension data
> via bysource list.

Hi Michal - 

coincidentally I've been looking into this area this week due to another
bug report (https://bugzilla.kernel.org/show_bug.cgi?id=60853).  Looking at
your proposed fix, the NAT extension data should have been cleaned
from the bysource list in nf_nat_cleanup_conntrack (via __nf_ct_ext_destroy)
before reaching the kfree.  Would you agree?

The reporter of #60853 suggested adding a synchronize_rcu to the end of the
nf_nat_cleanup_conntrack function, which seems sane.  I have been trying to 
reproduce the crash to test that theory.

Are you able to reproduce an OOPS in your testing?  Or is there a bug report
you are working from?

Phil

^ permalink raw reply

* Re: [net  8/8] i40e: include i40e in kernel proper
From: Joe Perches @ 2013-09-11 14:47 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem, Jesse Brandeburg, netdev, gospo, sassmann, Shannon Nelson,
	PJ Waskiewicz, e1000-devel
In-Reply-To: <1378909937.2026.8.camel@jtkirshe-mobl>

On Wed, 2013-09-11 at 07:32 -0700, Jeff Kirsher wrote:
> On Wed, 2013-09-11 at 05:20 -0700, Joe Perches wrote:
> > On Wed, 2013-09-11 at 02:50 -0700, Jeff Kirsher wrote:
> > > New driver build option is CONFIG_I40E
> > 
> > > diff --git a/Documentation/networking/00-INDEX b/Documentation/networking/00-INDEX
> > []
> > > +i40e.txt
> > > +	- README for the Intel Ethernet Controller XL710 Driver (i40e).
> > 
> > Just curious but why the XL710 / i40e name mismatch?
> 
> i40e stands for Intel 40 GbE Ethernet which is more generic than naming
> the driver the same as the first part (XL710).

Too bad if also means that when 100 comes out
this driver won't be expected to work.

^ permalink raw reply

* Re: Use-after-free in TUNSETIFF
From: Ben Hutchings @ 2013-09-11 14:44 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Wannes Rombouts, davem, jasowang, mst, edumazet, nhorman, netdev,
	Kevin Soules
In-Reply-To: <20130910173257.47e2dc08@nehalam.linuxnetplumber.net>

On Tue, 2013-09-10 at 17:32 -0700, Stephen Hemminger wrote:
[...]
> [1] A user with CAP_NET_ADMIN can basically hose the system many other ways.
> Capabilities are a failed security model.
> Almost all distro's limit CAP_NET_ADMIN to root anyway.

tun uses ns_capable(), not capable().  If user namespaces are enabled
then I think any user can create their own user & net namespaces, be
'root' in those namespaces and then invoke TUNSETIFF successfully.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox