* Re: [PATCH] VSOCK: Don't dec ack backlog twice for rejected connections
From: David Miller @ 2016-09-27 11:59 UTC (permalink / raw)
To: jhansen; +Cc: pv-drivers, netdev, linux-kernel, virtualization, stefanha,
gregkh
In-Reply-To: <1474959593-15311-1-git-send-email-jhansen@vmware.com>
From: Jorgen Hansen <jhansen@vmware.com>
Date: Mon, 26 Sep 2016 23:59:53 -0700
> If a pending socket is marked as rejected, we will decrease the
> sk_ack_backlog twice. So don't decrement it for rejected sockets
> in vsock_pending_work().
>
> Testing of the rejected socket path was done through code
> modifications.
>
> Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Jorgen Hansen <jhansen@vmware.com>
> Reviewed-by: Adit Ranadive <aditr@vmware.com>
> Reviewed-by: Aditya Sarwade <asarwade@vmware.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring
From: Rafał Miłecki @ 2016-09-27 11:58 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend Van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Network Development, Linux Kernel Mailing List,
Rafał Miłecki
In-Reply-To: <CACna6rwUR_GidhEFNTN+8mGQRpBdUzeM1-0NY65k6-McX7xGVg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 27 September 2016 at 13:44, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 27 September 2016 at 13:27, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>> Arend Van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> writes:
>>
>>> On 27-9-2016 11:14, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>
>>>> Flowrings contain skbs waiting for transmission that were passed to us
>>>> by netif. It means we checked every one of them looking for 802.1x
>>>> Ethernet type. When deleting flowring we have to use freeing function
>>>> that will check for 802.1x type as well.
>>>>
>>>> Freeing skbs without a proper check was leading to counter not being
>>>> properly decreased. This was triggering a WARNING every time
>>>> brcmf_netdev_wait_pend8021x was called.
>>>
>>> Acked-by: Arend van Spriel <arend-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>> ---
>>>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>>>
>>>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>>>> to WARNING on every add_key/del_key call. We was struggling with these
>>>> WARNINGs for some time and this fixes one of two problems causing them.
>>
>> Ok, I'll queue this for 4.9.
>>
>>> Please mark it for stable as well.
>>
>> I can add that. Any ideas how old releases stable releases should this
>> go to?
>
> I was analyzing this.
> 1) This patch uses brcmf_get_ifp which is available in 4.4+ only.
> 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac:
> Increase nr of supported flowrings.")
> 3) 4.4 would also require applying to the patch without broadcom/ subdir
>
> That said I suggest 4.5+. Any objections?
Let me see if patchwork with pick Cc tag as it does for others.
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # 4.5+
This may be worth backporting to 4.4 as well (as it's longterm), but
I'll do it separately due to patch not applying cleanly.
--
Rafał
^ permalink raw reply
* Re: [PATCH] fs/select: add vmalloc fallback for select(2)
From: Vlastimil Babka @ 2016-09-27 11:51 UTC (permalink / raw)
To: Nicholas Piggin, David Laight
Cc: Jason Baron, Hillf Danton, 'Alexander Viro',
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, 'Michal Hocko',
netdev@vger.kernel.org, Eric Dumazet
In-Reply-To: <20160927214229.2b0b49ac@roar.ozlabs.ibm.com>
On 09/27/2016 01:42 PM, Nicholas Piggin wrote:
> On Tue, 27 Sep 2016 11:37:24 +0000
> David Laight <David.Laight@ACULAB.COM> wrote:
>
>> From: Nicholas Piggin
>> > Sent: 27 September 2016 12:25
>> > On Tue, 27 Sep 2016 10:44:04 +0200
>> > Vlastimil Babka <vbabka@suse.cz> wrote:
>> >
>> >
>> > What's your customer doing with those selects? If they care at all about
>> > performance, I doubt they want select to attempt order-4 allocations, fail,
>> > then use vmalloc :)
>>
>> If they care about performance they shouldn't be passing select() lists that
>> are anywhere near that large.
>> If the number of actual fd is small - use poll().
>
> Right. Presumably it's some old app they're still using, no?
Process name suggests it's part of db2 database. It seems it has to implement
its own interface to select() syscall, because glibc itself seems to have a
FD_SETSIZE limit of 1024, which is probably why this wasn't an issue for all the
years...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v2] net: hns: mark symbols static where possible
From: David Miller @ 2016-09-27 11:50 UTC (permalink / raw)
To: baoyou.xie
Cc: yisen.zhuang, yankejian, huangdaode, lisheng011, lipeng321,
xieqianqian, fabf, colin.king, geliangtang, arnd, andrew,
chenny.xu, xypron.glpk, netdev, linux-kernel, xie.baoyou, han.fei,
tang.qiang007
In-Reply-To: <1474881218-21219-1-git-send-email-baoyou.xie@linaro.org>
From: Baoyou Xie <baoyou.xie@linaro.org>
Date: Mon, 26 Sep 2016 17:13:38 +0800
> We get a few warnings when building kernel with W=1:
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:76:21: warning: no previous prototype for 'hns_ae_get_handle' [-Wmissing-prototypes]
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:274:6: warning: no previous prototype for 'hns_ae_stop' [-Wmissing-prototypes]
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:302:6: warning: no previous prototype for 'hns_ae_toggle_ring_irq' [-Wmissing-prototypes]
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:490:6: warning: no previous prototype for 'hns_ae_update_stats' [-Wmissing-prototypes]
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:573:6: warning: no previous prototype for 'hns_ae_get_stats' [-Wmissing-prototypes]
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:605:6: warning: no previous prototype for 'hns_ae_get_strings' [-Wmissing-prototypes]
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:638:5: warning: no previous prototype for 'hns_ae_get_sset_count' [-Wmissing-prototypes]
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:687:6: warning: no previous prototype for 'hns_ae_update_led_status' [-Wmissing-prototypes]
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:698:5: warning: no previous prototype for 'hns_ae_cpld_set_led_id' [-Wmissing-prototypes]
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:710:6: warning: no previous prototype for 'hns_ae_get_regs' [-Wmissing-prototypes]
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:735:5: warning: no previous prototype for 'hns_ae_get_regs_len' [-Wmissing-prototypes]
> ....
>
> In fact, these functions are only used in the file in which they are
> declared and don't need a declaration, but can be made static.
> so this patch marks these functions with 'static'.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
This still doesn't apply to the net-next tree.
If you aren't actually building your patch against the net-next
tree, don't bother submitting these patches any more.
^ permalink raw reply
* Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring
From: Rafał Miłecki @ 2016-09-27 11:44 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend Van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
Network Development, Linux Kernel Mailing List,
Rafał Miłecki
In-Reply-To: <87inthy441.fsf@kamboji.qca.qualcomm.com>
On 27 September 2016 at 13:27, Kalle Valo <kvalo@codeaurora.org> wrote:
> Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> On 27-9-2016 11:14, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Flowrings contain skbs waiting for transmission that were passed to us
>>> by netif. It means we checked every one of them looking for 802.1x
>>> Ethernet type. When deleting flowring we have to use freeing function
>>> that will check for 802.1x type as well.
>>>
>>> Freeing skbs without a proper check was leading to counter not being
>>> properly decreased. This was triggering a WARNING every time
>>> brcmf_netdev_wait_pend8021x was called.
>>
>> Acked-by: Arend van Spriel <arend@broadcom.com>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>>
>>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>>> to WARNING on every add_key/del_key call. We was struggling with these
>>> WARNINGs for some time and this fixes one of two problems causing them.
>
> Ok, I'll queue this for 4.9.
>
>> Please mark it for stable as well.
>
> I can add that. Any ideas how old releases stable releases should this
> go to?
I was analyzing this.
1) This patch uses brcmf_get_ifp which is available in 4.4+ only.
2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac:
Increase nr of supported flowrings.")
3) 4.4 would also require applying to the patch without broadcom/ subdir
That said I suggest 4.5+. Any objections?
--
Rafał
^ permalink raw reply
* Re: [PATCH v3 net-next 0/3] net: bcmgenet: only use new api ethtool_{get|set}_link_ksettings
From: David Miller @ 2016-09-27 11:43 UTC (permalink / raw)
To: tremyfr; +Cc: f.fainelli, jaedon.shin, netdev, linux-kernel
In-Reply-To: <1474921917-4773-1-git-send-email-tremyfr@gmail.com>
From: Philippe Reynes <tremyfr@gmail.com>
Date: Mon, 26 Sep 2016 22:31:54 +0200
> Some times ago, a serie of patches were committed :
> - commit 62469c76007e ("net: ethernet: bcmgenet: use phydev from struct net_device")
> - commit 6b352ebccbcf ("net: ethernet: broadcom: bcmgenet: use new api ethtool_{get|set}_link_ksettings")
> The first patch add a regression on this driver, so it should be reverted.
> As the second patch depend on the former, it should be reverted too.
>
> The first patch is buggy because there is a "trick" in this driver.
> The structure phydev is kept in the private data when the interface
> go down, and used when the interface go up to enable the phy before
> the function phy_connect is called.
>
> I don't have this hardware, neither the datasheet. So I won't
> update the driver to avoid this trick.
>
> But the real goal of the first serie was to move to the new api
> ethtool_{get|set}_link_ksettings. So I provide a new version of
> the patch without the "cleaning" of driver to use the phydev
> store in the net_device structure.
>
> Changelog:
> v3:
> - use priv instead of dev (so all the code use the same phydev)
> v2:
> - use Florian Fainelli patches for the revert instead of Jaedon Shin
> - simply use net: bcmgenet: for the prefix of the patch
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH] fs/select: add vmalloc fallback for select(2)
From: Nicholas Piggin @ 2016-09-27 11:42 UTC (permalink / raw)
To: David Laight
Cc: Vlastimil Babka, Jason Baron, Hillf Danton,
'Alexander Viro', linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
'Michal Hocko', netdev@vger.kernel.org, Eric Dumazet
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB010A97D@AcuExch.aculab.com>
On Tue, 27 Sep 2016 11:37:24 +0000
David Laight <David.Laight@ACULAB.COM> wrote:
> From: Nicholas Piggin
> > Sent: 27 September 2016 12:25
> > On Tue, 27 Sep 2016 10:44:04 +0200
> > Vlastimil Babka <vbabka@suse.cz> wrote:
> >
> > > On 09/23/2016 06:47 PM, Jason Baron wrote:
> > > > Hi,
> > > >
> > > > On 09/23/2016 03:24 AM, Nicholas Piggin wrote:
> > > >> On Fri, 23 Sep 2016 14:42:53 +0800
> > > >> "Hillf Danton" <hillf.zj@alibaba-inc.com> wrote:
> > > >>
> > > >>>>
> > > >>>> The select(2) syscall performs a kmalloc(size, GFP_KERNEL) where size grows
> > > >>>> with the number of fds passed. We had a customer report page allocation
> > > >>>> failures of order-4 for this allocation. This is a costly order, so it might
> > > >>>> easily fail, as the VM expects such allocation to have a lower-order fallback.
> > > >>>>
> > > >>>> Such trivial fallback is vmalloc(), as the memory doesn't have to be
> > > >>>> physically contiguous. Also the allocation is temporary for the duration of the
> > > >>>> syscall, so it's unlikely to stress vmalloc too much.
> > > >>>>
> > > >>>> Note that the poll(2) syscall seems to use a linked list of order-0 pages, so
> > > >>>> it doesn't need this kind of fallback.
> > > >>
> > > >> How about something like this? (untested)
> > >
> > > This pushes the limit further, but might just delay the problem. Could be an
> > > optimization on top if there's enough interest, though.
> >
> > What's your customer doing with those selects? If they care at all about
> > performance, I doubt they want select to attempt order-4 allocations, fail,
> > then use vmalloc :)
>
> If they care about performance they shouldn't be passing select() lists that
> are anywhere near that large.
> If the number of actual fd is small - use poll().
Right. Presumably it's some old app they're still using, no?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH net] Revert "net: ethernet: bcmgenet: use phydev from struct net_device"
From: David Miller @ 2016-09-27 11:42 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, tremyfr, jaedon.shin
In-Reply-To: <1474747110-6496-1-git-send-email-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sat, 24 Sep 2016 12:58:30 -0700
> This reverts commit 62469c76007e ("net: ethernet: bcmgenet: use phydev
> from struct net_device") because it causes GENETv1/2/3 adapters to
> expose the following behavior after an ifconfig down/up sequence:
>
> PING fainelli-linux (10.112.156.244): 56 data bytes
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.352 ms
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.472 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.496 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.517 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.536 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.557 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=752.448 ms (DUP!)
>
> This was previously fixed by commit 5dbebbb44a6a ("net: bcmgenet:
> Software reset EPHY after power on") but the commit we are reverting was
> essentially making this previous commit void, here is why.
>
> Without commit 62469c76007e we would have the following scenario after
> an ifconfig down then up sequence:
>
> - bcmgenet_open() calls bcmgenet_power_up() to make sure the PHY is
> initialized *before* we get to initialize the UniMAC, this is
> critical to ensure the PHY is in a correct state, priv->phydev is
> valid, this code executes fine
>
> - second time from bcmgenet_mii_probe(), through the normal
> phy_init_hw() call (which arguably could be optimized out)
>
> Everything is fine in that case. With commit 62469c76007e, we would have
> the following scenario to happen after an ifconfig down then up
> sequence:
>
> - bcmgenet_close() calls phy_disonnect() which makes dev->phydev become
> NULL
>
> - when bcmgenet_open() executes again and calls bcmgenet_mii_reset() from
> bcmgenet_power_up() to initialize the internal PHY, the NULL check
> becomes true, so we do not reset the PHY, yet we keep going on and
> initialize the UniMAC, causing MAC activity to occur
>
> - we call bcmgenet_mii_reset() from bcmgenet_mii_probe(), but this is
> too late, the PHY is botched, and causes the above bogus pings/packets
> transmission/reception to occur
>
> Reported-by: Jaedon Shin <jaedon.shin@gmail.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH 0/3] net: fec: updates to align IP header
From: David Miller @ 2016-09-27 11:40 UTC (permalink / raw)
To: eric
Cc: netdev, linux, andrew, fugang.duan, otavio, edumazet, troy.kisky,
u.kleine-koenig
In-Reply-To: <1474728139-9335-1-git-send-email-eric@nelint.com>
From: Eric Nelson <eric@nelint.com>
Date: Sat, 24 Sep 2016 07:42:16 -0700
> This patch series is the outcome of investigation into very high
> numbers of alignment faults on kernel 4.1.33 from the linux-fslc
> tree:
> https://github.com/freescale/linux-fslc/tree/4.1-1.0.x-imx
>
> The first two patches remove support for the receive accelerator (RACC) from
> the i.MX25 and i.MX27 SoCs which don't support the function.
>
> The third patch enables hardware alignment of the ethernet packet payload
> (and especially the IP header) to prevent alignment faults in the IP stack.
>
> Testing on i.MX6UL on the 4.1.33 kernel showed that this patch removed
> on the order of 70k alignment faults during a 100MiB transfer using
> wget.
>
> Testing on an i.MX6Q (SABRE Lite) board on net-next (4.8.0-rc7) showed
> a much more modest improvement from 10's of faults, and it's not clear
> why that's the case.
Series applied and queued up for -stable.
^ permalink raw reply
* RE: [PATCH] fs/select: add vmalloc fallback for select(2)
From: David Laight @ 2016-09-27 11:37 UTC (permalink / raw)
To: 'Nicholas Piggin', Vlastimil Babka
Cc: Jason Baron, Hillf Danton, 'Alexander Viro',
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, 'Michal Hocko',
netdev@vger.kernel.org, Eric Dumazet
In-Reply-To: <20160927212458.3ab42b41@roar.ozlabs.ibm.com>
From: Nicholas Piggin
> Sent: 27 September 2016 12:25
> On Tue, 27 Sep 2016 10:44:04 +0200
> Vlastimil Babka <vbabka@suse.cz> wrote:
>
> > On 09/23/2016 06:47 PM, Jason Baron wrote:
> > > Hi,
> > >
> > > On 09/23/2016 03:24 AM, Nicholas Piggin wrote:
> > >> On Fri, 23 Sep 2016 14:42:53 +0800
> > >> "Hillf Danton" <hillf.zj@alibaba-inc.com> wrote:
> > >>
> > >>>>
> > >>>> The select(2) syscall performs a kmalloc(size, GFP_KERNEL) where size grows
> > >>>> with the number of fds passed. We had a customer report page allocation
> > >>>> failures of order-4 for this allocation. This is a costly order, so it might
> > >>>> easily fail, as the VM expects such allocation to have a lower-order fallback.
> > >>>>
> > >>>> Such trivial fallback is vmalloc(), as the memory doesn't have to be
> > >>>> physically contiguous. Also the allocation is temporary for the duration of the
> > >>>> syscall, so it's unlikely to stress vmalloc too much.
> > >>>>
> > >>>> Note that the poll(2) syscall seems to use a linked list of order-0 pages, so
> > >>>> it doesn't need this kind of fallback.
> > >>
> > >> How about something like this? (untested)
> >
> > This pushes the limit further, but might just delay the problem. Could be an
> > optimization on top if there's enough interest, though.
>
> What's your customer doing with those selects? If they care at all about
> performance, I doubt they want select to attempt order-4 allocations, fail,
> then use vmalloc :)
If they care about performance they shouldn't be passing select() lists that
are anywhere near that large.
If the number of actual fd is small - use poll().
Otherwise you want one of the 'event' mechanisms in order to avoid setting
the markers on every fd after every event (can't remember how you do that
in Linux).
At least this isn't SYSV - poll() was O(n^2) in the number of fd
(because the fd were on a linked list).
David
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring
From: Arend Van Spriel @ 2016-09-27 11:33 UTC (permalink / raw)
To: Kalle Valo
Cc: Rafał Miłecki, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, netdev, linux-kernel,
Rafał Miłecki
In-Reply-To: <87inthy441.fsf@kamboji.qca.qualcomm.com>
On 27-9-2016 13:27, Kalle Valo wrote:
> Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> On 27-9-2016 11:14, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Flowrings contain skbs waiting for transmission that were passed to us
>>> by netif. It means we checked every one of them looking for 802.1x
>>> Ethernet type. When deleting flowring we have to use freeing function
>>> that will check for 802.1x type as well.
>>>
>>> Freeing skbs without a proper check was leading to counter not being
>>> properly decreased. This was triggering a WARNING every time
>>> brcmf_netdev_wait_pend8021x was called.
>>
>> Acked-by: Arend van Spriel <arend@broadcom.com>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>>
>>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>>> to WARNING on every add_key/del_key call. We was struggling with these
>>> WARNINGs for some time and this fixes one of two problems causing them.
>
> Ok, I'll queue this for 4.9.
>
>> Please mark it for stable as well.
>
> I can add that. Any ideas how old releases stable releases should this
> go to?
Not sure if the vendor directory move causes issues as stable can not
fallback to three-way merge. I assumed it would so my last stable tag
was only for 4.7 and I took care of older kernels at later time with
backported patch. I can do that for this one as well.
Regards,
Arend
^ permalink raw reply
* Re: [Gigaset307x-common] ISDN-Gigaset: Release memory in gigaset_initcs() after an allocation failure
From: SF Markus Elfring @ 2016-09-27 11:32 UTC (permalink / raw)
To: Tilman Schmidt
Cc: Paul Bolle, Karsten Keil, gigaset307x-common, netdev,
kernel-janitors, LKML, Julia Lawall
In-Reply-To: <1474971962.1601897.738261385.73EB49E0@webmail.messagingengine.com>
>> I got the impression that the exception handling was incomplete in the
>> implementation of the function "gigaset_initcs".
>
> That impression is wrong. Careful reading of the code will confirm that.
* Is it still correct nowadays that the function "gigaset_initcs" did not
call the function "kfree" after a later function call failed?
* Do you expect that allocated memory will be automatically reclaimed
after it would return a null pointer?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring
From: Kalle Valo @ 2016-09-27 11:27 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Rafał Miłecki, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, netdev, linux-kernel,
Rafał Miłecki
In-Reply-To: <86fd0b60-0f73-484b-1788-3ec740672232@broadcom.com>
Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
> On 27-9-2016 11:14, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Flowrings contain skbs waiting for transmission that were passed to us
>> by netif. It means we checked every one of them looking for 802.1x
>> Ethernet type. When deleting flowring we have to use freeing function
>> that will check for 802.1x type as well.
>>
>> Freeing skbs without a proper check was leading to counter not being
>> properly decreased. This was triggering a WARNING every time
>> brcmf_netdev_wait_pend8021x was called.
>
> Acked-by: Arend van Spriel <arend@broadcom.com>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>
>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>> to WARNING on every add_key/del_key call. We was struggling with these
>> WARNINGs for some time and this fixes one of two problems causing them.
Ok, I'll queue this for 4.9.
> Please mark it for stable as well.
I can add that. Any ideas how old releases stable releases should this
go to?
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] brcmfmac: replace WARNING on timeout with a simple error message
From: Arend Van Spriel @ 2016-09-27 11:25 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo
Cc: Franky Lin, Hante Meuleman, Pieter-Paul Giesberts, Franky Lin,
linux-wireless, brcm80211-dev-list.pdl, netdev, linux-kernel,
Rafał Miłecki
In-Reply-To: <20160927101224.3534-1-zajec5@gmail.com>
On 27-9-2016 12:12, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Even with timeout increased to 950 ms we get WARNINGs from time to time.
> It mostly happens on A-MPDU stalls (e.g. when station goes out of
> range). It may take up to 5-10 secods for the firmware to recover and
> for that time it doesn't process packets.
>
> It's still useful to have a message on time out as it may indicate some
> firmware problem and incorrect key update. Raising a WARNING however
> wasn't really that necessary, it doesn't point to any driver bug anymore
> and backtrace wasn't much useful.
Indeed the interesting part would be in another context. So:
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> index 6d046ba..9e6f60a 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> @@ -1161,7 +1161,8 @@ int brcmf_netdev_wait_pend8021x(struct brcmf_if *ifp)
> !brcmf_get_pend_8021x_cnt(ifp),
> MAX_WAIT_FOR_8021X_TX);
>
> - WARN_ON(!err);
> + if (!err)
> + brcmf_err("Timed out waiting for no pending 802.1x packets\n");
>
> return !err;
> }
>
^ permalink raw reply
* Re: [PATCH] fs/select: add vmalloc fallback for select(2)
From: Nicholas Piggin @ 2016-09-27 11:24 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Jason Baron, Hillf Danton, 'Alexander Viro',
linux-fsdevel, linux-kernel, linux-mm, 'Michal Hocko',
netdev, Eric Dumazet
In-Reply-To: <5014387d-43da-03f6-a74b-2dc4fbf4fe32@suse.cz>
On Tue, 27 Sep 2016 10:44:04 +0200
Vlastimil Babka <vbabka@suse.cz> wrote:
> On 09/23/2016 06:47 PM, Jason Baron wrote:
> > Hi,
> >
> > On 09/23/2016 03:24 AM, Nicholas Piggin wrote:
> >> On Fri, 23 Sep 2016 14:42:53 +0800
> >> "Hillf Danton" <hillf.zj@alibaba-inc.com> wrote:
> >>
> >>>>
> >>>> The select(2) syscall performs a kmalloc(size, GFP_KERNEL) where size grows
> >>>> with the number of fds passed. We had a customer report page allocation
> >>>> failures of order-4 for this allocation. This is a costly order, so it might
> >>>> easily fail, as the VM expects such allocation to have a lower-order fallback.
> >>>>
> >>>> Such trivial fallback is vmalloc(), as the memory doesn't have to be
> >>>> physically contiguous. Also the allocation is temporary for the duration of the
> >>>> syscall, so it's unlikely to stress vmalloc too much.
> >>>>
> >>>> Note that the poll(2) syscall seems to use a linked list of order-0 pages, so
> >>>> it doesn't need this kind of fallback.
> >>
> >> How about something like this? (untested)
>
> This pushes the limit further, but might just delay the problem. Could be an
> optimization on top if there's enough interest, though.
What's your customer doing with those selects? If they care at all about
performance, I doubt they want select to attempt order-4 allocations, fail,
then use vmalloc :)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 3/5] ISDN-Gigaset: Delete an error message for a failed memory allocation
From: Tilman Schmidt @ 2016-09-27 10:57 UTC (permalink / raw)
To: SF Markus Elfring
Cc: gigaset307x-common, netdev, Karsten Keil, Paul Bolle, LKML,
kernel-janitors, Julia Lawall, Wolfram Sang
In-Reply-To: <64f7ba12-1b0e-9fac-9b2e-d22e7fb3fcc7@users.sourceforge.net>
On Mon, Sep 26, 2016, at 17:42, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 26 Sep 2016 15:35:47 +0200
>
> Omit an extra message for a memory allocation failure in this function.
>
> Link:
> http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
The patch is fine but the link in the commit message is irrelevant.
Please remove it.
(Yes, I read through the whole presentation to verify that. It was fun,
even.)
--
Tilman Schmidt
tilman@imap.cc
^ permalink raw reply
* Re: [PATCH net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Daniel Borkmann @ 2016-09-27 10:39 UTC (permalink / raw)
To: Shmulik Ladkani, David Miller
Cc: jhs, xiyou.wangcong, edumazet, netdev, shmulik.ladkani
In-Reply-To: <20160927110711.12555f4e@pixies>
On 09/27/2016 10:07 AM, Shmulik Ladkani wrote:
> Hi David,
>
> On Tue, 27 Sep 2016 01:56:06 -0400 (EDT), davem@davemloft.net wrote:
>> The discussion on this patch has ventured off into what to do about
>> recursion.
>>
>> But it unclear to me where this specific patch, and this series,
>> stands right now. Someone please clear this up for me.
>
> Status:
> - Series adds "ingress redirect/mirror" support
> - Positive feedback for the feature
> - So far no comments regarding code itself
> - Questions raised regarding "recursion handling"
> Expressed that existing mirred code (i.e egress redirect) is *already*
> loop-unsafe (and also, some non-tc netdev constructs, as exampled by
> others).
> Discussion then wandered to "recursion handling".
Any reason why dev_forward_skb() is not preferred over direct
netif_receive_skb() you're using? It would, for example, implicitly
assure that pkt_type is always PACKET_HOST, etc.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH v5 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
From: Maciej Żenczykowski @ 2016-09-27 10:39 UTC (permalink / raw)
To: YOSHIFUJI Hideaki
Cc: David S . Miller, Linux NetDev, Erik Kline, Lorenzo Colitti,
Hannes Frederic Sowa
In-Reply-To: <fb240cbb-09ab-69c4-2b4c-44a50e5a308c@miraclelinux.com>
That wouldn't really simplify much.
This change currently has 5 lines.
3 of those would be needed anyway if we were to define anything < 0 to
mean infinite.
Yes, you could get rid of the two lines with minus_one in them, but
this way we can also use -2 to mean something else in the future if we
ever want to.
^ permalink raw reply
* Re: [PATCH net-next 1/4] net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit
From: Daniel Borkmann @ 2016-09-27 10:30 UTC (permalink / raw)
To: Shmulik Ladkani, David S. Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, netdev,
Shmulik Ladkani
In-Reply-To: <1474550512-7552-2-git-send-email-shmulik.ladkani@gmail.com>
On 09/22/2016 03:21 PM, Shmulik Ladkani wrote:
> From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
>
> 'tcfm_ok_push' specifies whether a mac_len sized push is needed upon
> egress to the target device (if action is performed at ingress).
>
> Rename it to 'tcfm_mac_header_xmit' as this is actually an attribute of
> the target device.
> This allows to decouple the attribute from the action to be taken.
>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> ---
> include/net/tc_act/tc_mirred.h | 2 +-
> net/sched/act_mirred.c | 10 +++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
> index 62770ad..5275158 100644
> --- a/include/net/tc_act/tc_mirred.h
> +++ b/include/net/tc_act/tc_mirred.h
> @@ -8,7 +8,7 @@ struct tcf_mirred {
> struct tc_action common;
> int tcfm_eaction;
> int tcfm_ifindex;
> - int tcfm_ok_push;
> + int tcfm_mac_header_xmit;
Since you already touch this here and in patch 2/4 anyway, maybe
make that a bool along the way?
Perhaps instead of tcfm_mac_header_xmit, tcfm_mac_header_push
might be a better name?
> struct net_device __rcu *tcfm_dev;
> struct list_head tcfm_list;
> };
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index 667dc38..7b03b13 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -63,7 +63,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
> struct tc_mirred *parm;
> struct tcf_mirred *m;
> struct net_device *dev;
> - int ret, ok_push = 0;
> + int ret, mac_header_xmit = 0;
> bool exists = false;
>
> if (nla == NULL)
> @@ -102,10 +102,10 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
> case ARPHRD_IPGRE:
> case ARPHRD_VOID:
> case ARPHRD_NONE:
> - ok_push = 0;
> + mac_header_xmit = 0;
> break;
> default:
> - ok_push = 1;
> + mac_header_xmit = 1;
> break;
> }
> } else {
> @@ -136,7 +136,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
> dev_put(rcu_dereference_protected(m->tcfm_dev, 1));
> dev_hold(dev);
> rcu_assign_pointer(m->tcfm_dev, dev);
> - m->tcfm_ok_push = ok_push;
> + m->tcfm_mac_header_xmit = mac_header_xmit;
> }
>
> if (ret == ACT_P_CREATED) {
> @@ -181,7 +181,7 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
> goto out;
>
> if (!(at & AT_EGRESS)) {
> - if (m->tcfm_ok_push)
> + if (m->tcfm_mac_header_xmit)
> skb_push_rcsum(skb2, skb->mac_len);
> }
>
>
^ permalink raw reply
* Re: [Gigaset307x-common] ISDN-Gigaset: Release memory in gigaset_initcs() after an allocation failure
From: Tilman Schmidt @ 2016-09-27 10:26 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Paul Bolle, Karsten Keil, gigaset307x-common, netdev,
kernel-janitors, LKML, Julia Lawall
In-Reply-To: <ad12e1a0-f5d5-176a-7d37-be2f3cb0af5d@users.sourceforge.net>
Hi,
as longtime maintainer of the code in question I feel compelled to chime
in at this point.
On Tue, Sep 27, 2016, at 11:34, SF Markus Elfring wrote:
> >> Will it matter here if the function "kfree" will be called for the
> >> data structure members "bcs" and "inbuf" after a later function call
> >> failed within the implementation of "gigaset_initcs"?
> >
> > My translation of this question is: could you please hold my hand while
> > I read the code of a driver I do not use - a driver for hardware that I
> > don't even have, and therefor cannot really test - after I submitted a
> > patch that appears to be broken?
>
> I got the impression that the exception handling was incomplete in the
> implementation of the function "gigaset_initcs".
That impression is wrong. Careful reading of the code will confirm that.
> Does anybody (besides me) care for improving the software situation
> there?
There's no urgent need for improvement. The code is stable and there's
no demonstrated bug to be fixed.
You could improve the coding style, but that is of secondary importance,
and if you want to do that, as a minimum you have to make sure that you
don't introduce new bugs.
Thanks,
Tilman
--
Tilman Schmidt
tilman@imap.cc
^ permalink raw reply
* Re: [PATCH] Fix link error in 32bit arch because of 64bit division
From: Liping Zhang @ 2016-09-27 10:23 UTC (permalink / raw)
To: Vishwanath Pai
Cc: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik, johunt,
Netfilter Developer Mailing List, coreteam,
Linux Kernel Network Developers, Vishwanath Pai
In-Reply-To: <20160927074203.GA3968@akamai.com>
Hi Vishwanath Pai,
2016-09-27 15:42 GMT+08:00 Vishwanath Pai <vpai@akamai.com>:
> Fix link error in 32bit arch because of 64bit division
This should be "netfilter: xt_hashlimit: fix ... "
>
> --- a/net/netfilter/xt_hashlimit.c
> +++ b/net/netfilter/xt_hashlimit.c
> @@ -465,19 +465,20 @@ static u64 user2credits(u64 user, int revision)
> {
> if (revision == 1) {
> /* If multiplying would overflow... */
> - if (user > 0xFFFFFFFF / (HZ*CREDITS_PER_JIFFY_v1))
> + if (user > div64_u64(0xFFFFFFFF, (HZ*CREDITS_PER_JIFFY_v1)))
Here divisor and dividend are all 32-bit integer, so covert "/" to div64_u64
seems unnecessary.
> /* Divide first. */
> - return (user / XT_HASHLIMIT_SCALE) *\
> + return div64_u64(user, XT_HASHLIMIT_SCALE) *\
> HZ * CREDITS_PER_JIFFY_v1;
>
> - return (user * HZ * CREDITS_PER_JIFFY_v1) \
> - / XT_HASHLIMIT_SCALE;
> + return div64_u64((user * HZ * CREDITS_PER_JIFFY_v1),
> + XT_HASHLIMIT_SCALE);
> } else {
> - if (user > 0xFFFFFFFFFFFFFFFF / (HZ*CREDITS_PER_JIFFY))
> - return (user / XT_HASHLIMIT_SCALE_v2) *\
> + if (user > div64_u64(0xFFFFFFFFFFFFFFFF, (HZ*CREDITS_PER_JIFFY)))
0xFFFFFFFFFFFFFFFF and "HZ*CREDITS_PER_JIFFY" are both
constant, and GCC will do constant folding optimization, so I think
convert "/" to div64_u64 here is also unnecessary.
> + return div64_u64(user, XT_HASHLIMIT_SCALE_v2) *\
> HZ * CREDITS_PER_JIFFY;
>
> - return (user * HZ * CREDITS_PER_JIFFY) / XT_HASHLIMIT_SCALE_v2;
> + return div64_u64((user * HZ * CREDITS_PER_JIFFY),
> + XT_HASHLIMIT_SCALE_v2);
> }
> }
>
^ permalink raw reply
* Re: [PATCH v3] fs/select: add vmalloc fallback for select(2)
From: Michal Hocko @ 2016-09-27 10:22 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Alexander Viro, Andrew Morton, linux-fsdevel, linux-kernel,
linux-mm, netdev, Eric Dumazet, David Laight, Hillf Danton,
Nicholas Piggin, Jason Baron
In-Reply-To: <20160927084536.5923-1-vbabka@suse.cz>
On Tue 27-09-16 10:45:36, Vlastimil Babka wrote:
> The select(2) syscall performs a kmalloc(size, GFP_KERNEL) where size grows
> with the number of fds passed. We had a customer report page allocation
> failures of order-4 for this allocation. This is a costly order, so it might
> easily fail, as the VM expects such allocation to have a lower-order fallback.
>
> Such trivial fallback is vmalloc(), as the memory doesn't have to be physically
> contiguous and the allocation is temporary for the duration of the syscall
> only. There were some concerns, whether this would have negative impact on the
> system by exposing vmalloc() to userspace. Although an excessive use of vmalloc
> can cause some system wide performance issues - TLB flushes etc. - a large
> order allocation is not for free either and an excessive reclaim/compaction can
> have a similar effect. Also note that the size is effectively limited by
> RLIMIT_NOFILE which defaults to 1024 on the systems I checked. That means the
> bitmaps will fit well within single page and thus the vmalloc() fallback could
> be only excercised for processes where root allows a higher limit.
>
> Note that the poll(2) syscall seems to use a linked list of order-0 pages, so
> it doesn't need this kind of fallback.
>
> [eric.dumazet@gmail.com: fix failure path logic]
> [akpm@linux-foundation.org: use proper type for size]
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Yes this makes sense to me. It could be argued that this could be
simplified to not rely on high order allocations at all but this is
simple enough (and backportable to stable trees) and should work
reasonably well.
So FWIW
Acked-by: Michal Hocko <mhocko@suse.com>
I would even argue to use __GFP_NORETRY for size > PAGE_SIZE because
giving a userspace an access to high order pages which can invoke OOM
killer is not a great idea. Something for a separate patch though.
> ---
> fs/select.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/fs/select.c b/fs/select.c
> index 8ed9da50896a..3d4f85defeab 100644
> --- a/fs/select.c
> +++ b/fs/select.c
> @@ -29,6 +29,7 @@
> #include <linux/sched/rt.h>
> #include <linux/freezer.h>
> #include <net/busy_poll.h>
> +#include <linux/vmalloc.h>
>
> #include <asm/uaccess.h>
>
> @@ -554,7 +555,7 @@ int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
> fd_set_bits fds;
> void *bits;
> int ret, max_fds;
> - unsigned int size;
> + size_t size, alloc_size;
> struct fdtable *fdt;
> /* Allocate small arguments on the stack to save memory and be faster */
> long stack_fds[SELECT_STACK_ALLOC/sizeof(long)];
> @@ -581,7 +582,14 @@ int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
> if (size > sizeof(stack_fds) / 6) {
> /* Not enough space in on-stack array; must use kmalloc */
> ret = -ENOMEM;
> - bits = kmalloc(6 * size, GFP_KERNEL);
> + if (size > (SIZE_MAX / 6))
> + goto out_nofds;
> +
> + alloc_size = 6 * size;
> + bits = kmalloc(alloc_size, GFP_KERNEL|__GFP_NOWARN);
> + if (!bits && alloc_size > PAGE_SIZE)
> + bits = vmalloc(alloc_size);
> +
> if (!bits)
> goto out_nofds;
> }
> @@ -618,7 +626,7 @@ int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
>
> out:
> if (bits != stack_fds)
> - kfree(bits);
> + kvfree(bits);
> out_nofds:
> return ret;
> }
> --
> 2.10.0
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v5 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
From: YOSHIFUJI Hideaki @ 2016-09-27 10:20 UTC (permalink / raw)
To: Maciej Żenczykowski, Maciej Żenczykowski,
David S . Miller
Cc: hideaki.yoshifuji, netdev, Erik Kline, Lorenzo Colitti,
Hannes Frederic Sowa
In-Reply-To: <1474969963-17402-4-git-send-email-zenczykowski@gmail.com>
Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> This allows setting /proc/sys/net/ipv6/conf/*/router_solicitations
> to -1 meaning an unlimited number of retransmits.
>
We could say "< 0 means infinite" and we can reduce changes here.
--yoshfuji
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
> net/ipv6/addrconf.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 8bd2d06eefe7..1e59c0034916 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3687,7 +3687,7 @@ static void addrconf_rs_timer(unsigned long data)
> if (idev->if_flags & IF_RA_RCVD)
> goto out;
>
> - if (idev->rs_probes++ < idev->cnf.rtr_solicits) {
> + if (idev->rs_probes++ < idev->cnf.rtr_solicits || idev->cnf.rtr_solicits == -1) {
> write_unlock(&idev->lock);
> if (!ipv6_get_lladdr(dev, &lladdr, IFA_F_TENTATIVE))
> ndisc_send_rs(dev, &lladdr,
> @@ -3949,7 +3949,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
> send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp);
> send_rs = send_mld &&
> ipv6_accept_ra(ifp->idev) &&
> - ifp->idev->cnf.rtr_solicits > 0 &&
> + ifp->idev->cnf.rtr_solicits != 0 &&
> (dev->flags&IFF_LOOPBACK) == 0;
> read_unlock_bh(&ifp->idev->lock);
>
> @@ -5099,7 +5099,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
> return -EINVAL;
> if (!ipv6_accept_ra(idev))
> return -EINVAL;
> - if (idev->cnf.rtr_solicits <= 0)
> + if (idev->cnf.rtr_solicits == 0)
> return -EINVAL;
>
> write_lock_bh(&idev->lock);
> @@ -5699,6 +5699,7 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
> return ret;
> }
>
> +static const int minus_one = -1;
> static const int one = 1;
> static const int two_five_five = 255;
>
> @@ -5759,7 +5760,8 @@ static const struct ctl_table addrconf_sysctl[] = {
> .data = &ipv6_devconf.rtr_solicits,
> .maxlen = sizeof(int),
> .mode = 0644,
> - .proc_handler = proc_dointvec,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = (void *)&minus_one,
> },
> {
> .procname = "router_solicitation_interval",
>
--
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION
^ permalink raw reply
* Re: [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff
From: Hannes Frederic Sowa @ 2016-09-27 10:20 UTC (permalink / raw)
To: Maciej Żenczykowski
Cc: David S . Miller, Linux NetDev, Erik Kline, Lorenzo Colitti, vpai
In-Reply-To: <CANP3RGfHPZLHFnp4Rq8_dRNCro6Uh8O8767NgoGu7gvzu5h9BQ@mail.gmail.com>
[cc Vishwanath Pai]
On 27.09.2016 11:42, Maciej Żenczykowski wrote:
>> Please just use do_div here and go back to the first version of the
>> patch. Variable names could be more aligned with the RFC maybe?
>
> So I tried:
>
> static inline s32 rfc3315_s14_backoff_init(s32 irt)
> {
> /* multiply 'initial retransmission time' by 0.9 .. 1.1 */
> u64 tmp = (900000 + prandom_u32() % 200001) * (u64)irt;
> do_div(tmp, 1000000);
> return (s32)tmp;
> }
>
> static inline s32 rfc3315_s14_backoff_update(s32 rt, s32 mrt)
> {
> /* multiply 'retransmission timeout' by 1.9 .. 2.1 */
> u64 tmp = (1900000 + prandom_u32() % 200001) * (u64)rt;
> do_div(tmp, 1000000);
> if ((s32)tmp > mrt) {
> /* multiply 'maximum retransmission time' by 0.9 .. 1.1 */
> tmp = (900000 + prandom_u32() % 200001) * (u64)mrt;
> do_div(tmp, 1000000);
> }
> return (s32)tmp;
> }
>
> but then building for i386 I get:
>
> ERROR: "__udivdi3" [net/netfilter/xt_hashlimit.ko] undefined!
Hmm, evidently we have some u64 divisions in xt_hashlimit.c which should
be replaced by do_div?
^ permalink raw reply
* Re: [PATCH v5 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
From: YOSHIFUJI Hideaki @ 2016-09-27 10:18 UTC (permalink / raw)
To: Maciej Żenczykowski, Maciej Żenczykowski,
David S . Miller
Cc: hideaki.yoshifuji, netdev, Erik Kline, Lorenzo Colitti,
Hannes Frederic Sowa
In-Reply-To: <1474969963-17402-3-git-send-email-zenczykowski@gmail.com>
Hi,
Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> replace with extra1/2 magic
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
> net/ipv6/addrconf.c | 21 ++++++---------------
> 1 file changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 11fa1a5564d4..8bd2d06eefe7 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -5467,20 +5467,6 @@ int addrconf_sysctl_forward(struct ctl_table *ctl, int write,
> }
>
> static
> -int addrconf_sysctl_hop_limit(struct ctl_table *ctl, int write,
> - void __user *buffer, size_t *lenp, loff_t *ppos)
> -{
> - struct ctl_table lctl;
> - int min_hl = 1, max_hl = 255;
> -
> - lctl = *ctl;
> - lctl.extra1 = &min_hl;
> - lctl.extra2 = &max_hl;
> -
> - return proc_dointvec_minmax(&lctl, write, buffer, lenp, ppos);
> -}
> -
> -static
> int addrconf_sysctl_mtu(struct ctl_table *ctl, int write,
> void __user *buffer, size_t *lenp, loff_t *ppos)
> {
> @@ -5713,6 +5699,9 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
> return ret;
> }
>
> +static const int one = 1;
> +static const int two_five_five = 255;
> +
> static const struct ctl_table addrconf_sysctl[] = {
> {
> .procname = "forwarding",
> @@ -5726,7 +5715,9 @@ static const struct ctl_table addrconf_sysctl[] = {
> .data = &ipv6_devconf.hop_limit,
> .maxlen = sizeof(int),
> .mode = 0644,
> - .proc_handler = addrconf_sysctl_hop_limit,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = (void *)&one,
> + .extra2 = (void *)&two_five_five,
> },
> {
> .procname = "mtu",
>
Please submit this in a different series of patches
(like 1/7).
--yoshfuji
^ 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