* [PATCH net-next 6/6] doc, net: Update ndo_start_xmit return type and values
From: Ben Hutchings @ 2012-04-06 0:40 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <HEAD <1333672667.2652.22.camel@bwh-desktop.uk.solarflarecom.com>
Commit dc1f8bf68b311b1537cb65893430b6796118498a ('netdev: change
transmit to limited range type') changed the required return type and
9a1654ba0b50402a6bd03c7b0fe9b0200a5ea7b1 ('net: Optimize
hard_start_xmit() return checking') changed the valid numerical
return values.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
Documentation/networking/driver.txt | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/Documentation/networking/driver.txt b/Documentation/networking/driver.txt
index 2128e41..da59e28 100644
--- a/Documentation/networking/driver.txt
+++ b/Documentation/networking/driver.txt
@@ -2,16 +2,16 @@ Document about softnet driver issues
Transmit path guidelines:
-1) The ndo_start_xmit method must never return '1' under any
- normal circumstances. It is considered a hard error unless
+1) The ndo_start_xmit method must not return NETDEV_TX_BUSY under
+ any normal circumstances. It is considered a hard error unless
there is no way your device can tell ahead of time when it's
transmit function will become busy.
Instead it must maintain the queue properly. For example,
for a driver implementing scatter-gather this means:
- static int drv_hard_start_xmit(struct sk_buff *skb,
- struct net_device *dev)
+ static netdev_tx_t drv_hard_start_xmit(struct sk_buff *skb,
+ struct net_device *dev)
{
struct drv *dp = netdev_priv(dev);
@@ -23,7 +23,7 @@ Transmit path guidelines:
unlock_tx(dp);
printk(KERN_ERR PFX "%s: BUG! Tx Ring full when queue awake!\n",
dev->name);
- return 1;
+ return NETDEV_TX_BUSY;
}
... queue packet to card ...
@@ -35,6 +35,7 @@ Transmit path guidelines:
...
unlock_tx(dp);
...
+ return NETDEV_TX_OK;
}
And then at the end of your TX reclamation event handling:
@@ -61,9 +62,9 @@ Transmit path guidelines:
2) An ndo_start_xmit method must not modify the shared parts of a
cloned SKB.
-3) Do not forget that once you return 0 from your ndo_start_xmit
- method, it is your driver's responsibility to free up the SKB
- and in some finite amount of time.
+3) Do not forget that once you return NETDEV_TX_OK from your
+ ndo_start_xmit method, it is your driver's responsibility to free
+ up the SKB and in some finite amount of time.
For example, this means that it is not allowed for your TX
mitigation scheme to let TX packets "hang out" in the TX
@@ -71,8 +72,9 @@ Transmit path guidelines:
This error can deadlock sockets waiting for send buffer room
to be freed up.
- If you return 1 from the ndo_start_xmit method, you must not keep
- any reference to that SKB and you must not attempt to free it up.
+ If you return NETDEV_TX_BUSY from the ndo_start_xmit method, you
+ must not keep any reference to that SKB and you must not attempt
+ to free it up.
Probing guidelines:
--
1.7.7.6
--
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 related
* Re: [net-next 3/3] ixgbe: add syfs interface for to export read only driver information
From: David Miller @ 2012-04-06 0:59 UTC (permalink / raw)
To: mark.d.rustad
Cc: bhutchings, florian, jeffrey.t.kirsher, donald.c.skidmore, netdev,
gospo, sassmann
In-Reply-To: <81212BCD-D022-4B71-955A-96B6C8E2A5DC@intel.com>
From: "Rustad, Mark D" <mark.d.rustad@intel.com>
Date: Thu, 5 Apr 2012 23:52:40 +0000
> The redundancy with ethtool doesn't bother me much, because I think
> sysfs is a more general way to export driver information, and I find
> it very convenient to get as much as is practical from a single
> source. I really like doing "grep . * */*" in sysfs and getting as
> much information as possible about a driver.
That's not our position, we have flexible, extensible, interfaces
such as ethtool and netlink so that people don't get the urge
to toss random unnecessary junk into sysfs.
There are no damn APIs for what driver guys want to put into
sysfs, making it terrible from a user's perspective.
Whereas ethtool and netlink give you something consistent and
reliable.
Therefore there will be hard pushback on anything one tries to
export via sysfs, it is almost guarenteed that ethtool and
netlink fit the bill properly.
^ permalink raw reply
* Re: sendmmsg: put_user vs __put_user
From: David Miller @ 2012-04-06 1:01 UTC (permalink / raw)
To: luto; +Cc: drepper, netdev, linux-kernel
In-Reply-To: <4F7E3561.8000803@mit.edu>
From: Andy Lutomirski <luto@mit.edu>
Date: Thu, 05 Apr 2012 17:14:25 -0700
> On 03/30/2012 05:51 PM, David Miller wrote:
>> From: Ulrich Drepper <drepper@gmail.com>
>> Date: Fri, 30 Mar 2012 09:36:11 -0400
>>
>>> Shouldn't the compat code in the sendmmsg implementation use the same
>>> code as the normal code? In which case you probably want something
>>> like this:
>>
>> Compat processes are not able to generate virtual addresses anywhere
>> near the range where the kernel resides, so the address range
>> verification done by put_user() is completely superfluous and
>> therefore not necessary. The normal exception handling done by the
>> access is completely sufficient.
>
> I disagree. The following exploit causes a bogus page fault to a kernel
> address. I think this isn't exploitable right now on x86-64 because the
> page fault handler fixes it up, but I wouldn't be surprised if this
> crashes or at least warns on some architecture. (Actually trashing
> kernel memory is probably impossible with this on x86-64 chips because
> this can only overrun user space by four bytes, and there's a giant gap
> of impossible addresses above user space in x86-64.
I can guarentee this doesn't do anything on sparc64 either because
userspace is completely segregated from kernel space in a way that
every single foo_user() call cannot access kernel space no matter
what address it can trick into being passed there.
I still really don't see an issue with this, sorry.
^ permalink raw reply
* [PATCH] ath5k: Remove extraneous statements from ath5k_hw_proc_4word_tx_status and ath5k_hw_proc_2word_status.
From: Qasim Javed @ 2012-04-06 1:40 UTC (permalink / raw)
To: linux-wireless, ath5k-devel; +Cc: netdev, linux-kernel, Qasim Javed
Signed-off-by: Qasim Javed <qasimj@gmail.com>
---
drivers/net/wireless/ath/ath5k/desc.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/desc.c b/drivers/net/wireless/ath/ath5k/desc.c
index f8bfa3a..97b92c3 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -441,10 +441,8 @@ ath5k_hw_proc_2word_tx_status(struct ath5k_hw *ah,
struct ath5k_desc *desc,
struct ath5k_tx_status *ts)
{
- struct ath5k_hw_2w_tx_ctl *tx_ctl;
struct ath5k_hw_tx_status *tx_status;
- tx_ctl = &desc->ud.ds_tx5210.tx_ctl;
tx_status = &desc->ud.ds_tx5210.tx_stat;
/* No frame has been send or error */
@@ -495,11 +493,9 @@ ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
struct ath5k_desc *desc,
struct ath5k_tx_status *ts)
{
- struct ath5k_hw_4w_tx_ctl *tx_ctl;
struct ath5k_hw_tx_status *tx_status;
u32 txstat0, txstat1;
- tx_ctl = &desc->ud.ds_tx5212.tx_ctl;
tx_status = &desc->ud.ds_tx5212.tx_stat;
txstat1 = ACCESS_ONCE(tx_status->tx_status_1);
--
1.7.8.2
^ permalink raw reply related
* Re: [PATCH] tcp: allow splice() to build full TSO packets
From: Eric Dumazet @ 2012-04-06 1:59 UTC (permalink / raw)
To: David Miller
Cc: netdev, ncardwell, therbert, ycheng, hkchu, maze, maheshb,
ilpo.jarvinen, nanditad
In-Reply-To: <20120405.190509.409578034793502486.davem@davemloft.net>
On Thu, 2012-04-05 at 19:05 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 05 Apr 2012 15:05:35 +0200
>
> > Hmm, thinking again about this, I did more tests and it appears we need
> > to differentiate the SPLICE_F_MORE flag (user request) and the internal
> > marker provided by splice logic (handling a batch of pages)
> >
> > A program doing splice(... SPLICE_F_MORE) should really call tcp_push()
> > at the end of its work.
>
> This is the kind of problem I was hoping we weren't introducing
> when I asked about sendfile() et al. the other day :-)
Yes, sorry for this.
Yet sendfile() did not have this problem (or so I believe), only the
splice(SPLICE_F_MORE) did.
Thanks
^ permalink raw reply
* Re: [PATCH net-next] rps: introduce a new sysctl switch rps_workaround_buggy_driver
From: Li Yu @ 2012-04-06 2:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1333624027.18626.579.camel@edumazet-glaptop>
于 2012年04月05日 19:07, Eric Dumazet 写道:
> On Thu, 2012-04-05 at 18:31 +0800, Li Yu wrote:
>> We encountered a buggy NIC driver or hardware/firmware, it keeps
>> non-zero constant skb->rxhash for long time, so if we enabled RPS,
>> the targeted CPU keeps same for long time too.
>>
>> This patch introduces a sysctl switch to workaround for such problem,
>> if the switch was on, RPS core discards the skb->rxhash that is
>> computed by NIC hardware.
>>
>> Hope this patch also can help others, thanks.
>
> Really ?
>
> to disable this driver rxhash, you should try :
>
> ethtool -K eth0 rxhash off
>
>
Great! I really did not know this new option ago,
and wrote a kprobe workaround module. It seem that
ethtool 2.6.39 have such support at least.
Thank you very much!
Yu
>
>
^ permalink raw reply
* Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac
From: Jason Lin @ 2012-04-06 2:24 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev
In-Reply-To: <CAGCDX1=EHJY1C=-w_YazeC9ByP=4T+hj8bjvZ=cizziSfa1J3g@mail.gmail.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB2312, Size: 5877 bytes --]
Should phy_enable_interrupts() and phy_disable_interrupts() open
to MAC driver?
And MAC driver should initialize a workqueue to ack PHY's interrupt.
Is this a better way?
ÔÚ 2012Äê4ÔÂ5ÈÕÉÏÎç9:30£¬Jason Lin <kernel.jason@gmail.com> µÀ£º
> But, I need to enable PHY's interrupt by setting PHY's registers.
> And after producing a interrupt, there needs a workqueue
> to ack the PHY's interrupt.
> The phy_start_interrupts() can enable PHY's interrupt.
> If I do the following:
> 1) set phydev->irq = PHY_IGNORE_INTERRUPT;
> 2) invoke phy_connect();
> 3) phy_conenct() -> phy_connect_direct() ->
> if (phydev->irq > 0)
> phy_start_interrupts(phydev);
> 4) PHY_IGNORE_INTERRUPT = -2, it will not enable PHY's interrupt.
> 5) phy_start_interrupts() will connect to a callback function
> config_intr() of each PHY library.
>
> For example, drivers/net/phy/marvell.c, marvell_config_intr()
> Need to set register 0x12 to 0x6400 to enable corresponding interrupts.
>
> Any comments are appreciated.
> Thanks.
>
> ÔÚ 2012Äê4ÔÂ3ÈÕÏÂÎç4:49£¬Florian Fainelli <florian@openwrt.org> µÀ£º
>> Hi,
>>
>> Le 04/03/12 04:11, Jason Lin a ¨¦crit :
>>
>>> 1) Add a new definition PHY_MAC if phy shares the same
>>> interrupt with mac.
>>> 2) Add do_phy_workqueue(), that mac can invoke this function
>>> in its ISR when link status changed or other conditions.
>>>
>>> Does this seems reasonable?
>>
>>
>> No, I think this is well handled by the PHY_IGNORE_INTERRUPT case
>> (documented in Documentation/networking/phy.txt) by doing the following:
>>
>> - distinguish between MAC and PHY interrupts in your MAC interrupt handler
>> (most likely you have separate bits for PHY interrupts)
>> - once you see a PHY interrupt, call the appropriate PHY state machine
>> callbacks to update the PHY state machine
>>
>>>
>>>
>>> ---------------------------
>>> drivers/net/phy/phy.c | 27 +++++++++++++++++++++++++++
>>> drivers/net/phy/phy_device.c | 4 ++--
>>> include/linux/phy.h | 3 +++
>>> 3 files changed, 32 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>> index 7670aac..c8009e9 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -527,6 +527,28 @@ static irqreturn_t phy_interrupt(int irq, void
>>> *phy_dat)
>>> return IRQ_HANDLED;
>>> }
>>>
>>> +/*
>>> + * do_phy_workqueue - PHY interrupt handler used by MAC
>>> + * @irq: interrupt line
>>> + * @phydev: phy_device pointer
>>> + *
>>> + * Description: When a PHY use the same interrupt with MAC,
>>> + * the handler is invoked by MAC, and schedules a work task
>>> + * to clear the PHY's interrupt.
>>> + * This handler is invoked only in MAC's ISR.
>>> + */
>>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev) {
>>> +
>>> + BUG_ON(!in_interrupt());
>>> +
>>> + if (PHY_HALTED == phydev->state)
>>> + return IRQ_NONE;
>>> +
>>> + schedule_work(&phydev->phy_queue);
>>> + return IRQ_HANDLED;
>>> +}
>>> +EXPORT_SYMBOL(do_phy_workqueue);
>>> +
>>> /**
>>> * phy_enable_interrupts - Enable the interrupts from the PHY side
>>> * @phydev: target phy_device struct
>>> @@ -589,6 +611,11 @@ int phy_start_interrupts(struct phy_device *phydev)
>>>
>>> INIT_WORK(&phydev->phy_queue, phy_change);
>>>
>>> + if (phydev->irq == PHY_MAC) {
>>> + err = phy_enable_interrupts(phydev);
>>> + return err;
>>> + }
>>> +
>>> atomic_set(&phydev->irq_disable, 0);
>>> if (request_irq(phydev->irq, phy_interrupt,
>>> IRQF_SHARED,
>>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>>> index 993c52c..1857097 100644
>>> --- a/drivers/net/phy/phy_device.c
>>> +++ b/drivers/net/phy/phy_device.c
>>> @@ -345,7 +345,7 @@ int phy_connect_direct(struct net_device *dev,
>>> struct phy_device *phydev,
>>>
>>> phy_prepare_link(phydev, handler);
>>> phy_start_machine(phydev, NULL);
>>> - if (phydev->irq> 0)
>>> + if ((phydev->irq> 0) || (phydev->irq == PHY_MAC))
>>> phy_start_interrupts(phydev);
>>>
>>> return 0;
>>> @@ -399,7 +399,7 @@ EXPORT_SYMBOL(phy_connect);
>>> */
>>> void phy_disconnect(struct phy_device *phydev)
>>> {
>>> - if (phydev->irq> 0)
>>> + if ((phydev->irq> 0) || (phydev->irq == PHY_MAC))
>>> phy_stop_interrupts(phydev);
>>>
>>> phy_stop_machine(phydev);
>>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>>> index 7da5fa8..155822c 100644
>>> --- a/include/linux/phy.h
>>> +++ b/include/linux/phy.h
>>> @@ -44,9 +44,11 @@
>>> * Set phydev->irq to PHY_POLL if interrupts are not supported,
>>> * or not desired for this PHY. Set to PHY_IGNORE_INTERRUPT if
>>> * the attached driver handles the interrupt
>>> + * Set to PHY_MAC if using the same interrupt with MAC
>>> */
>>> #define PHY_POLL -1
>>> #define PHY_IGNORE_INTERRUPT -2
>>> +#define PHY_MAC -3
>>>
>>> #define PHY_HAS_INTERRUPT 0x00000001
>>> #define PHY_HAS_MAGICANEG 0x00000002
>>> @@ -510,6 +512,7 @@ int phy_ethtool_sset(struct phy_device *phydev,
>>> struct ethtool_cmd *cmd);
>>> int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
>>> int phy_mii_ioctl(struct phy_device *phydev,
>>> struct ifreq *ifr, int cmd);
>>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev);
>>> int phy_start_interrupts(struct phy_device *phydev);
>>> void phy_print_status(struct phy_device *phydev);
>>> void phy_device_free(struct phy_device *phydev);
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Expose ltr/obff interface by sysfs
From: Hao, Xudong @ 2012-04-06 2:43 UTC (permalink / raw)
To: linux-pci@vger.kernel.org, netdev@vger.kernel.org,
e1000-devel@lists.sourceforge.net
Cc: Jesse Barnes
Hi,
I'm working on virtualization Xen/KVM. I saw there are ltr/obff enabling/disabling function in pci.c, but no called till now. I want to know if anybody(driver developer) are working for using it? Can driver change the LTR latency value dynamically?
/*
LTR(Latency tolerance reporting) allows devices to send messages to the root complex indicating their latency tolerance for snooped & unsnooped memory transactions.
OBFF (optimized buffer flush/fill), where supported, can help improve energy efficiency by giving devices information about when interrupts and other activity will have a reduced power impact.
*/
One way to control ltr/obff is used by driver, however, I'm considering that in virtualization, how guest OS driver control them. I have an idea that expose an inode interface by sysfs, like "reset" inode implemented in pci-sysfs.c, so that system user/administrator can enable/disable ltr/obff or set latency value on userspace, but not limited on driver. Comments?
< pls CC me when reply this mail, thanks >
Best Regards,
Xudong Hao
------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH V4 net-next 05/26] bfin_mac: Support the get_ts_info ethtool method.
From: Paul Gortmaker @ 2012-04-06 4:08 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, David Miller, Ben Hutchings, Martin Porter, Jacob Keller,
Jeff Kirsher, John Ronciak, e1000-devel, linux-next
In-Reply-To: <88e5b2cfcabda03826bb576b21e2e1d990f07117.1333529506.git.richardcochran@gmail.com>
On Wed, Apr 4, 2012 at 4:59 AM, Richard Cochran
<richardcochran@gmail.com> wrote:
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---
> drivers/net/ethernet/adi/bfin_mac.c | 20 ++++++++++++++++++++
> 1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
> index ab4daec..db22278 100644
> --- a/drivers/net/ethernet/adi/bfin_mac.c
> +++ b/drivers/net/ethernet/adi/bfin_mac.c
> @@ -548,6 +548,25 @@ static int bfin_mac_ethtool_setwol(struct net_device *dev,
> return 0;
> }
>
> +static int bfin_mac_ethtool_get_ts_info(struct net_device *dev,
> + struct ethtool_ts_info *info);
-----------------------------------> ^^^^^^^
Trailing semicolon. Compile fail in linux-next due to a85bbddd.
http://kisskb.ellerman.id.au/kisskb/buildresult/6040086/
http://kisskb.ellerman.id.au/kisskb/buildresult/6040080/
http://kisskb.ellerman.id.au/kisskb/buildresult/6040087/
Thanks.
Paul.
> +{
> + info->so_timestamping =
> + SOF_TIMESTAMPING_TX_HARDWARE |
> + SOF_TIMESTAMPING_RX_HARDWARE |
> + SOF_TIMESTAMPING_SYS_HARDWARE;
> + info->phc_index = -1;
> + info->tx_types =
> + (1 << HWTSTAMP_TX_OFF) |
> + (1 << HWTSTAMP_TX_ON);
> + info->rx_filters =
> + (1 << HWTSTAMP_FILTER_NONE) |
> + (1 << HWTSTAMP_FILTER_PTP_V1_L4_EVENT) |
> + (1 << HWTSTAMP_FILTER_PTP_V2_L2_EVENT) |
> + (1 << HWTSTAMP_FILTER_PTP_V2_L4_EVENT);
> + return 0;
> +}
> +
> static const struct ethtool_ops bfin_mac_ethtool_ops = {
> .get_settings = bfin_mac_ethtool_getsettings,
> .set_settings = bfin_mac_ethtool_setsettings,
> @@ -555,6 +574,7 @@ static const struct ethtool_ops bfin_mac_ethtool_ops = {
> .get_drvinfo = bfin_mac_ethtool_getdrvinfo,
> .get_wol = bfin_mac_ethtool_getwol,
> .set_wol = bfin_mac_ethtool_setwol,
> + .get_ts_info = bfin_mac_ethtool_get_ts_info,
> };
>
> /**************************************************************************/
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V4 net-next 05/26] bfin_mac: Support the get_ts_info ethtool method.
From: David Miller @ 2012-04-06 4:18 UTC (permalink / raw)
To: paul.gortmaker
Cc: mporter, e1000-devel, netdev, jacob.e.keller, john.ronciak,
bhutchings, linux-next
In-Reply-To: <CAP=VYLq1kCqLxi31HcGavKk8e3ydX43Xz-7rwayLJPWAnfk28Q@mail.gmail.com>
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Fri, 6 Apr 2012 00:08:52 -0400
> On Wed, Apr 4, 2012 at 4:59 AM, Richard Cochran
> <richardcochran@gmail.com> wrote:
>> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
>> ---
>> drivers/net/ethernet/adi/bfin_mac.c | 20 ++++++++++++++++++++
>> 1 files changed, 20 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
>> index ab4daec..db22278 100644
>> --- a/drivers/net/ethernet/adi/bfin_mac.c
>> +++ b/drivers/net/ethernet/adi/bfin_mac.c
>> @@ -548,6 +548,25 @@ static int bfin_mac_ethtool_setwol(struct net_device *dev,
>> return 0;
>> }
>>
>> +static int bfin_mac_ethtool_get_ts_info(struct net_device *dev,
>> + struct ethtool_ts_info *info);
>
> -----------------------------------> ^^^^^^^
>
> Trailing semicolon. Compile fail in linux-next due to a85bbddd.
I just pushed out the obvious fix for this.
------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH V4 net-next 04/26] gianfar: Support the get_ts_info ethtool method.
From: Paul Gortmaker @ 2012-04-06 4:25 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, David Miller, Ben Hutchings, Martin Porter, Jacob Keller,
Jeff Kirsher, John Ronciak, e1000-devel, linux-next
In-Reply-To: <c268d71203b066b9052a8cabd5f6c71de2a8fa77.1333529506.git.richardcochran@gmail.com>
On Wed, Apr 4, 2012 at 4:59 AM, Richard Cochran
<richardcochran@gmail.com> wrote:
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---
> drivers/net/ethernet/freescale/gianfar.h | 3 ++
> drivers/net/ethernet/freescale/gianfar_ethtool.c | 29 ++++++++++++++++++++++
> drivers/net/ethernet/freescale/gianfar_ptp.c | 2 +
> 3 files changed, 34 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
> index 4c9f8d4..2136c7f 100644
> --- a/drivers/net/ethernet/freescale/gianfar.h
> +++ b/drivers/net/ethernet/freescale/gianfar.h
> @@ -1210,4 +1210,7 @@ struct filer_table {
> struct gfar_filer_entry fe[MAX_FILER_CACHE_IDX + 20];
> };
>
> +/* The gianfar_ptp module will set this variable */
> +extern int gfar_phc_index;
> +
> #endif /* __GIANFAR_H */
> diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
> index 8d74efd..27f49c7 100644
> --- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
> +++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
> @@ -1739,6 +1739,34 @@ static int gfar_get_nfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
> return ret;
> }
>
> +int gfar_phc_index = -1;
> +
> +static int gfar_get_ts_info(struct net_device *dev,
> + struct ethtool_ts_info *info)
> +{
> + struct gfar_private *priv = netdev_priv(dev);
> +
> + if (!(priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)) {
> + info->so_timestamping =
> + SOF_TIMESTAMPING_RX_SOFTWARE |
> + SOF_TIMESTAMPING_SOFTWARE;
Commit 66636287 in linux-next appears to cause this:
drivers/net/ethernet/freescale/gianfar_ethtool.c:1751:4: error:
'SOF_TIMESTAMPING_RX_SOFTWARE' undeclared (first use in this function)
drivers/net/ethernet/freescale/gianfar_ethtool.c:1752:4: error:
'SOF_TIMESTAMPING_SOFTWARE' undeclared (first use in this function)
Needs an include of <linux/net_tstamp.h> ?
http://kisskb.ellerman.id.au/kisskb/buildresult/6039723/
http://kisskb.ellerman.id.au/kisskb/buildresult/6039704/
http://kisskb.ellerman.id.au/kisskb/buildresult/6040028/
I'd normally just provide a tested and signed off patch, but I'm kind of
limited to just browsing the build results at this particular moment. Sorry
about that.
Paul.
> + info->phc_index = -1;
> + return 0;
> + }
> + info->so_timestamping =
> + SOF_TIMESTAMPING_TX_HARDWARE |
> + SOF_TIMESTAMPING_RX_HARDWARE |
> + SOF_TIMESTAMPING_RAW_HARDWARE;
> + info->phc_index = gfar_phc_index;
> + info->tx_types =
> + (1 << HWTSTAMP_TX_OFF) |
> + (1 << HWTSTAMP_TX_ON);
> + info->rx_filters =
> + (1 << HWTSTAMP_FILTER_NONE) |
> + (1 << HWTSTAMP_FILTER_ALL);
> + return 0;
> +}
> +
> const struct ethtool_ops gfar_ethtool_ops = {
> .get_settings = gfar_gsettings,
> .set_settings = gfar_ssettings,
> @@ -1761,4 +1789,5 @@ const struct ethtool_ops gfar_ethtool_ops = {
> #endif
> .set_rxnfc = gfar_set_nfc,
> .get_rxnfc = gfar_get_nfc,
> + .get_ts_info = gfar_get_ts_info,
> };
> diff --git a/drivers/net/ethernet/freescale/gianfar_ptp.c b/drivers/net/ethernet/freescale/gianfar_ptp.c
> index 5fd620b..c08e5d4 100644
> --- a/drivers/net/ethernet/freescale/gianfar_ptp.c
> +++ b/drivers/net/ethernet/freescale/gianfar_ptp.c
> @@ -515,6 +515,7 @@ static int gianfar_ptp_probe(struct platform_device *dev)
> err = PTR_ERR(etsects->clock);
> goto no_clock;
> }
> + gfar_phc_clock = ptp_clock_index(etsects->clock);
>
> dev_set_drvdata(&dev->dev, etsects);
>
> @@ -538,6 +539,7 @@ static int gianfar_ptp_remove(struct platform_device *dev)
> gfar_write(&etsects->regs->tmr_temask, 0);
> gfar_write(&etsects->regs->tmr_ctrl, 0);
>
> + gfar_phc_clock = -1;
> ptp_clock_unregister(etsects->clock);
> iounmap(etsects->regs);
> release_resource(etsects->rsrc);
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V4 net-next 04/26] gianfar: Support the get_ts_info ethtool method.
From: David Miller @ 2012-04-06 4:35 UTC (permalink / raw)
To: paul.gortmaker
Cc: mporter, e1000-devel, netdev, jacob.e.keller, john.ronciak,
bhutchings, linux-next
In-Reply-To: <CAP=VYLrNBjHgPJFzwBrQ4=5rouCD81PLW4f_TPZMkF7yObUdcA@mail.gmail.com>
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Fri, 6 Apr 2012 00:25:32 -0400
> Needs an include of <linux/net_tstamp.h> ?
I've pushed out that fix.
------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac
From: Jason Lin @ 2012-04-06 5:53 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev
In-Reply-To: <CAGCDX1kZo5f8_BpMgq75o0EoTRh9X=VbH2-rXDBYoKdCosO+VQ@mail.gmail.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB2312, Size: 6778 bytes --]
Dear:
In include/linux/phy.h
There has some comments:
/*
* Set phydev->irq to PHY_POLL if interrupts are not supported,
* or not desired for this PHY. Set to PHY_IGNORE_INTERRUPT if
* the attached driver handles the interrupt
*/
Florian, you are right.
I have to set phydev->irq to PHY_IGNORE_INTERRUPT.
This seems to work if one MAC driver is stick to specific PHY driver.
So one can use phywrite() and phyread() to control the PHY.
In my case, MAC driver handles the interrupt, but not to stick to the
specific PHY driver. So MAC driver needs a set of general functions
to set PHY's registers to enable and ack PHY's interrupts.
Dose it make sense?
ÔÚ 2012Äê4ÔÂ6ÈÕÉÏÎç10:24£¬Jason Lin <kernel.jason@gmail.com> µÀ£º
> Should phy_enable_interrupts() and phy_disable_interrupts() open
> to MAC driver?
> And MAC driver should initialize a workqueue to ack PHY's interrupt.
> Is this a better way?
>
> ÔÚ 2012Äê4ÔÂ5ÈÕÉÏÎç9:30£¬Jason Lin <kernel.jason@gmail.com> µÀ£º
>> But, I need to enable PHY's interrupt by setting PHY's registers.
>> And after producing a interrupt, there needs a workqueue
>> to ack the PHY's interrupt.
>> The phy_start_interrupts() can enable PHY's interrupt.
>> If I do the following:
>> 1) set phydev->irq = PHY_IGNORE_INTERRUPT;
>> 2) invoke phy_connect();
>> 3) phy_conenct() -> phy_connect_direct() ->
>> if (phydev->irq > 0)
>> phy_start_interrupts(phydev);
>> 4) PHY_IGNORE_INTERRUPT = -2, it will not enable PHY's interrupt.
>> 5) phy_start_interrupts() will connect to a callback function
>> config_intr() of each PHY library.
>>
>> For example, drivers/net/phy/marvell.c, marvell_config_intr()
>> Need to set register 0x12 to 0x6400 to enable corresponding interrupts.
>>
>> Any comments are appreciated.
>> Thanks.
>>
>> ÔÚ 2012Äê4ÔÂ3ÈÕÏÂÎç4:49£¬Florian Fainelli <florian@openwrt.org> µÀ£º
>>> Hi,
>>>
>>> Le 04/03/12 04:11, Jason Lin a ¨¦crit :
>>>
>>>> 1) Add a new definition PHY_MAC if phy shares the same
>>>> interrupt with mac.
>>>> 2) Add do_phy_workqueue(), that mac can invoke this function
>>>> in its ISR when link status changed or other conditions.
>>>>
>>>> Does this seems reasonable?
>>>
>>>
>>> No, I think this is well handled by the PHY_IGNORE_INTERRUPT case
>>> (documented in Documentation/networking/phy.txt) by doing the following:
>>>
>>> - distinguish between MAC and PHY interrupts in your MAC interrupt handler
>>> (most likely you have separate bits for PHY interrupts)
>>> - once you see a PHY interrupt, call the appropriate PHY state machine
>>> callbacks to update the PHY state machine
>>>
>>>>
>>>>
>>>> ---------------------------
>>>> drivers/net/phy/phy.c | 27 +++++++++++++++++++++++++++
>>>> drivers/net/phy/phy_device.c | 4 ++--
>>>> include/linux/phy.h | 3 +++
>>>> 3 files changed, 32 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>>> index 7670aac..c8009e9 100644
>>>> --- a/drivers/net/phy/phy.c
>>>> +++ b/drivers/net/phy/phy.c
>>>> @@ -527,6 +527,28 @@ static irqreturn_t phy_interrupt(int irq, void
>>>> *phy_dat)
>>>> return IRQ_HANDLED;
>>>> }
>>>>
>>>> +/*
>>>> + * do_phy_workqueue - PHY interrupt handler used by MAC
>>>> + * @irq: interrupt line
>>>> + * @phydev: phy_device pointer
>>>> + *
>>>> + * Description: When a PHY use the same interrupt with MAC,
>>>> + * the handler is invoked by MAC, and schedules a work task
>>>> + * to clear the PHY's interrupt.
>>>> + * This handler is invoked only in MAC's ISR.
>>>> + */
>>>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev) {
>>>> +
>>>> + BUG_ON(!in_interrupt());
>>>> +
>>>> + if (PHY_HALTED == phydev->state)
>>>> + return IRQ_NONE;
>>>> +
>>>> + schedule_work(&phydev->phy_queue);
>>>> + return IRQ_HANDLED;
>>>> +}
>>>> +EXPORT_SYMBOL(do_phy_workqueue);
>>>> +
>>>> /**
>>>> * phy_enable_interrupts - Enable the interrupts from the PHY side
>>>> * @phydev: target phy_device struct
>>>> @@ -589,6 +611,11 @@ int phy_start_interrupts(struct phy_device *phydev)
>>>>
>>>> INIT_WORK(&phydev->phy_queue, phy_change);
>>>>
>>>> + if (phydev->irq == PHY_MAC) {
>>>> + err = phy_enable_interrupts(phydev);
>>>> + return err;
>>>> + }
>>>> +
>>>> atomic_set(&phydev->irq_disable, 0);
>>>> if (request_irq(phydev->irq, phy_interrupt,
>>>> IRQF_SHARED,
>>>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>>>> index 993c52c..1857097 100644
>>>> --- a/drivers/net/phy/phy_device.c
>>>> +++ b/drivers/net/phy/phy_device.c
>>>> @@ -345,7 +345,7 @@ int phy_connect_direct(struct net_device *dev,
>>>> struct phy_device *phydev,
>>>>
>>>> phy_prepare_link(phydev, handler);
>>>> phy_start_machine(phydev, NULL);
>>>> - if (phydev->irq> 0)
>>>> + if ((phydev->irq> 0) || (phydev->irq == PHY_MAC))
>>>> phy_start_interrupts(phydev);
>>>>
>>>> return 0;
>>>> @@ -399,7 +399,7 @@ EXPORT_SYMBOL(phy_connect);
>>>> */
>>>> void phy_disconnect(struct phy_device *phydev)
>>>> {
>>>> - if (phydev->irq> 0)
>>>> + if ((phydev->irq> 0) || (phydev->irq == PHY_MAC))
>>>> phy_stop_interrupts(phydev);
>>>>
>>>> phy_stop_machine(phydev);
>>>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>>>> index 7da5fa8..155822c 100644
>>>> --- a/include/linux/phy.h
>>>> +++ b/include/linux/phy.h
>>>> @@ -44,9 +44,11 @@
>>>> * Set phydev->irq to PHY_POLL if interrupts are not supported,
>>>> * or not desired for this PHY. Set to PHY_IGNORE_INTERRUPT if
>>>> * the attached driver handles the interrupt
>>>> + * Set to PHY_MAC if using the same interrupt with MAC
>>>> */
>>>> #define PHY_POLL -1
>>>> #define PHY_IGNORE_INTERRUPT -2
>>>> +#define PHY_MAC -3
>>>>
>>>> #define PHY_HAS_INTERRUPT 0x00000001
>>>> #define PHY_HAS_MAGICANEG 0x00000002
>>>> @@ -510,6 +512,7 @@ int phy_ethtool_sset(struct phy_device *phydev,
>>>> struct ethtool_cmd *cmd);
>>>> int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
>>>> int phy_mii_ioctl(struct phy_device *phydev,
>>>> struct ifreq *ifr, int cmd);
>>>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev);
>>>> int phy_start_interrupts(struct phy_device *phydev);
>>>> void phy_print_status(struct phy_device *phydev);
>>>> void phy_device_free(struct phy_device *phydev);
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next 0/6] Documentation fixes
From: David Miller @ 2012-04-06 6:43 UTC (permalink / raw)
To: bhutchings; +Cc: netdev
In-Reply-To: <1333672667.2652.22.camel@bwh-desktop.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Fri, 6 Apr 2012 01:37:47 +0100
> These changes update Documentation/networking/driver.txt and
> Documentation/networking/netdevices.txt to reflect the last 5 or so
> years of API changes (though no new features are documented).
>
> They also remove documentation of a wrinkle in ethtool_ops that I
> recently fixed.
>
> These could safely be applied to either net or net-next; they can't
> break anything but another 3 months of dust isn't going to make much
> difference.
Thanks for doing this work Ben, I've applied it all to 'net'.
^ permalink raw reply
* Re: [PATCH V4 net-next 05/26] bfin_mac: Support the get_ts_info ethtool method.
From: Richard Cochran @ 2012-04-06 7:04 UTC (permalink / raw)
To: David Miller
Cc: paul.gortmaker, netdev, bhutchings, mporter, jacob.e.keller,
jeffrey.t.kirsher, john.ronciak, e1000-devel, linux-next
In-Reply-To: <20120406.001805.562264431747635985.davem@davemloft.net>
On Fri, Apr 06, 2012 at 12:18:05AM -0400, David Miller wrote:
> > Trailing semicolon. Compile fail in linux-next due to a85bbddd.
>
> I just pushed out the obvious fix for this.
Paul, thanks for compile testing, and David, thanks for fixing these.
Next time I promise to do at least the compile testing for the various
different architectures.
Richard
^ permalink raw reply
* Re: linux-next: build failure after merge of the final tree (net-next tree related)
From: Giuseppe CAVALLARO @ 2012-04-06 7:09 UTC (permalink / raw)
To: David Miller
Cc: sfr, netdev, linux-next, linux-kernel, deepak.sikri,
shiraz.hashim, vikas.manocha
In-Reply-To: <20120404.235933.350585244341009071.davem@davemloft.net>
On 4/5/2012 5:59 AM, David Miller wrote:
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Thu, 5 Apr 2012 13:25:15 +1000
>
>> After merging the final tree, today's linux-next build (powerpc
>> allyesconfig) failed like this:
>>
>> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c: In function 'stmmac_probe_config_dt':
>> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:53:7: error: 'struct plat_stmmacenet_data' has no member named 'pbl'
>>
>> Caused by commit 8327eb65e795 ("stmmac: re-work the internal GMAC DMA
>> platf parameters"). Clearly I am missing something, because I can't see
>> how that patch is correct since nothing ever assigns to the new dma_cfg
>> pointer but it is dereferenced.
>>
>> I have added this patch to make it build today, but it is obviously not
>> correct.
>
> Sorry Stephen.
>
> Giuseppe don't submit patches to me that don't even compile.
>
> Please submit a proper fix to me for this _now_.
Sorry Dave!
I don't know what happened (I believed to test all).
I'm sending you the patches soon!!!
Peppe
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [net-next] stmmac: fix build when CONFIG_OF is enable
From: Giuseppe CAVALLARO @ 2012-04-06 7:25 UTC (permalink / raw)
To: netdev; +Cc: davem, sfr, Giuseppe Cavallaro
In-Reply-To: <20120405132515.e61465875407a452c1a6f78e@canb.auug.org.au>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 116529a..12bd221 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -50,7 +50,7 @@ static int __devinit stmmac_probe_config_dt(struct platform_device *pdev,
* once needed on other platforms.
*/
if (of_device_is_compatible(np, "st,spear600-gmac")) {
- plat->pbl = 8;
+ plat->dma_cfg->pbl = 8;
plat->has_gmac = 1;
plat->pmt = 1;
}
--
1.7.4.4
^ permalink raw reply related
* Re: linux-next: build failure after merge of the final tree (net-next tree related)
From: Giuseppe CAVALLARO @ 2012-04-06 7:30 UTC (permalink / raw)
To: Giuseppe CAVALLARO
Cc: David Miller, sfr, netdev, linux-next, linux-kernel, deepak.sikri,
shiraz.hashim, vikas.manocha
In-Reply-To: <4F7E96B9.9060907@st.com>
On 4/6/2012 9:09 AM, Giuseppe CAVALLARO wrote:
> On 4/5/2012 5:59 AM, David Miller wrote:
>> From: Stephen Rothwell <sfr@canb.auug.org.au>
>> Date: Thu, 5 Apr 2012 13:25:15 +1000
>>
>>> After merging the final tree, today's linux-next build (powerpc
>>> allyesconfig) failed like this:
>>>
>>> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c: In function 'stmmac_probe_config_dt':
>>> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:53:7: error: 'struct plat_stmmacenet_data' has no member named 'pbl'
>>>
>>> Caused by commit 8327eb65e795 ("stmmac: re-work the internal GMAC DMA
>>> platf parameters"). Clearly I am missing something, because I can't see
>>> how that patch is correct since nothing ever assigns to the new dma_cfg
>>> pointer but it is dereferenced.
>>>
>>> I have added this patch to make it build today, but it is obviously not
>>> correct.
>>
>> Sorry Stephen.
>>
>> Giuseppe don't submit patches to me that don't even compile.
>>
>> Please submit a proper fix to me for this _now_.
>
> Sorry Dave!
> I don't know what happened (I believed to test all).
> I'm sending you the patches soon!!!
Please Stephen,
can you test it on your kernel. I didn't build when CONFIG_OF is enable.
I tested on ARM/SH/x86 but never with CONFIG_OF on.
The problem was due to the hard-coded values adding with devicetree support.
Sorry for this problem, next time I'll build also on OF.
I had also done a grep in all the arch to verify the ARCH that used the
pbl/fixed_burst/burst_len fields :-(. These have been moved in a
separate structure.
Let me know if the patch is ok.
Peppe
>
> Peppe
>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [net-next] stmmac: fix build when CONFIG_OF is enable
From: David Miller @ 2012-04-06 7:34 UTC (permalink / raw)
To: peppe.cavallaro; +Cc: netdev, sfr
In-Reply-To: <1333697117-25592-1-git-send-email-peppe.cavallaro@st.com>
From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Fri, 6 Apr 2012 09:25:17 +0200
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Applied, thanks.
^ permalink raw reply
* [PATCH net-next v2] tcp: bind() use stronger condition for bind_conflict
From: Alexandru Copot @ 2012-04-06 7:47 UTC (permalink / raw)
To: davem, eric.dumazet
Cc: fbl, kuznet, jmorris, yoshfuji, kaber, netdev, Alex Copot,
Daniel Baluta
From: Alex Copot <alex.mihai.c@gmail.com>
We must try harder to get unique (addr, port) pairs when
doing port autoselection for sockets with SO_REUSEADDR
option set.
We achieve this by adding a relaxation parameter to
inet_csk_bind_conflict. When 'relax' parameter is off
we return a conflict whenever the current searched
pair (addr, port) is not unique.
This tries to address the problems reported in patch:
8d238b25b1ec22a73b1c2206f111df2faaff8285
Revert "tcp: bind() fix when many ports are bound"
Tests where ran for creating and binding(0) many sockets
on 100 IPs. The results are, on average:
* 60000 sockets, 600 ports / IP:
* 0.210 s, 620 (IP, port) duplicates without patch
* 0.219 s, no duplicates with patch
* 100000 sockets, 1000 ports / IP:
* 0.371 s, 1720 duplicates without patch
* 0.373 s, no duplicates with patch
* 200000 sockets, 2000 ports / IP:
* 0.766 s, 6900 duplicates without patch
* 0.768 s, no duplicates with patch
* 500000 sockets, 5000 ports / IP:
* 2.227 s, 41500 duplicates without patch
* 2.284 s, no duplicates with patch
Signed-off-by: Alex Copot <alex.mihai.c@gmail.com>
Signed-off-by: Daniel Baluta <dbaluta@ixiacom.com>
---
Changes from v1:
- Fixed coding style
- Replaced int flag with bool
- Added test results
---
include/net/inet6_connection_sock.h | 2 +-
include/net/inet_connection_sock.h | 4 ++--
net/ipv4/inet_connection_sock.c | 17 +++++++++++++----
net/ipv6/inet6_connection_sock.c | 2 +-
4 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/include/net/inet6_connection_sock.h b/include/net/inet6_connection_sock.h
index 3207e58..1866a67 100644
--- a/include/net/inet6_connection_sock.h
+++ b/include/net/inet6_connection_sock.h
@@ -23,7 +23,7 @@ struct sock;
struct sockaddr;
extern int inet6_csk_bind_conflict(const struct sock *sk,
- const struct inet_bind_bucket *tb);
+ const struct inet_bind_bucket *tb, bool relax);
extern struct dst_entry* inet6_csk_route_req(struct sock *sk,
const struct request_sock *req);
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index dbf9aab..46c9e2c 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -60,7 +60,7 @@ struct inet_connection_sock_af_ops {
#endif
void (*addr2sockaddr)(struct sock *sk, struct sockaddr *);
int (*bind_conflict)(const struct sock *sk,
- const struct inet_bind_bucket *tb);
+ const struct inet_bind_bucket *tb, bool relax);
};
/** inet_connection_sock - INET connection oriented sock
@@ -245,7 +245,7 @@ extern struct request_sock *inet_csk_search_req(const struct sock *sk,
const __be32 raddr,
const __be32 laddr);
extern int inet_csk_bind_conflict(const struct sock *sk,
- const struct inet_bind_bucket *tb);
+ const struct inet_bind_bucket *tb, bool relax);
extern int inet_csk_get_port(struct sock *sk, unsigned short snum);
extern struct dst_entry* inet_csk_route_req(struct sock *sk,
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 19d66ce..13ef772 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -53,7 +53,7 @@ void inet_get_local_port_range(int *low, int *high)
EXPORT_SYMBOL(inet_get_local_port_range);
int inet_csk_bind_conflict(const struct sock *sk,
- const struct inet_bind_bucket *tb)
+ const struct inet_bind_bucket *tb, bool relax)
{
struct sock *sk2;
struct hlist_node *node;
@@ -79,6 +79,13 @@ int inet_csk_bind_conflict(const struct sock *sk,
sk2_rcv_saddr == sk_rcv_saddr(sk))
break;
}
+ if (!relax && reuse && sk2->sk_reuse &&
+ sk2->sk_state != TCP_LISTEN) {
+ const __be32 sk2_rcv_saddr = sk_rcv_saddr(sk2);
+ if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) ||
+ sk2_rcv_saddr == sk_rcv_saddr(sk))
+ break;
+ }
}
}
return node != NULL;
@@ -122,12 +129,13 @@ again:
(tb->num_owners < smallest_size || smallest_size == -1)) {
smallest_size = tb->num_owners;
smallest_rover = rover;
- if (atomic_read(&hashinfo->bsockets) > (high - low) + 1) {
+ if (atomic_read(&hashinfo->bsockets) > (high - low) + 1 &&
+ !inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb, false)) {
snum = smallest_rover;
goto tb_found;
}
}
- if (!inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) {
+ if (!inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb, false)) {
snum = rover;
goto tb_found;
}
@@ -178,12 +186,13 @@ tb_found:
goto success;
} else {
ret = 1;
- if (inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) {
+ if (inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb, true)) {
if (sk->sk_reuse && sk->sk_state != TCP_LISTEN &&
smallest_size != -1 && --attempts >= 0) {
spin_unlock(&head->lock);
goto again;
}
+
goto fail_unlock;
}
}
diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index 02dd203..e6cee52 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -28,7 +28,7 @@
#include <net/inet6_connection_sock.h>
int inet6_csk_bind_conflict(const struct sock *sk,
- const struct inet_bind_bucket *tb)
+ const struct inet_bind_bucket *tb, bool relax)
{
const struct sock *sk2;
const struct hlist_node *node;
--
1.7.9.6
^ permalink raw reply related
* [PATCH] netlink: fix races after skb queueing
From: Eric Dumazet @ 2012-04-06 8:17 UTC (permalink / raw)
To: David Miller; +Cc: netdev
As soon as an skb is queued into socket receive_queue, another thread
can consume it, so we are not allowed to reference skb anymore, or risk
use after free.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/netlink/af_netlink.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 32bb753..faa48f7 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -829,12 +829,19 @@ int netlink_attachskb(struct sock *sk, struct sk_buff *skb,
return 0;
}
-int netlink_sendskb(struct sock *sk, struct sk_buff *skb)
+static int __netlink_sendskb(struct sock *sk, struct sk_buff *skb)
{
int len = skb->len;
skb_queue_tail(&sk->sk_receive_queue, skb);
sk->sk_data_ready(sk, len);
+ return len;
+}
+
+int netlink_sendskb(struct sock *sk, struct sk_buff *skb)
+{
+ int len = __netlink_sendskb(sk, skb);
+
sock_put(sk);
return len;
}
@@ -957,8 +964,7 @@ static int netlink_broadcast_deliver(struct sock *sk, struct sk_buff *skb)
if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf &&
!test_bit(0, &nlk->state)) {
skb_set_owner_r(skb, sk);
- skb_queue_tail(&sk->sk_receive_queue, skb);
- sk->sk_data_ready(sk, skb->len);
+ __netlink_sendskb(sk, skb);
return atomic_read(&sk->sk_rmem_alloc) > (sk->sk_rcvbuf >> 1);
}
return -1;
@@ -1698,10 +1704,8 @@ static int netlink_dump(struct sock *sk)
if (sk_filter(sk, skb))
kfree_skb(skb);
- else {
- skb_queue_tail(&sk->sk_receive_queue, skb);
- sk->sk_data_ready(sk, skb->len);
- }
+ else
+ __netlink_sendskb(sk, skb);
return 0;
}
@@ -1715,10 +1719,8 @@ static int netlink_dump(struct sock *sk)
if (sk_filter(sk, skb))
kfree_skb(skb);
- else {
- skb_queue_tail(&sk->sk_receive_queue, skb);
- sk->sk_data_ready(sk, skb->len);
- }
+ else
+ __netlink_sendskb(sk, skb);
if (cb->done)
cb->done(cb);
^ permalink raw reply related
* Re: [PATCH] netlink: fix races after skb queueing
From: David Miller @ 2012-04-06 8:21 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1333700266.5312.19.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 06 Apr 2012 10:17:46 +0200
> As soon as an skb is queued into socket receive_queue, another thread
> can consume it, so we are not allowed to reference skb anymore, or risk
> use after free.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Good catch Eric.
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH] m68k/atari: EtherNEC - rewrite to use mainstream ne.c, take two
From: Geert Uytterhoeven @ 2012-04-06 8:28 UTC (permalink / raw)
To: Michael Schmitz; +Cc: Paul Gortmaker, linux-m68k, netdev
In-Reply-To: <4F7E184B.6000903@gmail.com>
On Fri, Apr 6, 2012 at 00:10, Michael Schmitz <schmitzmic@googlemail.com> wrote:
>> Indeed, with a small modification (keep multi-platform kernels in mind):
>>
>> #ifdef CONFIG_ATARI
>> #define EI_IRQ_FLAGS (MACH_IS_ATARI ? IRQF_SHARED : 0)
>> #else
>> #define EI_IRQ_FLAGS 0
>> #endif
>>
>
> Right you are. Is any other m68k platform using ne.c directly, or do you
> plan to convert all other NE2000 based drivers to ne.c now?
Plain ne is also used on Q40.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] netlink: fix races after skb queueing
From: Eric Dumazet @ 2012-04-06 8:34 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20120406.042144.742495137515648292.davem@davemloft.net>
On Fri, 2012-04-06 at 04:21 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 06 Apr 2012 10:17:46 +0200
>
> > As soon as an skb is queued into socket receive_queue, another thread
> > can consume it, so we are not allowed to reference skb anymore, or risk
> > use after free.
> >
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> Good catch Eric.
>
> Applied and queued up for -stable.
Same problem with sock_queue_err_skb(), I'll send a patch for it
Thanks
^ permalink raw reply
* [PATCH] net: fix a race in sock_queue_err_skb()
From: Eric Dumazet @ 2012-04-06 8:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev
As soon as an skb is queued into socket error queue, another thread
can consume it, so we are not allowed to reference skb anymore, or risk
use after free.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/core/skbuff.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index f223cdc..baf8d28 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3161,6 +3161,8 @@ static void sock_rmem_free(struct sk_buff *skb)
*/
int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)
{
+ int len = skb->len;
+
if (atomic_read(&sk->sk_rmem_alloc) + skb->truesize >=
(unsigned)sk->sk_rcvbuf)
return -ENOMEM;
@@ -3175,7 +3177,7 @@ int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)
skb_queue_tail(&sk->sk_error_queue, skb);
if (!sock_flag(sk, SOCK_DEAD))
- sk->sk_data_ready(sk, skb->len);
+ sk->sk_data_ready(sk, len);
return 0;
}
EXPORT_SYMBOL(sock_queue_err_skb);
^ 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