Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] wanxl: use m68k-linux-gnu-as if available
From: David Miller @ 2017-10-18 12:07 UTC (permalink / raw)
  To: kilobyte; +Cc: khc, netdev
In-Reply-To: <20171018113326.2rkpe7wrlzihwdyb@angband.pl>

From: Adam Borowski <kilobyte@angband.pl>
Date: Wed, 18 Oct 2017 13:33:26 +0200

> On Wed, Oct 18, 2017 at 12:08:12PM +0100, David Miller wrote:
>> From: Adam Borowski <kilobyte@angband.pl>
>> Date: Fri, 13 Oct 2017 01:24:34 +0200
>> 
>> > This fixes build failure on Debian based systems: GNU as is the only m68k
>> > assembler available in the archive (package binutils-m68k-linux-gnu).
>> > 
>> > Signed-off-by: Adam Borowski <kilobyte@angband.pl>
>> 
>> The Kconfig help text for the config option controlling the build
>> firmware is extremely clear what the requirements are for enabling
>> that option.
>> 
>> And that is: "It requires as68k, ld68k and hexdump programs."
> 
> Yeah, but as far as I know, as68k is a name of "GNU as"; Debian calls the
> same m68k-linux-gnu-as (and in general, $(GNU_TRIPLET)-as for all usual
> cross toolchains shipped).  And as Debian derivatives make the majority of
> distro installs these days, supporting its naming scheme would be
> convenient.
> 
> Or, if you believe no one should try to build this firmware from source,
> it might be better to annotate it to be excluded from randconfig builds.

We don't even know if whatever "as68k" is would be the same thing
as GNU as and generate the same binaries.

If you really must, somehow make sure the proper tools are available
at build or Kconfig time.

^ permalink raw reply

* Re: [PATCH] dql: make dql_init return void
From: Hiroaki SHIMODA @ 2017-10-18 12:15 UTC (permalink / raw)
  To: stephen; +Cc: tom, davem, netdev, sthemmin
In-Reply-To: <20171018001652.23740-1-sthemmin@microsoft.com>

On Tue, 17 Oct 2017 17:16:52 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> dql_init always returned 0, and the only place that uses it
> in network core code didn't care about the return value anyway.
> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Acked-by: Hiroaki SHIMODA <shimoda.hiroaki@gmail.com>

^ permalink raw reply

* Re: [PATCH net-next] tools: bpftool: use more common tag format
From: David Miller @ 2017-10-18 12:20 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: netdev, oss-drivers, alexei.starovoitov, daniel
In-Reply-To: <20171016171254.26225-1-jakub.kicinski@netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 16 Oct 2017 10:12:54 -0700

> Program tag is usually displayed as string of bytes without
> any separators (e.g. as "aa5520b1090cfeb6" vs MAC addr-like
> format bpftool uses currently: "aa:55:20:b1:09:0c:fe:b6").
> Make bptfool use the more common format both for displaying
> the tag and selecting the program by tag.
> 
> This was pointed out in review but I misunderstood the comment.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>

Applied.

^ permalink raw reply

* Re: [PATCH net v2] bpf: disallow arithmetic operations on context pointer
From: David Miller @ 2017-10-18 12:21 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: netdev, oss-drivers, alexei.starovoitov, daniel, ecree
In-Reply-To: <20171016181655.16366-1-jakub.kicinski@netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 16 Oct 2017 11:16:55 -0700

> Commit f1174f77b50c ("bpf/verifier: rework value tracking")
> removed the crafty selection of which pointer types are
> allowed to be modified.  This is OK for most pointer types
> since adjust_ptr_min_max_vals() will catch operations on
> immutable pointers.  One exception is PTR_TO_CTX which is
> now allowed to be offseted freely.
> 
> The intent of aforementioned commit was to allow context
> access via modified registers.  The offset passed to
> ->is_valid_access() verifier callback has been adjusted
> by the value of the variable offset.
> 
> What is missing, however, is taking the variable offset
> into account when the context register is used.  Or in terms
> of the code adding the offset to the value passed to the
> ->convert_ctx_access() callback.  This leads to the following
> eBPF user code:
> 
>      r1 += 68
>      r0 = *(u32 *)(r1 + 8)
>      exit
> 
> being translated to this in kernel space:
> 
>    0: (07) r1 += 68
>    1: (61) r0 = *(u32 *)(r1 +180)
>    2: (95) exit
> 
> Offset 8 is corresponding to 180 in the kernel, but offset
> 76 is valid too.  Verifier will "accept" access to offset
> 68+8=76 but then "convert" access to offset 8 as 180.
> Effective access to offset 248 is beyond the kernel context.
> (This is a __sk_buff example on a debug-heavy kernel -
> packet mark is 8 -> 180, 76 would be data.)
> 
> Dereferencing the modified context pointer is not as easy
> as dereferencing other types, because we have to translate
> the access to reading a field in kernel structures which is
> usually at a different offset and often of a different size.
> To allow modifying the pointer we would have to make sure
> that given eBPF instruction will always access the same
> field or the fields accessed are "compatible" in terms of
> offset and size...
> 
> Disallow dereferencing modified context pointers and add
> to selftests the test case described here.
> 
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Applied.

> ---
> Dave, a merge note - in net-next this will need env to be passed
> to verbose().

Thanks for the note.

^ permalink raw reply

* Re: [PATCH 1/7] devlink: Add permanent config parameter get/set operations
From: Steve Lin @ 2017-10-18 12:39 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, Jiri Pirko, David S . Miller, Michael Chan, John Linville,
	Andy Gospodarek
In-Reply-To: <20171018071140.GB2028@nanopsycho>

On Wed, Oct 18, 2017 at 3:11 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Tue, Oct 17, 2017 at 10:44:23PM CEST, steven.lin1@broadcom.com wrote:
> Steve. As I originally requested, could you please split this to:
> 1) single patch adding config get/set commands, without any config attributes
> 2) single patch per config attribute - please don't add them in bulk.
>    We also need very strict description for every single attribute so
>    other vendors know what it is and can re-use it. There is need to
>    avoid duplication here. Also, please send just few attribites in the
>    first run, not like 40 you are sending now. Impossible to review.

I broke the patch set up into functional blocks of attributes, in
order to avoid having ~40 patches of just a couple lines each.  But, I
will split further for each individual attribute, and just submit a
few initially, per your request.

>
> Also, why didn't you put it into nested attribute we were discussing?
>

I thought I did :) , using the DPIPE_HEADERS nested attribute as an
example.  I'll reach out to you off-list to understand what I'm
missing.

>>
>>+static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
>>+
>>+static int devlink_nl_sing_param_get(struct sk_buff *msg,
>
> I was wondering what song it will sing :) Just add "le", it's just 2
> chars :)
>

Will do, thanks. ;)

Steve

^ permalink raw reply

* Re: [PATCH 4/7] devlink: Adding perm config of link settings
From: Steve Lin @ 2017-10-18 12:39 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, Jiri Pirko, David S . Miller, Michael Chan, John Linville,
	Andy Gospodarek
In-Reply-To: <20171018073151.GC2028@nanopsycho>

On Wed, Oct 18, 2017 at 3:31 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>
> You need to split the config option to those that are per-port and to
> those that are per-asic. For each family, you have to use ither
> devlink_port of devlink handle. Also, you need to split into those that are
> permanent and to those who are teporary (until reset). I think you might
> need some flags for that.
>

All these are permanent; none are temporary - that's (partially) why
we consider these to be devlink/device parameters rather than a
netdev/ethtool thing, since they take effect after the next reset and
before any drivers are loaded (i.e. the card uses these parameters for
its default/startup configuration).

I will take a closer look at splitting these between per-port and per-asic.

Thanks,
Steve

^ permalink raw reply

* Re: [PATCH 6/7] bnxt: Move generic devlink code to new file
From: Steve Lin @ 2017-10-18 12:39 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, Jiri Pirko, David S . Miller, Michael Chan, John Linville,
	Andy Gospodarek
In-Reply-To: <20171018073348.GD2028@nanopsycho>

On Wed, Oct 18, 2017 at 3:33 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Tue, Oct 17, 2017 at 10:44:28PM CEST, steven.lin1@broadcom.com wrote:
>>Moving generic devlink code (registration) out of VR-R code
>>into new bnxt_devlink file.
>
> You can send this patch separatelly and let it be applied before the
> patchset.

Ok, will do.  Thanks again for all the feedback, Jiri.

Steve

^ permalink raw reply

* Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates
From: Marc Kleine-Budde @ 2017-10-18 12:44 UTC (permalink / raw)
  To: Franklin S Cooper Jr, Mario Hüttel, Yang, Wenyou,
	Sekhar Nori, wg, socketcan, quentin.schulz, edumazet, linux-can,
	netdev, linux-kernel
  Cc: Wenyou Yang, Dong Aisheng
In-Reply-To: <ed19ead3-d8f6-c027-74cf-8aee131c8a59@ti.com>


[-- Attachment #1.1: Type: text/plain, Size: 4910 bytes --]

On 09/21/2017 02:48 AM, Franklin S Cooper Jr wrote:
> 
> 
> On 09/20/2017 04:37 PM, Mario Hüttel wrote:
>>
>>
>> On 09/20/2017 10:19 PM, Franklin S Cooper Jr wrote:
>>> Hi Wenyou,
>>>
>>> On 09/17/2017 10:47 PM, Yang, Wenyou wrote:
>>>>
>>>> On 2017/9/14 13:06, Sekhar Nori wrote:
>>>>> On Thursday 14 September 2017 03:28 AM, Franklin S Cooper Jr wrote:
>>>>>> On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
>>>>>>> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
>>>>>>> resulted in errors. Scoping the signals I noticed that only a single
>>>>>>> bit
>>>>>>> was being transmitted and with a bit more investigation realized the
>>>>>>> actual
>>>>>>> MCAN IP would go back to initialization mode automatically.
>>>>>>>
>>>>>>> It appears this issue is due to the MCAN needing to use the Transmitter
>>>>>>> Delay Compensation Mode as defined in the MCAN User's Guide. When this
>>>>>>> mode is used the User's Guide indicates that the Transmitter Delay
>>>>>>> Compensation Offset register should be set. The document mentions
>>>>>>> that this
>>>>>>> register should be set to (1/dbitrate)/2*(Func Clk Freq).
>>>>>>>
>>>>>>> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document
>>>>>>> indicates
>>>>>>> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
>>>>>>> Therefore, only enable this mode and only set TDCO when the data bit
>>>>>>> rate
>>>>>>> is above 2.5 Mbps.
>>>>>>>
>>>>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>>>>> ---
>>>>>>> I'm pretty surprised that this hasn't been implemented already since
>>>>>>> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
>>>>>>> supports up to 10 Mbps.
>>>>>>>
>>>>>>> So it will be nice to get comments from users of this driver to
>>>>>>> understand
>>>>>>> if they have been able to use CAN-FD beyond 2.5 Mbps without this
>>>>>>> patch.
>>>>>>> If they haven't what did they do to get around it if they needed higher
>>>>>>> speeds.
>>>>>>>
>>>>>>> Meanwhile I plan on testing this using a more "realistic" CAN bus to
>>>>>>> insure
>>>>>>> everything still works at 5 Mbps which is the max speed of my CAN
>>>>>>> transceiver.
>>>>>> ping. Anyone has any thoughts on this?
>>>>> I added Dong who authored the m_can driver and Wenyou who added the only
>>>>> in-kernel user of the driver for any help.
>>>> I tested it on SAMA5D2 Xplained board both with and without this patch, 
>>>> both work with the 4M bps data bit rate.
>>> Thank you for testing this out. Its interesting that you have been able
>>> to use higher speeds without this patch. What is the CAN transceiver
>>> being used on the SAMA5D2 Xplained board? I tried looking at the
>>> schematic but it seems the CAN signals are used on an extension board
>>> which I can't find the schematic for. Also do you mind sharing your test
>>> setup? Were you doing a short point to point test?
>>>
>>> Thank You,
>>> Franklin
>> Hello Franklin,
>>
>> your patch definitely makes sense.
>>
>> I forgot the TDC in my patches because it was not present in the
>> previous driver versions and because I didn't encounter any
>> problems when testing it myself.
>>
>> The error is highly dependent on the hardware (transceiver) setup.
>> So it is definitely possible that some people don't encounter errors
>> without your patch.
> 
> So the Transmission Delay Compensation feature Value register is suppose
> to take into consideration the transceiver delay automatically and add
> the value of TDCO on top of that. So why would TDCO be dependent on the
> transceiver? I've heard conflicting things regarding TDC so any
> clarification on what actually impacts it would be appreciated.
> 
> Also part of the issue I'm having is how can we properly configure TDCO?
> Configuring TDCO is essentially figuring out what Secondary Sample Point
> to use. However, it is unclear what value to set SSP to and which use
> cases a given SSP will work or doesn't work. I've seen various
> recommendations from Bosch on choosing SSP but ultimately it seems they
> suggestion "real world testing" to come up with a proper value. Not
> setting TDCO causes problems for my device and improperly setting TDCO
> causes problems for my device. So its likely any value I use could end
> up breaking something for someone else.
> 
> Currently I leaning to a DT property that can be used for setting SSP.
> Perhaps use a generic default value and allow individuals to override it
> via DT?

Sounds reasonable. What's the status of this series?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v3 0/6] adapt DPAA drivers for DSA
From: David Miller @ 2017-10-18 12:45 UTC (permalink / raw)
  To: madalin.bucur
  Cc: netdev, f.fainelli, andrew, vivien.didelot, junote, linux-kernel
In-Reply-To: <1508178970-28945-1-git-send-email-madalin.bucur@nxp.com>

From: Madalin Bucur <madalin.bucur@nxp.com>
Date: Mon, 16 Oct 2017 21:36:04 +0300

> Junote Cai reported that he was not able to get a DSA setup involving the
> DPAA/FMAN driver to work and narrowed it down to of_find_net_device_by_node()
> call in DSA setup. The initial attempt to fix this by adding of_node to the
> platform device results in a second, failed, probing of the FMan MAC driver
> against the new platform device created for the DPAA Ethernet driver.
> Solve these issues by removing the of_node pointer from the platform device
> and changing the net_dev dev to the of_device dev to ensure the DSA init
> will be able to find the DPAA net_dev using of_find_net_device_by_node().
> Several changes were required to enable this solution: refactoring the
> adjust_link (also resulted in lesser, cleaner code) and renaming the fman
> kernel modules to keep the legacy udev rules happy.
> 
> Changes in v2:
> 
>  - fix issue on error path in "dpaa_eth: change device used" patch
>  - cleanup the dpaa_eth_probe() error paths
> 
> Changes in v3:
> 
>  - remove obsolete comment in moved code
>  - add explanation for module rename

Series applied.

^ permalink raw reply

* Re: [PATCH net-next v6 0/5] bpf: security: New file mode and LSM hooks for eBPF object permission control
From: David Miller @ 2017-10-18 12:47 UTC (permalink / raw)
  To: chenbofeng.kernel
  Cc: netdev, Selinux, linux-security-module, jeffv, alexei.starovoitov,
	lorenzo, daniel, sds, james.l.morris, paul, fengc
In-Reply-To: <20171016191135.8046-1-chenbofeng.kernel@gmail.com>

From: Chenbo Feng <chenbofeng.kernel@gmail.com>
Date: Mon, 16 Oct 2017 12:11:30 -0700

> Much like files and sockets, eBPF objects are accessed, controlled, and
> shared via a file descriptor (FD). Unlike files and sockets, the
> existing mechanism for eBPF object access control is very limited.
> Currently there are two options for granting accessing to eBPF
> operations: grant access to all processes, or only CAP_SYS_ADMIN
> processes. The CAP_SYS_ADMIN-only mode is not ideal because most users
> do not have this capability and granting a user CAP_SYS_ADMIN grants too
> many other security-sensitive permissions. It also unnecessarily allows
> all CAP_SYS_ADMIN processes access to eBPF functionality. Allowing all
> processes to access to eBPF objects is also undesirable since it has
> potential to allow unprivileged processes to consume kernel memory, and
> opens up attack surface to the kernel.
> 
> Adding LSM hooks maintains the status quo for systems which do not use
> an LSM, preserving compatibility with userspace, while allowing security
> modules to choose how best to handle permissions on eBPF objects. Here
> is a possible use case for the lsm hooks with selinux module:
> 
> The network-control daemon (netd) creates and loads an eBPF object for
> network packet filtering and analysis. It passes the object FD to an
> unprivileged network monitor app (netmonitor), which is not allowed to
> create, modify or load eBPF objects, but is allowed to read the traffic
> stats from the map.
> 
> Selinux could use these hooks to grant the following permissions:
> allow netd self:bpf_map { create read write};
> allow netmonitor netd:fd use;
> allow netmonitor netd:bpf_map read;
> 
> In this patch series, A file mode is added to bpf map to store the
> accessing mode. With this file mode flags, the map can be obtained read
> only, write only or read and write. With the help of this file mode,
> several security hooks can be added to the eBPF syscall implementations
> to do permissions checks. These LSM hooks are mainly focused on checking
> the process privileges before it obtains the fd for a specific bpf
> object. No matter from a file location or from a eBPF id. Besides that,
> a general check hook is also implemented at the start of bpf syscalls so
> that each security module can have their own implementation on the reset
> of bpf object related functionalities.
> 
> In order to store the ownership and security information about eBPF
> maps, a security field pointer is added to the struct bpf_map. And the
> last two patch set are implementation of selinux check on these hooks
> introduced, plus an additional check when eBPF object is passed between
> processes using unix socket as well as binder IPC.

Series applied.

^ permalink raw reply

* Re: [PATCH] decnet: af_decnet: mark expected switch fall-throughs
From: David Miller @ 2017-10-18 12:58 UTC (permalink / raw)
  To: garsilva; +Cc: linux-decnet-user, netdev, linux-kernel
In-Reply-To: <20171016201122.GA29615@embeddedor.com>

From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
Date: Mon, 16 Oct 2017 15:11:22 -0500

> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>

Applied.

^ permalink raw reply

* Re: [PATCH 1/7] devlink: Add permanent config parameter get/set operations
From: Jiri Pirko @ 2017-10-18 12:58 UTC (permalink / raw)
  To: Steve Lin
  Cc: netdev, Jiri Pirko, David S . Miller, Michael Chan, John Linville,
	Andy Gospodarek
In-Reply-To: <CA+Jmh7FRAAYyQv=qecU6RHFc_yj5P6Kp=8nB41i0driu9gycOQ@mail.gmail.com>

Wed, Oct 18, 2017 at 02:39:09PM CEST, steven.lin1@broadcom.com wrote:
>On Wed, Oct 18, 2017 at 3:11 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Tue, Oct 17, 2017 at 10:44:23PM CEST, steven.lin1@broadcom.com wrote:
>> Steve. As I originally requested, could you please split this to:
>> 1) single patch adding config get/set commands, without any config attributes
>> 2) single patch per config attribute - please don't add them in bulk.
>>    We also need very strict description for every single attribute so
>>    other vendors know what it is and can re-use it. There is need to
>>    avoid duplication here. Also, please send just few attribites in the
>>    first run, not like 40 you are sending now. Impossible to review.
>
>I broke the patch set up into functional blocks of attributes, in
>order to avoid having ~40 patches of just a couple lines each.  But, I
>will split further for each individual attribute, and just submit a
>few initially, per your request.
>
>>
>> Also, why didn't you put it into nested attribute we were discussing?
>>
>
>I thought I did :) , using the DPIPE_HEADERS nested attribute as an
>example.  I'll reach out to you off-list to understand what I'm
>missing.

I missed that. But you need a separate attr enum as well.

^ permalink raw reply

* Re: [PATCH] ipv4: mark expected switch fall-throughs
From: David Miller @ 2017-10-18 12:59 UTC (permalink / raw)
  To: garsilva
  Cc: kuznet, yoshfuji, pablo, kadlec, fw, netdev, linux-kernel,
	netfilter-devel, coreteam
In-Reply-To: <20171016204855.GA7470@embeddedor.com>

From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
Date: Mon, 16 Oct 2017 15:48:55 -0500

> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Notice that in some cases I placed the "fall through" comment
> on its own line, which is what GCC is expecting to find.
> 
> Addresses-Coverity-ID: 115108
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>

Applied.

^ permalink raw reply

* Re: [PATCH 4/7] devlink: Adding perm config of link settings
From: Jiri Pirko @ 2017-10-18 13:01 UTC (permalink / raw)
  To: Steve Lin
  Cc: netdev, Jiri Pirko, David S . Miller, Michael Chan, John Linville,
	Andy Gospodarek
In-Reply-To: <CA+Jmh7Eit_BBvkJNUdF4QYwCuP5T7WCkR4O_X9DK8O0CEzAjTA@mail.gmail.com>

Wed, Oct 18, 2017 at 02:39:42PM CEST, steven.lin1@broadcom.com wrote:
>On Wed, Oct 18, 2017 at 3:31 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> You need to split the config option to those that are per-port and to
>> those that are per-asic. For each family, you have to use ither
>> devlink_port of devlink handle. Also, you need to split into those that are
>> permanent and to those who are teporary (until reset). I think you might
>> need some flags for that.
>>
>
>All these are permanent; none are temporary - that's (partially) why
>we consider these to be devlink/device parameters rather than a
>netdev/ethtool thing, since they take effect after the next reset and
>before any drivers are loaded (i.e. the card uses these parameters for
>its default/startup configuration).

Understood. But I think that this iface should be capable to serve the
options of non-permanent as well. Or this could be 2 separate interfaces
with 2 separate cmd pair. Thoughts?


>
>I will take a closer look at splitting these between per-port and per-asic.
>
>Thanks,
>Steve

^ permalink raw reply

* Re: [PATCH net-next 3/3] ipv6: obsolete cached dst when removing them from fib tree
From: Paolo Abeni @ 2017-10-18 13:03 UTC (permalink / raw)
  To: Wei Wang, Eric Dumazet, Martin KaFai Lau, Xin Long
  Cc: Linux Kernel Network Developers, David S. Miller,
	Hannes Frederic Sowa
In-Reply-To: <CAEA6p_AS9c6pOym4t6FGW-bWfU=YmmL96Fc4vMcnDMCc8ywVqg@mail.gmail.com>

On Tue, 2017-10-17 at 13:48 -0700, Wei Wang wrote:
> On Tue, Oct 17, 2017 at 1:02 PM, Paolo Abeni <pabeni@redhat.com> wrote:
> > Meanwhile others sockets may grab more references to (and use) the same
> > aged-out dst.
> > 
> 
> I don't think other sockets could grab more reference to this dst
> because this dst should already be removed from the fib6 tree.

With the current net-next code, the dst is not removed from the fib
tree while someone else is holding it and dst_check() does not fail
after that the cached dst is aged out. If a socket cache grab a
reference to the CACHE dst, it will not release it untill the next
sernum change, regardless of the dst aging.

> > The commit 1e2ea8ad37be ("ipv6: set dst.obsolete when a cached route
> > has expired") was the solution to the above issue prior to the recent
> > refactor.
> > 
> 
> I don't really understand how this commit is solving the above issue.
> This commit still only ages out cached route if &rt->dst.__refcnt ==
> 1. So if socket is holding refcnt to this dst and dst_check() is not
> getting called,  this cached route still won't get deleted.

Setting obsolete to DST_OBSOLETE_KILL forced whoever was holding the
dst reference to drop it on the next dst_check(), so that refcnt could
go down. 

Cheers,

Paolo

^ permalink raw reply

* Re: [PATCH net-next] tcp: Use pI6c in tcp tracepoint
From: David Miller @ 2017-10-18 13:03 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, xiyou.wangcong
In-Reply-To: <1508189042-19591-1-git-send-email-dsahern@gmail.com>

From: David Ahern <dsahern@gmail.com>
Date: Mon, 16 Oct 2017 14:24:02 -0700

> The compact form for IPv6 addresses is more user friendly than the full
> version. For example:
>    compact: 2001:db8:1::1
>       full: 2001:0db8:0001:0000:0000:0000:0000:0004i
> 
> Update the tcp tracepoint to show the compact form.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next v6 0/5] bpf: security: New file mode and LSM hooks for eBPF object permission control
From: David Miller @ 2017-10-18 13:11 UTC (permalink / raw)
  To: chenbofeng.kernel
  Cc: netdev, Selinux, linux-security-module, jeffv, alexei.starovoitov,
	lorenzo, daniel, sds, james.l.morris, paul, fengc
In-Reply-To: <20171018.134729.837318478487425125.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Wed, 18 Oct 2017 13:47:29 +0100 (WEST)

> Series applied.

I had to revert as this breaks the build:

net/netfilter/xt_bpf.c: In function ‘__bpf_mt_check_path’:
net/netfilter/xt_bpf.c:59:7: error: too few arguments to function ‘bpf_obj_get_user’
  fd = bpf_obj_get_user(path);
       ^~~~~~~~~~~~~~~~

^ permalink raw reply

* Re: [PATCH] ipv6: mark expected switch fall-throughs
From: David Miller @ 2017-10-18 13:13 UTC (permalink / raw)
  To: garsilva
  Cc: steffen.klassert, herbert, kuznet, yoshfuji, pablo, kadlec, fw,
	netdev, linux-kernel, netfilter-devel, coreteam
In-Reply-To: <20171016213652.GA9967@embeddedor.com>

From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
Date: Mon, 16 Oct 2017 16:36:52 -0500

> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Notice that in some cases I placed the "fall through" comment
> on its own line, which is what GCC is expecting to find.
> 
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net: ipx: mark expected switch fall-through
From: David Miller @ 2017-10-18 13:14 UTC (permalink / raw)
  To: garsilva; +Cc: netdev, linux-kernel
In-Reply-To: <20171016215316.GA12885@embeddedor.com>

From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
Date: Mon, 16 Oct 2017 16:53:16 -0500

> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>

Applied.

^ permalink raw reply

* Re: [PATCH 1/7] devlink: Add permanent config parameter get/set operations
From: Steve Lin @ 2017-10-18 13:14 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, Jiri Pirko, David S . Miller, Michael Chan, John Linville,
	Andy Gospodarek
In-Reply-To: <20171018125825.GA1975@nanopsycho.orion>

On Wed, Oct 18, 2017 at 8:58 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Wed, Oct 18, 2017 at 02:39:09PM CEST, steven.lin1@broadcom.com wrote:
>>On Wed, Oct 18, 2017 at 3:11 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Tue, Oct 17, 2017 at 10:44:23PM CEST, steven.lin1@broadcom.com wrote:
>>> Steve. As I originally requested, could you please split this to:
>>> 1) single patch adding config get/set commands, without any config attributes
>>> 2) single patch per config attribute - please don't add them in bulk.
>>>    We also need very strict description for every single attribute so
>>>    other vendors know what it is and can re-use it. There is need to
>>>    avoid duplication here. Also, please send just few attribites in the
>>>    first run, not like 40 you are sending now. Impossible to review.
>>
>>I broke the patch set up into functional blocks of attributes, in
>>order to avoid having ~40 patches of just a couple lines each.  But, I
>>will split further for each individual attribute, and just submit a
>>few initially, per your request.
>>
>>>
>>> Also, why didn't you put it into nested attribute we were discussing?
>>>
>>
>>I thought I did :) , using the DPIPE_HEADERS nested attribute as an
>>example.  I'll reach out to you off-list to understand what I'm
>>missing.
>
> I missed that. But you need a separate attr enum as well.
>

I did have this as the nested attr enum in the original patch:

        /* Permanent Configuration Parameters */
        DEVLINK_ATTR_PERM_CFG,                          /* nested */

However, I only used the nested construct in the response from kernel
to userspace, not in the request from userspace to kernel.  (This was
based on looking at the various DPIPE_* nested attributes as
examples.)

Thinking about it after seeing your comment, I'm thinking I should
also use the nested attribute construct in the original request from
userspace to kernel as well, although I didn't see any previous
examples of this in devlink.

So I'll plan to use nesting in that direction as well.

Thanks,
Steve

^ permalink raw reply

* Re: [PATCH v2 net-next] tcp: Check daddr_cache before use in tracepoint
From: David Miller @ 2017-10-18 13:15 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, xiyou.wangcong
In-Reply-To: <1508193127-17626-1-git-send-email-dsahern@gmail.com>

From: David Ahern <dsahern@gmail.com>
Date: Mon, 16 Oct 2017 15:32:07 -0700

> Running perf in one window to capture tcp_retransmit_skb tracepoint:
>     $ perf record -e tcp:tcp_retransmit_skb -a
> 
> And causing a retransmission on an active TCP session (e.g., dropping
> packets in the receiver, changing MTU on the interface to 500 and back
> to 1500) triggers a panic:
 ...
> Check that daddr_cache is non-NULL before de-referencing.
> 
> Fixes: e086101b150a ("tcp: add a tracepoint for tcp retransmission")
> Signed-off-by: David Ahern <dsahern@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 0/4] bpf: move context info out of the verifier
From: David Miller @ 2017-10-18 13:17 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: netdev, oss-drivers, alexei.starovoitov, daniel
In-Reply-To: <20171016234056.1964-1-jakub.kicinski@netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 16 Oct 2017 16:40:52 -0700

> Daniel pointed out during the review of my previous patchset that
> the knowledge about context doesn't really belong directly in the
> verifier.  This patch set takes a bit of a drastic approach to
> move the info out of there.  I want to be able to use different
> set of verifier_ops for program analysis.  To do that, I have
> to first move the test_run callback to a separate structure.  Then
> verifier ops can be declared in the verifier directly and
> different sets can be picked for verification vs analysis.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] tcp: fix tcp_xmit_retransmit_queue() after rbtree introduction
From: David Miller @ 2017-10-18 13:19 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, ycheng
In-Reply-To: <1508207915.31614.87.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 16 Oct 2017 19:38:35 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> I tried to hard avoiding a call to rb_first() (via tcp_rtx_queue_head)
> in tcp_xmit_retransmit_queue(). But this was probably too bold.
> 
> Quoting Yuchung :
> 
> We might miss re-arming the RTO if tp->retransmit_skb_hint is not NULL.
> This can happen when RACK marks the first packet lost again and resets
> tp->retransmit_skb_hint for example (tcp_rack_mark_skb_lost())
> 
> Fixes: 75c119afe14f ("tcp: implement rb-tree based retransmit queue")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Yuchung Cheng <ycheng@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH 4/7] devlink: Adding perm config of link settings
From: Steve Lin @ 2017-10-18 13:22 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, Jiri Pirko, David S . Miller, Michael Chan, John Linville,
	Andy Gospodarek
In-Reply-To: <20171018130108.GC1975@nanopsycho.orion>

On Wed, Oct 18, 2017 at 9:01 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Wed, Oct 18, 2017 at 02:39:42PM CEST, steven.lin1@broadcom.com wrote:
>>On Wed, Oct 18, 2017 at 3:31 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>
>>> You need to split the config option to those that are per-port and to
>>> those that are per-asic. For each family, you have to use ither
>>> devlink_port of devlink handle. Also, you need to split into those that are
>>> permanent and to those who are teporary (until reset). I think you might
>>> need some flags for that.
>>>
>>
>>All these are permanent; none are temporary - that's (partially) why
>>we consider these to be devlink/device parameters rather than a
>>netdev/ethtool thing, since they take effect after the next reset and
>>before any drivers are loaded (i.e. the card uses these parameters for
>>its default/startup configuration).
>
> Understood. But I think that this iface should be capable to serve the
> options of non-permanent as well. Or this could be 2 separate interfaces
> with 2 separate cmd pair. Thoughts?
>

I would prefer to keep this command for permanent config only, and use
a separate command for transient configuration.  I think that
transient device configuration should be tackled in the separate
discussion that was started in the RFC version of this patchset,
related to moving ethtool ops to devlink/netlink.

I think that's a separate topic that requires a little more thought
and discussion, but really isn't that related to this current patchset
for permanent device configuration.  What do you think?

Steve

^ permalink raw reply

* Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates
From: Sekhar Nori @ 2017-10-18 13:24 UTC (permalink / raw)
  To: Marc Kleine-Budde, Franklin S Cooper Jr, Mario Hüttel,
	Yang, Wenyou, wg, socketcan, quentin.schulz, edumazet, linux-can,
	netdev, linux-kernel
  Cc: Wenyou Yang, Dong Aisheng, Quadros, Roger
In-Reply-To: <08432016-e733-b827-b9aa-bc359dd837f5@pengutronix.de>

Hi Marc,

On Wednesday 18 October 2017 06:14 PM, Marc Kleine-Budde wrote:
> On 09/21/2017 02:48 AM, Franklin S Cooper Jr wrote:
>>
>>
>> On 09/20/2017 04:37 PM, Mario Hüttel wrote:
>>>
>>>
>>> On 09/20/2017 10:19 PM, Franklin S Cooper Jr wrote:
>>>> Hi Wenyou,
>>>>
>>>> On 09/17/2017 10:47 PM, Yang, Wenyou wrote:
>>>>>
>>>>> On 2017/9/14 13:06, Sekhar Nori wrote:
>>>>>> On Thursday 14 September 2017 03:28 AM, Franklin S Cooper Jr wrote:
>>>>>>> On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
>>>>>>>> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
>>>>>>>> resulted in errors. Scoping the signals I noticed that only a single
>>>>>>>> bit
>>>>>>>> was being transmitted and with a bit more investigation realized the
>>>>>>>> actual
>>>>>>>> MCAN IP would go back to initialization mode automatically.
>>>>>>>>
>>>>>>>> It appears this issue is due to the MCAN needing to use the Transmitter
>>>>>>>> Delay Compensation Mode as defined in the MCAN User's Guide. When this
>>>>>>>> mode is used the User's Guide indicates that the Transmitter Delay
>>>>>>>> Compensation Offset register should be set. The document mentions
>>>>>>>> that this
>>>>>>>> register should be set to (1/dbitrate)/2*(Func Clk Freq).
>>>>>>>>
>>>>>>>> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document
>>>>>>>> indicates
>>>>>>>> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
>>>>>>>> Therefore, only enable this mode and only set TDCO when the data bit
>>>>>>>> rate
>>>>>>>> is above 2.5 Mbps.
>>>>>>>>
>>>>>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>>>>>> ---
>>>>>>>> I'm pretty surprised that this hasn't been implemented already since
>>>>>>>> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
>>>>>>>> supports up to 10 Mbps.
>>>>>>>>
>>>>>>>> So it will be nice to get comments from users of this driver to
>>>>>>>> understand
>>>>>>>> if they have been able to use CAN-FD beyond 2.5 Mbps without this
>>>>>>>> patch.
>>>>>>>> If they haven't what did they do to get around it if they needed higher
>>>>>>>> speeds.
>>>>>>>>
>>>>>>>> Meanwhile I plan on testing this using a more "realistic" CAN bus to
>>>>>>>> insure
>>>>>>>> everything still works at 5 Mbps which is the max speed of my CAN
>>>>>>>> transceiver.
>>>>>>> ping. Anyone has any thoughts on this?
>>>>>> I added Dong who authored the m_can driver and Wenyou who added the only
>>>>>> in-kernel user of the driver for any help.
>>>>> I tested it on SAMA5D2 Xplained board both with and without this patch, 
>>>>> both work with the 4M bps data bit rate.
>>>> Thank you for testing this out. Its interesting that you have been able
>>>> to use higher speeds without this patch. What is the CAN transceiver
>>>> being used on the SAMA5D2 Xplained board? I tried looking at the
>>>> schematic but it seems the CAN signals are used on an extension board
>>>> which I can't find the schematic for. Also do you mind sharing your test
>>>> setup? Were you doing a short point to point test?
>>>>
>>>> Thank You,
>>>> Franklin
>>> Hello Franklin,
>>>
>>> your patch definitely makes sense.
>>>
>>> I forgot the TDC in my patches because it was not present in the
>>> previous driver versions and because I didn't encounter any
>>> problems when testing it myself.
>>>
>>> The error is highly dependent on the hardware (transceiver) setup.
>>> So it is definitely possible that some people don't encounter errors
>>> without your patch.
>>
>> So the Transmission Delay Compensation feature Value register is suppose
>> to take into consideration the transceiver delay automatically and add
>> the value of TDCO on top of that. So why would TDCO be dependent on the
>> transceiver? I've heard conflicting things regarding TDC so any
>> clarification on what actually impacts it would be appreciated.
>>
>> Also part of the issue I'm having is how can we properly configure TDCO?
>> Configuring TDCO is essentially figuring out what Secondary Sample Point
>> to use. However, it is unclear what value to set SSP to and which use
>> cases a given SSP will work or doesn't work. I've seen various
>> recommendations from Bosch on choosing SSP but ultimately it seems they
>> suggestion "real world testing" to come up with a proper value. Not
>> setting TDCO causes problems for my device and improperly setting TDCO
>> causes problems for my device. So its likely any value I use could end
>> up breaking something for someone else.
>>
>> Currently I leaning to a DT property that can be used for setting SSP.
>> Perhaps use a generic default value and allow individuals to override it
>> via DT?
> 
> Sounds reasonable. What's the status of this series?

I have had some offline discussions with Franklin on this, and I am not
fully convinced that DT is the way to go here (although I don't have the
agreement with Franklin there).

There are two components in configuring the secondary sample point. It
is the transceiver loopback delay and an offset (example half of the bit
time in data phase).

While the transceiver loopback delay is pretty board dependent (and thus
amenable to DT encoding), I am not quite sure the offset can be
configured in DT because its not really board dependent.

Unfortunately, offset calculation does not seem to be an exact science.
There are recommendations ranging from using 50% of bit time to making
it same as the sample point configured. This means users who need to
change the SSP due to offset variations need to change  their DT even
without anything changing on their board.

Since we have a netlink socket interface to configure sample point, I
wonder if that should be extended to configure SSP too (or at least the
offset part of SSP)?

Thanks,
Sekhar

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox