* Re: [PATCH 3/4] net/fec: set phy_speed to the optimal frequency 2.5 MHz
From: Troy Kisky @ 2011-09-19 22:39 UTC (permalink / raw)
To: Shawn Guo; +Cc: netdev, David S. Miller, linux-arm-kernel, patches
In-Reply-To: <1316346852-17090-4-git-send-email-shawn.guo@linaro.org>
On 9/18/2011 4:54 AM, Shawn Guo wrote:
> With the unnecessary 1 bit left-shift on fep->phy_speed during the
> calculation, the phy_speed always runs at the half frequency of the
> optimal one 2.5 MHz.
>
> The patch removes that 1 bit left-shift to get the optimal phy_speed.
>
> Signed-off-by: Shawn Guo<shawn.guo@linaro.org>
> ---
> drivers/net/fec.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 5ef0e34..04206e4 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -1007,7 +1007,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
> /*
> * Set MII speed to 2.5 MHz (= clk_get_rate() / 2 * phy_speed)
> */
> - fep->phy_speed = DIV_ROUND_UP(clk_get_rate(fep->clk), 5000000)<< 1;
> + fep->phy_speed = DIV_ROUND_UP(clk_get_rate(fep->clk), 5000000);
> writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
>
> fep->mii_bus = mdiobus_alloc();
Do you need to round up to an even value? Is the hardware documentation
wrong?
Does this need a quirk? What boards has this been verified to fix?
Thanks
Troy
^ permalink raw reply
* Re: [PATCH] intel: Convert <FOO>_LENGTH_OF_ADDRESS to ETH_ALEN
From: Jeff Kirsher @ 2011-09-19 23:33 UTC (permalink / raw)
To: Joe Perches
Cc: Brandeburg, Jesse, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com
In-Reply-To: <1316471288.22881.3.camel@Joe-Laptop>
[-- Attachment #1: Type: text/plain, Size: 708 bytes --]
On Mon, 2011-09-19 at 15:28 -0700, Joe Perches wrote:
> Use the normal #defines not module specific ones.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> Here are the other, not ixgb uses of LENGTH_OF_ADDRESS
> converted to ETH_ALEN. Compile tested allyesconfig only.
>
> drivers/net/ethernet/intel/e1000/e1000_hw.h | 1 -
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 2 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 8 +++-----
> drivers/net/ethernet/intel/ixgbevf/defines.h | 1 -
> drivers/net/ethernet/intel/ixgbevf/vf.c | 4 ++--
> 5 files changed, 6 insertions(+), 10 deletions(-)
Thanks Joe. I have added the patch to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [net-next 11/13] igb: Make Tx budget for NAPI user adjustable
From: Jeff Kirsher @ 2011-09-19 23:36 UTC (permalink / raw)
To: Duyck, Alexander H
Cc: David Miller, bhutchings@solarflare.com, netdev@vger.kernel.org,
gospo@redhat.com
In-Reply-To: <4E77C1D4.503@intel.com>
[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]
On Mon, 2011-09-19 at 15:27 -0700, Duyck, Alexander H wrote:
> On 09/19/2011 02:00 PM, David Miller wrote:
> > From: Alexander Duyck<alexander.h.duyck@intel.com>
> > Date: Mon, 19 Sep 2011 09:32:18 -0700
> >> The fact is ixgbe has been using this parameter this way for over 2
> >> years now and the main goal of this patch was just to synchronize how
> >> things work on igb and ixgbe.
> >>
> >> Our hardware doesn't have a mechanism for firing an interrupt after X
> >> number of frames so instead we simply have modified things so that we
> >> will only process X number of frames and then fire another
> >> interrupt/poll if needed. As such we aren't that far out of
> >> compliance with the meaning of how this parameter is supposed to be
> >> used.
> > All I can say is this was a huge mistake you therefore need to revert
> > the IXGBE change, these ethtool settings are not for changing NAPI or
> > software interrupt behavior.
> >
> > And if you guys plan to be difficult on this and refuse to remove the
> > IXGBE bits, I'm letting you guys know ahead of time that I'll do it
> > for you.
> >
> > If the hardware can't support this facility, neither should these
> > ethtool hooks, because the whole point is to avoid hardware interrupts
> > from firing using these parameters.
> >
> > Propose new mechanisms to control NAPI behavior if you want.
> I'll remove the ixgbe code if that is what you want. It may be a month
> or so before I can get to it though since I am slammed with work so if
> you are in a hurry for it you might want to work with Jeff Kirsher to
> have the code removed.
Alex- I will work on this to resolve the issues the Ben and Dave have
pointed out.
>
> As far as this current patch goes I honestly don't have the time to add
> or rewrite yet another ethtool interface so I will probably just see
> about dropping the ethtool portion of this patch and update the
> description in order to make it acceptable.
>
> Thanks,
>
> Alex
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [net-next 11/13] igb: Make Tx budget for NAPI user adjustable
From: Stephen Hemminger @ 2011-09-19 23:42 UTC (permalink / raw)
To: jeffrey t kirsher
Cc: David Miller, bhutchings, netdev, gospo, Alexander H Duyck
In-Reply-To: <1316475390.2184.8.camel@jtkirshe-mobl>
I would like to see a general solution to allow configuring
napi weight. The Rx weight isn't easily configurable either.
Probably needs to be through ethtool callback since actual value range
and dev -> napi relationship is device specific.
^ permalink raw reply
* Re: [net-next 11/13] igb: Make Tx budget for NAPI user adjustable
From: David Miller @ 2011-09-19 23:47 UTC (permalink / raw)
To: stephen.hemminger
Cc: jeffrey.t.kirsher, bhutchings, netdev, gospo, alexander.h.duyck
In-Reply-To: <19fca01e-5827-4e28-b12c-2b65cca96878@tahiti.vyatta.com>
From: Stephen Hemminger <stephen.hemminger@vyatta.com>
Date: Mon, 19 Sep 2011 16:42:31 -0700 (PDT)
> I would like to see a general solution to allow configuring
> napi weight. The Rx weight isn't easily configurable either.
> Probably needs to be through ethtool callback since actual value range
> and dev -> napi relationship is device specific.
Agreed, it probably has to be per-queue too.
^ permalink raw reply
* Re: MSI-X vector count on 82754L
From: Jeff Kirsher @ 2011-09-19 23:56 UTC (permalink / raw)
To: Arnaud Lacombe; +Cc: Allan, Bruce W, netdev@vger.kernel.org, Vogel, Jack
In-Reply-To: <CACqU3MUfQn_r2oJ+ptuzYhkkUoucfL_NTMnHoCa0+TouP0s0kA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2229 bytes --]
On Sun, 2011-09-18 at 22:34 -0700, Arnaud Lacombe wrote:
> [fixing address of the netdev@ list, sorry - A.]
>
> On Mon, Sep 19, 2011 at 1:31 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> > Hi folks,
> >
> > In:
> >
> > commit 4662e82b2cb41c60826e50474dd86dd5c6372b0c
> > Author: Bruce Allan <bruce.w.allan@intel.com>
> > Date: Tue Aug 26 18:37:06 2008 -0700
> >
> > e1000e: add support for new 82574L part
> >
> > This new part has the same feature set as previous parts with the addition
> > of MSI-X support.
> >
> > Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> >
> > MSI-X support was added to the e1000e driver, later on slightly reworked by:
> >
> > commit 8e86acd7d5968e08b3e1604e685a8c45f6fd7f40
> > Author: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Date: Mon Aug 2 14:27:23 2010 +0000
> >
> > e1000e: Fix irq_synchronize in MSI-X case
> >
> > Based on original patch/work from Jean Delvare <jdelvare@suse.de>
> > Synchronize all IRQs when in MSI-X IRQ mode.
> >
> > Jean's original patch hard coded the sync with the 3 possible vectors,
> > this patch incorporates more flexibility for the future and aligns
> > with how igb stores the number of vectors into the adapter structure.
> >
> > CC: Jean Delvare <jdelvare@suse.de>
> > Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> > Acked-by: Bruce Allan <bruce.w.allan@intel.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > [It is unclear to what "Jean" does this commit log refers to, as the
> > support MSI-X was added Bruce, anyway...]
> >
> > Why only 3 vectors are used when both the product brief and the
> > datasheet advertise up to 5 MSI-X vector usable by the chip ? That is
> > 2*RX + 2*TX.
> >
> > Thanks in advance,
> > - Arnaud
> >
The e1000e driver is only a single queue driver so it does not need more
than 3 vectors, that is the reason we only use the 3 vectors.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] net: fix lockdep issue in __neigh_event_send
From: Maciej Żenczykowski @ 2011-09-20 0:01 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev, MuraliRaja Muniraju
In-Reply-To: <1316466207.2455.15.camel@edumazet-laptop>
> To my knowledge, no linux pristine linux kernel has this bug.
>
> dst_release() is lockless.
You are probably right there, I missed that this was caused by some
other old crappy code we've been trying to get rid of.
> So could you explain why its so important we add this 'bugfix' if it
> only applies to a Google kernel ?
Freeing the skb later seemed to be pretty painless...
> Anything I missed from my previous analysis ?
>
> Thanks !
Probably not.
Now that the dst cache is going away this probably doesn't really
matter either way.
Maciej
^ permalink raw reply
* Re: [net-next 01/11] ixgb: convert to ndo_fix_features
From: Jeff Kirsher @ 2011-09-20 0:02 UTC (permalink / raw)
To: Michał Mirosław
Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <20110917132153.GA32572@rere.qmqm.pl>
[-- Attachment #1: Type: text/plain, Size: 827 bytes --]
On Sat, 2011-09-17 at 06:21 -0700, Michał Mirosław wrote:
> On Fri, Sep 16, 2011 at 07:15:42PM -0700, Jeff Kirsher wrote:
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Private rx_csum flags are now duplicate of netdev->features &
> NETIF_F_RXCSUM.
> > Removing this needs deeper surgery.
> [...]
>
> Hmm. Looks like this patch is not complete: part removing
> get/set_flags
> from ethtool_ops is missing. I don't remember if this was included or
> not
> in the patch I sent originally. I'm attaching it now, rebased on
> current
> net-next (not tested).
>
> Best Regards,
> Michał Mirosław
>
> ---
>
> Finish conversion to unified ethtool ops: convert get_flags.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Thanks Michal, I have added the patch to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [net-next 11/13] igb: Make Tx budget for NAPI user adjustable
From: Ben Hutchings @ 2011-09-20 0:10 UTC (permalink / raw)
To: Stephen Hemminger
Cc: jeffrey t kirsher, David Miller, netdev, gospo, Alexander H Duyck
In-Reply-To: <19fca01e-5827-4e28-b12c-2b65cca96878@tahiti.vyatta.com>
On Mon, 2011-09-19 at 16:42 -0700, Stephen Hemminger wrote:
> I would like to see a general solution to allow configuring
> napi weight. The Rx weight isn't easily configurable either.
Indeed.
> Probably needs to be through ethtool callback since actual value range
> and dev -> napi relationship is device specific.
The maximum meaningful value is device specific but I'm not sure that
really matters.
And as David said it's really a many-to-one mapping of queue -> NAPI.
At netconf we talked about having 'irq' as an attribute of each queue
but maybe we should expose NAPI contexts through sysfs and make queues
refer to them instead. NAPI contexts would be named (in the same way as
the corresponding IRQ handlers) and have irq, weight, etc.
(Still short of time to work on this myself, alas.)
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
* Re: [PATCH v2 net-next] af_unix: dont send SCM_CREDENTIALS by default
From: Valdis.Kletnieks @ 2011-09-20 2:10 UTC (permalink / raw)
To: Tim Chen
Cc: Eric Dumazet, David Miller, zheng.z.yan, yanzheng, netdev, sfr,
jirislaby, sedat.dilek, alex.shi
In-Reply-To: <1316468398.4680.6.camel@schen9-DESK>
[-- Attachment #1: Type: text/plain, Size: 983 bytes --]
On Mon, 19 Sep 2011 14:39:58 PDT, Tim Chen said:
> Do we have to worry about the case where peer socket changes its flag
> to SOCK_PASSCRED while packets are in flight? If there isn't such
> pathological use case, the patch looks fine to me.
I wouldn't think so - if you're sending a packet, and retroactively trying to
change the flag and expect it to work, your program is too ugly to live. After
all, if the scheduler had cut off your timeslice and scheduledthe receiving
process before you set the flag, that packet would be delivered and done with
anyhow, and no amount of wishing will set that flag on an already-delivered
packet.
What *is* worth checking is that we DTRT if a process/thread is doing a send on
one CPU, and another process/thread with a shared file descriptor for that
socket is diddling the flag. But if we just define it as "atomic op to change
the flag and other observers get whatever value their CPU sees at that
instant", I'm OK with that too.. ;)
[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply
* Why not refresh ipv6 local link address of a bridge when a new interface added to the bridge?
From: Eric Xiong @ 2011-09-20 2:39 UTC (permalink / raw)
To: netdev
In-Reply-To: <CA+UXGK1XAgCxYUoOAP6yQSGhzDB-3p3oDqOoOc_e=rTT3DPx7Q@mail.gmail.com>
Hi all:
Sorry for disturb! when adding a interface to bridge, maybe the bridge
mac address will be changed. the bridge code don't invoke
call_netdevice_notifiers to notify ipv6 code to refresh local link
address. I don't know why. Thanks for your help!
BRs.
^ permalink raw reply
* Re: [PATCH 3/4] net/fec: set phy_speed to the optimal frequency 2.5 MHz
From: Shawn Guo @ 2011-09-20 2:57 UTC (permalink / raw)
To: Troy Kisky; +Cc: Shawn Guo, netdev, David S. Miller, linux-arm-kernel, patches
In-Reply-To: <4E77C4A2.1060802@boundarydevices.com>
On Mon, Sep 19, 2011 at 03:39:30PM -0700, Troy Kisky wrote:
> On 9/18/2011 4:54 AM, Shawn Guo wrote:
> >With the unnecessary 1 bit left-shift on fep->phy_speed during the
> >calculation, the phy_speed always runs at the half frequency of the
> >optimal one 2.5 MHz.
> >
> >The patch removes that 1 bit left-shift to get the optimal phy_speed.
> >
> >Signed-off-by: Shawn Guo<shawn.guo@linaro.org>
> >---
> > drivers/net/fec.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> >diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> >index 5ef0e34..04206e4 100644
> >--- a/drivers/net/fec.c
> >+++ b/drivers/net/fec.c
> >@@ -1007,7 +1007,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
> > /*
> > * Set MII speed to 2.5 MHz (= clk_get_rate() / 2 * phy_speed)
> > */
> >- fep->phy_speed = DIV_ROUND_UP(clk_get_rate(fep->clk), 5000000)<< 1;
> >+ fep->phy_speed = DIV_ROUND_UP(clk_get_rate(fep->clk), 5000000);
> > writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
> >
> > fep->mii_bus = mdiobus_alloc();
> Do you need to round up to an even value? Is the hardware
> documentation wrong?
The round up is something existed, and the patch does not touch that
part.
> Does this need a quirk? What boards has this been verified to fix?
>
I tested this on i.mx28, i.mx53 and i.mx6q. Do you see problem on
your platform?
--
Regards,
Shawn
^ permalink raw reply
* Re: pull request: wireless-next 2011-09-19
From: John W. Linville @ 2011-09-20 3:19 UTC (permalink / raw)
To: David Miller; +Cc: linux-wireless, netdev
In-Reply-To: <20110919.171124.1877822329573878893.davem@davemloft.net>
On Mon, Sep 19, 2011 at 05:11:24PM -0400, David Miller wrote:
> From: "John W. Linville" <linville@tuxdriver.com>
> Date: Mon, 19 Sep 2011 16:36:39 -0400
>
> > Here is another big batch of updates intended for 3.2 -- still
> > clearing-out the backlog...
> >
> > This round includes lots of updates to ath9k, b43, iwlagn, and rt2x00.
> > Some cleanups go to mac80211, along with a number of mesh-mode fixes
> > from Javier.
> >
> > Please let me know if there are problems!
>
> What about the iwl build failure reported by Eric Dumazet?
>
> If that's not handled here, I want that fixed before we add even
> more regressions :-)
It's there. :-)
commit ff620849110649b5f94989ddfd7a72b2bd43bd42
Author: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Date: Tue Sep 6 09:31:25 2011 -0700
iwlagn: fix compilation when debug flags is unset
Trivial fixes to allow compilation without warnings when debug
compilation flag isn't set.
Also fix the compilation when debugfs flag isn't set.
Fix a warning: unused priv pointer on the way.
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Greetings
From: Lee Shing @ 2011-09-20 3:58 UTC (permalink / raw)
Greetings,
I am Mr. Lee Shing, Principal Assurance manager for the Nanyang Commercial Bank of China. An investment was placed under my management eight years ago by a customer who is now deceased. I need your
assistance in investing these funds. If you're interested contact me via my private email lshing1515@yahoo.com.hk
If you're not interested in my offer do not respond to this email.Awaiting your urgent reply.
Thanks and my regards,
Lee Shing
^ permalink raw reply
* Re: [PATCH v2 net-next] af_unix: dont send SCM_CREDENTIALS by default
From: Eric Dumazet @ 2011-09-20 4:16 UTC (permalink / raw)
To: Valdis.Kletnieks
Cc: Tim Chen, David Miller, zheng.z.yan, yanzheng, netdev, sfr,
jirislaby, sedat.dilek, alex.shi
In-Reply-To: <6030.1316484637@turing-police.cc.vt.edu>
Le lundi 19 septembre 2011 à 22:10 -0400, Valdis.Kletnieks@vt.edu a
écrit :
> On Mon, 19 Sep 2011 14:39:58 PDT, Tim Chen said:
> > Do we have to worry about the case where peer socket changes its flag
> > to SOCK_PASSCRED while packets are in flight? If there isn't such
> > pathological use case, the patch looks fine to me.
>
> I wouldn't think so - if you're sending a packet, and retroactively trying to
> change the flag and expect it to work, your program is too ugly to live. After
> all, if the scheduler had cut off your timeslice and scheduledthe receiving
> process before you set the flag, that packet would be delivered and done with
> anyhow, and no amount of wishing will set that flag on an already-delivered
> packet.
>
> What *is* worth checking is that we DTRT if a process/thread is doing a send on
> one CPU, and another process/thread with a shared file descriptor for that
> socket is diddling the flag. But if we just define it as "atomic op to change
> the flag and other observers get whatever value their CPU sees at that
> instant", I'm OK with that too.. ;)
>
Note : The man page does states :
"To receive a struct ucred message the SO_PASSCRED option must be
enabled on the socket."
But it doesnt say if the SO_PASSCRED option must be enabled before the
sender sends its message, or before receiver attempts to read it.
Once a message is queued on an unix socket, flipping SO_PASSCRED cant
change its content (adding or removing credentials), since sender might
already have disappeared.
So current code includes credentials in all sent messages, just in case
receiver actually fetch credentials.
There are probably programs that assume they can set SO_PASSCRED right
before calling recvmsg(). Are we taking risk to break them, or are we
gentle and provide a sysctl option to ease the transition, I dont
know...
^ permalink raw reply
* [PATCH net-next] ipv6: fix a possible double free
From: rongqing.li @ 2011-09-20 5:52 UTC (permalink / raw)
To: netdev, eric.dumazet, stable
From: Roy.Li <rongqing.li@windriver.com>
When calling snmp6_alloc_dev fails, the snmp6 relevant memory
are freed by snmp6_alloc_dev. Calling in6_dev_finish_destroy
will free these memory twice.
Double free will lead that undefined behavior occurs.
Signed-off-by: Roy.Li <rongqing.li@windriver.com>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv6/addrconf.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f012ebd..12368c5 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -374,8 +374,8 @@ static struct inet6_dev * ipv6_add_dev(struct net_device *dev)
"%s(): cannot allocate memory for statistics; dev=%s.\n",
__func__, dev->name));
neigh_parms_release(&nd_tbl, ndev->nd_parms);
- ndev->dead = 1;
- in6_dev_finish_destroy(ndev);
+ dev_put(dev);
+ kfree(ndev);
return NULL;
}
--
1.7.1
^ permalink raw reply related
* Re: discrepancy in ip(7) wrt. IP DF flag for UDP sockets
From: Michael Kerrisk @ 2011-09-20 6:14 UTC (permalink / raw)
To: Neil Horman
Cc: Benjamin Poirier, linux-man-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110919130313.GA27819-B26myB8xz7F8NnZeBjwnZQMhkBWG/bsMQH7oEaQurus@public.gmane.org>
Hello Benjamin, Neil,
On Mon, Sep 19, 2011 at 3:03 PM, Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote:
> On Mon, Sep 19, 2011 at 08:19:40AM -0400, Benjamin Poirier wrote:
>> Hi,
>>
>> I noticed what appears to be a discrepancy between the ip(7) man page
>> and the kernel code with regards to the IP DF flag for UDP sockets.
>>
>> The man page says that "The don't-fragment flag is set on all outgoing
>> datagrams" and that the ip_no_pmtu_disc sysctl affects only SOCK_STREAM
>> sockets. This is quickly disproved by doing:
>> echo 1 > /proc/sys/net/ipv4/ip_no_pmtu_disc
>> firing up netcat and looking at a few outgoing udp packets in wireshark
>> (they don't have the DF flag set).
Could you describe the required change in terms of how the man page
text should look--i.e., rewrite the passage as you think it should
look?
Thanks,
Michael
>> 1) in the words of `man 7 ip`:
>> IP_MTU_DISCOVER (since Linux 2.2)
>> Set or receive the Path MTU Discovery setting for a socket.
>> When enabled, Linux will perform Path MTU Discovery as defined
>> in RFC 1191 on this socket. The don't-fragment flag is set on
>> all outgoing datagrams. The system-wide default is controlled
>> by the /proc/sys/net/ipv4/ip_no_pmtu_disc file for SOCK_STREAM
>> sockets, and disabled on all others.
>>
>> This is the text present in the latest version of the online manpages,
>> http://webcache.googleusercontent.com/search?q=cache:http://www.kernel.org/doc/man-pages/reporting_bugs.html&ie=UTF-8
>>
>> 2) in net/ipv4/af_inet.c:inet_create():
>> if (ipv4_config.no_pmtu_disc)
>> inet->pmtudisc = IP_PMTUDISC_DONT;
>> else
>> inet->pmtudisc = IP_PMTUDISC_WANT;
>>
>> and pmtudisc is left alone from there on for UDP sockets.
>>
>> What should be adjusted, the man page or the code?
>>
> The man page is wrong I think
>
> By my read, the code:
> 1) Affects UDP and TCP the same way (which makes sense to me)
>
> 2) Is doing exactly what you asked it to, since you set no_pmtu_disc, which
> means the stack should be free to fragment a frame as it sees fit according to
> the MTU metric of the route its traversing, hence the cleared DF bit in the
> fraem.
>
> RFC 1191 can apply equally well to udp, as tcp, and is evident in that you can
> set the per-socket option IP_MTU_DISCOVER to any of the 4 acceptible values
> offered (DONT/WANT/DO/PROBE), so theres no reason the sysctl governing the
> default value at creation shouldn't apply as well.
> Neil
>
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" 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: RFS issue: no HW filter for paused stream
From: Amir Vadai @ 2011-09-20 6:53 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Tom Herbert, oren, liranl, netdev, amirv, Diego Crupnicoff
In-Reply-To: <1316447572.2764.21.camel@bwh-desktop>
This will unset the current CPU of the rflow that belongs to the desired
CPU.
The problem is when the stream resumes and it goes to the wrong RXQ - in
our HW, it will be according to RSS, as long as there is no specific
flow steering rule for the stream.
We need to unset the current CPU of the rflow of the actual RXQ that the
packet arrived at:
diff --git a/net/core/dev.c b/net/core/dev.c
index 4b9981c..a6b3bc8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2685,6 +2685,12 @@ set_rps_cpu(struct net_device *dev, struct
sk_buff *skb,
rflow = &flow_table->flows[flow_id];
rflow->cpu = next_cpu;
rflow->filter = rc;
+ /* If this flow (or a flow with the same hash value) becomes
+ * active on the CPU as before, we want to restore the
+ * hardware filter. Unset the current CPU to ensure that
+ * set_rps_cpu() will be called then.
+ */
+ old_rflow->cpu = RPS_NO_CPU;
if (old_rflow->filter == rflow->filter)
old_rflow->filter = RPS_NO_FILTER;
out:
Or even better, not set it in the first place - but I'm not sure I
undersdtand the implications on RPS:
diff --git a/net/core/dev.c b/net/core/dev.c
index 4b9981c..748acdb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2654,7 +2654,7 @@ set_rps_cpu(struct net_device *dev, struct sk_buff
*skb,
{
u16 tcpu;
- tcpu = rflow->cpu = next_cpu;
+ tcpu = next_cpu;
if (tcpu != RPS_NO_CPU) {
#ifdef CONFIG_RFS_ACCEL
struct netdev_rx_queue *rxqueue;
- Amir
On 09/19/2011 06:52 PM, Ben Hutchings wrote:
> On Mon, 2011-09-19 at 08:13 -0700, Tom Herbert wrote:
>> Ben: Once a accel RFS flow expires (because flow is idle?), how
>> should it get re-instantiated if thread's CPU doesn't change?
>
> Good question.
>
>> Tom
>>
>> On Sun, Sep 18, 2011 at 11:05 PM, Amir Vadai<amirv@dev.mellanox.co.il> wrote:
>>> (Resending in plain text)
>>>
>>> Tom Hi,
>>> When a stream is paused, and its rule is expired while it is paused,
>>> no new rule will be configured to the HW when traffic resume.
>>>
>>> Scenario:
>>> 1. Start iperf.
>>> 2. Pause it using Ctrl-Z
>>> 3. Start another iperf (to make sure first stream rule is expired)
>>> 4. Stop the second stream.
>>> 5. Resume first stream. Traffic is not steered to the right rx-queue.
>>>
>>> From looking at the code:
>>> - When first stream started, RSS steered traffic to rx-queue 'x'.
>>> Because iperf server was running on a different CPU, a new rule was
>>> added and current-cpu was set to desired-cpu.
>>> - After paused, rule was expired and removed from HW by net driver.
>>> But current-cpu wasn't cleared and still is equal to desired-cpu.
>>> - When stream was resumed, traffic was steered again by RSS, and
>>> because current-cpu was equal to desired-cpu, ndo_rx_flow_steer
>>> wasn't called and no rule was configured to the HW.
>>>
>>> Why isn't current-cpu cleared when expiring a rule?
>
> Because I wrongly assumed that rules could be independently expired by
> the driver and the RPS/RFS core code.
>
> Try this (I haven't tested it myself yet):
>
> From: Ben Hutchings<bhutchings@solarflare.com>
> Date: Mon, 19 Sep 2011 16:44:13 +0100
> Subject: [PATCH net-next] RPS: When a hardware filter is expired, ensure it
> can be re-added later
>
> Amir Vadai wrote:
>> When a stream is paused, and its rule is expired while it is paused,
>> no new rule will be configured to the HW when traffic resume.
> [...]
>> - When stream was resumed, traffic was steered again by RSS, and
>> because current-cpu was equal to desired-cpu, ndo_rx_flow_steer
>> wasn't called and no rule was configured to the HW.
>>
>> Why isn't current-cpu cleared when expiring a rule?
>
> When rps_may_expire_flow() matches a filter to a flow that is found to
> be idle, unset the current CPU for that flow.
>
> Reported-by: Amir Vadai<amirv@dev.mellanox.co.il>
> ---
> net/core/dev.c | 21 ++++++++++++++++-----
> 1 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b2e262e..3caf65a 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2817,11 +2817,22 @@ bool rps_may_expire_flow(struct net_device *dev, u16 rxq_index,
> if (flow_table&& flow_id<= flow_table->mask) {
> rflow =&flow_table->flows[flow_id];
> cpu = ACCESS_ONCE(rflow->cpu);
> - if (rflow->filter == filter_id&& cpu != RPS_NO_CPU&&
> - ((int)(per_cpu(softnet_data, cpu).input_queue_head -
> - rflow->last_qtail)<
> - (int)(10 * flow_table->mask)))
> - expire = false;
> + if (rflow->filter == filter_id&& cpu != RPS_NO_CPU) {
> + if ((int)(per_cpu(softnet_data, cpu).input_queue_head -
> + rflow->last_qtail)<
> + (int)(10 * flow_table->mask)) {
> + expire = false;
> + } else {
> + /* If this flow (or a flow with the
> + * same hash value) becomes active
> + * on the CPU as before, we want to
> + * restore the hardware filter. Unset
> + * the current CPU to ensure that
> + * set_rps_cpu() will be called then.
> + */
> + rflow->cpu = RPS_NO_CPU;
> + }
> + }
> }
> rcu_read_unlock();
> return expire;
^ permalink raw reply related
* [net-next 0/9][pull request] Intel Wired LAN Driver Update
From: Jeff Kirsher @ 2011-09-20 7:11 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, gospo
The following series contains updates to igb only. This is a smaller
subset of an earlier push request I sent, minus the problem/dependent
patches. The missing patches are:
ixgb: eliminate checkstack warnings (Jesse has a v2 in test now)
igb: Make Tx budget for NAPI user adjustable (I am reworking)
these patches were not included as well since they were somewhat dependent
on the patch I am currently re-working.
igb: split buffer_info into tx_buffer_info and rx_buffer_info
igb: Consolidate creation of Tx context descriptors into a single
The following are changes since commit 765cf9976e937f1cfe9159bf4534967c8bf8eb6d:
tcp: md5: remove one indirection level in tcp_md5sig_pool
and are available in the git repository at:
git://github.com/Jkirsher/net-next.git
Alexander Duyck (9):
igb: Update RXDCTL/TXDCTL configurations
igb: Update max_frame_size to account for an optional VLAN tag if
present
igb: drop support for single buffer mode
igb: streamline Rx buffer allocation and cleanup
igb: update ring and adapter structure to improve performance
igb: Refactor clean_rx_irq to reduce overhead and improve performance
igb: drop the "adv" off function names relating to descriptors
igb: Replace E1000_XX_DESC_ADV with IGB_XX_DESC
igb: Remove multi_tx_table and simplify igb_xmit_frame
drivers/net/ethernet/intel/igb/igb.h | 117 +++---
drivers/net/ethernet/intel/igb/igb_ethtool.c | 15 +-
drivers/net/ethernet/intel/igb/igb_main.c | 524 ++++++++++++--------------
3 files changed, 311 insertions(+), 345 deletions(-)
--
1.7.6.2
^ permalink raw reply
* [net-next 1/9] igb: Update RXDCTL/TXDCTL configurations
From: Jeff Kirsher @ 2011-09-20 7:11 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, Jeff Kirsher
In-Reply-To: <1316502690-25488-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change cleans up the RXDCTL and TXDCTL configurations and optimizes RX
performance by allowing back write-backs on all hardware other than 82576.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 5 +++--
drivers/net/ethernet/intel/igb/igb_main.c | 23 +++++++++--------------
2 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 265e151..577fd3e 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -100,11 +100,12 @@ struct vf_data_storage {
*/
#define IGB_RX_PTHRESH 8
#define IGB_RX_HTHRESH 8
-#define IGB_RX_WTHRESH 1
#define IGB_TX_PTHRESH 8
#define IGB_TX_HTHRESH 1
+#define IGB_RX_WTHRESH ((hw->mac.type == e1000_82576 && \
+ adapter->msix_entries) ? 1 : 4)
#define IGB_TX_WTHRESH ((hw->mac.type == e1000_82576 && \
- adapter->msix_entries) ? 1 : 16)
+ adapter->msix_entries) ? 1 : 16)
/* this is the size past which hardware will drop packets when setting LPE=0 */
#define MAXIMUM_ETHERNET_VLAN_SIZE 1522
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 3cb1bc9..aa78c10 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2666,14 +2666,12 @@ void igb_configure_tx_ring(struct igb_adapter *adapter,
struct igb_ring *ring)
{
struct e1000_hw *hw = &adapter->hw;
- u32 txdctl;
+ u32 txdctl = 0;
u64 tdba = ring->dma;
int reg_idx = ring->reg_idx;
/* disable the queue */
- txdctl = rd32(E1000_TXDCTL(reg_idx));
- wr32(E1000_TXDCTL(reg_idx),
- txdctl & ~E1000_TXDCTL_QUEUE_ENABLE);
+ wr32(E1000_TXDCTL(reg_idx), 0);
wrfl();
mdelay(10);
@@ -2685,7 +2683,7 @@ void igb_configure_tx_ring(struct igb_adapter *adapter,
ring->head = hw->hw_addr + E1000_TDH(reg_idx);
ring->tail = hw->hw_addr + E1000_TDT(reg_idx);
- writel(0, ring->head);
+ wr32(E1000_TDH(reg_idx), 0);
writel(0, ring->tail);
txdctl |= IGB_TX_PTHRESH;
@@ -3028,12 +3026,10 @@ void igb_configure_rx_ring(struct igb_adapter *adapter,
struct e1000_hw *hw = &adapter->hw;
u64 rdba = ring->dma;
int reg_idx = ring->reg_idx;
- u32 srrctl, rxdctl;
+ u32 srrctl = 0, rxdctl = 0;
/* disable the queue */
- rxdctl = rd32(E1000_RXDCTL(reg_idx));
- wr32(E1000_RXDCTL(reg_idx),
- rxdctl & ~E1000_RXDCTL_QUEUE_ENABLE);
+ wr32(E1000_RXDCTL(reg_idx), 0);
/* Set DMA base address registers */
wr32(E1000_RDBAL(reg_idx),
@@ -3045,7 +3041,7 @@ void igb_configure_rx_ring(struct igb_adapter *adapter,
/* initialize head and tail */
ring->head = hw->hw_addr + E1000_RDH(reg_idx);
ring->tail = hw->hw_addr + E1000_RDT(reg_idx);
- writel(0, ring->head);
+ wr32(E1000_RDH(reg_idx), 0);
writel(0, ring->tail);
/* set descriptor configuration */
@@ -3076,13 +3072,12 @@ void igb_configure_rx_ring(struct igb_adapter *adapter,
/* set filtering for VMDQ pools */
igb_set_vmolr(adapter, reg_idx & 0x7, true);
- /* enable receive descriptor fetching */
- rxdctl = rd32(E1000_RXDCTL(reg_idx));
- rxdctl |= E1000_RXDCTL_QUEUE_ENABLE;
- rxdctl &= 0xFFF00000;
rxdctl |= IGB_RX_PTHRESH;
rxdctl |= IGB_RX_HTHRESH << 8;
rxdctl |= IGB_RX_WTHRESH << 16;
+
+ /* enable receive descriptor fetching */
+ rxdctl |= E1000_RXDCTL_QUEUE_ENABLE;
wr32(E1000_RXDCTL(reg_idx), rxdctl);
}
--
1.7.6.2
^ permalink raw reply related
* [net-next 2/9] igb: Update max_frame_size to account for an optional VLAN tag if present
From: Jeff Kirsher @ 2011-09-20 7:11 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, Jeff Kirsher
In-Reply-To: <1316502690-25488-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This patch modifies the max_frame_size in order account for an optional
VLAN tag. In order to support this we must also increase the
MAX_STD_JUMBO_FRAME_SIZE to account for the 4 extra bytes.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 2 --
drivers/net/ethernet/intel/igb/igb_main.c | 19 ++++++++++++-------
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 577fd3e..8e90b85 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -117,8 +117,6 @@ struct vf_data_storage {
#define IGB_RXBUFFER_2048 2048
#define IGB_RXBUFFER_16384 16384
-#define MAX_STD_JUMBO_FRAME_SIZE 9234
-
/* How many Tx Descriptors do we need to call netif_wake_queue ? */
#define IGB_TX_QUEUE_WAKE 16
/* How many Rx Buffers do we bundle into one write to the hardware ? */
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index aa78c10..6156275 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2396,7 +2396,8 @@ static int __devinit igb_sw_init(struct igb_adapter *adapter)
adapter->rx_itr_setting = IGB_DEFAULT_ITR;
adapter->tx_itr_setting = IGB_DEFAULT_ITR;
- adapter->max_frame_size = netdev->mtu + ETH_HLEN + ETH_FCS_LEN;
+ adapter->max_frame_size = netdev->mtu + ETH_HLEN + ETH_FCS_LEN +
+ VLAN_HLEN;
adapter->min_frame_size = ETH_ZLEN + ETH_FCS_LEN;
spin_lock_init(&adapter->stats64_lock);
@@ -2962,16 +2963,19 @@ static inline int igb_set_vf_rlpml(struct igb_adapter *adapter, int size,
**/
static void igb_rlpml_set(struct igb_adapter *adapter)
{
- u32 max_frame_size;
+ u32 max_frame_size = adapter->max_frame_size;
struct e1000_hw *hw = &adapter->hw;
u16 pf_id = adapter->vfs_allocated_count;
- max_frame_size = adapter->max_frame_size + VLAN_TAG_SIZE;
-
- /* if vfs are enabled we set RLPML to the largest possible request
- * size and set the VMOLR RLPML to the size we need */
if (pf_id) {
igb_set_vf_rlpml(adapter, max_frame_size, pf_id);
+ /*
+ * If we're in VMDQ or SR-IOV mode, then set global RLPML
+ * to our max jumbo frame size, in case we need to enable
+ * jumbo frames on one of the rings later.
+ * This will not pass over-length frames into the default
+ * queue because it's gated by the VMOLR.RLPML.
+ */
max_frame_size = MAX_JUMBO_FRAME_SIZE;
}
@@ -4461,7 +4465,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
{
struct igb_adapter *adapter = netdev_priv(netdev);
struct pci_dev *pdev = adapter->pdev;
- int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
+ int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
u32 rx_buffer_len, i;
if ((new_mtu < 68) || (max_frame > MAX_JUMBO_FRAME_SIZE)) {
@@ -4469,6 +4473,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
return -EINVAL;
}
+#define MAX_STD_JUMBO_FRAME_SIZE 9238
if (max_frame > MAX_STD_JUMBO_FRAME_SIZE) {
dev_err(&pdev->dev, "MTU > 9216 not supported.\n");
return -EINVAL;
--
1.7.6.2
^ permalink raw reply related
* [net-next 3/9] igb: drop support for single buffer mode
From: Jeff Kirsher @ 2011-09-20 7:11 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, Jeff Kirsher
In-Reply-To: <1316502690-25488-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change removes support for single buffer mode from igb and makes the
driver function in packet split always. The advantage to doing this is
that we can reduce total memory allocation overhead significantly as we
will only need to allocate one 1K slab per packet and then make use of a
reusable half page instead of allocating a 2K slab per packet.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 7 +-
drivers/net/ethernet/intel/igb/igb_ethtool.c | 5 +-
drivers/net/ethernet/intel/igb/igb_main.c | 102 ++++++--------------------
3 files changed, 28 insertions(+), 86 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 8e90b85..50632b1 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -111,11 +111,9 @@ struct vf_data_storage {
#define MAXIMUM_ETHERNET_VLAN_SIZE 1522
/* Supported Rx Buffer Sizes */
-#define IGB_RXBUFFER_64 64 /* Used for packet split */
-#define IGB_RXBUFFER_128 128 /* Used for packet split */
-#define IGB_RXBUFFER_1024 1024
-#define IGB_RXBUFFER_2048 2048
+#define IGB_RXBUFFER_512 512
#define IGB_RXBUFFER_16384 16384
+#define IGB_RX_HDR_LEN IGB_RXBUFFER_512
/* How many Tx Descriptors do we need to call netif_wake_queue ? */
#define IGB_TX_QUEUE_WAKE 16
@@ -221,7 +219,6 @@ struct igb_ring {
struct {
struct igb_rx_queue_stats rx_stats;
struct u64_stats_sync rx_syncp;
- u32 rx_buffer_len;
};
};
};
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 414b022..04bc7a5 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -1368,7 +1368,6 @@ static int igb_setup_desc_rings(struct igb_adapter *adapter)
rx_ring->count = IGB_DEFAULT_RXD;
rx_ring->dev = &adapter->pdev->dev;
rx_ring->netdev = adapter->netdev;
- rx_ring->rx_buffer_len = IGB_RXBUFFER_2048;
rx_ring->reg_idx = adapter->vfs_allocated_count;
if (igb_setup_rx_resources(rx_ring)) {
@@ -1597,7 +1596,7 @@ static int igb_clean_test_rings(struct igb_ring *rx_ring,
/* unmap rx buffer, will be remapped by alloc_rx_buffers */
dma_unmap_single(rx_ring->dev,
buffer_info->dma,
- rx_ring->rx_buffer_len,
+ IGB_RX_HDR_LEN,
DMA_FROM_DEVICE);
buffer_info->dma = 0;
@@ -1635,7 +1634,7 @@ static int igb_run_loopback_test(struct igb_adapter *adapter)
struct igb_ring *tx_ring = &adapter->test_tx_ring;
struct igb_ring *rx_ring = &adapter->test_rx_ring;
int i, j, lc, good_cnt, ret_val = 0;
- unsigned int size = 1024;
+ unsigned int size = IGB_RX_HDR_LEN;
netdev_tx_t tx_ret_val;
struct sk_buff *skb;
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 6156275..022c442 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -517,16 +517,14 @@ rx_ring_summary:
DUMP_PREFIX_ADDRESS,
16, 1,
phys_to_virt(buffer_info->dma),
- rx_ring->rx_buffer_len, true);
- if (rx_ring->rx_buffer_len
- < IGB_RXBUFFER_1024)
- print_hex_dump(KERN_INFO, "",
- DUMP_PREFIX_ADDRESS,
- 16, 1,
- phys_to_virt(
- buffer_info->page_dma +
- buffer_info->page_offset),
- PAGE_SIZE/2, true);
+ IGB_RX_HDR_LEN, true);
+ print_hex_dump(KERN_INFO, "",
+ DUMP_PREFIX_ADDRESS,
+ 16, 1,
+ phys_to_virt(
+ buffer_info->page_dma +
+ buffer_info->page_offset),
+ PAGE_SIZE/2, true);
}
}
@@ -707,7 +705,6 @@ static int igb_alloc_queues(struct igb_adapter *adapter)
ring->queue_index = i;
ring->dev = &adapter->pdev->dev;
ring->netdev = adapter->netdev;
- ring->rx_buffer_len = MAXIMUM_ETHERNET_VLAN_SIZE;
ring->flags = IGB_RING_FLAG_RX_CSUM; /* enable rx checksum */
/* set flag indicating ring supports SCTP checksum offload */
if (adapter->hw.mac.type >= e1000_82576)
@@ -3049,22 +3046,13 @@ void igb_configure_rx_ring(struct igb_adapter *adapter,
writel(0, ring->tail);
/* set descriptor configuration */
- if (ring->rx_buffer_len < IGB_RXBUFFER_1024) {
- srrctl = ALIGN(ring->rx_buffer_len, 64) <<
- E1000_SRRCTL_BSIZEHDRSIZE_SHIFT;
+ srrctl = IGB_RX_HDR_LEN << E1000_SRRCTL_BSIZEHDRSIZE_SHIFT;
#if (PAGE_SIZE / 2) > IGB_RXBUFFER_16384
- srrctl |= IGB_RXBUFFER_16384 >>
- E1000_SRRCTL_BSIZEPKT_SHIFT;
+ srrctl |= IGB_RXBUFFER_16384 >> E1000_SRRCTL_BSIZEPKT_SHIFT;
#else
- srrctl |= (PAGE_SIZE / 2) >>
- E1000_SRRCTL_BSIZEPKT_SHIFT;
+ srrctl |= (PAGE_SIZE / 2) >> E1000_SRRCTL_BSIZEPKT_SHIFT;
#endif
- srrctl |= E1000_SRRCTL_DESCTYPE_HDR_SPLIT_ALWAYS;
- } else {
- srrctl = ALIGN(ring->rx_buffer_len, 1024) >>
- E1000_SRRCTL_BSIZEPKT_SHIFT;
- srrctl |= E1000_SRRCTL_DESCTYPE_ADV_ONEBUF;
- }
+ srrctl |= E1000_SRRCTL_DESCTYPE_HDR_SPLIT_ALWAYS;
if (hw->mac.type == e1000_82580)
srrctl |= E1000_SRRCTL_TIMESTAMP;
/* Only set Drop Enable if we are supporting multiple queues */
@@ -3268,7 +3256,7 @@ static void igb_clean_rx_ring(struct igb_ring *rx_ring)
if (buffer_info->dma) {
dma_unmap_single(rx_ring->dev,
buffer_info->dma,
- rx_ring->rx_buffer_len,
+ IGB_RX_HDR_LEN,
DMA_FROM_DEVICE);
buffer_info->dma = 0;
}
@@ -4466,7 +4454,6 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
struct igb_adapter *adapter = netdev_priv(netdev);
struct pci_dev *pdev = adapter->pdev;
int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
- u32 rx_buffer_len, i;
if ((new_mtu < 68) || (max_frame > MAX_JUMBO_FRAME_SIZE)) {
dev_err(&pdev->dev, "Invalid MTU setting\n");
@@ -4485,30 +4472,6 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
/* igb_down has a dependency on max_frame_size */
adapter->max_frame_size = max_frame;
- /* NOTE: netdev_alloc_skb reserves 16 bytes, and typically NET_IP_ALIGN
- * means we reserve 2 more, this pushes us to allocate from the next
- * larger slab size.
- * i.e. RXBUFFER_2048 --> size-4096 slab
- */
-
- if (adapter->hw.mac.type == e1000_82580)
- max_frame += IGB_TS_HDR_LEN;
-
- if (max_frame <= IGB_RXBUFFER_1024)
- rx_buffer_len = IGB_RXBUFFER_1024;
- else if (max_frame <= MAXIMUM_ETHERNET_VLAN_SIZE)
- rx_buffer_len = MAXIMUM_ETHERNET_VLAN_SIZE;
- else
- rx_buffer_len = IGB_RXBUFFER_128;
-
- if ((max_frame == ETH_FRAME_LEN + ETH_FCS_LEN + IGB_TS_HDR_LEN) ||
- (max_frame == MAXIMUM_ETHERNET_VLAN_SIZE + IGB_TS_HDR_LEN))
- rx_buffer_len = MAXIMUM_ETHERNET_VLAN_SIZE + IGB_TS_HDR_LEN;
-
- if ((adapter->hw.mac.type == e1000_82580) &&
- (rx_buffer_len == IGB_RXBUFFER_128))
- rx_buffer_len += IGB_RXBUFFER_64;
-
if (netif_running(netdev))
igb_down(adapter);
@@ -4516,9 +4479,6 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
netdev->mtu, new_mtu);
netdev->mtu = new_mtu;
- for (i = 0; i < adapter->num_rx_queues; i++)
- adapter->rx_ring[i]->rx_buffer_len = rx_buffer_len;
-
if (netif_running(netdev))
igb_up(adapter);
else
@@ -5781,8 +5741,7 @@ static void igb_rx_hwtstamp(struct igb_q_vector *q_vector, u32 staterr,
igb_systim_to_hwtstamp(adapter, skb_hwtstamps(skb), regval);
}
-static inline u16 igb_get_hlen(struct igb_ring *rx_ring,
- union e1000_adv_rx_desc *rx_desc)
+static inline u16 igb_get_hlen(union e1000_adv_rx_desc *rx_desc)
{
/* HW will not DMA in data larger than the given buffer, even if it
* parses the (NFS, of course) header to be larger. In that case, it
@@ -5790,8 +5749,8 @@ static inline u16 igb_get_hlen(struct igb_ring *rx_ring,
*/
u16 hlen = (le16_to_cpu(rx_desc->wb.lower.lo_dword.hdr_info) &
E1000_RXDADV_HDRBUFLEN_MASK) >> E1000_RXDADV_HDRBUFLEN_SHIFT;
- if (hlen > rx_ring->rx_buffer_len)
- hlen = rx_ring->rx_buffer_len;
+ if (hlen > IGB_RX_HDR_LEN)
+ hlen = IGB_RX_HDR_LEN;
return hlen;
}
@@ -5841,14 +5800,10 @@ static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector,
if (buffer_info->dma) {
dma_unmap_single(dev, buffer_info->dma,
- rx_ring->rx_buffer_len,
+ IGB_RX_HDR_LEN,
DMA_FROM_DEVICE);
buffer_info->dma = 0;
- if (rx_ring->rx_buffer_len >= IGB_RXBUFFER_1024) {
- skb_put(skb, length);
- goto send_up;
- }
- skb_put(skb, igb_get_hlen(rx_ring, rx_desc));
+ skb_put(skb, igb_get_hlen(rx_desc));
}
if (length) {
@@ -5879,7 +5834,7 @@ static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector,
next_buffer->dma = 0;
goto next_desc;
}
-send_up:
+
if (staterr & E1000_RXDEXT_ERR_FRAME_ERR_MASK) {
dev_kfree_skb_irq(skb);
goto next_desc;
@@ -5943,17 +5898,14 @@ void igb_alloc_rx_buffers_adv(struct igb_ring *rx_ring, int cleaned_count)
struct igb_buffer *buffer_info;
struct sk_buff *skb;
unsigned int i;
- int bufsz;
i = rx_ring->next_to_use;
buffer_info = &rx_ring->buffer_info[i];
- bufsz = rx_ring->rx_buffer_len;
-
while (cleaned_count--) {
rx_desc = E1000_RX_DESC_ADV(*rx_ring, i);
- if ((bufsz < IGB_RXBUFFER_1024) && !buffer_info->page_dma) {
+ if (!buffer_info->page_dma) {
if (!buffer_info->page) {
buffer_info->page = netdev_alloc_page(netdev);
if (unlikely(!buffer_info->page)) {
@@ -5983,7 +5935,7 @@ void igb_alloc_rx_buffers_adv(struct igb_ring *rx_ring, int cleaned_count)
skb = buffer_info->skb;
if (!skb) {
- skb = netdev_alloc_skb_ip_align(netdev, bufsz);
+ skb = netdev_alloc_skb_ip_align(netdev, IGB_RX_HDR_LEN);
if (unlikely(!skb)) {
u64_stats_update_begin(&rx_ring->rx_syncp);
rx_ring->rx_stats.alloc_failed++;
@@ -5996,7 +5948,7 @@ void igb_alloc_rx_buffers_adv(struct igb_ring *rx_ring, int cleaned_count)
if (!buffer_info->dma) {
buffer_info->dma = dma_map_single(rx_ring->dev,
skb->data,
- bufsz,
+ IGB_RX_HDR_LEN,
DMA_FROM_DEVICE);
if (dma_mapping_error(rx_ring->dev,
buffer_info->dma)) {
@@ -6009,14 +5961,8 @@ void igb_alloc_rx_buffers_adv(struct igb_ring *rx_ring, int cleaned_count)
}
/* Refresh the desc even if buffer_addrs didn't change because
* each write-back erases this info. */
- if (bufsz < IGB_RXBUFFER_1024) {
- rx_desc->read.pkt_addr =
- cpu_to_le64(buffer_info->page_dma);
- rx_desc->read.hdr_addr = cpu_to_le64(buffer_info->dma);
- } else {
- rx_desc->read.pkt_addr = cpu_to_le64(buffer_info->dma);
- rx_desc->read.hdr_addr = 0;
- }
+ rx_desc->read.pkt_addr = cpu_to_le64(buffer_info->page_dma);
+ rx_desc->read.hdr_addr = cpu_to_le64(buffer_info->dma);
i++;
if (i == rx_ring->count)
--
1.7.6.2
^ permalink raw reply related
* [net-next 5/9] igb: update ring and adapter structure to improve performance
From: Jeff Kirsher @ 2011-09-20 7:11 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, Jeff Kirsher
In-Reply-To: <1316502690-25488-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change is meant to improve performance by splitting the Tx and Rx
rings into 3 sections. The first is primarily a read only section
containing basic things like the indexes, a pointer to the dev and netdev
structures, and basic information. The second section contains the stats
and next_to_use and next_to_clean values. The third section is primarily
unused values that can just be placed at the end of the ring and are not
used in the hot path.
The adapter structure has several sections that are read in the hot path.
In order to improve performance there I am combining the frequent read
hot path items into a single cache line.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 83 ++++++++++++++++-------------
drivers/net/ethernet/intel/igb/igb_main.c | 4 +-
2 files changed, 46 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index b2f2a8c..7036fd5 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -187,26 +187,26 @@ struct igb_q_vector {
};
struct igb_ring {
- struct igb_q_vector *q_vector; /* backlink to q_vector */
- struct net_device *netdev; /* back pointer to net_device */
- struct device *dev; /* device pointer for dma mapping */
- dma_addr_t dma; /* phys address of the ring */
- void *desc; /* descriptor ring memory */
- unsigned int size; /* length of desc. ring in bytes */
- u16 count; /* number of desc. in the ring */
+ struct igb_q_vector *q_vector; /* backlink to q_vector */
+ struct net_device *netdev; /* back pointer to net_device */
+ struct device *dev; /* device pointer for dma mapping */
+ struct igb_buffer *buffer_info; /* array of buffer info structs */
+ void *desc; /* descriptor ring memory */
+ unsigned long flags; /* ring specific flags */
+ void __iomem *tail; /* pointer to ring tail register */
+
+ u16 count; /* number of desc. in the ring */
+ u8 queue_index; /* logical index of the ring*/
+ u8 reg_idx; /* physical index of the ring */
+ u32 size; /* length of desc. ring in bytes */
+
+ /* everything past this point are written often */
+ u16 next_to_clean ____cacheline_aligned_in_smp;
u16 next_to_use;
- u16 next_to_clean;
- u8 queue_index;
- u8 reg_idx;
- void __iomem *head;
- void __iomem *tail;
- struct igb_buffer *buffer_info; /* array of buffer info structs */
unsigned int total_bytes;
unsigned int total_packets;
- u32 flags;
-
union {
/* TX */
struct {
@@ -221,6 +221,8 @@ struct igb_ring {
struct u64_stats_sync rx_syncp;
};
};
+ /* Items past this point are only used during ring alloc / free */
+ dma_addr_t dma; /* phys address of the ring */
};
#define IGB_RING_FLAG_RX_CSUM 0x00000001 /* RX CSUM enabled */
@@ -248,15 +250,15 @@ static inline int igb_desc_unused(struct igb_ring *ring)
/* board specific private data structure */
struct igb_adapter {
- struct timer_list watchdog_timer;
- struct timer_list phy_info_timer;
unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
- u16 mng_vlan_id;
- u32 bd_number;
- u32 wol;
- u32 en_mng_pt;
- u16 link_speed;
- u16 link_duplex;
+
+ struct net_device *netdev;
+
+ unsigned long state;
+ unsigned int flags;
+
+ unsigned int num_q_vectors;
+ struct msix_entry *msix_entries;
/* Interrupt Throttle Rate */
u32 rx_itr_setting;
@@ -264,27 +266,36 @@ struct igb_adapter {
u16 tx_itr;
u16 rx_itr;
- struct work_struct reset_task;
- struct work_struct watchdog_task;
- bool fc_autoneg;
- u8 tx_timeout_factor;
- struct timer_list blink_timer;
- unsigned long led_status;
-
/* TX */
- struct igb_ring *tx_ring[16];
u32 tx_timeout_count;
+ int num_tx_queues;
+ struct igb_ring *tx_ring[16];
/* RX */
- struct igb_ring *rx_ring[16];
- int num_tx_queues;
int num_rx_queues;
+ struct igb_ring *rx_ring[16];
u32 max_frame_size;
u32 min_frame_size;
+ struct timer_list watchdog_timer;
+ struct timer_list phy_info_timer;
+
+ u16 mng_vlan_id;
+ u32 bd_number;
+ u32 wol;
+ u32 en_mng_pt;
+ u16 link_speed;
+ u16 link_duplex;
+
+ struct work_struct reset_task;
+ struct work_struct watchdog_task;
+ bool fc_autoneg;
+ u8 tx_timeout_factor;
+ struct timer_list blink_timer;
+ unsigned long led_status;
+
/* OS defined structs */
- struct net_device *netdev;
struct pci_dev *pdev;
struct cyclecounter cycles;
struct timecounter clock;
@@ -306,15 +317,11 @@ struct igb_adapter {
int msg_enable;
- unsigned int num_q_vectors;
struct igb_q_vector *q_vector[MAX_Q_VECTORS];
- struct msix_entry *msix_entries;
u32 eims_enable_mask;
u32 eims_other;
/* to not mess up cache alignment, always add to the bottom */
- unsigned long state;
- unsigned int flags;
u32 eeprom_wol;
struct igb_ring *multi_tx_table[IGB_ABS_MAX_TX_QUEUES];
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index af8c2f7..9fa2ad0 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2679,7 +2679,6 @@ void igb_configure_tx_ring(struct igb_adapter *adapter,
tdba & 0x00000000ffffffffULL);
wr32(E1000_TDBAH(reg_idx), tdba >> 32);
- ring->head = hw->hw_addr + E1000_TDH(reg_idx);
ring->tail = hw->hw_addr + E1000_TDT(reg_idx);
wr32(E1000_TDH(reg_idx), 0);
writel(0, ring->tail);
@@ -3040,7 +3039,6 @@ void igb_configure_rx_ring(struct igb_adapter *adapter,
ring->count * sizeof(union e1000_adv_rx_desc));
/* initialize head and tail */
- ring->head = hw->hw_addr + E1000_RDH(reg_idx);
ring->tail = hw->hw_addr + E1000_RDT(reg_idx);
wr32(E1000_RDH(reg_idx), 0);
writel(0, ring->tail);
@@ -5653,7 +5651,7 @@ static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
" jiffies <%lx>\n"
" desc.status <%x>\n",
tx_ring->queue_index,
- readl(tx_ring->head),
+ rd32(E1000_TDH(tx_ring->reg_idx)),
readl(tx_ring->tail),
tx_ring->next_to_use,
tx_ring->next_to_clean,
--
1.7.6.2
^ permalink raw reply related
* [net-next 4/9] igb: streamline Rx buffer allocation and cleanup
From: Jeff Kirsher @ 2011-09-20 7:11 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, Jeff Kirsher
In-Reply-To: <1316502690-25488-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change is meant to streamline the Rx buffer allocation and cleanup.
This is accomplished by reducing the number of writes by only having the Rx
descriptor ring written by software during allocation, and it will only be
read during cleanup.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 2 +-
drivers/net/ethernet/intel/igb/igb_main.c | 190 ++++++++++++++++-------------
2 files changed, 104 insertions(+), 88 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 50632b1..b2f2a8c 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -370,7 +370,7 @@ extern void igb_setup_rctl(struct igb_adapter *);
extern netdev_tx_t igb_xmit_frame_ring_adv(struct sk_buff *, struct igb_ring *);
extern void igb_unmap_and_free_tx_resource(struct igb_ring *,
struct igb_buffer *);
-extern void igb_alloc_rx_buffers_adv(struct igb_ring *, int);
+extern void igb_alloc_rx_buffers_adv(struct igb_ring *, u16);
extern void igb_update_stats(struct igb_adapter *, struct rtnl_link_stats64 *);
extern bool igb_has_link(struct igb_adapter *adapter);
extern void igb_set_ethtool_ops(struct net_device *);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 022c442..af8c2f7 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3243,16 +3243,15 @@ static void igb_free_all_rx_resources(struct igb_adapter *adapter)
**/
static void igb_clean_rx_ring(struct igb_ring *rx_ring)
{
- struct igb_buffer *buffer_info;
unsigned long size;
- unsigned int i;
+ u16 i;
if (!rx_ring->buffer_info)
return;
/* Free all the Rx ring sk_buffs */
for (i = 0; i < rx_ring->count; i++) {
- buffer_info = &rx_ring->buffer_info[i];
+ struct igb_buffer *buffer_info = &rx_ring->buffer_info[i];
if (buffer_info->dma) {
dma_unmap_single(rx_ring->dev,
buffer_info->dma,
@@ -5764,7 +5763,7 @@ static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector,
struct igb_buffer *buffer_info , *next_buffer;
struct sk_buff *skb;
bool cleaned = false;
- int cleaned_count = 0;
+ u16 cleaned_count = igb_desc_unused(rx_ring);
int current_node = numa_node_id();
unsigned int total_bytes = 0, total_packets = 0;
unsigned int i;
@@ -5848,7 +5847,6 @@ static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector,
igb_rx_checksum_adv(rx_ring, staterr, skb);
skb->protocol = eth_type_trans(skb, netdev);
- skb_record_rx_queue(skb, rx_ring->queue_index);
if (staterr & E1000_RXD_STAT_VP) {
u16 vid = le16_to_cpu(rx_desc->wb.upper.vlan);
@@ -5858,8 +5856,6 @@ static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector,
napi_gro_receive(&q_vector->napi, skb);
next_desc:
- rx_desc->wb.upper.status_error = 0;
-
/* return some buffers to hardware, one at a time is too slow */
if (cleaned_count >= IGB_RX_BUFFER_WRITE) {
igb_alloc_rx_buffers_adv(rx_ring, cleaned_count);
@@ -5873,110 +5869,130 @@ next_desc:
}
rx_ring->next_to_clean = i;
- cleaned_count = igb_desc_unused(rx_ring);
-
- if (cleaned_count)
- igb_alloc_rx_buffers_adv(rx_ring, cleaned_count);
-
- rx_ring->total_packets += total_packets;
- rx_ring->total_bytes += total_bytes;
u64_stats_update_begin(&rx_ring->rx_syncp);
rx_ring->rx_stats.packets += total_packets;
rx_ring->rx_stats.bytes += total_bytes;
u64_stats_update_end(&rx_ring->rx_syncp);
+ rx_ring->total_packets += total_packets;
+ rx_ring->total_bytes += total_bytes;
+
+ if (cleaned_count)
+ igb_alloc_rx_buffers_adv(rx_ring, cleaned_count);
+
return cleaned;
}
+static bool igb_alloc_mapped_skb(struct igb_ring *rx_ring,
+ struct igb_buffer *bi)
+{
+ struct sk_buff *skb = bi->skb;
+ dma_addr_t dma = bi->dma;
+
+ if (dma)
+ return true;
+
+ if (likely(!skb)) {
+ skb = netdev_alloc_skb_ip_align(rx_ring->netdev,
+ IGB_RX_HDR_LEN);
+ bi->skb = skb;
+ if (!skb) {
+ rx_ring->rx_stats.alloc_failed++;
+ return false;
+ }
+
+ /* initialize skb for ring */
+ skb_record_rx_queue(skb, rx_ring->queue_index);
+ }
+
+ dma = dma_map_single(rx_ring->dev, skb->data,
+ IGB_RX_HDR_LEN, DMA_FROM_DEVICE);
+
+ if (dma_mapping_error(rx_ring->dev, dma)) {
+ rx_ring->rx_stats.alloc_failed++;
+ return false;
+ }
+
+ bi->dma = dma;
+ return true;
+}
+
+static bool igb_alloc_mapped_page(struct igb_ring *rx_ring,
+ struct igb_buffer *bi)
+{
+ struct page *page = bi->page;
+ dma_addr_t page_dma = bi->page_dma;
+ unsigned int page_offset = bi->page_offset ^ (PAGE_SIZE / 2);
+
+ if (page_dma)
+ return true;
+
+ if (!page) {
+ page = netdev_alloc_page(rx_ring->netdev);
+ bi->page = page;
+ if (unlikely(!page)) {
+ rx_ring->rx_stats.alloc_failed++;
+ return false;
+ }
+ }
+
+ page_dma = dma_map_page(rx_ring->dev, page,
+ page_offset, PAGE_SIZE / 2,
+ DMA_FROM_DEVICE);
+
+ if (dma_mapping_error(rx_ring->dev, page_dma)) {
+ rx_ring->rx_stats.alloc_failed++;
+ return false;
+ }
+
+ bi->page_dma = page_dma;
+ bi->page_offset = page_offset;
+ return true;
+}
+
/**
* igb_alloc_rx_buffers_adv - Replace used receive buffers; packet split
* @adapter: address of board private structure
**/
-void igb_alloc_rx_buffers_adv(struct igb_ring *rx_ring, int cleaned_count)
+void igb_alloc_rx_buffers_adv(struct igb_ring *rx_ring, u16 cleaned_count)
{
- struct net_device *netdev = rx_ring->netdev;
union e1000_adv_rx_desc *rx_desc;
- struct igb_buffer *buffer_info;
- struct sk_buff *skb;
- unsigned int i;
+ struct igb_buffer *bi;
+ u16 i = rx_ring->next_to_use;
- i = rx_ring->next_to_use;
- buffer_info = &rx_ring->buffer_info[i];
+ rx_desc = E1000_RX_DESC_ADV(*rx_ring, i);
+ bi = &rx_ring->buffer_info[i];
+ i -= rx_ring->count;
while (cleaned_count--) {
- rx_desc = E1000_RX_DESC_ADV(*rx_ring, i);
-
- if (!buffer_info->page_dma) {
- if (!buffer_info->page) {
- buffer_info->page = netdev_alloc_page(netdev);
- if (unlikely(!buffer_info->page)) {
- u64_stats_update_begin(&rx_ring->rx_syncp);
- rx_ring->rx_stats.alloc_failed++;
- u64_stats_update_end(&rx_ring->rx_syncp);
- goto no_buffers;
- }
- buffer_info->page_offset = 0;
- } else {
- buffer_info->page_offset ^= PAGE_SIZE / 2;
- }
- buffer_info->page_dma =
- dma_map_page(rx_ring->dev, buffer_info->page,
- buffer_info->page_offset,
- PAGE_SIZE / 2,
- DMA_FROM_DEVICE);
- if (dma_mapping_error(rx_ring->dev,
- buffer_info->page_dma)) {
- buffer_info->page_dma = 0;
- u64_stats_update_begin(&rx_ring->rx_syncp);
- rx_ring->rx_stats.alloc_failed++;
- u64_stats_update_end(&rx_ring->rx_syncp);
- goto no_buffers;
- }
- }
+ if (!igb_alloc_mapped_skb(rx_ring, bi))
+ break;
- skb = buffer_info->skb;
- if (!skb) {
- skb = netdev_alloc_skb_ip_align(netdev, IGB_RX_HDR_LEN);
- if (unlikely(!skb)) {
- u64_stats_update_begin(&rx_ring->rx_syncp);
- rx_ring->rx_stats.alloc_failed++;
- u64_stats_update_end(&rx_ring->rx_syncp);
- goto no_buffers;
- }
+ /* Refresh the desc even if buffer_addrs didn't change
+ * because each write-back erases this info. */
+ rx_desc->read.hdr_addr = cpu_to_le64(bi->dma);
- buffer_info->skb = skb;
- }
- if (!buffer_info->dma) {
- buffer_info->dma = dma_map_single(rx_ring->dev,
- skb->data,
- IGB_RX_HDR_LEN,
- DMA_FROM_DEVICE);
- if (dma_mapping_error(rx_ring->dev,
- buffer_info->dma)) {
- buffer_info->dma = 0;
- u64_stats_update_begin(&rx_ring->rx_syncp);
- rx_ring->rx_stats.alloc_failed++;
- u64_stats_update_end(&rx_ring->rx_syncp);
- goto no_buffers;
- }
- }
- /* Refresh the desc even if buffer_addrs didn't change because
- * each write-back erases this info. */
- rx_desc->read.pkt_addr = cpu_to_le64(buffer_info->page_dma);
- rx_desc->read.hdr_addr = cpu_to_le64(buffer_info->dma);
+ if (!igb_alloc_mapped_page(rx_ring, bi))
+ break;
+
+ rx_desc->read.pkt_addr = cpu_to_le64(bi->page_dma);
+ rx_desc++;
+ bi++;
i++;
- if (i == rx_ring->count)
- i = 0;
- buffer_info = &rx_ring->buffer_info[i];
+ if (unlikely(!i)) {
+ rx_desc = E1000_RX_DESC_ADV(*rx_ring, 0);
+ bi = rx_ring->buffer_info;
+ i -= rx_ring->count;
+ }
+
+ /* clear the hdr_addr for the next_to_use descriptor */
+ rx_desc->read.hdr_addr = 0;
}
-no_buffers:
+ i += rx_ring->count;
+
if (rx_ring->next_to_use != i) {
rx_ring->next_to_use = i;
- if (i == 0)
- i = (rx_ring->count - 1);
- else
- i--;
/* Force memory writes to complete before letting h/w
* know there are new descriptors to fetch. (Only
--
1.7.6.2
^ permalink raw reply related
* [net-next 6/9] igb: Refactor clean_rx_irq to reduce overhead and improve performance
From: Jeff Kirsher @ 2011-09-20 7:11 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, Jeff Kirsher
In-Reply-To: <1316502690-25488-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change is meant to be a general cleanup and performance improvement
for clean_rx_irq. The previous patch should have updated the allocation so
that the rings can be treated as read-only within the clean_rx_irq
function. In addition I am re-ordering the operations such that several
goals are accomplished including reducing the overhead for packet
accounting, reducing the number of items on the stack, and improving
overall performance.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 96 ++++++++++++++---------------
1 files changed, 47 insertions(+), 49 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 9fa2ad0..dd85df0 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -138,7 +138,7 @@ static void igb_setup_dca(struct igb_adapter *);
#endif /* CONFIG_IGB_DCA */
static bool igb_clean_tx_irq(struct igb_q_vector *);
static int igb_poll(struct napi_struct *, int);
-static bool igb_clean_rx_irq_adv(struct igb_q_vector *, int *, int);
+static bool igb_clean_rx_irq_adv(struct igb_q_vector *, int);
static int igb_ioctl(struct net_device *, struct ifreq *, int cmd);
static void igb_tx_timeout(struct net_device *);
static void igb_reset_task(struct work_struct *);
@@ -5481,28 +5481,27 @@ static int igb_poll(struct napi_struct *napi, int budget)
struct igb_q_vector *q_vector = container_of(napi,
struct igb_q_vector,
napi);
- int tx_clean_complete = 1, work_done = 0;
+ bool clean_complete = true;
#ifdef CONFIG_IGB_DCA
if (q_vector->adapter->flags & IGB_FLAG_DCA_ENABLED)
igb_update_dca(q_vector);
#endif
if (q_vector->tx_ring)
- tx_clean_complete = igb_clean_tx_irq(q_vector);
+ clean_complete = !!igb_clean_tx_irq(q_vector);
if (q_vector->rx_ring)
- igb_clean_rx_irq_adv(q_vector, &work_done, budget);
+ clean_complete &= igb_clean_rx_irq_adv(q_vector, budget);
- if (!tx_clean_complete)
- work_done = budget;
+ /* If all work not completed, return budget and keep polling */
+ if (!clean_complete)
+ return budget;
/* If not enough Rx work done, exit the polling mode */
- if (work_done < budget) {
- napi_complete(napi);
- igb_ring_irq_enable(q_vector);
- }
+ napi_complete(napi);
+ igb_ring_irq_enable(q_vector);
- return work_done;
+ return 0;
}
/**
@@ -5751,37 +5750,26 @@ static inline u16 igb_get_hlen(union e1000_adv_rx_desc *rx_desc)
return hlen;
}
-static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector,
- int *work_done, int budget)
+static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector, int budget)
{
struct igb_ring *rx_ring = q_vector->rx_ring;
- struct net_device *netdev = rx_ring->netdev;
- struct device *dev = rx_ring->dev;
- union e1000_adv_rx_desc *rx_desc , *next_rxd;
- struct igb_buffer *buffer_info , *next_buffer;
- struct sk_buff *skb;
- bool cleaned = false;
- u16 cleaned_count = igb_desc_unused(rx_ring);
- int current_node = numa_node_id();
+ union e1000_adv_rx_desc *rx_desc;
+ const int current_node = numa_node_id();
unsigned int total_bytes = 0, total_packets = 0;
- unsigned int i;
u32 staterr;
- u16 length;
+ u16 cleaned_count = igb_desc_unused(rx_ring);
+ u16 i = rx_ring->next_to_clean;
- i = rx_ring->next_to_clean;
- buffer_info = &rx_ring->buffer_info[i];
rx_desc = E1000_RX_DESC_ADV(*rx_ring, i);
staterr = le32_to_cpu(rx_desc->wb.upper.status_error);
while (staterr & E1000_RXD_STAT_DD) {
- if (*work_done >= budget)
- break;
- (*work_done)++;
- rmb(); /* read descriptor and rx_buffer_info after status DD */
+ struct igb_buffer *buffer_info = &rx_ring->buffer_info[i];
+ struct sk_buff *skb = buffer_info->skb;
+ union e1000_adv_rx_desc *next_rxd;
- skb = buffer_info->skb;
- prefetch(skb->data - NET_IP_ALIGN);
buffer_info->skb = NULL;
+ prefetch(skb->data);
i++;
if (i == rx_ring->count)
@@ -5789,42 +5777,48 @@ static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector,
next_rxd = E1000_RX_DESC_ADV(*rx_ring, i);
prefetch(next_rxd);
- next_buffer = &rx_ring->buffer_info[i];
- length = le16_to_cpu(rx_desc->wb.upper.length);
- cleaned = true;
- cleaned_count++;
+ /*
+ * This memory barrier is needed to keep us from reading
+ * any other fields out of the rx_desc until we know the
+ * RXD_STAT_DD bit is set
+ */
+ rmb();
- if (buffer_info->dma) {
- dma_unmap_single(dev, buffer_info->dma,
+ if (!skb_is_nonlinear(skb)) {
+ __skb_put(skb, igb_get_hlen(rx_desc));
+ dma_unmap_single(rx_ring->dev, buffer_info->dma,
IGB_RX_HDR_LEN,
DMA_FROM_DEVICE);
buffer_info->dma = 0;
- skb_put(skb, igb_get_hlen(rx_desc));
}
- if (length) {
- dma_unmap_page(dev, buffer_info->page_dma,
- PAGE_SIZE / 2, DMA_FROM_DEVICE);
- buffer_info->page_dma = 0;
+ if (rx_desc->wb.upper.length) {
+ u16 length = le16_to_cpu(rx_desc->wb.upper.length);
skb_fill_page_desc(skb, skb_shinfo(skb)->nr_frags,
buffer_info->page,
buffer_info->page_offset,
length);
+ skb->len += length;
+ skb->data_len += length;
+ skb->truesize += length;
+
if ((page_count(buffer_info->page) != 1) ||
(page_to_nid(buffer_info->page) != current_node))
buffer_info->page = NULL;
else
get_page(buffer_info->page);
- skb->len += length;
- skb->data_len += length;
- skb->truesize += length;
+ dma_unmap_page(rx_ring->dev, buffer_info->page_dma,
+ PAGE_SIZE / 2, DMA_FROM_DEVICE);
+ buffer_info->page_dma = 0;
}
if (!(staterr & E1000_RXD_STAT_EOP)) {
+ struct igb_buffer *next_buffer;
+ next_buffer = &rx_ring->buffer_info[i];
buffer_info->skb = next_buffer->skb;
buffer_info->dma = next_buffer->dma;
next_buffer->skb = skb;
@@ -5833,7 +5827,7 @@ static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector,
}
if (staterr & E1000_RXDEXT_ERR_FRAME_ERR_MASK) {
- dev_kfree_skb_irq(skb);
+ dev_kfree_skb_any(skb);
goto next_desc;
}
@@ -5844,7 +5838,7 @@ static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector,
igb_rx_checksum_adv(rx_ring, staterr, skb);
- skb->protocol = eth_type_trans(skb, netdev);
+ skb->protocol = eth_type_trans(skb, rx_ring->netdev);
if (staterr & E1000_RXD_STAT_VP) {
u16 vid = le16_to_cpu(rx_desc->wb.upper.vlan);
@@ -5853,7 +5847,12 @@ static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector,
}
napi_gro_receive(&q_vector->napi, skb);
+ budget--;
next_desc:
+ if (!budget)
+ break;
+
+ cleaned_count++;
/* return some buffers to hardware, one at a time is too slow */
if (cleaned_count >= IGB_RX_BUFFER_WRITE) {
igb_alloc_rx_buffers_adv(rx_ring, cleaned_count);
@@ -5862,7 +5861,6 @@ next_desc:
/* use prefetched values */
rx_desc = next_rxd;
- buffer_info = next_buffer;
staterr = le32_to_cpu(rx_desc->wb.upper.status_error);
}
@@ -5877,7 +5875,7 @@ next_desc:
if (cleaned_count)
igb_alloc_rx_buffers_adv(rx_ring, cleaned_count);
- return cleaned;
+ return !!budget;
}
static bool igb_alloc_mapped_skb(struct igb_ring *rx_ring,
--
1.7.6.2
^ 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