* [PATCH net 1/6] net: fool proof dev_valid_name()
From: Eric Dumazet @ 2018-04-05 13:39 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Steffen Klassert, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180405133931.207634-1-edumazet@google.com>
We want to use dev_valid_name() to validate tunnel names,
so better use strnlen(name, IFNAMSIZ) than strlen(name) to make
sure to not upset KASAN.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 9b04a9fd1dfd0e065a7fe798dd840a07f0e0a4df..969462ebb296250fe5f3b7c4621e9ba9720a2dbe 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1027,7 +1027,7 @@ bool dev_valid_name(const char *name)
{
if (*name == '\0')
return false;
- if (strlen(name) >= IFNAMSIZ)
+ if (strnlen(name, IFNAMSIZ) == IFNAMSIZ)
return false;
if (!strcmp(name, ".") || !strcmp(name, ".."))
return false;
--
2.17.0.484.g0c8726318c-goog
^ permalink raw reply related
* [PATCH net 0/6] net: better validate user provided tunnel names
From: Eric Dumazet @ 2018-04-05 13:39 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Steffen Klassert, Eric Dumazet, Eric Dumazet
This series changes dev_valid_name() to not attempt reading
a possibly too long user-provided device name, then use
this helper in five different tunnel providers.
Eric Dumazet (6):
net: fool proof dev_valid_name()
ip_tunnel: better validate user provided tunnel names
ipv6: sit: better validate user provided tunnel names
ip6_gre: better validate user provided tunnel names
ip6_tunnel: better validate user provided tunnel names
vti6: better validate user provided tunnel names
net/core/dev.c | 2 +-
net/ipv4/ip_tunnel.c | 11 ++++++-----
net/ipv6/ip6_gre.c | 8 +++++---
net/ipv6/ip6_tunnel.c | 11 +++++++----
net/ipv6/ip6_vti.c | 7 +++++--
net/ipv6/sit.c | 8 +++++---
6 files changed, 29 insertions(+), 18 deletions(-)
--
2.17.0.484.g0c8726318c-goog
^ permalink raw reply
* [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin
From: Esben Haabendal @ 2018-04-05 13:35 UTC (permalink / raw)
To: netdev
Cc: Esben Haabendal, Rasmus Villemoes, Andrew Lunn, Florian Fainelli,
open list
From: Esben Haabendal <eha@deif.com>
The LED2[2]/INTn pin on Marvell 88E1318S as well as 88E1510/12/14/18 needs
to be configured to be usable as interrupt not only when WOL is enabled,
but whenever we rely on interrupts from the PHY.
Signed-off-by: Esben Haabendal <eha@deif.com>
Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
drivers/net/phy/marvell.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 0e0978d8a0eb..f03a510f1247 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -457,6 +457,21 @@ static int marvell_of_reg_init(struct phy_device *phydev)
}
#endif /* CONFIG_OF_MDIO */
+static int m88e1318_config_intr(struct phy_device *phydev)
+{
+ int err;
+
+ err = marvell_config_intr(phydev);
+ if (err)
+ return err;
+
+ /* Setup LED[2] as interrupt pin (active low) */
+ return phy_modify(phydev, MII_88E1318S_PHY_LED_TCR,
+ MII_88E1318S_PHY_LED_TCR_FORCE_INT,
+ MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
+ MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);
+}
+
static int m88e1121_config_aneg_rgmii_delays(struct phy_device *phydev)
{
int mscr;
@@ -2090,7 +2105,7 @@ static struct phy_driver marvell_drivers[] = {
.config_aneg = &m88e1318_config_aneg,
.read_status = &marvell_read_status,
.ack_interrupt = &marvell_ack_interrupt,
- .config_intr = &marvell_config_intr,
+ .config_intr = &m88e1318_config_intr,
.did_interrupt = &m88e1121_did_interrupt,
.get_wol = &m88e1318_get_wol,
.set_wol = &m88e1318_set_wol,
@@ -2189,7 +2204,7 @@ static struct phy_driver marvell_drivers[] = {
.config_aneg = &m88e1510_config_aneg,
.read_status = &marvell_read_status,
.ack_interrupt = &marvell_ack_interrupt,
- .config_intr = &marvell_config_intr,
+ .config_intr = &m88e1318_config_intr,
.did_interrupt = &m88e1121_did_interrupt,
.get_wol = &m88e1318_get_wol,
.set_wol = &m88e1318_set_wol,
--
2.16.3
^ permalink raw reply related
* Re: [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals
From: Felix Fietkau @ 2018-04-05 13:27 UTC (permalink / raw)
To: Joe Perches, netdev, linux-wireless, b43-dev, bridge,
netfilter-devel, coreteam
Cc: brcm80211-dev-list.pdl, linux-kernel, brcm80211-dev-list
In-Reply-To: <cover.1522479607.git.joe@perches.com>
On 2018-03-31 09:05, Joe Perches wrote:
> There are many local static and non-static arrays that are used for
> Ethernet broadcast address output or comparison.
>
> Centralize the array into a single separate file and remove the local
> arrays.
I suspect that for many targets and configurations, the local arrays
might actually be smaller than exporting a global. You have to factor in
not just the .text size, but the fact that referencing an exported
symbol needs a .reloc entry as well, which also eats up some space (at
least when the code is being built as module).
In my opinion, your series probably causes more bloat in common
configurations instead of reducing it.
You're also touching several places that could easily use
eth_broadcast_addr and eth_zero_addr. I think making those changes would
be more productive than what you did in this series.
- Felix
^ permalink raw reply
* net_dim() may use uninitialized data
From: Geert Uytterhoeven @ 2018-04-05 13:13 UTC (permalink / raw)
To: Tal Gilboa; +Cc: netdev, Linux Kernel Mailing List
Hi Tal,
With gcc-4.1.2:
drivers/net/ethernet/broadcom/bcmsysport.c: In function ‘bcm_sysport_poll’:
include/linux/net_dim.h:354: warning: ‘curr_stats.ppms’ may be
used uninitialized in this function
include/linux/net_dim.h:354: warning: ‘curr_stats.bpms’ may be
used uninitialized in this function
include/linux/net_dim.h:354: warning: ‘curr_stats.epms’ may be
used uninitialized in this function
Indeed, ...
| static inline void net_dim_calc_stats(struct net_dim_sample *start,
| struct net_dim_sample *end,
| struct net_dim_stats *curr_stats)
| {
| /* u32 holds up to 71 minutes, should be enough */
| u32 delta_us = ktime_us_delta(end->time, start->time);
| u32 npkts = BIT_GAP(BITS_PER_TYPE(u32), end->pkt_ctr, start->pkt_ctr);
| u32 nbytes = BIT_GAP(BITS_PER_TYPE(u32), end->byte_ctr,
| start->byte_ctr);
|
| if (!delta_us)
| return;
... if delta_us is zero, none of the below will be initialized ...
| curr_stats->ppms = DIV_ROUND_UP(npkts * USEC_PER_MSEC, delta_us);
| curr_stats->bpms = DIV_ROUND_UP(nbytes * USEC_PER_MSEC, delta_us);
| curr_stats->epms = DIV_ROUND_UP(NET_DIM_NEVENTS * USEC_PER_MSEC,
| delta_us);
| }
|
| static inline void net_dim(struct net_dim *dim,
| struct net_dim_sample end_sample)
| {
| struct net_dim_stats curr_stats;
| u16 nevents;
|
| switch (dim->state) {
| case NET_DIM_MEASURE_IN_PROGRESS:
| nevents = BIT_GAP(BITS_PER_TYPE(u16),
| end_sample.event_ctr,
| dim->start_sample.event_ctr);
| if (nevents < NET_DIM_NEVENTS)
| break;
| net_dim_calc_stats(&dim->start_sample, &end_sample,
| &curr_stats);
... in the output parameter curr_stats ...
| if (net_dim_decision(&curr_stats, dim)) {
... and net_dim_decision will make some funky decisions based on
uninitialized data.
What are the proper values to initialize curr_stats with?
Alternatively, perhaps the call to net_dim_decision() should be made
dependent on delta_us being non-zero?
| dim->state = NET_DIM_APPLY_NEW_PROFILE;
| schedule_work(&dim->work);
| break;
| }
| /* fall through */
| case NET_DIM_START_MEASURE:
| dim->state = NET_DIM_MEASURE_IN_PROGRESS;
| break;
| case NET_DIM_APPLY_NEW_PROFILE:
| break;
| }
| }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] dt: bindings: add new dt entries for brcmfmac
From: Kalle Valo @ 2018-04-05 13:10 UTC (permalink / raw)
To: Ulf Hansson
Cc: Arend van Spriel, Florian Fainelli, Alexey Roslyakov, Andrew Lunn,
Rob Herring, Mark Rutland, Franky Lin, Hante Meuleman,
Chi-Hsien Lin, Wright Feng, netdev, linux-wireless, devicetree,
Linux Kernel Mailing List, brcm80211-dev-list.pdl,
brcm80211-dev-list
In-Reply-To: <CAPDyKFr2GVB0+UUpT3GgGBT17TMcrky6X4L_pw+ztTL6ejbCwQ@mail.gmail.com>
Ulf Hansson <ulf.hansson@linaro.org> writes:
> On 20 March 2018 at 10:55, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>>>>> If I get it right, you mean something like this:
>>>>>
>>>>> mmc3: mmc@1c12000 {
>>>>> ...
>>>>> broken-sg-support;
>>>>> sd-head-align = 4;
>>>>> sd-sgentry-align = 512;
>>>>>
>>>>> brcmf: wifi@1 {
>>>>> ...
>>>>> };
>>>>> };
>>>>>
>>>>> Where dt: bindings documentation for these entries should reside?
>>>>> In generic MMC bindings? Well, this is the very special case and
>>>>> mmc-linux maintainer will unlikely to accept these changes.
>>>>> Also, extra kernel code modification might be required. It could make
>>>>> quite trivial change much more complex.
>>>>
>>>> If the MMC maintainers are not copied on this patch series, it will
>>>> likely be hard for them to identify this patch series and chime in...
>>>
>>> The main question is whether this is indeed a "very special case" as
>>> Alexey claims it to be or that it is likely to be applicable to other
>>> device and host combinations as you are suggesting.
>>>
>>> If these properties are imposed by the host or host controller it
>>> would make sense to have these in the mmc bindings.
>>
>> BTW, last year we were discussing something similar (I mean related to
>> alignment requirements) with ath10k SDIO patches and at the time the
>> patch submitter was proposing to have a bounce buffer in ath10k to
>> workaround that. I don't remember the details anymore, they are on the
>> ath10k mailing list archive if anyone is curious to know, but I would
>> not be surprised if they are similar as here. So there might be a need
>> to solve this in a generic way (but not sure of course as I haven't
>> checked the details).
>
> I re-call something about these as well, here are the patches. Perhaps
> I should pick some of them up...
>
> https://patchwork.kernel.org/patch/10123137/
> https://patchwork.kernel.org/patch/10123139/
> https://patchwork.kernel.org/patch/10123141/
> https://patchwork.kernel.org/patch/10123143/
Actually I was talking about a different patch, found it now:
ath10k_sdio: DMA bounce buffers for read write
https://patchwork.kernel.org/patch/9979543/
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next] netns: filter uevents correctly
From: Kirill Tkhai @ 2018-04-05 13:01 UTC (permalink / raw)
To: Christian Brauner, ebiederm, davem, gregkh, netdev, linux-kernel
Cc: avagin, serge
In-Reply-To: <20180404194857.29375-1-christian.brauner@ubuntu.com>
On 04.04.2018 22:48, Christian Brauner wrote:
> commit 07e98962fa77 ("kobject: Send hotplug events in all network namespaces")
>
> enabled sending hotplug events into all network namespaces back in 2010.
> Over time the set of uevents that get sent into all network namespaces has
> shrunk. We have now reached the point where hotplug events for all devices
> that carry a namespace tag are filtered according to that namespace.
>
> Specifically, they are filtered whenever the namespace tag of the kobject
> does not match the namespace tag of the netlink socket. One example are
> network devices. Uevents for network devices only show up in the network
> namespaces these devices are moved to or created in.
>
> However, any uevent for a kobject that does not have a namespace tag
> associated with it will not be filtered and we will *try* to broadcast it
> into all network namespaces.
>
> The original patchset was written in 2010 before user namespaces were a
> thing. With the introduction of user namespaces sending out uevents became
> partially isolated as they were filtered by user namespaces:
>
> net/netlink/af_netlink.c:do_one_broadcast()
>
> if (!net_eq(sock_net(sk), p->net)) {
> if (!(nlk->flags & NETLINK_F_LISTEN_ALL_NSID))
> return;
>
> if (!peernet_has_id(sock_net(sk), p->net))
> return;
>
> if (!file_ns_capable(sk->sk_socket->file, p->net->user_ns,
> CAP_NET_BROADCAST))
> j return;
> }
>
> The file_ns_capable() check will check whether the caller had
> CAP_NET_BROADCAST at the time of opening the netlink socket in the user
> namespace of interest. This check is fine in general but seems insufficient
> to me when paired with uevents. The reason is that devices always belong to
> the initial user namespace so uevents for kobjects that do not carry a
> namespace tag should never be sent into another user namespace. This has
> been the intention all along. But there's one case where this breaks,
> namely if a new user namespace is created by root on the host and an
> identity mapping is established between root on the host and root in the
> new user namespace. Here's a reproducer:
>
> sudo unshare -U --map-root
> udevadm monitor -k
> # Now change to initial user namespace and e.g. do
> modprobe kvm
> # or
> rmmod kvm
>
> will allow the non-initial user namespace to retrieve all uevents from the
> host. This seems very anecdotal given that in the general case user
> namespaces do not see any uevents and also can't really do anything useful
> with them.
>
> Additionally, it is now possible to send uevents from userspace. As such we
> can let a sufficiently privileged (CAP_SYS_ADMIN in the owning user
> namespace of the network namespace of the netlink socket) userspace process
> make a decision what uevents should be sent.
>
> This makes me think that we should simply ensure that uevents for kobjects
> that do not carry a namespace tag are *always* filtered by user namespace
> in kobj_bcast_filter(). Specifically:
> - If the owning user namespace of the uevent socket is not init_user_ns the
> event will always be filtered.
> - If the network namespace the uevent socket belongs to was created in the
> initial user namespace but was opened from a non-initial user namespace
> the event will be filtered as well.
> Put another way, uevents for kobjects not carrying a namespace tag are now
> always only sent to the initial user namespace. The regression potential
> for this is near to non-existent since user namespaces can't really do
> anything with interesting devices.
>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> lib/kobject_uevent.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
> index 15ea216a67ce..cb98cddb6e3b 100644
> --- a/lib/kobject_uevent.c
> +++ b/lib/kobject_uevent.c
> @@ -251,7 +251,15 @@ static int kobj_bcast_filter(struct sock *dsk, struct sk_buff *skb, void *data)
> return sock_ns != ns;
> }
>
> - return 0;
> + /*
> + * The kobject does not carry a namespace tag so filter by user
> + * namespace below.
> + */
> + if (sock_net(dsk)->user_ns != &init_user_ns)
> + return 1;
> +
> + /* Check if socket was opened from non-initial user namespace. */
> + return sk_user_ns(dsk) != &init_user_ns;
> }
> #endif
So, this prohibits to listen events of all devices except network-related
in containers? If it's so, I don't think it's a good solution. Uevents is not
net-devices-only related interface and it's used for all devices in system.
People may want to delegate block devices to nested user_ns, for example.
Better we should think about something like "generic device <-> user_ns" connection,
and to filter events by this user_ns.
Thanks,
Kirill
^ permalink raw reply
* Re: [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address
From: Kalle Valo @ 2018-04-05 12:48 UTC (permalink / raw)
To: Joe Perches
Cc: Christian Lamparter, Amitkumar Karwar, Nishant Sarmukadam,
Ganapathi Bhat, Xinming Hu, Ping-Ke Shih, Jussi Kivilinna,
linux-wireless, netdev, linux-kernel, b43-dev
In-Reply-To: <87po3desix.fsf@kamboji.qca.qualcomm.com>
Kalle Valo <kvalo@codeaurora.org> writes:
> Joe Perches <joe@perches.com> writes:
>
>> Use the new ether_broadcast_addr global instead to save some object code.
>>
>> Signed-off-by: Joe Perches <joe@perches.com>
>> ---
>> drivers/net/wireless/admtek/adm8211.c | 3 +--
>> drivers/net/wireless/ath/carl9170/mac.c | 4 +---
>> drivers/net/wireless/broadcom/b43/main.c | 3 +--
>> drivers/net/wireless/marvell/mwifiex/cfg80211.c | 3 +--
>> drivers/net/wireless/realtek/rtlwifi/core.c | 5 ++---
>> drivers/net/wireless/rndis_wlan.c | 6 +-----
>> drivers/net/wireless/ti/wl1251/main.c | 5 +----
>> drivers/net/wireless/ti/wlcore/main.c | 5 +----
>> 8 files changed, 9 insertions(+), 25 deletions(-)
>
> It would be really helpful if you could CLEARLY document in the patches
> (or in the cover letter but then you need to cc all parties) to what
> tree the patches are meant for. Otherwise I, and other maintainers as
> well, need to waste time trying to guess what's your plan.
Forgot to mention that for me the best approach is to have the tree name
in subject, just like Dave and Linus both recommend:
[PATCH 06/12 wireless-drivers-next] wireless: Convert simple uses of a static const Ethernet broadcast address
This way I can immeaditely see from patchwork to which tree the patch
should go which helps tremendously. And if the tree name is too long you
can always shorten it to w-d and w-d-next.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support
From: Andrew Lunn @ 2018-04-05 12:48 UTC (permalink / raw)
To: Laurentiu Tudor
Cc: Stuart Yoder, Arnd Bergmann, Ioana Ciornei, gregkh,
Linux Kernel Mailing List, Ruxandra Ioana Ciocoi Radulescu,
Razvan Stefanescu, Roy Pledge, Networking
In-Reply-To: <5AC61393.7090509@nxp.com>
> > Hi Laurentiu
> >
> > So i can use switchdev without it? I can modprobe the switchdev
> > driver, all the physical interfaces will appear, and i can use ip addr
> > add etc. I do not need to use a user space tool at all in order to use
> > the network functionality?
>
> Absolutely!
Great.
Then the easiest way forwards is to simply drop the IOCTL code for the
moment. Get the basic support for the hardware into the kernel
first. Then come back later to look at dynamic behaviour which needs
some form of configuration.
> In normal use cases the system designer, depending on the requirements,
> configures the various devices that it desires through a firmware
> configuration (think something like a device tree). The devices
> configured are presented on the mc-bus and probed normally by the
> kernel. The standard networking linux tools can be used as expected.
So what you should probably do is start a discussion on what this
device tree binding looks like. But you need to be careful even
here. Device tree describes the hardware, not how you configure the
hardware. So maybe DT does not actually fit.
Andrew
^ permalink raw reply
* Re: [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address
From: Kalle Valo @ 2018-04-05 12:39 UTC (permalink / raw)
To: Joe Perches
Cc: Christian Lamparter, Amitkumar Karwar, Nishant Sarmukadam,
Ganapathi Bhat, Xinming Hu, Ping-Ke Shih, Jussi Kivilinna,
linux-wireless, netdev, linux-kernel, b43-dev
In-Reply-To: <79196f134a513d50968e8e208a0e56b3c0236ee3.1522479607.git.joe@perches.com>
Joe Perches <joe@perches.com> writes:
> Use the new ether_broadcast_addr global instead to save some object code.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/net/wireless/admtek/adm8211.c | 3 +--
> drivers/net/wireless/ath/carl9170/mac.c | 4 +---
> drivers/net/wireless/broadcom/b43/main.c | 3 +--
> drivers/net/wireless/marvell/mwifiex/cfg80211.c | 3 +--
> drivers/net/wireless/realtek/rtlwifi/core.c | 5 ++---
> drivers/net/wireless/rndis_wlan.c | 6 +-----
> drivers/net/wireless/ti/wl1251/main.c | 5 +----
> drivers/net/wireless/ti/wlcore/main.c | 5 +----
> 8 files changed, 9 insertions(+), 25 deletions(-)
It would be really helpful if you could CLEARLY document in the patches
(or in the cover letter but then you need to cc all parties) to what
tree the patches are meant for. Otherwise I, and other maintainers as
well, need to waste time trying to guess what's your plan.
I will now drop the four wireless patches from my queue. So if you want
to me to take them please resubmit.
--
Kalle Valo
^ permalink raw reply
* Re: [v2] net: thunderx: nicvf_main: Fix potential NULL pointer dereferences
From: Vadim Lomovtsev @ 2018-04-05 12:38 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Eric Dumazet, Sunil Goutham, Robert Richter, linux-arm-kernel,
netdev, linux-kernel
In-Reply-To: <20180403220423.GA718@embeddedor.com>
Hi guys,
Please give me couple days to workout solution for this.
I'll post patch for this as soon as I done with my testing.
WBR,
Vadim
On Tue, Apr 03, 2018 at 05:04:23PM -0500, Gustavo A. R. Silva wrote:
> Add null check on kmalloc() return value in order to prevent
> a null pointer dereference.
>
> Addresses-Coverity-ID: 1467429 ("Dereference null return value")
> Fixes: 37c3347eb247 ("net: thunderx: add ndo_set_rx_mode callback implementation for VF")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> Changes in v2:
> - Add a null check on a second kmalloc a few lines below. Thanks to
> Eric Dumazet for pointing this out.
>
> drivers/net/ethernet/cavium/thunder/nicvf_main.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> index 1e9a31f..f7b5ca5 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> @@ -1999,10 +1999,14 @@ static void nicvf_set_rx_mode(struct net_device *netdev)
> struct xcast_addr *xaddr;
>
> mc_list = kmalloc(sizeof(*mc_list), GFP_ATOMIC);
> + if (unlikely(!mc_list))
> + return;
> INIT_LIST_HEAD(&mc_list->list);
> netdev_hw_addr_list_for_each(ha, &netdev->mc) {
> xaddr = kmalloc(sizeof(*xaddr),
> GFP_ATOMIC);
> + if (unlikely(!xaddr))
> + return;
> xaddr->addr =
> ether_addr_to_u64(ha->addr);
> list_add_tail(&xaddr->list,
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add register dump support for MICROCHIP LAN78xx
From: Andrew Lunn @ 2018-04-05 12:37 UTC (permalink / raw)
To: Raghuram Chary J; +Cc: davem, netdev, unglinuxdriver, woojung.huh
In-Reply-To: <20180405061128.5479-1-raghuramchary.jallipalli@microchip.com>
On Thu, Apr 05, 2018 at 11:41:28AM +0530, Raghuram Chary J wrote:
Hi Raghuram
> + fprintf(stdout, "PHY Registers:\n");
> + fprintf(stdout, "--------------\n");
> + fprintf(stdout, "Mode Control = 0x%04X\n", *lan78xx_reg++);
> + fprintf(stdout, "Mode Status = 0x%04X\n", *lan78xx_reg++);
> + fprintf(stdout, "Device identifier1 = 0x%04X\n", *lan78xx_reg++);
> + fprintf(stdout, "Device identifier2 = 0x%04X\n", *lan78xx_reg++);
> + fprintf(stdout, "Auto-Neg Advertisement = 0x%04X\n",
> + *lan78xx_reg++);
> + fprintf(stdout, "Auto-Neg Link Partner Ability = 0x%04X\n",
> + *lan78xx_reg++);
> + fprintf(stdout, "Auto-Neg Expansion = 0x%04X\n", *lan78xx_reg++);
> + fprintf(stdout, "Auto-Neg Next Page TX = 0x%04X\n", *lan78xx_reg++);
> + fprintf(stdout, "Auto-Neg Link Partner Next Page RX = 0x%04X\n",
> + *lan78xx_reg++);
> + fprintf(stdout, "1000BASE-T Control = 0x%04X\n", *lan78xx_reg++);
> + fprintf(stdout, "1000BASE-T Status = 0x%04X\n", *lan78xx_reg++);
> + fprintf(stdout, "Reserved = 0x%04X\n", *lan78xx_reg++);
> + fprintf(stdout, "Reserved = 0x%04X\n", *lan78xx_reg++);
> + fprintf(stdout, "MMD Access Control = 0x%04X\n", *lan78xx_reg++);
> + fprintf(stdout, "MMD Access Address/Data = 0x%04X\n", *lan78xx_reg++);
Do the above registers conform to the normal MII definitions? Have you
looked at the code in netsemi.c? Maybe a generic helper can be defined
which dumps standard PHY registers?
Andrew
^ permalink raw reply
* Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support
From: gregkh @ 2018-04-05 12:30 UTC (permalink / raw)
To: Laurentiu Tudor
Cc: Andrew Lunn, Stuart Yoder, Arnd Bergmann, Ioana Ciornei,
Linux Kernel Mailing List, Ruxandra Ioana Ciocoi Radulescu,
Razvan Stefanescu, Roy Pledge, Networking
In-Reply-To: <5AC5FAA8.80409@nxp.com>
On Thu, Apr 05, 2018 at 10:30:01AM +0000, Laurentiu Tudor wrote:
> Hello,
>
> My 2c below.
>
> On 04/04/2018 03:42 PM, Andrew Lunn wrote:
> >> I hear you. It is more complicated this way...having all these individual
> >> objects vs just a single "bundle" of them that represents a NIC. But, that's
> >> the way the DPAA2 hardware is, and we're implementing kernel support for
> >> the hardware as it is.
> >
> > Hi Stuart
> >
> > I see we are not making any progress here.
> >
> > So what i suggest is you post the kernel code and configuration tool
> > concept to netdev for a full review. You want reviews from David
> > Miller, Jiri Pirko, Jakub Kicinski, David Ahern, etc.
> >
>
> I think that the discussion steered too much towards networking related
> topics, while this ioctl doesn't have much to do with networking.
> It's just an ioctl for our mc-bus bus driver that is used to manage the
> devices on this bus through userspace tools.
> In addition, I'd drop any mention of our reference user space app
> (restool) to emphasize that this ioctl is not added just for a
> particular user space app. I think Stuart also mentioned this.
I'm not going to take a "generic device configuration ioctl" patch
unless it is documented to all exactly what it does, and why it is
there.
thanks,
greg k-h
^ permalink raw reply
* Re: marvell switch
From: Andrew Lunn @ 2018-04-05 12:22 UTC (permalink / raw)
To: Ran Shalit; +Cc: netdev
In-Reply-To: <CAJ2oMh+binZ1QhEUfnHxU5TVfhCT1FPuiWCyAwyxxzcxEUn5Gg@mail.gmail.com>
On Thu, Apr 05, 2018 at 05:47:24AM +0300, Ran Shalit wrote:
> Hello,
>
> I am trying to use marvell switch in linux,
> Is it that the kernel drivers from marvell switch are used just to
> enable all ports, or do they also provide APIs to userspace to enable
> specific ports only.
> I have not find examples or wiki for marvell switch, so I am not too
> sure as what are the drivers meant for.
Hi Ran
The Marvell driver makes each port act like a normal Linux network
interface. So if you want to enable a port, do
ip link set lan0 up
Want to add an ip address to a port
ip addr add 10.42.42.42/24 dev lan0
Want to bridge two ports
ip link add name br0 type bridge
ip link set dev br0 up
ip link set dev lan0 master br0
ip link set dev lan1 master br0
Just treat them as normal interfaces.
Andrew
^ permalink raw reply
* Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support
From: Laurentiu Tudor @ 2018-04-05 12:16 UTC (permalink / raw)
To: Andrew Lunn
Cc: Stuart Yoder, Arnd Bergmann, Ioana Ciornei, gregkh,
Linux Kernel Mailing List, Ruxandra Ioana Ciocoi Radulescu,
Razvan Stefanescu, Roy Pledge, Networking
In-Reply-To: <20180405114736.GA12178@lunn.ch>
On 04/05/2018 02:47 PM, Andrew Lunn wrote:
> On Thu, Apr 05, 2018 at 10:30:01AM +0000, Laurentiu Tudor wrote:
>> Hello,
>>
>> My 2c below.
>>
>> On 04/04/2018 03:42 PM, Andrew Lunn wrote:
>>>> I hear you. It is more complicated this way...having all these individual
>>>> objects vs just a single "bundle" of them that represents a NIC. But, that's
>>>> the way the DPAA2 hardware is, and we're implementing kernel support for
>>>> the hardware as it is.
>>>
>>> Hi Stuart
>>>
>>> I see we are not making any progress here.
>>>
>>> So what i suggest is you post the kernel code and configuration tool
>>> concept to netdev for a full review. You want reviews from David
>>> Miller, Jiri Pirko, Jakub Kicinski, David Ahern, etc.
>>>
>>
>> I think that the discussion steered too much towards networking related
>> topics, while this ioctl doesn't have much to do with networking.
>
> Hi Laurentiu
>
> So i can use switchdev without it? I can modprobe the switchdev
> driver, all the physical interfaces will appear, and i can use ip addr
> add etc. I do not need to use a user space tool at all in order to use
> the network functionality?
Absolutely!
In normal use cases the system designer, depending on the requirements,
configures the various devices that it desires through a firmware
configuration (think something like a device tree). The devices
configured are presented on the mc-bus and probed normally by the
kernel. The standard networking linux tools can be used as expected.
The ioctl is necessary only for more advanced use cases that are
supported by this bus. Think "more dynamic" scenarios that involve
linking & unlinking various devices at runtime, maybe some
virtualization scenarios. Unfortunately I'm not the architect type of
guy so I don't have more specific examples to better illustrate ...
---
Best Regards, Laurentiu
^ permalink raw reply
* Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support
From: Andrew Lunn @ 2018-04-05 11:47 UTC (permalink / raw)
To: Laurentiu Tudor
Cc: Stuart Yoder, Arnd Bergmann, Ioana Ciornei, gregkh,
Linux Kernel Mailing List, Ruxandra Ioana Ciocoi Radulescu,
Razvan Stefanescu, Roy Pledge, Networking
In-Reply-To: <5AC5FAA8.80409@nxp.com>
On Thu, Apr 05, 2018 at 10:30:01AM +0000, Laurentiu Tudor wrote:
> Hello,
>
> My 2c below.
>
> On 04/04/2018 03:42 PM, Andrew Lunn wrote:
> >> I hear you. It is more complicated this way...having all these individual
> >> objects vs just a single "bundle" of them that represents a NIC. But, that's
> >> the way the DPAA2 hardware is, and we're implementing kernel support for
> >> the hardware as it is.
> >
> > Hi Stuart
> >
> > I see we are not making any progress here.
> >
> > So what i suggest is you post the kernel code and configuration tool
> > concept to netdev for a full review. You want reviews from David
> > Miller, Jiri Pirko, Jakub Kicinski, David Ahern, etc.
> >
>
> I think that the discussion steered too much towards networking related
> topics, while this ioctl doesn't have much to do with networking.
Hi Laurentiu
So i can use switchdev without it? I can modprobe the switchdev
driver, all the physical interfaces will appear, and i can use ip addr
add etc. I do not need to use a user space tool at all in order to use
the network functionality?
Andrew
^ permalink raw reply
* [PATCH 2/2] net: phy: dp83640: Read strapped configuration settings
From: esben.haabendal @ 2018-04-05 11:44 UTC (permalink / raw)
To: Richard Cochran, Andrew Lunn, Florian Fainelli,
open list:PTP HARDWARE CLOCK SUPPORT, open list
Cc: Esben Haabendal, Rasmus Villemoes
In-Reply-To: <20180405114424.8519-1-esben.haabendal@gmail.com>
From: Esben Haabendal <eha@deif.com>
Read configration settings, to allow automatic forced speed/duplex setup
by hardware strapping.
Signed-off-by: Esben Haabendal <eha@deif.com>
Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
drivers/net/phy/dp83640.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 654f42d00092..01e21b4998ad 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -1134,6 +1134,10 @@ static int dp83640_probe(struct phy_device *phydev)
if (!dp83640)
goto no_memory;
+ err = genphy_read_config(phydev);
+ if (err)
+ goto no_config;
+
dp83640->phydev = phydev;
INIT_DELAYED_WORK(&dp83640->ts_work, rx_timestamp_work);
@@ -1166,6 +1170,7 @@ static int dp83640_probe(struct phy_device *phydev)
no_register:
clock->chosen = NULL;
+no_config:
kfree(dp83640);
no_memory:
dp83640_clock_put(clock);
--
2.16.3
^ permalink raw reply related
* [PATCH 1/2] net: phy: Helper function for reading strapped configuration values
From: esben.haabendal @ 2018-04-05 11:44 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, open list:ETHERNET PHY LIBRARY,
open list
Cc: Esben Haabendal, Rasmus Villemoes
From: Esben Haabendal <eha@deif.com>
Add a function for use in PHY driver probe functions, reading current
autoneg, speed and duplex configuration from BMCR register.
Useful for PHY that supports hardware strapped configuration, enabling
Linux to respect that configuration (i.e. strapped non-autoneg
configuration).
Signed-off-by: Esben Haabendal <eha@deif.com>
Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
drivers/net/phy/phy_device.c | 41 +++++++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 1 +
2 files changed, 42 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 74664a6c0cdc..cc52ff2a2344 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1673,6 +1673,47 @@ int genphy_config_init(struct phy_device *phydev)
}
EXPORT_SYMBOL(genphy_config_init);
+/**
+ * genphy_read_config - read configuration from PHY
+ * @phydev: target phy_device struct
+ *
+ * Description: Reads MII_BMCR and sets phydev autoneg, speed and duplex
+ * accordingly. For use in driver probe functions, to respect strapped
+ * configuration settings.
+ */
+int genphy_read_config(struct phy_device *phydev)
+{
+ int bmcr;
+
+ bmcr = phy_read(phydev, MII_BMCR);
+ if (bmcr < 0)
+ return bmcr;
+
+ if (BMCR_ANENABLE & bmcr) {
+ phydev->autoneg = AUTONEG_ENABLE;
+
+ phydev->speed = 0;
+ phydev->duplex = -1;
+ } else {
+ phydev->autoneg = AUTONEG_DISABLE;
+
+ if (bmcr & BMCR_FULLDPLX)
+ phydev->duplex = DUPLEX_FULL;
+ else
+ phydev->duplex = DUPLEX_HALF;
+
+ if (bmcr & BMCR_SPEED1000)
+ phydev->speed = SPEED_1000;
+ else if (bmcr & BMCR_SPEED100)
+ phydev->speed = SPEED_100;
+ else
+ phydev->speed = SPEED_10;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(genphy_read_config);
+
/* This is used for the phy device which doesn't support the MMD extended
* register access, but it does have side effect when we are trying to access
* the MMD register via indirect method.
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7c4c2379e010..5a8821c8105d 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -970,6 +970,7 @@ void phy_attached_info(struct phy_device *phydev);
/* Clause 22 PHY */
int genphy_config_init(struct phy_device *phydev);
+int genphy_read_config(struct phy_device *phydev);
int genphy_setup_forced(struct phy_device *phydev);
int genphy_restart_aneg(struct phy_device *phydev);
int genphy_config_aneg(struct phy_device *phydev);
--
2.16.3
^ permalink raw reply related
* [RFC] ethtool: Support for driver private ioctl's
From: Jose Abreu @ 2018-04-05 10:47 UTC (permalink / raw)
To: David Miller, Jakub Jelinek, Jeff Garzik, Tim Hockin,
Eli Kupermann, Chris Leech, Scott Feldman, Ben Hutchings
Cc: netdev, Joao Pinto
Hi All,
I would like to know your opinion regarding adding support for
driver private ioctl's in ethtool.
Background: Synopsys Ethernet IP's have a certain number of
features which can be reconfigured at runtime. Giving you two
examples: One of the most recent one is the safety features,
which can be enabled/disabled and forced at runtime. Another one
is a Flexible RX Parser which can route specific packets to
specific RX DMA channels. Given that these are features specific
to our IP's it would not be useful to add an uniform API for this
because the users would only be one or two drivers ...
This new feature would change the help usage for ethtool so that
each driver private option would be shown, and then each driver
specific file would have a structure with all the available
options. Finally, each driver would have to handle the private
IOCTL's.
We already have this working locally and now I would like to know
your opinion about upstreaming this ... Do you think this can be
useful for anyone else? Or should we change direction to use, for
example, debugfs/configfs?
Thanks and Best Regards,
Jose Miguel Abreu
^ permalink raw reply
* Re: Best userspace programming API for XDP features query to kernel?
From: Daniel Borkmann via iovisor-dev @ 2018-04-05 10:37 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Eric Leblond, Victor Julien, Peter Manev
Cc: oisf-devel-ZwoEplunGu2j570ONfqVQLVmwVP6tfMwSoIsB4E12gc,
Jakub Kicinski, Alexei Starovoitov, netdev-u79uwXL29TY76Z2rM5mHXA,
iovisor-dev-9jONkmmOlFHEE9lA1F8Ukti2O/JbrIOy@public.gmane.org,
Saeed Mahameed, Daniel Borkmann
In-Reply-To: <20180404142856.284ee8c9-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On 04/04/2018 02:28 PM, Jesper Dangaard Brouer via iovisor-dev wrote:
> Hi Suricata people,
>
> When Eric Leblond (and I helped) integrated XDP in Suricata, we ran
> into the issue, that at Suricata load/start time, we cannot determine
> if the chosen XDP config options, like xdp-cpu-redirect[1], is valid on
> this HW (e.g require driver XDP_REDIRECT support and bpf cpumap).
>
> We would have liked a way to report that suricata.yaml config was
> invalid for this hardware/setup. Now, it just loads, and packets gets
> silently dropped by XDP (well a WARN_ONCE and catchable via tracepoints).
>
> My question to suricata developers: (Q1) Do you already have code that
> query the kernel or drivers for features?
>
> At the IOvisor call (2 weeks ago), we discussed two options of exposing
> XDP features avail in a given driver.
>
> Option#1: Extend existing ethtool -k/-K "offload and other features"
> with some XDP features, that userspace can query. (Do you already query
> offloads, regarding Q1)
>
> Option#2: Invent a new 'ip link set xdp' netlink msg with a query option.
I don't really mind if you go via ethtool, as long as we handle this
generically from there and e.g. call the dev's ndo_bpf handler such that
we keep all the information in one place. This can be a new ndo_bpf command
e.g. XDP_QUERY_FEATURES or such.
More specifically, how would such feature mask look like? How fine grained
would this be? When you add a new minor feature to, say, cpumap that not
all drivers support yet, we'd need a new flag each time, no? Same for meta data,
then potentially for the redirect memory return work, or the af_xdp bits, the
xdp_rxq_info would have needed it, etc. What about nfp in terms of XDP
offload capabilities, should they be included as well or is probing to load
the program and see if it loads/JITs as we do today just fine (e.g. you'd
otherwise end up with extra flags on a per BPF helper basis)? To make a
somewhat reliable assertion whether feature xyz would work, this would
explode in new feature bits long term. Additionally, if we end up with a
lot of feature flags, it will be very hard for users to determine whether
this particular set of features a driver supports actually represents a
fully supported native XDP driver.
What about keeping this high level to users? E.g. say you have 2 options
that drivers can expose as netdev_features_strings 'xdp-native-full' or
'xdp-native-partial'. If a driver truly supports all XDP features for a
given kernel e.g. v4.16, then a query like 'ethtool -k foo' will say
'xdp-native-full', if at least one feature is missing to be feature complete
from e.g. above list, then ethtool will tell 'xdp-native-partial', and if
not even ndo_bpf callback exists then no 'xdp-native-*' is reported.
Side-effect might be that it would give incentive to keep drivers in state
'xdp-native-full' instead of being downgraded to 'xdp-native-partial'.
Potentially, in the 'xdp-native-partial' state, we can expose a high-level
list of missing features that the driver does not support yet, which would
over time converge towards 'zero' and thus 'xdp-native-full' again. ethtool
itself could get a new XDP specific query option that, based on this info,
can then dump the full list of supported and not supported features. In order
for this to not explode, such features would need to be kept on a high-level
basis, meaning if e.g. cpumap gets extended along with support for a number
of drivers, then those that missed out would need to be temporarily
re-flagged with e.g. 'cpumap not supported' until it gets also implemented
there. That way, we don't explode in adding too fine-grained feature bit
combinations long term and make it easier to tell whether a driver supports
the full set in native XDP or not. Thoughts?
> (Q2) Do Suricata devs have any preference (or other options/ideas) for
> the way the kernel expose this info to userspace?
>
> [1] http://suricata.readthedocs.io/en/latest/capture-hardware/ebpf-xdp.html#the-xdp-cpu-redirect-case
^ permalink raw reply
* Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support
From: Laurentiu Tudor @ 2018-04-05 10:30 UTC (permalink / raw)
To: Andrew Lunn, Stuart Yoder
Cc: Arnd Bergmann, Ioana Ciornei, gregkh, Linux Kernel Mailing List,
Ruxandra Ioana Ciocoi Radulescu, Razvan Stefanescu, Roy Pledge,
Networking
In-Reply-To: <20180404124246.GA20869@lunn.ch>
Hello,
My 2c below.
On 04/04/2018 03:42 PM, Andrew Lunn wrote:
>> I hear you. It is more complicated this way...having all these individual
>> objects vs just a single "bundle" of them that represents a NIC. But, that's
>> the way the DPAA2 hardware is, and we're implementing kernel support for
>> the hardware as it is.
>
> Hi Stuart
>
> I see we are not making any progress here.
>
> So what i suggest is you post the kernel code and configuration tool
> concept to netdev for a full review. You want reviews from David
> Miller, Jiri Pirko, Jakub Kicinski, David Ahern, etc.
>
I think that the discussion steered too much towards networking related
topics, while this ioctl doesn't have much to do with networking.
It's just an ioctl for our mc-bus bus driver that is used to manage the
devices on this bus through userspace tools.
In addition, I'd drop any mention of our reference user space app
(restool) to emphasize that this ioctl is not added just for a
particular user space app. I think Stuart also mentioned this.
---
Thanks & Best Regards, Laurentiu
^ permalink raw reply
* [PATCH net] net: mvpp2: Fix parser entry init boundary check
From: Maxime Chevallier @ 2018-04-05 9:55 UTC (permalink / raw)
To: davem
Cc: Maxime Chevallier, netdev, linux-kernel, Antoine Tenart,
thomas.petazzoni, gregory.clement, miquel.raynal, nadavh, stefanc,
ymarkman, mw
Boundary check in mvpp2_prs_init_from_hw must be done according to the
passed "tid" parameter, not the mvpp2_prs_entry index, which is not yet
initialized at the time of the check.
Fixes: 47e0e14eb1a6 ("net: mvpp2: Make mvpp2_prs_hw_read a parser entry init function")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/ethernet/marvell/mvpp2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 7fc1bbf51c44..54a038943c06 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -1604,7 +1604,7 @@ static int mvpp2_prs_init_from_hw(struct mvpp2 *priv,
{
int i;
- if (pe->index > MVPP2_PRS_TCAM_SRAM_SIZE - 1)
+ if (tid > MVPP2_PRS_TCAM_SRAM_SIZE - 1)
return -EINVAL;
memset(pe, 0, sizeof(*pe));
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2] Bluetooth: Remove VLA usage in aes_cmac
From: Gustavo A. R. Silva @ 2018-04-05 9:51 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Johan Hedberg, David S. Miller, Bluez mailing list,
Network Development, linux-kernel
In-Reply-To: <347FC7F5-4BB3-4477-9EF1-BAAA98F1D107@holtmann.org>
On 04/05/2018 03:46 AM, Marcel Holtmann wrote:
>> By the way, what is you opinion on replacing crypto_shash_descsize(ctx) with PAGE_SIZE / 8 in SHASH_DESC_ON_STACK?
>>
>> Does it work for you?
>
> isn’t that just waste?
>
Agree.
> The macro itself is this.
>
> #define SHASH_DESC_ON_STACK(shash, ctx) \
> char __##shash##_desc[sizeof(struct shash_desc) + \
> crypto_shash_descsize(ctx)] CRYPTO_MINALIGN_ATTR; \
> struct shash_desc *shash = (struct shash_desc *)__##shash##_desc
>
> For AES-CMAC, we could always do this with a manual macro that gives us the right size. However that is error prone if any internals change. I think what has to happened that crypto_shash_decsize becomes something the compiler can evaluate at compile time.
>
Yeah. That would imply an analysis of the algorithm each of the callers
use. In the case of AES-CMAC, what is the maximum digest size?
I tried to find a fixed-length value for AES-CMAC but I didn't get any
output with git grep -n _DIGEST_SIZE | grep AES
Thanks
--
Gustavo
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/3] bpf/verifier: subprog/func_call simplifications
From: Edward Cree @ 2018-04-05 8:49 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Daniel Borkmann, netdev
In-Reply-To: <20180405052820.eurpes52ilhofbg3@ast-mbp.dhcp.thefacebook.com>
On 05/04/18 06:28, Alexei Starovoitov wrote:
> On Thu, Apr 05, 2018 at 12:58:46AM +0100, Edward Cree wrote:
>> On 04/04/18 00:37, Alexei Starovoitov wrote:
>>> hmm. that doesn't fail for me and any other bots didn't complain.
>>> Are you sure you're running the latest kernel and tests?
>> Ah, test_progs isn't actually rebuilding because __NR_bpf is undeclared;
>> something must be going wrong with header files.
>> Never mind.
>>
>>> hmm. what's wrong with bsearch? It's trivial and fast.
>> bsearch is O(log n), and the sort() call on the subprog_starts (which happens
>> every time add_subprog() is called) is O(n log n).
>> Whereas reading aux->subprogno is O(1).
>> As far as I'm concerned, that's a sign that the latter data structure is the
>> appropriate one.
> only if it can be done as separate _first_ pass before cfg.
As I think I already said, I'm working on a next version of the patch that
does just that.
> No. It's worse. Your proposed approach to bounded loops completely
> relying on 'explore all paths' whereas John's does not.
> Can 'explore all paths' work with 1M bpf insns? No.
> Whereas an approach that builds dom graph, detects natural loops
> and loop invariants - can.
... IF it's possible at all, which I'm still doubtful about.
> This sounds like arbitrary assumptions what this new approach would do.
> Instead of rejecting it outright try to solve this very hard problem.
I'm not rejecting it outright; I'm just saying, be prepared for the possibility
that it can't be solved, and try to leave a line of retreat / have a plan B in
the case that it proves to be subject to a no-free-lunch theorem. Because my
intuition is that an entropy argument may require all algos to have the same
superexponential worst-case running time as 'explore all paths' does.
> Before this discussion gets carried away too far let's get back to this patch set.
> Having seen all arguments I still think that only patch 3 is worth pursuing further.
I think the next version of the patch series (coming real soon now) answers at
least most of your objections (and indeed the above debate isn't relevant to it).
^ permalink raw reply
* Re: [PATCH v2] Bluetooth: Remove VLA usage in aes_cmac
From: Marcel Holtmann @ 2018-04-05 8:46 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Johan Hedberg, David S. Miller, Bluez mailing list,
Network Development, linux-kernel
In-Reply-To: <2af25866-4e8a-7f9c-9298-e45abfab20c7@embeddedor.com>
Hi Gustavo,
>> so I took this patch back out of bluetooth-next before sending the pull request. I think the discussion on how to fix SHASH_DESC_ON_STACK macro needs to complete first. Once that has concluded we can revisit if this patch is still needed or if another solution has been found. Same as with WiFi, these are not just one-shot calls where a memory allocation doesn’t matter. We need this for random address resolution and thus there can be many of the aes_cmac calls when seeing neighboring devices.
>
> Yeah. I agree.
>
> Based on Herbert's response to the discussion about SHASH_DESC_ON_STACK https://lkml.org/lkml/2018/3/27/300
>
> it seems it is feasible to fix that macro very easily. I will follow up on this.
>
> By the way, what is you opinion on replacing crypto_shash_descsize(ctx) with PAGE_SIZE / 8 in SHASH_DESC_ON_STACK?
>
> Does it work for you?
isn’t that just waste?
The macro itself is this.
#define SHASH_DESC_ON_STACK(shash, ctx) \
char __##shash##_desc[sizeof(struct shash_desc) + \
crypto_shash_descsize(ctx)] CRYPTO_MINALIGN_ATTR; \
struct shash_desc *shash = (struct shash_desc *)__##shash##_desc
For AES-CMAC, we could always do this with a manual macro that gives us the right size. However that is error prone if any internals change. I think what has to happened that crypto_shash_decsize becomes something the compiler can evaluate at compile time.
Regards
Marcel
^ 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