* Re: [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set
From: Sergei Shtylyov @ 2014-12-05 13:21 UTC (permalink / raw)
To: roopa, jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
aviadr
Cc: netdev, davem, shm, gospo
In-Reply-To: <1417746401-8140-3-git-send-email-roopa@cumulusnetworks.com>
Hello.
On 12/5/2014 5:26 AM, roopa@cumulusnetworks.com wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> This allows offloading to switch asic without having the user to set
> any flag. And this is done in the bridge driver to rollback kernel settings
> on hw offload failure if required in the future.
> With this, it also makes sure a notification goes out only after the
> attributes are set both in the kernel and hw.
> ---
> net/bridge/br_netlink.c | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 9f5eb55..ce173f0 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -407,9 +407,21 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh)
> afspec, RTM_SETLINK);
> }
>
> + if ((dev->features & NETIF_F_HW_SWITCH_OFFLOAD) &&
> + dev->netdev_ops->ndo_bridge_setlink) {
> + int ret = dev->netdev_ops->ndo_bridge_setlink(dev, nlh);
Need empty line after declaration.
> + if (ret && ret != -EOPNOTSUPP) {
> + /* XXX Fix this in the future to rollback
> + * kernel settings and return error
> + */
> + br_warn(p->br, "error offloading bridge attributes "
> + "on port %u(%s)\n", (unsigned int) p->port_no,
Don't break up the message strings. Also, your continuation lines should
start under the next character after ( on the first line.
> + p->dev->name);
> + }
> + }
> +
> if (err == 0)
> br_ifinfo_notify(RTM_NEWLINK, p);
> -
Why?
> out:
> return err;
> }
> @@ -433,6 +445,19 @@ int br_dellink(struct net_device *dev, struct nlmsghdr *nlh)
> err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
> afspec, RTM_DELLINK);
>
> + if (dev->features & NETIF_F_HW_SWITCH_OFFLOAD
> + && dev->netdev_ops->ndo_bridge_setlink) {
> + int ret = dev->netdev_ops->ndo_bridge_dellink(dev, nlh);
> + if (ret && ret != -EOPNOTSUPP) {
> + /* XXX Fix this in the future to rollback
> + * kernel settings and return error
> + */
> + br_warn(p->br, "error offloading bridge attributes "
> + "on port %u(%s)\n", (unsigned int) p->port_no,
> + p->dev->name);
Same comments here.
> + }
> + }
> +
> return err;
> }
> static int br_validate(struct nlattr *tb[], struct nlattr *data[])
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 3/3] rocker: set feature NETIF_F_HW_SWITCH_OFFLOAD
From: Sergei Shtylyov @ 2014-12-05 13:18 UTC (permalink / raw)
To: roopa, jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
aviadr
Cc: netdev, davem, shm, gospo
In-Reply-To: <1417746401-8140-4-git-send-email-roopa@cumulusnetworks.com>
Hello.
On 12/5/2014 5:26 AM, roopa@cumulusnetworks.com wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> This patch just sets the feature flag on rocker ports
You didn't sign off on any of your patches, hence they can't be applied.
> ---
> drivers/net/ethernet/rocker/rocker.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
> index fded127..3fe19b0 100644
> --- a/drivers/net/ethernet/rocker/rocker.c
> +++ b/drivers/net/ethernet/rocker/rocker.c
> @@ -4003,7 +4003,8 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
> NAPI_POLL_WEIGHT);
> rocker_carrier_init(rocker_port);
>
> - dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> + dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
> + NETIF_F_HW_SWITCH_OFFLOAD;
There was no need tho break the line.
[...]
WBR, Sergei
^ permalink raw reply
* [PATCH 1/1] net: macb: Remove obsolete comment from Kconfig
From: James Byrne @ 2014-12-05 13:03 UTC (permalink / raw)
To: Nicolas Ferre; +Cc: netdev, James Byrne
The Kconfig file says that Gigabit mode is not supported, but it has been
supported since commit 140b7552fdff04bbceeb842f0e04f0b4015fe97b ("net/macb:
Add support for Gigabit Ethernet mode").
Signed-off-by: James Byrne <james.byrne@origamienergy.com>
---
drivers/net/ethernet/cadence/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 9e089d2..6932be0 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -35,8 +35,8 @@ config MACB
---help---
The Cadence MACB ethernet interface is found on many Atmel AT32 and
AT91 parts. This driver also supports the Cadence GEM (Gigabit
- Ethernet MAC found in some ARM SoC devices). Note: the Gigabit mode
- is not yet supported. Say Y to include support for the MACB/GEM chip.
+ Ethernet MAC found in some ARM SoC devices). Say Y to include
+ support for the MACB/GEM chip.
To compile this driver as a module, choose M here: the module
will be called macb.
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads
From: Jamal Hadi Salim @ 2014-12-05 12:50 UTC (permalink / raw)
To: Jiri Pirko
Cc: roopa, sfeldma, bcrl, tgraf, john.fastabend, stephen, linville,
nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh, aviadr,
netdev, davem, shm, gospo
In-Reply-To: <20141205121730.GK1866@nanopsycho.orion>
On 12/05/14 07:17, Jiri Pirko wrote:
> Fri, Dec 05, 2014 at 01:06:50PM CET, jhs@mojatatu.com wrote:
>> On 12/04/14 21:26, roopa@cumulusnetworks.com wrote:
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This is a generic high level feature flag for all switch asic features today.
>>>
>>> switch drivers set this flag on switch ports. Logical devices like
>>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>>
>>> I had to use SWITCH in the name to avoid ambiguity with other feature
>>> flags. But, since i have been harping about not calling it 'switch',
>>> I am welcome to any suggestions :)
>>>
>>
>> I know in this use case it is a switch. But please dont use that term
>> for things that we are going to use as generic offload descriptors.
>> How about just simple: NETIF_F_HW_DEV_OFFLOAD_BIT
>
> What that should tell it stands for? I think we need something more
> specific.
>
Ok, the use case in this instance is very specific. My blinders
go off anytime i see switch or sw or swdev.
In which case maybe what Scott suggested but changing L2FWD
with FDB?
cheers,
jamal
^ permalink raw reply
* Re: [PATCH v2 6/6] net-PPP: Delete another unnecessary assignment in mppe_alloc()
From: SF Markus Elfring @ 2014-12-05 12:50 UTC (permalink / raw)
To: Dan Carpenter
Cc: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet,
LKML, kernel-janitors, Julia Lawall
In-Reply-To: <20141205122315.GC4912@mwanda>
>> The data structure element "sha1" was assigned a null pointer by the
>> mppe_alloc() after a function call "crypto_alloc_hash" failed.
>
> This patch is also buggy.
Do you really want to keep a variable reset after a detected failure?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v2 5/6] net-PPP: Delete an unnecessary assignment in mppe_alloc()
From: SF Markus Elfring @ 2014-12-05 12:44 UTC (permalink / raw)
To: Dan Carpenter
Cc: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet,
LKML, kernel-janitors, Julia Lawall
In-Reply-To: <20141205122214.GB4912@mwanda>
>> The data structure element "arc4" was assigned a null pointer by the
>> mppe_alloc() function if a previous function call "crypto_alloc_blkcipher"
>> failed.
>
> crypto_alloc_blkcipher() returns error pointers and not NULL.
That is true.
> This patch creates a bug.
Please explain: How?
Did you notice that the data structure element "arc4" was reset
to a null pointer if a failure was detected for the function
call "crypto_alloc_blkcipher"?
Do you find this specific assignment still necessary for exception
handling in the implementation of mppe_alloc() function?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH 2/2] gianfar: handle map error in gfar_start_xmit()
From: Denis Kirjanov @ 2014-12-05 12:42 UTC (permalink / raw)
To: Arseny Solokha; +Cc: Claudiu Manoil, netdev, linux-kernel
In-Reply-To: <1417775874-17775-3-git-send-email-asolokha@kb.kras.ru>
On 12/5/14, Arseny Solokha <asolokha@kb.kras.ru> wrote:
> From: Arseny Solokha <asolokha@kb.kras.ru>
>
> When DMA-API debugging is enabled in the kernel, it spews the following
> upon upping the link:
>
> WARNING: at lib/dma-debug.c:1135
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W O 3.18.0-rc7 #1
> task: c0720340 ti: effe2000 task.ti: c0750000
> NIP: c01d7c1c LR: c01d7c1c CTR: c02250fc
> REGS: effe3d40 TRAP: 0700 Tainted: G W O (3.18.0-rc7)
> MSR: 00021000 <CE,ME> CR: 22044242 XER: 20000000
>
> GPR00: c01d7c1c effe3df0 c0720340 00000095 c201e404 c201e9f0 00021000
> 01a9d000
> GPR08: 00000007 00000000 01a9d000 00000313 22044242 00583f60 05f41012
> 00000000
> GPR16: 00000000 000000ff 00000000 00000000 00000000 c5dc3b40 c5677720
> 00000001
> GPR24: c0730000 00029000 c0d0d828 c072c394 effe3e48 c075baec c0d0f020
> ee31e600
> NIP [c01d7c1c] check_unmap+0x5b4/0xae4
> LR [c01d7c1c] check_unmap+0x5b4/0xae4
> Call Trace:
> [effe3df0] [c01d7c1c] check_unmap+0x5b4/0xae4 (unreliable)
> [effe3e40] [c01d81c4] debug_dma_unmap_page+0x78/0x8c
> [effe3ec0] [c0286ba0] gfar_clean_tx_ring+0x120/0x3c0
> [effe3f30] [c0286f90] gfar_poll_tx_sq+0x48/0x94
> o[effe3f50] [c030c388] net_rx_action+0x130/0x1ac
> [effe3f80] [c00319e0] __do_softirq+0x134/0x240
> [effe3fe0] [c0031dd0] irq_exit+0xa4/0xc8
> [effe3ff0] [c000e01c] call_do_irq+0x24/0x3c
> [c0751e70] [c0004a04] do_IRQ+0x8c/0x108
> [c0751e90] [c0010068] ret_from_except+0x0/0x18
> --- interrupt: 501 at arch_cpu_idle+0x24/0x5c
> LR = arch_cpu_idle+0x24/0x5c
> [c0751f50] [c007d2e4] rcu_idle_enter+0xc8/0xcc (unreliable)
> [c0751f60] [c006587c] cpu_startup_entry+0x1d4/0x29c
> [c0751fb0] [c054399c] start_kernel+0x338/0x34c
> [c0751ff0] [c000046c] set_ivor+0x154/0x190
> Instruction dump:
> 394adb30 80fc0018 811c001c 3c60c04f 5529103a 7cca482e 38639e60 813c0020
> 815c0024 90c10008 4cc63182 48240b01 <0fe00000> 3c60c04f 3863971c 4cc63182
> ---[ end trace 3eb7bf62ba1b80f9 ]---
> Mapped at:
> [<c0287420>] gfar_start_xmit+0x424/0x910
> [<c030e964>] dev_hard_start_xmit+0x20c/0x3d8
> [<c032cc3c>] sch_direct_xmit+0x124/0x22c
> [<c030ede8>] __dev_queue_xmit+0x2b8/0x674
>
> Or the following upon starting transmission of some large chunks
> of data:
>
> fsl-gianfar ffe25000.ethernet: DMA-API: device driver failed to check map
> error[device address=0x0000000005fa8000]
> ------------[ cut here ]------------
> WARNING: at lib/dma-debug.c:1135
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G O 3.18.0-rc7 #35
> task: c071d340 ti: effe2000 task.ti: c074e000
> NIP: c01d7c1c LR: c01d7c1c CTR: c022339c
> REGS: effe3d40 TRAP: 0700 Tainted: G O (3.18.0-rc7)
> MSR: 00021000 <CE,ME> CR: 22044242 XER: 20000000
>
> GPR00: c01d7c1c effe3df0 c071d340 00000094 00000001 c0071750 00000000
> 00000001
> GPR08: 00000000 00000000 effe2000 00000000 20044242 00581f60 05fa8000
> 000001c4
> GPR16: 00000000 000000ff 00000000 000000e4 00000039 c5b1a9c0 c5679c60
> 00000002
> GPR24: c0730000 00029000 c0d0c528 c0729394 effe3e48 c0759aec c0d0d020
> ee315900
> NIP [c01d7c1c] check_unmap+0x5b4/0xae4
> LR [c01d7c1c] check_unmap+0x5b4/0xae4
> Call Trace:
> [effe3df0] [c01d7c1c] check_unmap+0x5b4/0xae4 (unreliable)
> [effe3e40] [c01d81c4] debug_dma_unmap_page+0x78/0x8c
> [effe3ec0] [c0284c78] gfar_clean_tx_ring+0x1b4/0x3c0
> [effe3f30] [c0284fd4] gfar_poll_tx_sq+0x48/0x94
> [effe3f50] [c030a5c4] net_rx_action+0x130/0x1ac
> [effe3f80] [c00319e0] __do_softirq+0x134/0x240
> [effe3fe0] [c0031dd0] irq_exit+0xa4/0xc8
> [effe3ff0] [c000e01c] call_do_irq+0x24/0x3c
> [c074fe70] [c0004a04] do_IRQ+0x8c/0x108
> [c074fe90] [c0010068] ret_from_except+0x0/0x18
> --- interrupt: 501 at arch_cpu_idle+0x24/0x5c
> LR = arch_cpu_idle+0x24/0x5c
> [c074ff50] [c007d2e4] rcu_idle_enter+0xc8/0xcc (unreliable)
> [c074ff60] [c006587c] cpu_startup_entry+0x1d4/0x29c
> [c074ffb0] [c054199c] start_kernel+0x338/0x34c
> [c074fff0] [c000046c] set_ivor+0x154/0x190
> Instruction dump:
> 394abb30 80fc0018 811c001c 3c60c04e 5529103a 7cca482e 386379f0 813c0020
> 815c0024 90c10008 4cc63182 4823ed39 <0fe00000> 3c60c04e 386372ac 4cc63182
> ---[ end trace 008c59ca7ca1f712 ]---
> Mapped at:
> [<c0285264>] gfar_start_xmit+0x224/0x95c
> [<c030cba0>] dev_hard_start_xmit+0x20c/0x3d8
> [<c032ae78>] sch_direct_xmit+0x124/0x22c
> [<c032b008>] __qdisc_run+0x88/0x1c0
> [<c0307920>] net_tx_action+0xf0/0x19c
>
> Ignore these mapping failures in hope we'll have more luck next time.
>
> Signed-off-by: Arseny Solokha <asolokha@kb.kras.ru>
> ---
> drivers/net/ethernet/freescale/gianfar.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.c
> b/drivers/net/ethernet/freescale/gianfar.c
> index f34ca55..9ea887e 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -2296,6 +2296,12 @@ static int gfar_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
> 0,
> frag_len,
> DMA_TO_DEVICE);
> + if (unlikely(dma_mapping_error(priv->dev, bufaddr))) {
> + /* As DMA mapping failed, pretend the TX path
> + * is busy to retry later
> + */
> + return NETDEV_TX_BUSY;
> + }
>
> /* set the TxBD length and buffer pointer */
> txbdp->bufPtr = bufaddr;
> @@ -2345,8 +2351,15 @@ static int gfar_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
> fcb->ptp = 1;
> }
>
> - txbdp_start->bufPtr = dma_map_single(priv->dev, skb->data,
> - skb_headlen(skb), DMA_TO_DEVICE);
> + bufaddr = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
> + DMA_TO_DEVICE);
> + if (unlikely(dma_mapping_error(priv->dev, bufaddr))) {
> + /* As DMA mapping failed, pretend the TX path is busy to retry
> + * later
> + */
> + return NETDEV_TX_BUSY;
> + }
> + txbdp_start->bufPtr = bufaddr;
>
> /* If time stamping is requested one additional TxBD must be set up. The
> * first TxBD points to the FCB and must have a data length of
> --
You have to return TX_BUSY in the case of the full hw queue.
Have you tested your changes with fault injection?
> 2.2.0
>
> --
> 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 2/3] bridge: offload bridge port attributes to switch asic if feature flag set
From: Jamal Hadi Salim @ 2014-12-05 12:41 UTC (permalink / raw)
To: Roopa Prabhu, John Fastabend
Cc: Scott Feldman, Jiří Pírko, Benjamin LaHaise,
Thomas Graf, stephen@networkplumber.org, John Linville,
nhorman@tuxdriver.com, Nicolas Dichtel, vyasevic@redhat.com,
Florian Fainelli, buytenh@wantstofly.org, Aviad Raveh, Netdev,
David S. Miller, shm, Andy Gospodarek
In-Reply-To: <54815A4E.3020806@cumulusnetworks.com>
On 12/05/14 02:10, Roopa Prabhu wrote:
> On 12/4/14, 10:55 PM, John Fastabend wrote:
>> On 12/04/2014 10:41 PM, Scott Feldman wrote:
>>> On Thu, Dec 4, 2014 at 6:26 PM, <roopa@cumulusnetworks.com> wrote:
>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>
>>>> This allows offloading to switch asic without having the user to set
>>>> any flag. And this is done in the bridge driver to rollback kernel
>>>> settings
>>>> on hw offload failure if required in the future.
>>>>
>>>> With this, it also makes sure a notification goes out only after the
>>>> attributes are set both in the kernel and hw.
>>>
>>> I like this approach as it streamlines the steps for the user in
>>> setting port flags. There is one case for FLOODING where you'll have
>>> to turn off flooding for both, and then turn on flooding in hw. You
>>> don't want flooding turned on on kernel and hw.
>>>
>>>> ---
>>>> net/bridge/br_netlink.c | 27 ++++++++++++++++++++++++++-
>>>> 1 file changed, 26 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>>> index 9f5eb55..ce173f0 100644
>>>> --- a/net/bridge/br_netlink.c
>>>> +++ b/net/bridge/br_netlink.c
>>>> @@ -407,9 +407,21 @@ int br_setlink(struct net_device *dev, struct
>>>> nlmsghdr *nlh)
>>>> afspec, RTM_SETLINK);
>>>> }
>>>>
>>>> + if ((dev->features & NETIF_F_HW_SWITCH_OFFLOAD) &&
>>>> + dev->netdev_ops->ndo_bridge_setlink) {
>>>> + int ret = dev->netdev_ops->ndo_bridge_setlink(dev,
>>>> nlh);
>>>
>>> I think you want to up-level this to net/core/rtnetlink.c because
>>> you're only enabling the feature for one instance of a driver that
>>> implements ndo_bridge_setlink: the bridge driver. If another driver
>>> was MASTER and implemented ndo_bridge_setlink, you'd want same check
>>> to push setting down to SELF port driver.
>>
>> Also if the user set SELF && MASTER flags && had HW_SWITCH_OFFLOAD bit
>> set we would call ndo_bridge_setlink twice on the dev. Maybe you can
>> catch this case in rtnetlink.c and only call it once.
>
> yes, thought about this and when i looked at iproute2 code, it is either
> master
> or self today and i don't think anybody else uses both flags with the
> current
> kernel implementation. But yes, that does not stop anybody from setting
> both flags.
> I will handle it better in v2.
folks, can we have probably 2-3 sets of patches?
#1 introduces the flags and doesnt change anything.
Others to introduce other features.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net-next] tipc: fix missing spinlock init and nullptr oops
From: Erik Hugne @ 2014-12-05 12:39 UTC (permalink / raw)
To: netdev, tipc-discussion, jon.maloy, ying.xue, richard.alpe
In-Reply-To: <1417622320-19730-1-git-send-email-erik.hugne@ericsson.com>
On Wed, Dec 03, 2014 at 04:58:40PM +0100, erik.hugne@ericsson.com wrote:
> From: Erik Hugne <erik.hugne@ericsson.com>
>
> commit 908344cdda80 ("tipc: fix bug in multicast congestion
> handling") introduced two bugs with the bclink wakeup
> function. This commit fixes the missing spinlock init for the
> waiting_sks list. We also eliminate the race condition
> between the waiting_sks length check/dequeue operations in
> tipc_bclink_wakeup_users by simply removing the redundant
> length check.
>
> Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
> Acked-by: Tero Aho <Tero.Aho@coriant.com>
Richard found another issue during regression testing closely related
to this, I have dropped it from patchwork and he will send in
v2.
//E
^ permalink raw reply
* Re: [patch net-next] net: sched: cls: remove unused op put from tcf_proto_ops
From: Jamal Hadi Salim @ 2014-12-05 12:31 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: davem
In-Reply-To: <1417725678-24968-1-git-send-email-jiri@resnulli.us>
On 12/04/14 15:41, Jiri Pirko wrote:
> It is never called and implementations are void. So just remove it.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
cheers,
jamal
^ permalink raw reply
* Re: [RFC][PATCH 07/13] iov_iter.c: get rid of bvec_copy_page_{to,from}_iter()
From: Sergei Shtylyov @ 2014-12-05 12:28 UTC (permalink / raw)
To: Al Viro, Linus Torvalds; +Cc: linux-kernel, linux-fsdevel, netdev
In-Reply-To: <1417724597-17099-7-git-send-email-viro@ZenIV.linux.org.uk>
Hello.
On 12/4/2014 11:23 PM, Al Viro wrote:
Looks like you've erred in the subject as you seem to be getting rid of
copy_page_{to|from}_iter_bvec() instead of bvec_copy_page_{to|from}_iter()...
> From: Al Viro <viro@zeniv.linux.org.uk>
> Just have copy_page_{to,from}_iter() fall back to kmap_atomic +
> copy_{to,from}_iter() + kunmap_atomic() in ITER_BVEC case. As
> the matter of fact, that's what we want to do for any iov_iter
> kind that isn't blocking - e.g. ITER_KVEC will also go that way
> once we recognize it on iov_iter.c primitives level
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> mm/iov_iter.c | 60 ++++++++++++++++++++++++-----------------------------------
> 1 file changed, 24 insertions(+), 36 deletions(-)
> diff --git a/mm/iov_iter.c b/mm/iov_iter.c
> index 39ad713..17b7144 100644
> --- a/mm/iov_iter.c
> +++ b/mm/iov_iter.c
> @@ -486,30 +486,33 @@ static size_t copy_from_iter_bvec(void *to, size_t bytes, struct iov_iter *i)
> return wanted;
> }
>
> -static size_t copy_page_to_iter_bvec(struct page *page, size_t offset,
> - size_t bytes, struct iov_iter *i)
> +size_t copy_to_iter(void *addr, size_t bytes, struct iov_iter *i)
> {
> - void *kaddr = kmap_atomic(page);
> - size_t wanted = copy_to_iter_bvec(kaddr + offset, bytes, i);
> - kunmap_atomic(kaddr);
> - return wanted;
> + if (i->type & ITER_BVEC)
> + return copy_to_iter_bvec(addr, bytes, i);
> + else
> + return copy_to_iter_iovec(addr, bytes, i);
> }
> +EXPORT_SYMBOL(copy_to_iter);
>
> -static size_t copy_page_from_iter_bvec(struct page *page, size_t offset,
> - size_t bytes, struct iov_iter *i)
> +size_t copy_from_iter(void *addr, size_t bytes, struct iov_iter *i)
> {
> - void *kaddr = kmap_atomic(page);
> - size_t wanted = copy_from_iter_bvec(kaddr + offset, bytes, i);
> - kunmap_atomic(kaddr);
> - return wanted;
> + if (i->type & ITER_BVEC)
> + return copy_from_iter_bvec(addr, bytes, i);
> + else
> + return copy_from_iter_iovec(addr, bytes, i);
> }
> +EXPORT_SYMBOL(copy_from_iter);
[...]
WBR, Sergei
^ permalink raw reply
* Re: [PATCH v2 6/6] net-PPP: Delete another unnecessary assignment in mppe_alloc()
From: Dan Carpenter @ 2014-12-05 12:23 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet,
LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5480DE25.2020802@users.sourceforge.net>
On Thu, Dec 04, 2014 at 11:20:21PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 4 Dec 2014 22:42:30 +0100
>
> The data structure element "sha1" was assigned a null pointer by the
> mppe_alloc() after a function call "crypto_alloc_hash" failed.
This patch is also buggy.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v2 5/6] net-PPP: Delete an unnecessary assignment in mppe_alloc()
From: Dan Carpenter @ 2014-12-05 12:22 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet,
LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5480DDC1.6040101@users.sourceforge.net>
On Thu, Dec 04, 2014 at 11:18:41PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 4 Dec 2014 22:33:34 +0100
>
> The data structure element "arc4" was assigned a null pointer by the
> mppe_alloc() function if a previous function call "crypto_alloc_blkcipher"
> failed.
No. crypto_alloc_blkcipher() returns error pointers and not NULL.
This patch creates a bug.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads
From: Jiri Pirko @ 2014-12-05 12:17 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: roopa, sfeldma, bcrl, tgraf, john.fastabend, stephen, linville,
nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh, aviadr,
netdev, davem, shm, gospo
In-Reply-To: <54819FDA.2030405@mojatatu.com>
Fri, Dec 05, 2014 at 01:06:50PM CET, jhs@mojatatu.com wrote:
>On 12/04/14 21:26, roopa@cumulusnetworks.com wrote:
>>From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>>This is a generic high level feature flag for all switch asic features today.
>>
>>switch drivers set this flag on switch ports. Logical devices like
>>bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>
>>I had to use SWITCH in the name to avoid ambiguity with other feature
>>flags. But, since i have been harping about not calling it 'switch',
>>I am welcome to any suggestions :)
>>
>
>I know in this use case it is a switch. But please dont use that term
>for things that we are going to use as generic offload descriptors.
>How about just simple: NETIF_F_HW_DEV_OFFLOAD_BIT
What that should tell it stands for? I think we need something more
specific.
>BTW: Didnt you already have a netdev specific offload feature flag?
>
>cheers,
>jamal
>
>>An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>>in net_device_flags.
>>---
>> include/linux/netdev_features.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>>diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>>index 8e30685..68db1de 100644
>>--- a/include/linux/netdev_features.h
>>+++ b/include/linux/netdev_features.h
>>@@ -66,6 +66,7 @@ enum {
>> NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
>> NETIF_F_HW_L2FW_DOFFLOAD_BIT, /* Allow L2 Forwarding in Hardware */
>> NETIF_F_BUSY_POLL_BIT, /* Busy poll */
>>+ NETIF_F_HW_SWITCH_OFFLOAD_BIT, /* HW switch offload */
>>
>> /*
>> * Add your fresh new feature above and remember to update
>>@@ -124,6 +125,7 @@ enum {
>> #define NETIF_F_HW_VLAN_STAG_TX __NETIF_F(HW_VLAN_STAG_TX)
>> #define NETIF_F_HW_L2FW_DOFFLOAD __NETIF_F(HW_L2FW_DOFFLOAD)
>> #define NETIF_F_BUSY_POLL __NETIF_F(BUSY_POLL)
>>+#define NETIF_F_HW_SWITCH_OFFLOAD __NETIF_F(HW_SWITCH_OFFLOAD)
>>
>> /* Features valid for ethtool to change */
>> /* = all defined minus driver/device-class-related */
>>
>
^ permalink raw reply
* Re: [bisected] xfrm: TCP connection initiating PMTU discovery stalls on v3.12+
From: Wolfgang Walter @ 2014-12-05 12:09 UTC (permalink / raw)
To: netdev; +Cc: Thomas Jarosch, Eric Dumazet, Herbert Xu, Steffen Klassert
In-Reply-To: <1769392.YOU9Lj6NQS@h2o.as.studentenwerk.mhn.de>
Hello,
as reverting this patch fixes this rather annoying problem: is it dangerous to
revert it as a workaround until the root cause is found?
Am Montag, 1. Dezember 2014, 17:41:23 schrieb Wolfgang Walter:
> Am Montag, 1. Dezember 2014, 14:17:28 schrieb Wolfgang Walter:
> > Am Samstag, 29. November 2014, 12:44:07 schrieb Thomas Jarosch:
> > > Hello,
> > >
> > > we're in the process of updating production level machines
> > > from kernel 3.4.101 to kernel 3.14.25. On one mail server
> > > we noticed that emails destined for an IPSec tunnel sometimes
> > > get stuck in the mail queue with TCP timeouts.
> > >
> > > To make a long story short: When the VPN connection is initially
> > > set up or re-newed, the path MTU for the xfrm tunnel is undetermined.
> > >
> > > As soon as a TCP client starts to send large packets,
> > > it triggers path MTU detection. Some middlebox on the
> > > way to the final server has a lower MTU and sends back
> > > an "ICMP fragmentation needed" packet as normal.
> > >
> > > With the old kernel, the packet size for the TCP connection inside
> > > the xfrm tunnel gets adjusted and all is fine. With kernel v3.12+,
> > > the connection stalls completely. Same thing with kernel v3.18-rc6.
> >
> > We see something similar with real nic (RTL8139). In our case only the
> > first tcp-connection which triggers PMTU stalls. Later tcp-connections
> > then work fine.
> >
> > I will revert that patch and see if that fixes the problem.
>
> Reverting the commit fixes the problem here, too.
>
> > > We wrote a small tool to mimic postfix's TCP behavior (see attached
> > > file).
> > > In the end it's a normal TCP client sending large packets.
> > > The server side is just "socat - tcp4-listen:667".
> > >
> > > If you run "socket_client" a second time, the path MTU
> > > for the xfrm tunnel is already known and packets flow normal, too.
> > >
> > >
> > > The "evil" commit in question is this one:
> > > ---------------------------------------------------------------------
> > > commit 8f26fb1c1ed81c33f5d87c5936f4d9d1b4118918
> > > Author: Eric Dumazet <edumazet@google.com>
> > > Date: Tue Oct 15 12:24:54 2013 -0700
> > >
> > > tcp: remove the sk_can_gso() check from tcp_set_skb_tso_segs()
> > >
> > > sk_can_gso() should only be used as a hint in tcp_sendmsg() to build
> > > GSO
> > >
> > > packets in the first place. (As a performance hint)
> > >
> > > Once we have GSO packets in write queue, we can not decide they are
> > > no
> > > longer GSO only because flow now uses a route which doesn't handle
> > > TSO/GSO.
> > >
> > > Core networking stack handles the case very well for us, all we need
> > > is keeping track of packet counts in MSS terms, regardless of
> > > segmentation done later (in GSO or hardware)
> > >
> > > Right now, if tcp_fragment() splits a GSO packet in two parts,
> > > @left and @right, and route changed through a non GSO device,
> > > both @left and @right have pcount set to 1, which is wrong,
> > > and leads to incorrect packet_count tracking.
> > >
> > > This problem was added in commit d5ac99a648 ("[TCP]: skb pcount with
> > > MTU
> > >
> > > discovery")
> > >
> > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > Signed-off-by: Neal Cardwell <ncardwell@google.com>
> > > Signed-off-by: Yuchung Cheng <ycheng@google.com>
> > > Reported-by: Maciej Żenczykowski <maze@google.com>
> > > Signed-off-by: David S. Miller <davem@davemloft.net>
> > >
> > > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> > > index 8fad1c1..d46f214 100644
> > > --- a/net/ipv4/tcp_output.c
> > > +++ b/net/ipv4/tcp_output.c
> > > @@ -989,8 +989,7 @@ static void tcp_set_skb_tso_segs(const struct sock
> > > *sk,
> > > struct sk_buff *skb, /* Make sure we own this skb before messing
> > > gso_size/gso_segs */ WARN_ON_ONCE(skb_cloned(skb));
> > >
> > > - if (skb->len <= mss_now || !sk_can_gso(sk) ||
> > > - skb->ip_summed == CHECKSUM_NONE) {
> > > + if (skb->len <= mss_now || skb->ip_summed == CHECKSUM_NONE) {
> > >
> > > /* Avoid the costly divide in the normal
> > >
> > > * non-TSO case.
> > > */
> > >
> > > ---------------------------------------------------------------------
> > >
> > > When I revert it, even kernel v3.18-rc6 starts working.
> > > But I doubt this is the root problem, may be just hiding another issue.
> > >
> > > --- Sample output of socket_client using vanilla v3.12 kernel ---
> > > [1417258063 SEND result: 4096, strerror: Success]
> > > tcp max seg: res: 0, max_seg: 1370
> > > [1417258063 SEND result: 4096, strerror: Success]
> > > tcp max seg: res: 0, max_seg: 1370
> > > [1417258063 SEND result: 4096, strerror: Success]
> > > tcp max seg: res: 0, max_seg: 1370
> > > [1417258063 SEND result: 4096, strerror: Success]
> > > tcp max seg: res: 0, max_seg: 1370
> > > [1417258063 SEND result: 4096, strerror: Success]
> > > tcp max seg: res: 0, max_seg: 1338
> > > [1417258063 SEND result: 4096, strerror: Success]
> > > tcp max seg: res: 0, max_seg: 1338
> > > *STUCK*
> > > --------------------------------------------------------
> > >
> > > The "machine" is running on KVM and using "virtio_net" as NIC driver.
> > > I've played with the ethtool offload settings:
> > >
> > > *** eth1 defaults ***
> > > Offload parameters for eth1:
> > > rx-checksumming: on
> > > tx-checksumming: on
> > > scatter-gather: on
> > > tcp-segmentation-offload: on
> > > udp-fragmentation-offload: on
> > > generic-segmentation-offload: on
> > > generic-receive-offload: on
> > > large-receive-offload: off
> > >
> > > *** eth1 working (no stalls) using vanilla kernel ***
> > > Offload parameters for eth1:
> > > rx-checksumming: on
> > > tx-checksumming: off <-- the magic switch
> > > scatter-gather: off
> > > tcp-segmentation-offload: off
> > > udp-fragmentation-offload: off
> > > generic-segmentation-offload: off
> > > generic-receive-offload: off
> > > large-receive-offload: off
> > >
> > > When I turn "tx-checksumming" back on, it fails again.
> > > Though that is probably also just a side effect.
> > >
> > > I can provide tcpdumps if needed but they are no real help
> > > since you can just see the kernel stops sending TCP packets.
> > > (and the outgoing TCP packets are encrypted in ESP packets)
> > >
> > >
> > > Any vague idea what might be the root cause?
> > >
> > > I also tried reverting commit 4d53eff48b5f03ce67f4f301d6acca1d2145cb7a
> > > ("xfrm: Don't queue retransmitted packets if the original is still on
> > > the
> > > host") but that didn't change the situation. In fact it wasn't even
> > > triggered.
> > >
> > > Please CC: comments. Thanks.
> > >
> > > Best regards,
> > > Thomas
> >
> > Regards,
>
> Regards,
Regards,
--
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
^ permalink raw reply
* Re: [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set
From: Jamal Hadi Salim @ 2014-12-05 12:07 UTC (permalink / raw)
To: roopa, jiri, sfeldma, bcrl, tgraf, john.fastabend, stephen,
linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
aviadr
Cc: netdev, davem, shm, gospo
In-Reply-To: <1417746401-8140-3-git-send-email-roopa@cumulusnetworks.com>
On 12/04/14 21:26, roopa@cumulusnetworks.com wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
Ok, so this one maintains status quo. Looks reasonable
to me.
cheers,
jamal
> This allows offloading to switch asic without having the user to set
> any flag. And this is done in the bridge driver to rollback kernel settings
> on hw offload failure if required in the future.
>
> With this, it also makes sure a notification goes out only after the
> attributes are set both in the kernel and hw.
> ---
> net/bridge/br_netlink.c | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 9f5eb55..ce173f0 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -407,9 +407,21 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh)
> afspec, RTM_SETLINK);
> }
>
> + if ((dev->features & NETIF_F_HW_SWITCH_OFFLOAD) &&
> + dev->netdev_ops->ndo_bridge_setlink) {
> + int ret = dev->netdev_ops->ndo_bridge_setlink(dev, nlh);
> + if (ret && ret != -EOPNOTSUPP) {
> + /* XXX Fix this in the future to rollback
> + * kernel settings and return error
> + */
> + br_warn(p->br, "error offloading bridge attributes "
> + "on port %u(%s)\n", (unsigned int) p->port_no,
> + p->dev->name);
> + }
> + }
> +
> if (err == 0)
> br_ifinfo_notify(RTM_NEWLINK, p);
> -
> out:
> return err;
> }
> @@ -433,6 +445,19 @@ int br_dellink(struct net_device *dev, struct nlmsghdr *nlh)
> err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
> afspec, RTM_DELLINK);
>
> + if (dev->features & NETIF_F_HW_SWITCH_OFFLOAD
> + && dev->netdev_ops->ndo_bridge_setlink) {
> + int ret = dev->netdev_ops->ndo_bridge_dellink(dev, nlh);
> + if (ret && ret != -EOPNOTSUPP) {
> + /* XXX Fix this in the future to rollback
> + * kernel settings and return error
> + */
> + br_warn(p->br, "error offloading bridge attributes "
> + "on port %u(%s)\n", (unsigned int) p->port_no,
> + p->dev->name);
> + }
> + }
> +
> return err;
> }
> static int br_validate(struct nlattr *tb[], struct nlattr *data[])
>
^ permalink raw reply
* Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads
From: Jamal Hadi Salim @ 2014-12-05 12:06 UTC (permalink / raw)
To: roopa, jiri, sfeldma, bcrl, tgraf, john.fastabend, stephen,
linville, nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh,
aviadr
Cc: netdev, davem, shm, gospo
In-Reply-To: <1417746401-8140-2-git-send-email-roopa@cumulusnetworks.com>
On 12/04/14 21:26, roopa@cumulusnetworks.com wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This is a generic high level feature flag for all switch asic features today.
>
> switch drivers set this flag on switch ports. Logical devices like
> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>
> I had to use SWITCH in the name to avoid ambiguity with other feature
> flags. But, since i have been harping about not calling it 'switch',
> I am welcome to any suggestions :)
>
I know in this use case it is a switch. But please dont use that term
for things that we are going to use as generic offload descriptors.
How about just simple: NETIF_F_HW_DEV_OFFLOAD_BIT
BTW: Didnt you already have a netdev specific offload feature flag?
cheers,
jamal
> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
> in net_device_flags.
> ---
> include/linux/netdev_features.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index 8e30685..68db1de 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -66,6 +66,7 @@ enum {
> NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
> NETIF_F_HW_L2FW_DOFFLOAD_BIT, /* Allow L2 Forwarding in Hardware */
> NETIF_F_BUSY_POLL_BIT, /* Busy poll */
> + NETIF_F_HW_SWITCH_OFFLOAD_BIT, /* HW switch offload */
>
> /*
> * Add your fresh new feature above and remember to update
> @@ -124,6 +125,7 @@ enum {
> #define NETIF_F_HW_VLAN_STAG_TX __NETIF_F(HW_VLAN_STAG_TX)
> #define NETIF_F_HW_L2FW_DOFFLOAD __NETIF_F(HW_L2FW_DOFFLOAD)
> #define NETIF_F_BUSY_POLL __NETIF_F(BUSY_POLL)
> +#define NETIF_F_HW_SWITCH_OFFLOAD __NETIF_F(HW_SWITCH_OFFLOAD)
>
> /* Features valid for ethtool to change */
> /* = all defined minus driver/device-class-related */
>
^ permalink raw reply
* Re: [PATCH] ixgbe, ixgbevf: Add new mbox API to enable MC promiscuous mode
From: Hiroshi Shimamoto @ 2014-12-05 11:39 UTC (permalink / raw)
To: Alexander Duyck, e1000-devel@lists.sourceforge.net
Cc: netdev@vger.kernel.org, Choi, Sy Jong, Hayato Momma,
linux-kernel@vger.kernel.org
In-Reply-To: <54809D57.9060804@gmail.com>
> Subject: Re: [E1000-devel] [PATCH] ixgbe, ixgbevf: Add new mbox API to enable MC promiscuous mode
>
> On 11/27/2014 02:39 AM, Hiroshi Shimamoto wrote:
> > From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> >
> > The limitation of the number of multicast address for VF is not enough
> > for the large scale server with SR-IOV feature.
> > IPv6 requires the multicast MAC address for each IP address to handle
> > the Neighbor Solicitation message.
> > We couldn't assign over 30 IPv6 addresses to a single VF interface.
> >
> > The easy way to solve this is enabling multicast promiscuous mode.
> > It is good to have a functionality to enable multicast promiscuous mode
> > for each VF from VF driver.
> >
> > This patch introduces the new mbox API, IXGBE_VF_SET_MC_PROMISC, to
> > enable/disable multicast promiscuous mode in VF. If multicast promiscuous
> > mode is enabled the VF can receive all multicast packets.
> >
> > With this patch, the ixgbevf driver automatically enable multicast
> > promiscuous mode when the number of multicast addresses is over than 30
> > if possible.
> >
> > This also bump the API version up to 1.2 to check whether the API,
> > IXGBE_VF_SET_MC_PROMISC is available.
> >
> > Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> > CC: Choi, Sy Jong <sy.jong.choi@intel.com>
> > Reviewed-by: Hayato Momma <h-momma@ce.jp.nec.com>
>
> This is a REALLY bad idea unless you plan to limit this to privileged VFs.
>
> I would recommend looking at adding an ndo operation to control this
> feature so that it could be disabled by default in the PF and only
> enabled on the host side if specifically requested. Otherwise the
Do you mean that PF driver should have the flag to enable or disable per VF
and disallow the request from VF?
> problem is I can easily see this leading security issues as the VFs
> might begin getting access to messages that they aren't supposed to.
OK, by the way, I think that the current ixgbe and ixgbevf implementation
has already such issue. The guest can add hash entry to receive MAC and it
can get every multicast MAC frame with the current mbox API.
Does your concern come from the easiness of doing that?
thanks,
Hiroshi
>
> - Alex
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
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
* [PATCH iproute2 REGRESSIONS] ss: Fix layout issues introduced by regression
From: Vadim Kochan @ 2014-12-05 11:16 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
This patch fixes the following issues which was introduced
by me in commits:
#1 (2dc854854b7f1b) ss: Fixed broken output for Netlink 'Peer Address:Port' column
ISSUE: Broken layout when all sockets are printed out
#2 (eef43b5052afb7) ss: Identify more netlink protocol names
ISSUE: PID is not printed if only numbers output was specified (-n)
Also aligned the width of the local/peer ports to be more wider.
I tested with a lot of option combinations (I may miss some test cases),
but layout seems to me better even on the previous released version
of iproute2/ss.
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
misc/ss.c | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/misc/ss.c b/misc/ss.c
index a99294d..8abaaff 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -101,8 +101,6 @@ int state_width;
int addrp_width;
int addr_width;
int serv_width;
-int paddr_width;
-int pserv_width;
int screen_width;
static const char *TCP_PROTO = "tcp";
@@ -2912,11 +2910,12 @@ static void netlink_show_one(struct filter *f,
printf("%-*s ", state_width, "UNCONN");
printf("%-6d %-6d ", rq, wq);
- if (resolve_services)
- {
+ if (resolve_services) {
printf("%*s:", addr_width, nl_proto_n2a(prot, prot_name,
sizeof(prot_name)));
- }
+ } else
+ printf("%*d:", addr_width, prot);
+
if (pid == -1) {
printf("%-*s ", serv_width, "*");
@@ -2947,10 +2946,10 @@ static void netlink_show_one(struct filter *f,
if (state == NETLINK_CONNECTED) {
printf("%*d:%-*d",
- paddr_width, dst_group, pserv_width, dst_pid);
+ addr_width, dst_group, serv_width, dst_pid);
} else {
printf("%*s*%-*s",
- paddr_width, "", pserv_width, "");
+ addr_width, "", serv_width, "");
}
char *pid_context = NULL;
@@ -3684,22 +3683,13 @@ int main(int argc, char *argv[])
printf("%-*s ", state_width, "State");
printf("%-6s %-6s ", "Recv-Q", "Send-Q");
- paddr_width = addr_width;
- pserv_width = serv_width;
-
- /* Netlink service column can be resolved as process name/pid thus it
- * can be much wider than address column which is just a
- * protocol name/id.
- */
- if (current_filter.dbs & (1<<NETLINK_DB)) {
- serv_width = addr_width - 10;
- paddr_width = 13;
- pserv_width = 13;
- }
+ /* Make enough space for the local/remote port field */
+ addr_width -= 13;
+ serv_width += 13;
printf("%*s:%-*s %*s:%-*s\n",
addr_width, "Local Address", serv_width, "Port",
- paddr_width, "Peer Address", pserv_width, "Port");
+ addr_width, "Peer Address", serv_width, "Port");
fflush(stdout);
--
2.1.3
^ permalink raw reply related
* Re: 3.12.33 - BUG xfrm_selector_match+0x25/0x2f6
From: Steffen Klassert @ 2014-12-05 10:53 UTC (permalink / raw)
To: Julian Anastasov
Cc: Smart Weblications GmbH - Florian Wiessner, netdev, LKML, stable
In-Reply-To: <alpine.LFD.2.11.1412042338370.4841@ja.home.ssi.bg>
On Fri, Dec 05, 2014 at 01:15:51AM +0200, Julian Anastasov wrote:
>
> Hello,
>
> On Thu, 4 Dec 2014, Steffen Klassert wrote:
>
> > > [16623.096721] Call Trace:
> > > [16623.096744] <IRQ>
> > > [16623.096749] [<ffffffff81547a7c>] ? xfrm_sk_policy_lookup+0x44/0x9b
> > > [16623.096802] [<ffffffff81547ef7>] ? xfrm_lookup+0x91/0x446
> > > [16623.096832] [<ffffffff81541316>] ? ip_route_me_harder+0x150/0x1b0
> > > [16623.096865] [<ffffffffa01b6457>] ? ip_vs_route_me_harder+0x86/0x91 [ip_vs]
> > > [16623.096899] [<ffffffffa01b797a>] ? ip_vs_out+0x2d3/0x5bc [ip_vs]
> > > [16623.096930] [<ffffffff81501420>] ? ip_rcv_finish+0x2b8/0x2b8
> >
> > I really wonder why the xfrm_sk_policy_lookup codepath is taken here.
> > It looks like this is the processing of an inbound ipv4 packet that
> > is going to be rerouted to the output path by ipvs, so this packet
> > should not have socket context at all.
>
> In above trace looks like IPVS-NAT is used between
> local client and some real server. IPVS handles this skb
> at LOCAL_IN and calls ip_vs_route_me_harder(). If we have
> skb->sk at LOCAL_IN, my first thought is about early demux.
Yes, that's possible. Can be checked by disabling early demux.
echo 0 > /proc/sys/net/ipv4/ip_early_demux
If I look what it tries to dereference when the crash happens,
this does not look like a pointer. But sk->sk_policy[dir]
should be either a pointer to kernel memory or NULL. So I
think that the skb->sk pointer is already bogus.
^ permalink raw reply
* RE: [PATCH] broadcom: Add BCM54616S phy support
From: fugang.duan @ 2014-12-05 10:44 UTC (permalink / raw)
To: Alessio Igor Bogani, Florian Fainelli; +Cc: netdev@vger.kernel.org
In-Reply-To: <1417770960-4578-1-git-send-email-alessio.bogani@elettra.eu>
From: Alessio Igor Bogani <alessio.bogani@elettra.eu>
> Sent: Friday, December 05, 2014 5:16 PM
> To: Florian Fainelli
> Cc: netdev@vger.kernel.org; Alessio Igor Bogani
> Subject: [PATCH] broadcom: Add BCM54616S phy support
>
> Signed-off-by: Alessio Igor Bogani <alessio.bogani@elettra.eu>
> ---
> drivers/net/phy/Kconfig | 4 ++--
> drivers/net/phy/broadcom.c | 14 ++++++++++++++
> include/linux/brcmphy.h | 1 +
> 3 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index
> 75472cf7..eb35dcb 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -68,8 +68,8 @@ config SMSC_PHY
> config BROADCOM_PHY
> tristate "Drivers for Broadcom PHYs"
> ---help---
> - Currently supports the BCM5411, BCM5421, BCM5461, BCM5464,
> BCM5481
> - and BCM5482 PHYs.
> + Currently supports the BCM5411, BCM5421, BCM5461, BCM54616S,
> BCM5464,
> + BCM5481 and BCM5482 PHYs.
>
> config BCM63XX_PHY
> tristate "Drivers for Broadcom 63xx SOCs internal PHY"
> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
> index 854f2c9..74cbf45 100644
> --- a/drivers/net/phy/broadcom.c
> +++ b/drivers/net/phy/broadcom.c
> @@ -549,6 +549,19 @@ static struct phy_driver broadcom_drivers[] = {
> .config_intr = bcm54xx_config_intr,
> .driver = { .owner = THIS_MODULE },
> }, {
> + .phy_id = PHY_ID_BCM54616S,
> + .phy_id_mask = 0xfffffff0,
> + .name = "Broadcom BCM54616S",
> + .features = PHY_GBIT_FEATURES |
> + SUPPORTED_Pause | SUPPORTED_Asym_Pause,
> + .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
> + .config_init = bcm54xx_config_init,
> + .config_aneg = genphy_config_aneg,
> + .read_status = genphy_read_status,
> + .ack_interrupt = bcm54xx_ack_interrupt,
> + .config_intr = bcm54xx_config_intr,
> + .driver = { .owner = THIS_MODULE },
> +}, {
> .phy_id = PHY_ID_BCM5464,
> .phy_id_mask = 0xfffffff0,
> .name = "Broadcom BCM5464",
> @@ -673,6 +686,7 @@ static struct mdio_device_id __maybe_unused
> broadcom_tbl[] = {
> { PHY_ID_BCM5411, 0xfffffff0 },
> { PHY_ID_BCM5421, 0xfffffff0 },
> { PHY_ID_BCM5461, 0xfffffff0 },
> + { PHY_ID_BCM54616S, 0xfffffff0 },
> { PHY_ID_BCM5464, 0xfffffff0 },
> { PHY_ID_BCM5482, 0xfffffff0 },
> { PHY_ID_BCM5482, 0xfffffff0 },
> diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h index
> 7ccd928..1c9920b 100644
> --- a/include/linux/brcmphy.h
> +++ b/include/linux/brcmphy.h
> @@ -11,6 +11,7 @@
> #define PHY_ID_BCM5421 0x002060e0
> #define PHY_ID_BCM5464 0x002060b0
> #define PHY_ID_BCM5461 0x002060c0
> +#define PHY_ID_BCM54616S 0x03625d10
> #define PHY_ID_BCM57780 0x03625d90
>
> #define PHY_ID_BCM7250 0xae025280
> --
Some other question:
Do you know the current phy driver support BCM54220B0KFBG phy ?
Regards,
Andy
^ permalink raw reply
* Re: 3.12.33 - BUG xfrm_selector_match+0x25/0x2f6
From: Steffen Klassert @ 2014-12-05 10:43 UTC (permalink / raw)
To: Smart Weblications GmbH - Florian Wiessner; +Cc: netdev, LKML, stable
In-Reply-To: <54808D8B.3080804@smart-weblications.de>
On Thu, Dec 04, 2014 at 05:36:27PM +0100, Smart Weblications GmbH - Florian Wiessner wrote:
> Hi,
>
> Am 04.12.2014 08:56, schrieb Steffen Klassert:
> >
> > I really wonder why the xfrm_sk_policy_lookup codepath is taken here.
> > It looks like this is the processing of an inbound ipv4 packet that
> > is going to be rerouted to the output path by ipvs, so this packet
> > should not have socket context at all.
> >
> > xfrm_sk_policy_lookup is called just if the packet has socket context
> > and the socket has an IPsec output policy configured. Do you use IPsec
> > socket policies?
> >
>
> Yes it is insane i do not know why this happens and i wonder as well - i do not
> have IPsec configured. I tried yesterday with only
>
> CONFIG_XFRM=y
> CONFIG_XFRM_ALGO=m
>
> and all other XFRM modules disabled, same problem.
>
> I now compiled kernel without xfrm to check if the problem is somewhere else.
>
> I have seen that on this box (debian squeeze) the racoon tool inserts xfrm
> polcies like so:
>
> ip xfrm policy show
> src ::/0 dst ::/0
> dir 4 priority 0 ptype main
> src ::/0 dst ::/0
> dir 3 priority 0 ptype main
> src ::/0 dst ::/0
> dir 4 priority 0 ptype main
> src ::/0 dst ::/0
> dir 3 priority 0 ptype main
> src ::/0 dst ::/0
> ...
Well, these are socket policies. The ike deamon uses them
for SA negotiation.
>
> I tried without racoon running and with ipsec userspace tools disabled, but the
> problem still exists without ipsec userspace tools.
Does this mean that it still happens if you have no IPsec policies
in the system?
>
> Interesting is maybe, that the longer the node is running and interfaces are
> added to a bridge, the more policies sum up. Here is an overview of other nodes,
> but without ipvs running:
Would be interesting to see them.
^ permalink raw reply
* [PATCH 2/2] gianfar: handle map error in gfar_start_xmit()
From: Arseny Solokha @ 2014-12-05 10:37 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, linux-kernel, Arseny Solokha
In-Reply-To: <1417775874-17775-1-git-send-email-asolokha@kb.kras.ru>
From: Arseny Solokha <asolokha@kb.kras.ru>
When DMA-API debugging is enabled in the kernel, it spews the following
upon upping the link:
WARNING: at lib/dma-debug.c:1135
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W O 3.18.0-rc7 #1
task: c0720340 ti: effe2000 task.ti: c0750000
NIP: c01d7c1c LR: c01d7c1c CTR: c02250fc
REGS: effe3d40 TRAP: 0700 Tainted: G W O (3.18.0-rc7)
MSR: 00021000 <CE,ME> CR: 22044242 XER: 20000000
GPR00: c01d7c1c effe3df0 c0720340 00000095 c201e404 c201e9f0 00021000 01a9d000
GPR08: 00000007 00000000 01a9d000 00000313 22044242 00583f60 05f41012 00000000
GPR16: 00000000 000000ff 00000000 00000000 00000000 c5dc3b40 c5677720 00000001
GPR24: c0730000 00029000 c0d0d828 c072c394 effe3e48 c075baec c0d0f020 ee31e600
NIP [c01d7c1c] check_unmap+0x5b4/0xae4
LR [c01d7c1c] check_unmap+0x5b4/0xae4
Call Trace:
[effe3df0] [c01d7c1c] check_unmap+0x5b4/0xae4 (unreliable)
[effe3e40] [c01d81c4] debug_dma_unmap_page+0x78/0x8c
[effe3ec0] [c0286ba0] gfar_clean_tx_ring+0x120/0x3c0
[effe3f30] [c0286f90] gfar_poll_tx_sq+0x48/0x94
o[effe3f50] [c030c388] net_rx_action+0x130/0x1ac
[effe3f80] [c00319e0] __do_softirq+0x134/0x240
[effe3fe0] [c0031dd0] irq_exit+0xa4/0xc8
[effe3ff0] [c000e01c] call_do_irq+0x24/0x3c
[c0751e70] [c0004a04] do_IRQ+0x8c/0x108
[c0751e90] [c0010068] ret_from_except+0x0/0x18
--- interrupt: 501 at arch_cpu_idle+0x24/0x5c
LR = arch_cpu_idle+0x24/0x5c
[c0751f50] [c007d2e4] rcu_idle_enter+0xc8/0xcc (unreliable)
[c0751f60] [c006587c] cpu_startup_entry+0x1d4/0x29c
[c0751fb0] [c054399c] start_kernel+0x338/0x34c
[c0751ff0] [c000046c] set_ivor+0x154/0x190
Instruction dump:
394adb30 80fc0018 811c001c 3c60c04f 5529103a 7cca482e 38639e60 813c0020
815c0024 90c10008 4cc63182 48240b01 <0fe00000> 3c60c04f 3863971c 4cc63182
---[ end trace 3eb7bf62ba1b80f9 ]---
Mapped at:
[<c0287420>] gfar_start_xmit+0x424/0x910
[<c030e964>] dev_hard_start_xmit+0x20c/0x3d8
[<c032cc3c>] sch_direct_xmit+0x124/0x22c
[<c030ede8>] __dev_queue_xmit+0x2b8/0x674
Or the following upon starting transmission of some large chunks
of data:
fsl-gianfar ffe25000.ethernet: DMA-API: device driver failed to check map error[device address=0x0000000005fa8000]
------------[ cut here ]------------
WARNING: at lib/dma-debug.c:1135
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G O 3.18.0-rc7 #35
task: c071d340 ti: effe2000 task.ti: c074e000
NIP: c01d7c1c LR: c01d7c1c CTR: c022339c
REGS: effe3d40 TRAP: 0700 Tainted: G O (3.18.0-rc7)
MSR: 00021000 <CE,ME> CR: 22044242 XER: 20000000
GPR00: c01d7c1c effe3df0 c071d340 00000094 00000001 c0071750 00000000 00000001
GPR08: 00000000 00000000 effe2000 00000000 20044242 00581f60 05fa8000 000001c4
GPR16: 00000000 000000ff 00000000 000000e4 00000039 c5b1a9c0 c5679c60 00000002
GPR24: c0730000 00029000 c0d0c528 c0729394 effe3e48 c0759aec c0d0d020 ee315900
NIP [c01d7c1c] check_unmap+0x5b4/0xae4
LR [c01d7c1c] check_unmap+0x5b4/0xae4
Call Trace:
[effe3df0] [c01d7c1c] check_unmap+0x5b4/0xae4 (unreliable)
[effe3e40] [c01d81c4] debug_dma_unmap_page+0x78/0x8c
[effe3ec0] [c0284c78] gfar_clean_tx_ring+0x1b4/0x3c0
[effe3f30] [c0284fd4] gfar_poll_tx_sq+0x48/0x94
[effe3f50] [c030a5c4] net_rx_action+0x130/0x1ac
[effe3f80] [c00319e0] __do_softirq+0x134/0x240
[effe3fe0] [c0031dd0] irq_exit+0xa4/0xc8
[effe3ff0] [c000e01c] call_do_irq+0x24/0x3c
[c074fe70] [c0004a04] do_IRQ+0x8c/0x108
[c074fe90] [c0010068] ret_from_except+0x0/0x18
--- interrupt: 501 at arch_cpu_idle+0x24/0x5c
LR = arch_cpu_idle+0x24/0x5c
[c074ff50] [c007d2e4] rcu_idle_enter+0xc8/0xcc (unreliable)
[c074ff60] [c006587c] cpu_startup_entry+0x1d4/0x29c
[c074ffb0] [c054199c] start_kernel+0x338/0x34c
[c074fff0] [c000046c] set_ivor+0x154/0x190
Instruction dump:
394abb30 80fc0018 811c001c 3c60c04e 5529103a 7cca482e 386379f0 813c0020
815c0024 90c10008 4cc63182 4823ed39 <0fe00000> 3c60c04e 386372ac 4cc63182
---[ end trace 008c59ca7ca1f712 ]---
Mapped at:
[<c0285264>] gfar_start_xmit+0x224/0x95c
[<c030cba0>] dev_hard_start_xmit+0x20c/0x3d8
[<c032ae78>] sch_direct_xmit+0x124/0x22c
[<c032b008>] __qdisc_run+0x88/0x1c0
[<c0307920>] net_tx_action+0xf0/0x19c
Ignore these mapping failures in hope we'll have more luck next time.
Signed-off-by: Arseny Solokha <asolokha@kb.kras.ru>
---
drivers/net/ethernet/freescale/gianfar.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index f34ca55..9ea887e 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -2296,6 +2296,12 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
0,
frag_len,
DMA_TO_DEVICE);
+ if (unlikely(dma_mapping_error(priv->dev, bufaddr))) {
+ /* As DMA mapping failed, pretend the TX path
+ * is busy to retry later
+ */
+ return NETDEV_TX_BUSY;
+ }
/* set the TxBD length and buffer pointer */
txbdp->bufPtr = bufaddr;
@@ -2345,8 +2351,15 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
fcb->ptp = 1;
}
- txbdp_start->bufPtr = dma_map_single(priv->dev, skb->data,
- skb_headlen(skb), DMA_TO_DEVICE);
+ bufaddr = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
+ DMA_TO_DEVICE);
+ if (unlikely(dma_mapping_error(priv->dev, bufaddr))) {
+ /* As DMA mapping failed, pretend the TX path is busy to retry
+ * later
+ */
+ return NETDEV_TX_BUSY;
+ }
+ txbdp_start->bufPtr = bufaddr;
/* If time stamping is requested one additional TxBD must be set up. The
* first TxBD points to the FCB and must have a data length of
--
2.2.0
^ permalink raw reply related
* [PATCH 1/2] gianfar: handle map error in gfar_new_rxbdp()
From: Arseny Solokha @ 2014-12-05 10:37 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, linux-kernel, Arseny Solokha
In-Reply-To: <1417775874-17775-1-git-send-email-asolokha@kb.kras.ru>
From: Arseny Solokha <asolokha@kb.kras.ru>
When DMA-API debugging is enabled in the kernel, it spews the following
upon upping the link:
fsl-gianfar ffe25000.ethernet: DMA-API: device driver failed to check map error[device address=0x0000000005f41012] [size=90 bytes] [map-
WARNING: at lib/dma-debug.c:1135
Modules linked in:
CPU: 1 PID: 0 Comm: swapper/1 Tainted: G O 3.18.0-rc7 #1
task: ee06f080 ti: effde000 task.ti: ee0b2000
NIP: c01d7c1c LR: c01d7c1c CTR: 00000000
REGS: effdfd40 TRAP: 0700 Tainted: G O (3.18.0-rc7)
MSR: 00021000 <CE,ME> CR: 42804442 XER: 00000000
GPR00: c01d7c1c effdfdf0 ee06f080 00000097 00000001 c0066dd8 00000000 00000001
GPR08: 00000000 00000000 effde000 0000029e 00000000 00000000 c55fa740 ee0d6818
GPR16: r00000600 c5a6a9c0 ee0d6830 ee0d6850 ffff8100 00000008 00000000 c5bbc800
GPR24: c0730000 00029000 c0d0b828 c072c394 effdfe48 c075baec c0d0f020 ee308300
NIP [c01d7c1c] check_unmap+0x5b4/0xae4
LR [c01d7c1c] check_unmap+0x5b4/0xae4
Call Trace:
[effdfdf0] [c01d7c1c] check_unmap+0x5b4/0xae4 (unreliable)
[effdfe40] [c01d81c4] debug_dma_unmap_page+0x78/0x8c
[effdfec0] [c028b270] gfar_clean_rx_ring+0x114/0x4c0
[effdff30] [c028b814] gfar_poll_rx_sq+0x3c/0xa4
[effdff50] [c030c388] net_rx_action+0x130/0x1ac
[effdff80] [c00319e0] __do_softirq+0x134/0x240
[effdffe0] [c0031dd0] irq_exit+0xa4/0xc8
[effdfff0] [c000e01c] call_do_irq+0x24/0x3c
[ee0b3e60] [c0004a04] do_IRQ+0x8c/0x108
[ee0b3e80] [c0010068] ret_from_except+0x0/0x18
--- interrupt: 501 at arch_cpu_idle+0x24/0x5c
LR = arch_cpu_idle+0x24/0x5c
[ee0b3f40] [c007d2e4] rcu_idle_enter+0xc8/0xcc (unreliable)
[ee0b3f50] [c006587c] cpu_startup_entry+0x1d4/0x29c
[ee0b3fa0] [c00111cc] start_secondary+0x364/0x478
[ee0b3ff0] [c000217c] __secondary_start+0x7c/0xc8
Instruction dump:
394adb30 80fc0018 811c001c 3c60c04f 5529103a 7cca482e 38639e60 813c0020
815c0024 90c10008 4cc63182 48240b01 <0fe00000> 3c60c04f 3863971c 4cc63182
---[ end trace 3eb7bf62ba1b80f8 ]---
oMapped at:
[<c02887cc>] gfar_new_rxbdp.isra.4+0x120/0x16c
[<c0288968>] gfar_init_bds+0x150/0x1b0
[<c028a800>] startup_gfar+0x334/0x3d8
[<c028ac64>] gfar_enet_open+0x2b8/0x460
[<c03100c0>] __dev_open+0xdc/0x150
And the underlying code indeed doesn't perform the check.
Signed-off-by: Arseny Solokha <asolokha@kb.kras.ru>
---
drivers/net/ethernet/freescale/gianfar.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 4fdf0aa..f34ca55 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -117,8 +117,8 @@ static void gfar_reset_task(struct work_struct *work);
static void gfar_timeout(struct net_device *dev);
static int gfar_close(struct net_device *dev);
struct sk_buff *gfar_new_skb(struct net_device *dev);
-static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
- struct sk_buff *skb);
+static int gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
+ struct sk_buff *skb);
static int gfar_set_mac_address(struct net_device *dev);
static int gfar_change_mtu(struct net_device *dev, int new_mtu);
static irqreturn_t gfar_error(int irq, void *dev_id);
@@ -214,6 +214,8 @@ static int gfar_init_bds(struct net_device *ndev)
gfar_init_rxbdp(rx_queue, rxbdp,
rxbdp->bufPtr);
} else {
+ int ret;
+
skb = gfar_new_skb(ndev);
if (!skb) {
netdev_err(ndev, "Can't allocate RX buffers\n");
@@ -221,7 +223,11 @@ static int gfar_init_bds(struct net_device *ndev)
}
rx_queue->rx_skbuff[j] = skb;
- gfar_new_rxbdp(rx_queue, rxbdp, skb);
+ ret = gfar_new_rxbdp(rx_queue, rxbdp, skb);
+ if (ret) {
+ netdev_err(ndev, "Buffer mapping error\n");
+ return ret;
+ }
}
rxbdp++;
@@ -2606,8 +2612,8 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
netdev_tx_completed_queue(txq, howmany, bytes_sent);
}
-static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
- struct sk_buff *skb)
+static int gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
+ struct sk_buff *skb)
{
struct net_device *dev = rx_queue->dev;
struct gfar_private *priv = netdev_priv(dev);
@@ -2615,7 +2621,12 @@ static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
buf = dma_map_single(priv->dev, skb->data,
priv->rx_buffer_size, DMA_FROM_DEVICE);
+ if (dma_mapping_error(priv->dev, buf))
+ return -EFAULT;
+
gfar_init_rxbdp(rx_queue, bdp, buf);
+
+ return 0;
}
static struct sk_buff *gfar_alloc_skb(struct net_device *dev)
@@ -2805,6 +2816,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
while (!((bdp->status & RXBD_EMPTY) || (--rx_work_limit < 0))) {
struct sk_buff *newskb;
+ int rxbdpret;
rmb();
@@ -2854,7 +2866,15 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
rx_queue->rx_skbuff[rx_queue->skb_currx] = newskb;
/* Setup the new bdp */
- gfar_new_rxbdp(rx_queue, bdp, newskb);
+ rxbdpret = gfar_new_rxbdp(rx_queue, bdp, newskb);
+ if (unlikely(rxbdpret)) {
+ /* We drop the frame if we failed to map a new DMA
+ * buffer
+ */
+ count_errors(bdp->status, dev);
+ dev_kfree_skb(newskb);
+ continue;
+ }
/* Update to the next pointer */
bdp = next_bd(bdp, base, rx_queue->rx_ring_size);
--
2.2.0
^ permalink raw reply related
* [PATCH 0/2] DMA API usage fixes in gianfar
From: Arseny Solokha @ 2014-12-05 10:37 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, linux-kernel, Arseny Solokha
Hello.
This patch set fixes DMA API usage issues in gianfar ethernet driver
reported by the kernel w/ DMA API debug enabled.
There were even reports that the kernel sometimes oopsed in the past
because of kernel paging request handling failures, though it was likely
observed on some ancient versions. And while I personally doesn't have
any strong evidence of this, there's no reason to let these possible
failures live any longer.
Arseny Solokha (2):
gianfar: handle map error in gfar_new_rxbdp()
gianfar: handle map error in gfar_start_xmit()
drivers/net/ethernet/freescale/gianfar.c | 49 ++++++++++++++++++++++++++------
1 file changed, 41 insertions(+), 8 deletions(-)
--
Regards,
Arseny Solokha.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox