* Re: BUG: double spinlock in "drivers/net/3c505.c"
From: David Miller @ 2010-06-18 3:12 UTC (permalink / raw)
To: chf.fritz; +Cc: strakh, philb, craigs, tridge, Alan.Cox, netdev, linux-kernel
In-Reply-To: <1276003605.3916.14.camel@lovely>
From: Christoph Fritz <chf.fritz@googlemail.com>
Date: Tue, 08 Jun 2010 15:26:45 +0200
> On Mon, 2010-06-07 at 15:17 +0400, Alexander Strakh wrote:
>> KERNEL_VERSION: 2.6.35-rc1
>> SUBJECT: duble spinlock in function elp_start_xmit
>
> Not only in elp_start_xmit. This driver is for a pretty old and slow isa
> ethernet card and I think nobody cares. To quote a comment from the
> source: "[...] the concurrency protection is particularly awful".
Indeed, I spent some time trying to unravel the locking mess
for these call chains to check_3c505_dma() and it was just
too much to sanely cure.
^ permalink raw reply
* Re: [PATCH 8/8] bridge: Fix netpoll support
From: Cong Wang @ 2010-06-18 3:06 UTC (permalink / raw)
To: Herbert Xu
Cc: Michael S. Tsirkin, Qianfeng Zhang, David S. Miller, netdev,
Stephen Hemminger, Matt Mackall, Paul E. McKenney
In-Reply-To: <20100617105526.GA1440@gondor.apana.org.au>
On 06/17/10 18:55, Herbert Xu wrote:
> On Thu, Jun 17, 2010 at 06:57:28PM +0800, Cong Wang wrote:
>>
>> Hmm, I get it now. So this helps to fix problem 3)?
>
> Yes, by allocating real netpoll structures for each device that
> we're polling, instead of sharing a single one amongst all of
> them.
>
> This is also the basis of the solution of the use-after-free bug.
>
Looks reasonable.
Thank you for your patch!
^ permalink raw reply
* Re: [Patch 2/2] mlx4: add dynamic LRO disable support
From: Cong Wang @ 2010-06-18 3:10 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Ben Hutchings, netdev, herbert.xu, nhorman, davem
In-Reply-To: <20100617120318.GA4059@dhcp-lab-161.englab.brq.redhat.com>
On 06/17/10 20:03, Stanislaw Gruszka wrote:
> On Thu, Jun 17, 2010 at 06:54:28PM +0800, Cong Wang wrote:
>>>> I don't think mdev->state_lock is used to protect dev->feature.
>>>> rtnl_lock is. I think switching to mlx4_ethtool_op_set_flags()
>>>> from the default one has already solved this.
>>>
>>> Ahh, you have right, may intention was use it to stop and start
>>> port. Code rather should look like below:
>>>
>>> if (netdev_running(dev)) {
>>> mutex_lock(&mdev->state_lock);
>>> mlx4_en_stop_port(dev);
>>> }
>>>
>>> dev->features ^= NETIF_F_LRO;
>>>
>>> if (netdev_running(dev)) {
>>> rc = mlx4_en_start_port(dev);
>>> mutex_unlock(&mdev->state_lock);
>>> if (rc)
>>> en_err(priv, "Failed to restart port\n");
>>> }
>>>
>>
>> Hmm, you mean ->features should be changed after port is stopped?
>
> Actually not ->features variable, but NETIF_F_LRO bit, as only this
> bit is used in rx path.
>
Yeah, this is what I meant.
>> Why?
>
> For reasons you talked before in this thread :) to do not change
> LRO in the middle of receiving packages.
>
Ohh... I missed this, seems reasonable, will fix this in the next update.
Thanks.
^ permalink raw reply
* [net-next-2.6 PATCH 1/4] e1000e: avoid polling h/w registers during link negotiation
From: Jeff Kirsher @ 2010-06-18 4:58 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Bruce Allan, Jeff Kirsher
From: Bruce Allan <bruce.w.allan@intel.com>
Avoid touching hardware registers when possible, otherwise link negotiation
can get messed up when user-level scripts are rapidly polling the driver to
see if/when link is up. Use the saved link state information instead when
possible.
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/e1000e/e1000.h | 1 -
drivers/net/e1000e/ethtool.c | 54 ++++++++++++++++++++++--------------------
drivers/net/e1000e/netdev.c | 2 +-
3 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
index 328b7f4..9ee133f 100644
--- a/drivers/net/e1000e/e1000.h
+++ b/drivers/net/e1000e/e1000.h
@@ -461,7 +461,6 @@ extern int e1000e_setup_tx_resources(struct e1000_adapter *adapter);
extern void e1000e_free_rx_resources(struct e1000_adapter *adapter);
extern void e1000e_free_tx_resources(struct e1000_adapter *adapter);
extern void e1000e_update_stats(struct e1000_adapter *adapter);
-extern bool e1000e_has_link(struct e1000_adapter *adapter);
extern void e1000e_set_interrupt_capability(struct e1000_adapter *adapter);
extern void e1000e_reset_interrupt_capability(struct e1000_adapter *adapter);
extern void e1000e_disable_aspm(struct pci_dev *pdev, u16 state);
diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index db86850..77c5829 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -118,7 +118,6 @@ static int e1000_get_settings(struct net_device *netdev,
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
- u32 status;
if (hw->phy.media_type == e1000_media_type_copper) {
@@ -156,22 +155,29 @@ static int e1000_get_settings(struct net_device *netdev,
ecmd->transceiver = XCVR_EXTERNAL;
}
- status = er32(STATUS);
- if (status & E1000_STATUS_LU) {
- if (status & E1000_STATUS_SPEED_1000)
- ecmd->speed = 1000;
- else if (status & E1000_STATUS_SPEED_100)
- ecmd->speed = 100;
- else
- ecmd->speed = 10;
+ ecmd->speed = -1;
+ ecmd->duplex = -1;
- if (status & E1000_STATUS_FD)
- ecmd->duplex = DUPLEX_FULL;
- else
- ecmd->duplex = DUPLEX_HALF;
+ if (netif_running(netdev)) {
+ if (netif_carrier_ok(netdev)) {
+ ecmd->speed = adapter->link_speed;
+ ecmd->duplex = adapter->link_duplex - 1;
+ }
} else {
- ecmd->speed = -1;
- ecmd->duplex = -1;
+ u32 status = er32(STATUS);
+ if (status & E1000_STATUS_LU) {
+ if (status & E1000_STATUS_SPEED_1000)
+ ecmd->speed = 1000;
+ else if (status & E1000_STATUS_SPEED_100)
+ ecmd->speed = 100;
+ else
+ ecmd->speed = 10;
+
+ if (status & E1000_STATUS_FD)
+ ecmd->duplex = DUPLEX_FULL;
+ else
+ ecmd->duplex = DUPLEX_HALF;
+ }
}
ecmd->autoneg = ((hw->phy.media_type == e1000_media_type_fiber) ||
@@ -179,7 +185,7 @@ static int e1000_get_settings(struct net_device *netdev,
/* MDI-X => 2; MDI =>1; Invalid =>0 */
if ((hw->phy.media_type == e1000_media_type_copper) &&
- !hw->mac.get_link_status)
+ netif_carrier_ok(netdev))
ecmd->eth_tp_mdix = hw->phy.is_mdix ? ETH_TP_MDI_X :
ETH_TP_MDI;
else
@@ -191,19 +197,15 @@ static int e1000_get_settings(struct net_device *netdev,
static u32 e1000_get_link(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
- struct e1000_mac_info *mac = &adapter->hw.mac;
+ struct e1000_hw *hw = &adapter->hw;
/*
- * If the link is not reported up to netdev, interrupts are disabled,
- * and so the physical link state may have changed since we last
- * looked. Set get_link_status to make sure that the true link
- * state is interrogated, rather than pulling a cached and possibly
- * stale link state from the driver.
+ * Avoid touching hardware registers when possible, otherwise
+ * link negotiation can get messed up when user-level scripts
+ * are rapidly polling the driver to see if link is up.
*/
- if (!netif_carrier_ok(netdev))
- mac->get_link_status = 1;
-
- return e1000e_has_link(adapter);
+ return netif_running(netdev) ? netif_carrier_ok(netdev) :
+ !!(er32(STATUS) & E1000_STATUS_LU);
}
static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u16 spddplx)
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 8faf224..2a71889 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -3964,7 +3964,7 @@ static void e1000_print_link_info(struct e1000_adapter *adapter)
((ctrl & E1000_CTRL_TFCE) ? "TX" : "None" )));
}
-bool e1000e_has_link(struct e1000_adapter *adapter)
+static bool e1000e_has_link(struct e1000_adapter *adapter)
{
struct e1000_hw *hw = &adapter->hw;
bool link_active = 0;
^ permalink raw reply related
* [net-next-2.6 PATCH 2/4] e1000e: do not touch PHY page 800 registers when link speed is 1000Mbps
From: Jeff Kirsher @ 2010-06-18 4:59 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Bruce Allan, Jeff Kirsher
In-Reply-To: <20100618045804.6728.37945.stgit@localhost.localdomain>
From: Bruce Allan <bruce.w.allan@intel.com>
The PHY on 82577/82578 has issues when the registers on page 800 are
accessed when in gigabit mode. Do not clear the Wakeup Control register
when resetting the part since it is on page 800 (and will be cleared on
reset anyway).
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/e1000e/netdev.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 2a71889..aa14976 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -3184,8 +3184,6 @@ void e1000e_reset(struct e1000_adapter *adapter)
e1000_get_hw_control(adapter);
ew32(WUC, 0);
- if (adapter->flags2 & FLAG2_HAS_PHY_WAKEUP)
- e1e_wphy(&adapter->hw, BM_WUC, 0);
if (mac->ops.init_hw(hw))
e_err("Hardware Error\n");
^ permalink raw reply related
* [net-next-2.6 PATCH 3/4] e1000e: packet split should not be used with early receive
From: Jeff Kirsher @ 2010-06-18 4:59 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Bruce Allan, Jeff Kirsher
In-Reply-To: <20100618045804.6728.37945.stgit@localhost.localdomain>
From: Bruce Allan <bruce.w.allan@intel.com>
Originally it was thought there were issues with ICHx/PCH parts with packet
split when jumbo frames were enabled but in fact it is really only when
early-receive is enabled (via ERT register) on these parts. Use packet
split with jumbos but only when early-receive is not enabled.
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/e1000e/netdev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index aa14976..71592ed 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -2772,7 +2772,7 @@ static void e1000_setup_rctl(struct e1000_adapter *adapter)
* per packet.
*/
pages = PAGE_USE_COUNT(adapter->netdev->mtu);
- if (!(adapter->flags & FLAG_IS_ICH) && (pages <= 3) &&
+ if (!(adapter->flags & FLAG_HAS_ERT) && (pages <= 3) &&
(PAGE_SIZE <= 16384) && (rctl & E1000_RCTL_LPE))
adapter->rx_ps_pages = pages;
else
^ permalink raw reply related
* [net-next-2.6 PATCH 4/4] e1000e: disable gig speed when in S0->Sx transition
From: Jeff Kirsher @ 2010-06-18 4:59 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, Bruce Allan, Jeff Kirsher
In-Reply-To: <20100618045804.6728.37945.stgit@localhost.localdomain>
From: Bruce Allan <bruce.w.allan@intel.com>
Most of this workaround is necessary for all ICHx/PCH parts so one of
the two MAC-type checks can be removed.
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/e1000e/ich8lan.c | 19 +++++--------------
1 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index c7292a1..6b5e108 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -3457,21 +3457,12 @@ void e1000e_disable_gig_wol_ich8lan(struct e1000_hw *hw)
{
u32 phy_ctrl;
- switch (hw->mac.type) {
- case e1000_ich8lan:
- case e1000_ich9lan:
- case e1000_ich10lan:
- case e1000_pchlan:
- phy_ctrl = er32(PHY_CTRL);
- phy_ctrl |= E1000_PHY_CTRL_D0A_LPLU |
- E1000_PHY_CTRL_GBE_DISABLE;
- ew32(PHY_CTRL, phy_ctrl);
+ phy_ctrl = er32(PHY_CTRL);
+ phy_ctrl |= E1000_PHY_CTRL_D0A_LPLU | E1000_PHY_CTRL_GBE_DISABLE;
+ ew32(PHY_CTRL, phy_ctrl);
- if (hw->mac.type == e1000_pchlan)
- e1000_phy_hw_reset_ich8lan(hw);
- default:
- break;
- }
+ if (hw->mac.type >= e1000_pchlan)
+ e1000_phy_hw_reset_ich8lan(hw);
}
/**
^ permalink raw reply related
* RE: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
From: Xin, Xiaohui @ 2010-06-18 5:26 UTC (permalink / raw)
To: Herbert Xu
Cc: Stephen Hemminger, netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, mst@redhat.com, mingo@elte.hu,
davem@davemloft.net, jdike@linux.intel.com
In-Reply-To: <20100617112119.GB1515@gondor.apana.org.au>
[-- Attachment #1: Type: text/plain, Size: 2302 bytes --]
>-----Original Message-----
>From: Herbert Xu [mailto:herbert@gondor.apana.org.au]
>Sent: Thursday, June 17, 2010 7:21 PM
>To: Xin, Xiaohui
>Cc: Stephen Hemminger; netdev@vger.kernel.org; kvm@vger.kernel.org;
>linux-kernel@vger.kernel.org; mst@redhat.com; mingo@elte.hu; davem@davemloft.net;
>jdike@linux.intel.com
>Subject: Re: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
>
>On Sun, Jun 13, 2010 at 04:58:36PM +0800, Xin, Xiaohui wrote:
>>
>> Herbert,
>> In this way, I think we should create 3 functions at least in drivers to allocate rx buffer, to
>receive the rx buffers, and to clean the rx buffers.
>>
>> We can also have another way here. We can provide a function to only substitute
>> alloc_page(), and a function to release the pages when cleaning the rx buffers.
>
>Yes that's exactly what I had in mind.
>
Herbert,
I have questions about the idea above:
1) Since netdev_alloc_skb() is still there, and we only modify alloc_page(),
then we don't need napi_gro_frags() any more, the driver's original receiving
function is ok. Right?
2) Is napi_gro_frags() only suitable for TCP protocol packet?
I have done a small test for ixgbe driver to let it only allocate paged buffers
and found kernel hangs when napi_gro_frags() receives an ARP packet.
3) As I have mentioned above, with this idea, netdev_alloc_skb() will allocate
as usual, the data pointed by skb->data will be copied into the first guest buffer.
That means we should reserve sufficient room in guest buffer. For PS mode
supported driver (for example ixgbe), the room will be more than 128. After 128bytes,
we will put the first frag data. Look into virtio-net.c the function page_to_skb()
and receive_mergeable(), that means we should modify guest virtio-net driver to
compute the offset as the parameter for skb_set_frag().
How do you think about this? Attached is a patch to how to modify the guest driver.
I reserve 512 bytes as an example, and transfer the header len of the skb in hdr->hdr_len.
Thanks
Xiaohui
>Cheers,
>--
>Visit Openswan at http://www.openswan.org/
>Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
>Home Page: http://gondor.apana.org.au/~herbert/
>PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[-- Attachment #2: mod-guest.diff --]
[-- Type: application/octet-stream, Size: 2031 bytes --]
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b0577dd..a4e6988 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -36,7 +36,9 @@ module_param(gso, bool, 0444);
/* FIXME: MTU in config. */
#define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
-#define GOOD_COPY_LEN 128
+#define GOOD_COPY_LEN 512
#define VIRTNET_SEND_COMMAND_SG_MAX 2
@@ -147,11 +149,12 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
{
struct sk_buff *skb;
struct skb_vnet_hdr *hdr;
- unsigned int copy, hdr_len, offset;
+ unsigned int copy, hdr_len, offset, end;
char *p;
p = page_address(page);
@@ -172,14 +175,25 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
len -= hdr_len;
p += offset;
- copy = len;
+ if (!hdr->hdr.hdr_len)
+ copy = len;
+ else {
+ copy = hdr->hdr.hdr_len;
+ end = skb_tailroom(skb);
+ printk(KERN_INFO "c %d, e %d\n", copy, end);
+ }
+
if (copy > skb_tailroom(skb))
copy = skb_tailroom(skb);
memcpy(skb_put(skb, copy), p, copy);
len -= copy;
- offset += copy;
+ if (!hdr->hdr.hdr_len)
+ offset += copy;
+ else
+ offset = end;
+ printk(KERN_INFO "off %d\n", offset);
while (len) {
set_skb_frag(skb, page, offset, &len);
page = (struct page *)page->private;
@@ -218,7 +238,12 @@ static int receive_mergeable(struct virtnet_info *vi, struct sk_buff *skb)
len = PAGE_SIZE;
set_skb_frag(skb, page, 0, &len);
-
+ if (hdr->hdr.hdr_len)
+ set_skb_frag(skb, page, 542, &len);
+ else
+ set_skb_frag(skb, page, 0, &len);
--vi->num;
}
return 0;
@@ -311,7 +340,15 @@ static void receive_buf(struct net_device *dev, void *buf, unsigned int len)
skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
skb_shinfo(skb)->gso_segs = 0;
}
-
+ skb->ip_summed = 1;
netif_receive_skb(skb);
return;
@@ -479,7 +516,8 @@ again:
received++;
}
- if (vi->num < vi->max / 2) {
+ if (vi->num < vi->max - 10) {
if (!try_fill_recv(vi, GFP_ATOMIC))
schedule_delayed_work(&vi->refill, 0);
}
^ permalink raw reply related
* Re: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
From: Herbert Xu @ 2010-06-18 5:59 UTC (permalink / raw)
To: Xin, Xiaohui
Cc: Stephen Hemminger, netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, mst@redhat.com, mingo@elte.hu,
davem@davemloft.net, jdike@linux.intel.com, Rusty Russell
In-Reply-To: <F2E9EB7348B8264F86B6AB8151CE2D7915089FE4CC@shsmsx502.ccr.corp.intel.com>
On Fri, Jun 18, 2010 at 01:26:49PM +0800, Xin, Xiaohui wrote:
>
> Herbert,
> I have questions about the idea above:
> 1) Since netdev_alloc_skb() is still there, and we only modify alloc_page(),
> then we don't need napi_gro_frags() any more, the driver's original receiving
> function is ok. Right?
Well I was actually thinking about converting all drivers that
need this to napi_gro_frags. But now that you mention it, yes
we could still keep the old interface to minimise the work.
> 2) Is napi_gro_frags() only suitable for TCP protocol packet?
> I have done a small test for ixgbe driver to let it only allocate paged buffers
> and found kernel hangs when napi_gro_frags() receives an ARP packet.
It should work with any packet. In fact, I'm pretty sure the
other drivers (e.g., cxgb3) use that interface for all packets.
> 3) As I have mentioned above, with this idea, netdev_alloc_skb() will allocate
> as usual, the data pointed by skb->data will be copied into the first guest buffer.
> That means we should reserve sufficient room in guest buffer. For PS mode
> supported driver (for example ixgbe), the room will be more than 128. After 128bytes,
> we will put the first frag data. Look into virtio-net.c the function page_to_skb()
> and receive_mergeable(), that means we should modify guest virtio-net driver to
> compute the offset as the parameter for skb_set_frag().
>
> How do you think about this? Attached is a patch to how to modify the guest driver.
> I reserve 512 bytes as an example, and transfer the header len of the skb in hdr->hdr_len.
Expanding the buffer size to 512 bytes to accomodate PS mode
looks reasonable to me.
However, I don't think we should increase the copy threshold to
512 bytes at the same time. I don't have any figures myself but
I think if we are to make such a change it should be a separate
one and come with supporting numbers.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [Bugme-new] [Bug 16187] New: Carrier detection failed in dhcpcd when link is up
From: Grant Grundler @ 2010-06-18 6:16 UTC (permalink / raw)
To: Andrew Morton
Cc: netdev, Grant Grundler, Kyle McMartin, bugzilla-daemon,
bugme-daemon, casteyde.christian
In-Reply-To: <20100615142418.f47e8abd.akpm@linux-foundation.org>
On Tue, Jun 15, 2010 at 02:24:18PM -0700, Andrew Morton wrote:
>
> (switched to email. Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
I've resync to linus' tree (2.6.35-rc3) and reviewed the output of:
git diff v2.6.34 drivers/net/tulip/
I don't see anything that would affect how link state
changes get reported to user space.
I'm not inclined to believe this is a tulip "bug" unless
core netdev behavior changed and tulip is not longer
doing the right thing.
hth,
grant
^ permalink raw reply
* Distributed Switch Architecture(DSA)
From: Joakim Tjernlund @ 2010-06-18 7:06 UTC (permalink / raw)
To: Lennert Buytenhek, netdev
I am trying to wrap my head around DSA and I need some help.
Assume the example from Lennert:
+-----------+ +-----------+
| | RGMII | |
| +-------+ +------ 1000baseT MDI ("WAN")
| | | 6-port +------ 1000baseT MDI ("LAN1")
| CPU | | ethernet +------ 1000baseT MDI ("LAN2")
| |MIImgmt| switch +------ 1000baseT MDI ("LAN3")
| +-------+ w/5 PHYs +------ 1000baseT MDI ("LAN4")
| | | |
+-----------+ +-----------+
If I understand this correctly I get at least 5 virtual I/Fs corresponding
to WAN, LAN1-4, but how is the RGMII I/F modelled?
I guess I will have one "real" ethX I/F which maps to RGMII but do I get one
virtual I/F too?
What use are these virtual I/Fs? Just to read status from the corresponding
ports? Can one TX and RX network pkgs over these I/Fs too?
Now I want to add STP/RSTP to the switch. How would one do that?
Jocke
^ permalink raw reply
* RE: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
From: Xin, Xiaohui @ 2010-06-18 7:14 UTC (permalink / raw)
To: Herbert Xu
Cc: Stephen Hemminger, netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, mst@redhat.com, mingo@elte.hu,
davem@davemloft.net, jdike@linux.intel.com, Rusty Russell
In-Reply-To: <20100618055929.GA11333@gondor.apana.org.au>
>-----Original Message-----
>From: Herbert Xu [mailto:herbert@gondor.apana.org.au]
>Sent: Friday, June 18, 2010 1:59 PM
>To: Xin, Xiaohui
>Cc: Stephen Hemminger; netdev@vger.kernel.org; kvm@vger.kernel.org;
>linux-kernel@vger.kernel.org; mst@redhat.com; mingo@elte.hu; davem@davemloft.net;
>jdike@linux.intel.com; Rusty Russell
>Subject: Re: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
>
>On Fri, Jun 18, 2010 at 01:26:49PM +0800, Xin, Xiaohui wrote:
>>
>> Herbert,
>> I have questions about the idea above:
>> 1) Since netdev_alloc_skb() is still there, and we only modify alloc_page(),
>> then we don't need napi_gro_frags() any more, the driver's original receiving
>> function is ok. Right?
>
>Well I was actually thinking about converting all drivers that
>need this to napi_gro_frags. But now that you mention it, yes
>we could still keep the old interface to minimise the work.
>
>> 2) Is napi_gro_frags() only suitable for TCP protocol packet?
>> I have done a small test for ixgbe driver to let it only allocate paged buffers
>> and found kernel hangs when napi_gro_frags() receives an ARP packet.
>
>It should work with any packet. In fact, I'm pretty sure the
>other drivers (e.g., cxgb3) use that interface for all packets.
>
Thanks for the verification. By the way, does that mean that nearly all drivers can use the
same napi_gro_frags() to receive buffers though currently each driver has it's own receiving
function?
>> 3) As I have mentioned above, with this idea, netdev_alloc_skb() will allocate
>> as usual, the data pointed by skb->data will be copied into the first guest buffer.
>> That means we should reserve sufficient room in guest buffer. For PS mode
>> supported driver (for example ixgbe), the room will be more than 128. After 128bytes,
>> we will put the first frag data. Look into virtio-net.c the function page_to_skb()
>> and receive_mergeable(), that means we should modify guest virtio-net driver to
>> compute the offset as the parameter for skb_set_frag().
>>
>> How do you think about this? Attached is a patch to how to modify the guest driver.
>> I reserve 512 bytes as an example, and transfer the header len of the skb in hdr->hdr_len.
>
>Expanding the buffer size to 512 bytes to accomodate PS mode
>looks reasonable to me.
>
>However, I don't think we should increase the copy threshold to
>512 bytes at the same time. I don't have any figures myself but
>I think if we are to make such a change it should be a separate
>one and come with supporting numbers.
>
Let me have a look to see if I can retain the copy threshold as 128 bytes
and copy the header data safely.
>Cheers,
>--
>Visit Openswan at http://www.openswan.org/
>Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
>Home Page: http://gondor.apana.org.au/~herbert/
>PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: Distributed Switch Architecture(DSA)
From: Lennert Buytenhek @ 2010-06-18 7:33 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: netdev
In-Reply-To: <OF4DC5B7A3.2D318B4E-ONC1257746.00253DA6-C1257746.002714B0@transmode.se>
On Fri, Jun 18, 2010 at 09:06:52AM +0200, Joakim Tjernlund wrote:
> I am trying to wrap my head around DSA and I need some help.
>
> Assume the example from Lennert:
>
> +-----------+ +-----------+
> | | RGMII | |
> | +-------+ +------ 1000baseT MDI ("WAN")
> | | | 6-port +------ 1000baseT MDI ("LAN1")
> | CPU | | ethernet +------ 1000baseT MDI ("LAN2")
> | |MIImgmt| switch +------ 1000baseT MDI ("LAN3")
> | +-------+ w/5 PHYs +------ 1000baseT MDI ("LAN4")
> | | | |
> +-----------+ +-----------+
>
> If I understand this correctly I get at least 5 virtual I/Fs corresponding
> to WAN, LAN1-4, but how is the RGMII I/F modelled?
The RGMII interface is just the interface that your "real" network
driver exports. In the case of the Kirkwood 6281 A0 Reference Design
(which I developed this code on), that would be eth0. After the DSA
driver is instantiated, you don't send or receive over eth0 directly
anymore -- eth0 becomes purely a transport for DSA-tagged packets.
> I guess I will have one "real" ethX I/F which maps to RGMII but do I
> get one virtual I/F too?
You get a virtual interface for each of the ports on the switch (that
are not CPU or inter-switch ports), i.e. all ports on the right of the
diagram -- wan, lan1, lan2, lan3, lan4. These interfaces are created
by net/dsa/slave.c and are called DSA interfaces or slave interfaces.
> What use are these virtual I/Fs? Just to read status from the
> corresponding ports?
That's one of the purposes, yes. There's a polling routine that
periodically checks the status of each of the ports on the switch (via
the MII management interface) and feeds back that status to the virtual
interfaces. I.e. if you plug a cable into lan3, you'll see a syslog
message about the link on the virtual interface lan3 having come up,
with the link speed, etc.
> Can one TX and RX network pkgs over these I/Fs too?
Sure -- that's the whole point.
> Now I want to add STP/RSTP to the switch. How would one do that?
First, you'll want the hardware bridging patches that I posted to
netdev@ a while back, e.g.:
http://patchwork.ozlabs.org/patch/16578/
They aren't in upstream-mergeable form in their current form, but they
do the job. These will propagate brctl addif/delif calls into the switch
chip, so that switching between those ports will be done in hardware.
Now if all you want is regular STP, with that patch you'll be done --
the ->bridge_set_stp_state() hook propagates the spanning tree state of
each of the DSA virtual interfaces into the switch chip automatically.
If you want to use a userspace STP implementation, you'll just have to
make sure that STP state (listening/learning/blocking/forwarding/etc) is
correctly propagated to the switch chip similarly to how it's done in the
patch.
(Ideally, these patches should be reworked to receive bridge configuration
and port status changes via netlink. Unfortunately, I was asked to return
all my Marvell hardware when I left Marvell, so someone else will have to
do this work.)
^ permalink raw reply
* Re: [PATCH 2/2] Driver core: reduce duplicated code
From: Uwe Kleine-König @ 2010-06-18 7:39 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel, Greg Kroah-Hartman, Magnus Damm, Rafael J. Wysocki,
Paul Mundt, Dmitry Torokhov, Eric Miao, netdev
In-Reply-To: <20100616205332.GB15837@kroah.com>
Hi Greg,
On Wed, Jun 16, 2010 at 01:53:32PM -0700, Greg KH wrote:
> On Tue, Jun 15, 2010 at 11:05:00AM +0200, Uwe Kleine-König wrote:
> > On Tue, Jun 15, 2010 at 10:47:56AM +0200, Uwe Kleine-König wrote:
> > > This makes the two similar functions platform_device_register_simple
> > > and platform_device_register_data one line inline functions using a new
> > > generic function platform_device_register_resndata.
> > I forgot to add some comments to this mail, ... sorry.
> >
> > - I'm not completely happy with the name of the new function. If
> > someone has a better name please tell me.
>
> I don't like it either, what is "resndata" supposed to stand for?
resources and data -> res'n'data -> resndata.
> > - can platform_device_register_resndata be moved to __init_or_module?
>
> I doubt it, but try it and see if a build warns about it.
for x86_64_defconfig + MODULES=n there are two section mismatches:
dock_add
regulatory_init
regulatory_init is only called from
static int __init cfg80211_init(void)
. (I just sent a patch[1] that moves regulatory_init to .init.text.)
dock_add (in drivers/acpi/dock.c) is a bit harder. I will take a look
later if it can go to .init.text (together with a few more functions),
too.
> > - I moved the kernel docs to the header but didn't test if they are
> > picked up when generating docs. Even if not, there is no better
> > place, is there?
>
> No, that's the proper place, but make sure the docbook source is also
> picking up the .h file, I don't know if it currently does.
It does not, will fix in a v2.
Thanks
Uwe
[1] http://mid.gmane.org/1276846735-12105-2-git-send-email-u.kleine-koenig@pengutronix.de
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
From: Herbert Xu @ 2010-06-18 7:45 UTC (permalink / raw)
To: Xin, Xiaohui
Cc: Stephen Hemminger, netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, mst@redhat.com, mingo@elte.hu,
davem@davemloft.net, jdike@linux.intel.com, Rusty Russell
In-Reply-To: <F2E9EB7348B8264F86B6AB8151CE2D7915089FE573@shsmsx502.ccr.corp.intel.com>
On Fri, Jun 18, 2010 at 03:14:18PM +0800, Xin, Xiaohui wrote:
>
> Thanks for the verification. By the way, does that mean that nearly all drivers can use the
> same napi_gro_frags() to receive buffers though currently each driver has it's own receiving
> function?
There is no reason why the napi_gro_frags can't be used by any
driver that supports receiving data into pages.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: Distributed Switch Architecture(DSA)
From: Joakim Tjernlund @ 2010-06-18 9:15 UTC (permalink / raw)
To: Lennert Buytenhek; +Cc: netdev
In-Reply-To: <20100618073309.GB14513@mail.wantstofly.org>
Lennert Buytenhek <buytenh@wantstofly.org> wrote on 2010/06/18 09:33:09:
>
> On Fri, Jun 18, 2010 at 09:06:52AM +0200, Joakim Tjernlund wrote:
>
> > I am trying to wrap my head around DSA and I need some help.
> >
> > Assume the example from Lennert:
> >
> > +-----------+ +-----------+
> > | | RGMII | |
> > | +-------+ +------ 1000baseT MDI ("WAN")
> > | | | 6-port +------ 1000baseT MDI ("LAN1")
> > | CPU | | ethernet +------ 1000baseT MDI ("LAN2")
> > | |MIImgmt| switch +------ 1000baseT MDI ("LAN3")
> > | +-------+ w/5 PHYs +------ 1000baseT MDI ("LAN4")
> > | | | |
> > +-----------+ +-----------+
> >
> > If I understand this correctly I get at least 5 virtual I/Fs corresponding
> > to WAN, LAN1-4, but how is the RGMII I/F modelled?
>
> The RGMII interface is just the interface that your "real" network
> driver exports. In the case of the Kirkwood 6281 A0 Reference Design
> (which I developed this code on), that would be eth0. After the DSA
> driver is instantiated, you don't send or receive over eth0 directly
> anymore -- eth0 becomes purely a transport for DSA-tagged packets.
hmm, but how do I send normal pkgs form the CPU to the switch then?
I envision I would get some interface in the CPU I can set an IP address
on and use as a normal I/F which would be switched by the HW switch to
the appropriate port.
>
>
> > I guess I will have one "real" ethX I/F which maps to RGMII but do I
> > get one virtual I/F too?
>
> You get a virtual interface for each of the ports on the switch (that
> are not CPU or inter-switch ports), i.e. all ports on the right of the
> diagram -- wan, lan1, lan2, lan3, lan4. These interfaces are created
> by net/dsa/slave.c and are called DSA interfaces or slave interfaces.
>
>
> > What use are these virtual I/Fs? Just to read status from the
> > corresponding ports?
>
> That's one of the purposes, yes. There's a polling routine that
> periodically checks the status of each of the ports on the switch (via
> the MII management interface) and feeds back that status to the virtual
> interfaces. I.e. if you plug a cable into lan3, you'll see a syslog
> message about the link on the virtual interface lan3 having come up,
> with the link speed, etc.
>
>
> > Can one TX and RX network pkgs over these I/Fs too?
>
> Sure -- that's the whole point.
TX:ing pkgs on such virtual I/F would go directly to the port, bypassing
normal switching?
What about RX? What decides which pkg to route through the switch and
which pgk to send up to the virtual I/F?
>
>
> > Now I want to add STP/RSTP to the switch. How would one do that?
>
> First, you'll want the hardware bridging patches that I posted to
> netdev@ a while back, e.g.:
>
> http://patchwork.ozlabs.org/patch/16578/
I see, will have to study this a bit closer. One question though,
does this disable MAC learning in the linux bridge?
Do you have any idea how to do DSA on a Broadcom switch?
The control plane is an attached with PCI and has a big
user space lib/apps to manage the switch.
>
> They aren't in upstream-mergeable form in their current form, but they
> do the job. These will propagate brctl addif/delif calls into the switch
> chip, so that switching between those ports will be done in hardware.
>
> Now if all you want is regular STP, with that patch you'll be done --
> the ->bridge_set_stp_state() hook propagates the spanning tree state of
> each of the DSA virtual interfaces into the switch chip automatically.
>
> If you want to use a userspace STP implementation, you'll just have to
> make sure that STP state (listening/learning/blocking/forwarding/etc) is
> correctly propagated to the switch chip similarly to how it's done in the
> patch.
>
> (Ideally, these patches should be reworked to receive bridge configuration
> and port status changes via netlink. Unfortunately, I was asked to return
> all my Marvell hardware when I left Marvell, so someone else will have to
> do this work.)
>
^ permalink raw reply
* Re: [PATCH] socketcan: add a driver for FlexCAN controllers.
From: Wolfgang Grandegger @ 2010-06-18 9:47 UTC (permalink / raw)
To: Hans J. Koch; +Cc: netdev, socketcan-core
In-Reply-To: <20100617105201.GA2015@bluebox.local>
Hi Hans-Jürgen,
On 06/17/2010 12:52 PM, Hans J. Koch wrote:
> This adds a driver for FlexCAN based CAN controllers,
> e.g. found in Freescale i.MX35 SoCs.
>
> The original version of this driver was posted by Sascha Hauer in July 2009:
> http://kerneltrap.org/mailarchive/linux-netdev/2009/7/29/6251621
>
> I took this version, added NAPI support, and fixed some problems found
> during testing. Well, here is the result. Please review.
I briefly browsed the patch and various bits and pieces are missing or
not correctly implemented. Marc already pointed out a few of them:
- I do not find can_put/get_echo_skb functions in the code. How is
IFF_ECHO supposed to work?
- Support for CAN_CTRLMODE_BERR_REPORTING and do_get_berr_counter()
seems to be missing.
- Make use of alloc_can_skb() and alloc_can_err_skb().
Wolfgang.
^ permalink raw reply
* Re: Distributed Switch Architecture(DSA)
From: Lennert Buytenhek @ 2010-06-18 9:59 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: netdev
In-Reply-To: <OF68B8EEAD.B003EFE7-ONC1257746.003109DB-C1257746.0032D371@transmode.se>
On Fri, Jun 18, 2010 at 11:15:09AM +0200, Joakim Tjernlund wrote:
> > > I am trying to wrap my head around DSA and I need some help.
> > >
> > > Assume the example from Lennert:
> > >
> > > +-----------+ +-----------+
> > > | | RGMII | |
> > > | +-------+ +------ 1000baseT MDI ("WAN")
> > > | | | 6-port +------ 1000baseT MDI ("LAN1")
> > > | CPU | | ethernet +------ 1000baseT MDI ("LAN2")
> > > | |MIImgmt| switch +------ 1000baseT MDI ("LAN3")
> > > | +-------+ w/5 PHYs +------ 1000baseT MDI ("LAN4")
> > > | | | |
> > > +-----------+ +-----------+
> > >
> > > If I understand this correctly I get at least 5 virtual I/Fs corresponding
> > > to WAN, LAN1-4, but how is the RGMII I/F modelled?
> >
> > The RGMII interface is just the interface that your "real" network
> > driver exports. In the case of the Kirkwood 6281 A0 Reference Design
> > (which I developed this code on), that would be eth0. After the DSA
> > driver is instantiated, you don't send or receive over eth0 directly
> > anymore -- eth0 becomes purely a transport for DSA-tagged packets.
>
> hmm, but how do I send normal pkgs form the CPU to the switch then?
Define what you mean by 'normal pkgs'.
> I envision I would get some interface in the CPU I can set an IP address
> on and use as a normal I/F which would be switched by the HW switch to
> the appropriate port.
Yes, these are the DSA/slave interfaces created by net/dsa/slave.c.
You are free to attach IP addresses to the wan/lanX interfaces, and
things will work as you'd expect them to.
> > > I guess I will have one "real" ethX I/F which maps to RGMII but do I
> > > get one virtual I/F too?
> >
> > You get a virtual interface for each of the ports on the switch (that
> > are not CPU or inter-switch ports), i.e. all ports on the right of the
> > diagram -- wan, lan1, lan2, lan3, lan4. These interfaces are created
> > by net/dsa/slave.c and are called DSA interfaces or slave interfaces.
> >
> >
> > > What use are these virtual I/Fs? Just to read status from the
> > > corresponding ports?
> >
> > That's one of the purposes, yes. There's a polling routine that
> > periodically checks the status of each of the ports on the switch (via
> > the MII management interface) and feeds back that status to the virtual
> > interfaces. I.e. if you plug a cable into lan3, you'll see a syslog
> > message about the link on the virtual interface lan3 having come up,
> > with the link speed, etc.
> >
> >
> > > Can one TX and RX network pkgs over these I/Fs too?
> >
> > Sure -- that's the whole point.
>
> TX:ing pkgs on such virtual I/F would go directly to the port, bypassing
> normal switching?
Define what you mean by 'normal switching'.
> What about RX? What decides which pkg to route through the switch and
> which pgk to send up to the virtual I/F?
By default, which is until you enable bridging on some subset of the
ports, all ports have their own address database, and all received
packets are passed directly up to the CPU, where the DSA code will
then make those packets be received on the DSA slave interfaces.
> > > Now I want to add STP/RSTP to the switch. How would one do that?
> >
> > First, you'll want the hardware bridging patches that I posted to
> > netdev@ a while back, e.g.:
> >
> > http://patchwork.ozlabs.org/patch/16578/
>
> I see, will have to study this a bit closer. One question though,
> does this disable MAC learning in the linux bridge?
No, why should it?
> Do you have any idea how to do DSA on a Broadcom switch?
I have no idea. When I originally submitted the DSA code for merging,
I contacted Broadcom people about adding support for Broadcom switch
chips to it, but I never heard back from them.
^ permalink raw reply
* Re: [PATCH] socketcan: add a driver for FlexCAN controllers.
From: Wolfgang Grandegger @ 2010-06-18 10:04 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Hans J. Koch, netdev, socketcan-core
In-Reply-To: <4C1A2CDF.2040008@pengutronix.de>
On 06/17/2010 04:10 PM, Marc Kleine-Budde wrote:
> Hey Hans,
>
> Hans J. Koch wrote:
>> + priv->can.do_set_bittiming = flexcan_set_bittiming;
Yes, but we need to fix it globally anyway.
> using this callback is error prone, with respect to 3-sample and
> friends, see discussion on socketcan-core.
>
>> + priv->can.bittiming_const = &flexcan_bittiming_const;
>> + priv->can.do_set_mode = flexcan_set_mode;
>> + priv->can.restart_ms = 500;
>
> should this be set in the driver?
No, the default is manual restart for all drivers.
Wolfgang.
^ permalink raw reply
* Re: [PATCH] socketcan: add a driver for FlexCAN controllers.
From: Marc Kleine-Budde @ 2010-06-18 10:16 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4C1B4098.3090800-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1302 bytes --]
Wolfgang Grandegger wrote:
> Hi Hans-Jürgen,
>
> On 06/17/2010 12:52 PM, Hans J. Koch wrote:
>> This adds a driver for FlexCAN based CAN controllers,
>> e.g. found in Freescale i.MX35 SoCs.
>>
>> The original version of this driver was posted by Sascha Hauer in July 2009:
>> http://kerneltrap.org/mailarchive/linux-netdev/2009/7/29/6251621
>>
>> I took this version, added NAPI support, and fixed some problems found
>> during testing. Well, here is the result. Please review.
>
> I briefly browsed the patch and various bits and pieces are missing or
> not correctly implemented. Marc already pointed out a few of them:
>
> - I do not find can_put/get_echo_skb functions in the code. How is
> IFF_ECHO supposed to work?
the driver uses hardware loopback
> - Support for CAN_CTRLMODE_BERR_REPORTING and do_get_berr_counter()
> seems to be missing.
>
> - Make use of alloc_can_skb() and alloc_can_err_skb().
the last two points are already addressed in my version of the driver.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* Re: [Bugme-new] [Bug 16216] New: wrong source addr of UDP packets when using policy routing
From: Unknown @ 2010-06-18 9:56 UTC (permalink / raw)
To: Patrick McHardy
Cc: Eric Dumazet, Andrew Morton, netdev, bugzilla-daemon,
bugme-daemon
In-Reply-To: <4C190D34.8080100@trash.net>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1255 bytes --]
Okey. Did you people came into any conclusions?
Is there a patch I can test?
I tried to find 914a9ab386a288d0f22252fc268ecbc048cdcbd5
in few stable trees but was unable to.
---------- Original message ----------
From: Patrick McHardy <kaber@trash.net>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, netdev@vger.kernel.org,
bugzilla-daemon@bugzilla.kernel.org, bugme-daemon@bugzilla.kernel.org,
borg@uu3.net
Subject: Re: [Bugme-new] [Bug 16216] New: wrong source addr of UDP packets when
using policy routing
Date: Wed, 16 Jun 2010 19:43:16 +0200
Message-ID: <4C190D34.8080100@trash.net>
Eric Dumazet wrote:
> Le mercredi 16 juin 2010 18:46 +0200, Patrick McHardy a écrit :
>
>
> > This is know behaviour, fwmarks don't work for source address selection
> > since before the source address is chosen, you don't even have a packet
> > which could be marked.
> >
>
> We know have sk->sk_mark routing (socket based), so we might change
> sk->sk_mark with appropriate iptables target when one packet is
> received... not very clean but worth to mention...
>
That would still be too late. The proper way would be to have the application
set the socket mark.
^ permalink raw reply
* Re: [PATCH] socketcan: add a driver for FlexCAN controllers.
From: Wolfgang Grandegger @ 2010-06-18 10:33 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4C1B4796.3060506-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 06/18/2010 12:16 PM, Marc Kleine-Budde wrote:
> Wolfgang Grandegger wrote:
>> Hi Hans-Jürgen,
>>
>> On 06/17/2010 12:52 PM, Hans J. Koch wrote:
>>> This adds a driver for FlexCAN based CAN controllers,
>>> e.g. found in Freescale i.MX35 SoCs.
>>>
>>> The original version of this driver was posted by Sascha Hauer in July 2009:
>>> http://kerneltrap.org/mailarchive/linux-netdev/2009/7/29/6251621
>>>
>>> I took this version, added NAPI support, and fixed some problems found
>>> during testing. Well, here is the result. Please review.
>>
>> I briefly browsed the patch and various bits and pieces are missing or
>> not correctly implemented. Marc already pointed out a few of them:
>>
>> - I do not find can_put/get_echo_skb functions in the code. How is
>> IFF_ECHO supposed to work?
>
> the driver uses hardware loopback
OK, then
dev = alloc_candev(sizeof(struct flexcan_priv), 0);
should be used (and TX_ECHO_SKB_MAX removed) in Hans-Jürgens driver.
>
>> - Support for CAN_CTRLMODE_BERR_REPORTING and do_get_berr_counter()
>> seems to be missing.
>>
>> - Make use of alloc_can_skb() and alloc_can_err_skb().
>
> the last two points are already addressed in my version of the driver.
I do not see support for CAN_CTRLMODE_BERR_REPORTING in your driver
(which has nothing to do with do_get_berr_counter).
Wolfgang.
^ permalink raw reply
* Re: [PATCH] socketcan: add a driver for FlexCAN controllers.
From: Marc Kleine-Budde @ 2010-06-18 10:44 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4C1B4B85.3010905-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1976 bytes --]
Wolfgang Grandegger wrote:
> On 06/18/2010 12:16 PM, Marc Kleine-Budde wrote:
>> Wolfgang Grandegger wrote:
>>> Hi Hans-Jürgen,
>>>
>>> On 06/17/2010 12:52 PM, Hans J. Koch wrote:
>>>> This adds a driver for FlexCAN based CAN controllers,
>>>> e.g. found in Freescale i.MX35 SoCs.
>>>>
>>>> The original version of this driver was posted by Sascha Hauer in July 2009:
>>>> http://kerneltrap.org/mailarchive/linux-netdev/2009/7/29/6251621
>>>>
>>>> I took this version, added NAPI support, and fixed some problems found
>>>> during testing. Well, here is the result. Please review.
>>> I briefly browsed the patch and various bits and pieces are missing or
>>> not correctly implemented. Marc already pointed out a few of them:
>>>
>>> - I do not find can_put/get_echo_skb functions in the code. How is
>>> IFF_ECHO supposed to work?
>> the driver uses hardware loopback
>
> OK, then
>
> dev = alloc_candev(sizeof(struct flexcan_priv), 0);
>
> should be used (and TX_ECHO_SKB_MAX removed) in Hans-Jürgens driver.
>
>>> - Support for CAN_CTRLMODE_BERR_REPORTING and do_get_berr_counter()
>>> seems to be missing.
>>>
>>> - Make use of alloc_can_skb() and alloc_can_err_skb().
>> the last two points are already addressed in my version of the driver.
>
> I do not see support for CAN_CTRLMODE_BERR_REPORTING in your driver
> (which has nothing to do with do_get_berr_counter).
oh yes...sorry, got confused.
However I implemented CAN_CTRLMODE_BERR_REPORTING, i.e. turning of the
bit error interrupts by default. This has the effect that no bus warning
and bus passive interrupt was signalled.
I should add a comment to my driver.
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* [v3 Patch 1/2] s2io: add dynamic LRO disable support
From: Amerigo Wang @ 2010-06-18 10:55 UTC (permalink / raw)
To: netdev
Cc: nhorman, sgruszka, herbert.xu, Amerigo Wang, bhutchings,
Ramkrishna.Vepa, davem
This patch adds dynamic LRO diable support for s2io net driver.
I don't have s2io card, so only did compiling test. Anyone who wants
to test this is more than welcome.
This is based on Neil's initial work.
Signed-off-by: WANG Cong <amwang@redhat.com>
Signed-off-by: Neil Horman <nhorman@redhat.com>
Acked-by: Neil Horman <nhorman@redhat.com>
Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Ramkrishna Vepa <Ramkrishna.Vepa@exar.com>
---
diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 668327c..e380f37 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -795,7 +795,6 @@ static int init_shared_mem(struct s2io_nic *nic)
ring->rx_curr_put_info.ring_len = rx_cfg->num_rxd - 1;
ring->nic = nic;
ring->ring_no = i;
- ring->lro = lro_enable;
blk_cnt = rx_cfg->num_rxd / (rxd_count[nic->rxd_mode] + 1);
/* Allocating all the Rx blocks */
@@ -6684,6 +6683,41 @@ static int s2io_ethtool_op_set_tso(struct net_device *dev, u32 data)
return 0;
}
+static int s2io_ethtool_set_flags(struct net_device *dev, u32 data)
+{
+ struct s2io_nic *sp = netdev_priv(dev);
+ int rc = 0;
+ int changed = 0;
+
+ if (data & (ETH_FLAG_NTUPLE | ETH_FLAG_RXHASH))
+ return -EOPNOTSUPP;
+
+ if (data & ETH_FLAG_LRO) {
+ if (lro_enable) {
+ if (!(dev->features & NETIF_F_LRO)) {
+ dev->features |= NETIF_F_LRO;
+ changed = 1;
+ }
+ } else
+ rc = -EINVAL;
+ } else if (dev->features & NETIF_F_LRO) {
+ dev->features &= ~NETIF_F_LRO;
+ changed = 1;
+ }
+
+ if (changed && netif_running(dev)) {
+ s2io_stop_all_tx_queue(sp);
+ s2io_card_down(sp);
+ sp->lro = dev->features & NETIF_F_LRO;
+ rc = s2io_card_up(sp);
+ if (rc)
+ s2io_reset(sp);
+ else
+ s2io_start_all_tx_queue(sp);
+ }
+
+ return rc;
+}
static const struct ethtool_ops netdev_ethtool_ops = {
.get_settings = s2io_ethtool_gset,
@@ -6701,6 +6735,8 @@ static const struct ethtool_ops netdev_ethtool_ops = {
.get_rx_csum = s2io_ethtool_get_rx_csum,
.set_rx_csum = s2io_ethtool_set_rx_csum,
.set_tx_csum = s2io_ethtool_op_set_tx_csum,
+ .set_flags = s2io_ethtool_set_flags,
+ .get_flags = ethtool_op_get_flags,
.set_sg = ethtool_op_set_sg,
.get_tso = s2io_ethtool_op_get_tso,
.set_tso = s2io_ethtool_op_set_tso,
@@ -7229,6 +7265,7 @@ static int s2io_card_up(struct s2io_nic *sp)
struct ring_info *ring = &mac_control->rings[i];
ring->mtu = dev->mtu;
+ ring->lro = sp->lro;
ret = fill_rx_buffers(sp, ring, 1);
if (ret) {
DBG_PRINT(ERR_DBG, "%s: Out of memory in Open\n",
^ permalink raw reply related
* [v3 Patch 2/2] mlx4: add dynamic LRO disable support
From: Amerigo Wang @ 2010-06-18 10:55 UTC (permalink / raw)
To: netdev
Cc: nhorman, sgruszka, herbert.xu, Amerigo Wang, bhutchings,
Ramkrishna.Vepa, davem
In-Reply-To: <20100618105935.6496.4725.sendpatchset@localhost.localdomain>
This patch adds dynamic LRO diable support for mlx4 net driver.
It also fixes a bug of mlx4, which checks NETIF_F_LRO flag in rx
path without rtnl lock.
I don't have mlx4 card, so only did compiling test. Anyone who wants
to test this is more than welcome.
This is based on Neil's initial work too.
Signed-off-by: WANG Cong <amwang@redhat.com>
Signed-off-by: Neil Horman <nhorman@redhat.com>
Acked-by: Neil Horman <nhorman@redhat.com>
Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Ben Hutchings <bhutchings@solarflare.com>
---
diff --git a/drivers/net/mlx4/en_ethtool.c b/drivers/net/mlx4/en_ethtool.c
index d5afd03..d68b796 100644
--- a/drivers/net/mlx4/en_ethtool.c
+++ b/drivers/net/mlx4/en_ethtool.c
@@ -387,6 +387,41 @@ static void mlx4_en_get_ringparam(struct net_device *dev,
param->tx_pending = mdev->profile.prof[priv->port].tx_ring_size;
}
+static int mlx4_ethtool_op_set_flags(struct net_device *dev, u32 data)
+{
+ struct mlx4_en_priv *priv = netdev_priv(dev);
+ struct mlx4_en_dev *mdev = priv->mdev;
+ int rc = 0;
+ int changed = 0;
+
+ if (data & (ETH_FLAG_NTUPLE | ETH_FLAG_RXHASH))
+ return -EOPNOTSUPP;
+
+ if (data & ETH_FLAG_LRO) {
+ if (!(dev->features & NETIF_F_LRO))
+ changed = 1;
+ } else if (dev->features & NETIF_F_LRO) {
+ changed = 1;
+ mdev->profile.num_lro = 0;
+ }
+
+ if (changed) {
+ if (netif_running(dev)) {
+ mutex_lock(&mdev->state_lock);
+ mlx4_en_stop_port(dev);
+ }
+ dev->features ^= NETIF_F_LRO;
+ if (netif_running(dev)) {
+ rc = mlx4_en_start_port(dev);
+ if (rc)
+ en_err(priv, "Failed to restart port\n");
+ mutex_unlock(&mdev->state_lock);
+ }
+ }
+
+ return rc;
+}
+
const struct ethtool_ops mlx4_en_ethtool_ops = {
.get_drvinfo = mlx4_en_get_drvinfo,
.get_settings = mlx4_en_get_settings,
@@ -415,7 +450,7 @@ const struct ethtool_ops mlx4_en_ethtool_ops = {
.get_ringparam = mlx4_en_get_ringparam,
.set_ringparam = mlx4_en_set_ringparam,
.get_flags = ethtool_op_get_flags,
- .set_flags = ethtool_op_set_flags,
+ .set_flags = mlx4_ethtool_op_set_flags,
};
^ permalink raw reply related
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