* Re: [PATCH bpf-next] bpf: add optional memory accounting for maps
From: Y Song @ 2019-01-31 7:15 UTC (permalink / raw)
To: Martynas Pumputis; +Cc: netdev, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20190130140251.23784-1-m@lambda.lt>
On Wed, Jan 30, 2019 at 6:05 AM Martynas Pumputis <m@lambda.lt> wrote:
>
> Previously, memory allocated for a map was not accounted. Therefore,
> this memory could not be taken into consideration by the cgroups
> memory controller.
>
> This patch introduces the "BPF_F_ACCOUNT_MEM" flag which enables
> the memory accounting for a map, and it can be set during
> the map creation ("BPF_MAP_CREATE") in "map_flags".
>
> When enabled, we account only that amount of memory which is charged
> against the "RLIMIT_MEMLOCK" limit.
>
> To validate the change, first we create the memory cgroup "test-map":
>
> # mkdir /sys/fs/cgroup/memory/test-map
>
> And then we run the following program against the cgroup:
>
> $ cat test_map.c
> <..>
> int main() {
> usleep(3 * 1000000);
> assert(bpf_create_map(BPF_MAP_TYPE_HASH, 8, 16, 65536, 0) > 0);
> usleep(3 * 1000000);
> }
> # cgexec -g memory:test-map ./test_map &
> # cat /sys/fs/cgroup/memory/test-map/memory{,.kmem}.usage_in_bytes
> 397312
> 258048
>
> <after 3 sec the map has been created>
>
> # bpftool map list
> 19: hash flags 0x0
> key 8B value 16B max_entries 65536 memlock 5771264B
> # cat /sys/fs/cgroup/memory/test-map/memory{,.kmem}.usage_in_bytes
> 401408
> 262144
>
> As we can see, the memory allocated for map is not accounted, as
> 397312B + 5771264B > 401408B.
>
> Next, we enabled the accounting and re-run the test:
>
> $ cat test_map.c
> <..>
> int main() {
> usleep(3 * 1000000);
> assert(bpf_create_map(BPF_MAP_TYPE_HASH, 8, 16, 65536, BPF_F_ACCOUNT_MEM) > 0);
> usleep(3 * 1000000);
> }
> # cgexec -g memory:test-map ./test_map &
> # cat /sys/fs/cgroup/memory/test-map/memory{,.kmem}.usage_in_bytes
> 450560
> 307200
>
> <after 3 sec the map has been created>
>
> # bpftool map list
> 20: hash flags 0x80
> key 8B value 16B max_entries 65536 memlock 5771264B
> # cat /sys/fs/cgroup/memory/test-map/memory{,.kmem}.usage_in_bytes
> 6221824
> 6078464
>
> This time, the memory (including kmem) is accounted, as
> 450560B + 5771264B <= 6221824B
The above test result is for cgroup v1.
I also tested the patch for cgroup v2. It works fine too.
>
> Signed-off-by: Martynas Pumputis <m@lambda.lt>
> ---
> include/linux/bpf.h | 5 +++--
> include/uapi/linux/bpf.h | 2 ++
> kernel/bpf/arraymap.c | 14 +++++++++-----
> kernel/bpf/bpf_lru_list.c | 11 +++++++++--
> kernel/bpf/bpf_lru_list.h | 1 +
> kernel/bpf/cpumap.c | 12 +++++++++---
> kernel/bpf/devmap.c | 10 ++++++++--
> kernel/bpf/hashtab.c | 19 ++++++++++++++-----
> kernel/bpf/lpm_trie.c | 19 +++++++++++++------
> kernel/bpf/queue_stack_maps.c | 5 +++--
> kernel/bpf/reuseport_array.c | 3 ++-
> kernel/bpf/stackmap.c | 12 ++++++++----
> kernel/bpf/syscall.c | 12 ++++++++----
> kernel/bpf/xskmap.c | 9 +++++++--
> net/core/sock_map.c | 13 +++++++++----
> tools/include/uapi/linux/bpf.h | 3 +++
> 16 files changed, 108 insertions(+), 42 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index e734f163bd0b..353a3f4304fe 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -79,7 +79,8 @@ struct bpf_map {
> u32 btf_value_type_id;
> struct btf *btf;
> bool unpriv_array;
> - /* 55 bytes hole */
> + bool account_mem;
> + /* 54 bytes hole */
>
> /* The 3rd and 4th cacheline with misc members to avoid false sharing
> * particularly with refcounting.
> @@ -506,7 +507,7 @@ void bpf_map_put(struct bpf_map *map);
> int bpf_map_precharge_memlock(u32 pages);
> int bpf_map_charge_memlock(struct bpf_map *map, u32 pages);
> void bpf_map_uncharge_memlock(struct bpf_map *map, u32 pages);
> -void *bpf_map_area_alloc(size_t size, int numa_node);
> +void *bpf_map_area_alloc(size_t size, int numa_node, bool account_mem);
> void bpf_map_area_free(void *base);
> void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr);
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 91c43884f295..a374ccbaa51b 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -278,6 +278,8 @@ enum bpf_attach_type {
> #define BPF_F_NO_COMMON_LRU (1U << 1)
> /* Specify numa node during map creation */
> #define BPF_F_NUMA_NODE (1U << 2)
> +/* Enable memory accounting for map */
> +#define BPF_F_ACCOUNT_MEM (1U << 7)
Could you put the above definition after BPF_F_ZERO_SEED (1U << 6)?
The same for tools.
With this change, you can add my ack:
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH btf 0/3] Add BTF types deduplication algorithm
From: Yonghong Song @ 2019-01-31 7:18 UTC (permalink / raw)
To: Andrii Nakryiko, netdev@vger.kernel.org, Alexei Starovoitov,
Martin Lau, acme@kernel.org, daniel@iogearbox.net, Kernel Team,
Edward Cree
In-Reply-To: <20190131065837.3380288-1-andriin@fb.com>
add Edward.
On 1/30/19 10:58 PM, Andrii Nakryiko wrote:
> This patch series adds BTF deduplication algorithm to libbpf. This algorithm
> allows to take BTF type information containing duplicate per-compilation unit
> information and reduce it to equivalent set of BTF types with no duplication without
> loss of information. It also deduplicates strings and removes those strings that
> are not referenced from any BTF type (and line information in .BTF.ext section,
> if any).
>
> Algorithm also resolves struct/union forward declarations into concrete BTF types
> across multiple compilation units to facilitate better deduplication ratio. If
> undesired, this resolution can be disabled through specifying corresponding options.
>
> When applied to BTF data emitted by pahole's DWARF->BTF converter, it reduces
> the overall size of .BTF section by about 65x, from about 112MB to 1.75MB, leaving
> only 29247 out of initial 3073497 BTF type descriptors.
>
> Algorithm with minor differences and preliminary results before FUNC/FUNC_PROTO
> support is also described more verbosely at:
> https://facebookmicrosites.github.io/bpf/blog/2018/11/14/btf-enhancement.html
>
> Andrii Nakryiko (3):
> btf: extract BTF type size calculation
> btf: add BTF types deduplication algorithm
> selftests/btf: add initial BTF dedup tests
>
> tools/lib/bpf/btf.c | 1851 +++++++++++++++++++++++-
> tools/lib/bpf/btf.h | 11 +
> tools/lib/bpf/libbpf.map | 3 +
> tools/testing/selftests/bpf/test_btf.c | 535 ++++++-
> 4 files changed, 2333 insertions(+), 67 deletions(-)
>
^ permalink raw reply
* [PATCH net] l2tp: copy 4 more bytes to linear part if necessary
From: Jacob Wen @ 2019-01-31 7:18 UTC (permalink / raw)
To: netdev; +Cc: gnault
The size of L2TPv2 header with all optional fields is 14 bytes.
l2tp_udp_recv_core only moves 10 bytes to the linear part of a
skb. This may lead to l2tp_recv_common read data outside of a skb.
This patch make sure that there is at least 14 bytes in the linear
part of a skb to meet the maximum need of l2tp_udp_recv_core and
l2tp_recv_common. The minimum size of both PPP HDLC-like frame and
Ethernet frame is larger than 14 bytes, so we are safe to do so.
Also remove L2TP_HDR_SIZE_NOSEQ, it is unused now.
Fixes: fd558d186df2 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts")
Suggested-by: Guillaume Nault <gnault@redhat.com>
Signed-off-by: Jacob Wen <jian.w.wen@oracle.com>
---
net/l2tp/l2tp_core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index dd5ba0c11ab3..fed6becc5daf 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -83,8 +83,7 @@
#define L2TP_SLFLAG_S 0x40000000
#define L2TP_SL_SEQ_MASK 0x00ffffff
-#define L2TP_HDR_SIZE_SEQ 10
-#define L2TP_HDR_SIZE_NOSEQ 6
+#define L2TP_HDR_SIZE_MAX 14
/* Default trace flags */
#define L2TP_DEFAULT_DEBUG_FLAGS 0
@@ -808,7 +807,7 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
__skb_pull(skb, sizeof(struct udphdr));
/* Short packet? */
- if (!pskb_may_pull(skb, L2TP_HDR_SIZE_SEQ)) {
+ if (!pskb_may_pull(skb, L2TP_HDR_SIZE_MAX)) {
l2tp_info(tunnel, L2TP_MSG_DATA,
"%s: recv short packet (len=%d)\n",
tunnel->name, skb->len);
--
2.17.1
^ permalink raw reply related
* Re: r8169 Driver - Poor Network Performance Since Kernel 4.19
From: David Chang @ 2019-01-31 7:23 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Peter Ceiley, Realtek linux nic maintainers, netdev
In-Reply-To: <4d832c16-8830-b746-a818-6026c2e6725c@gmail.com>
Hi Heiner,
On Jan 31, 2019 at 07:35:30 +0100, Heiner Kallweit wrote:
> Hi David, two more things:
>
> 1. Could you please test a recent linux-next kernel?
> 2. Please get a register dump (ethtool -d <if>) from 4.18 and 4.19
> and compare them.
I'm sorry that I do not have the issue machine handy. I would ask
our user to do the test. Thanks!
Regards,
David
>
> Heiner
>
>
> On 31.01.2019 07:21, Heiner Kallweit wrote:
> > David, thanks for the link to the bug ticket.
> > I think only a proper bisect can help to find the offending commit.
> >
> > Heiner
> >
> >
> > On 31.01.2019 03:32, David Chang wrote:
> >> Hi,
> >>
> >> We had a similr case here.
> >> - Realtek r8169 receive performance regression in kernel 4.19
> >> https://bugzilla.suse.com/show_bug.cgi?id=1119649
> >>
> >> kernel: r8169 0000:01:00.0 eth0: RTL8168h/8111h, XID 54100880
> >> The major symptom is there are many rx_missed count.
> >>
> >>
> >> On Jan 30, 2019 at 20:15:45 +0100, Heiner Kallweit wrote:
> >>> Hi Peter,
> >>>
> >>> recently I had somebody where pcie_aspm=off for whatever reason didn't
> >>> do the trick, can you also check with pcie_aspm.policy=performance.
> >>
> >> We will give it a try later.
> >>
> >>> And please check with "ethtool -S <if>" whether the chip statistics
> >>> show a significant number of errors.
> >>>
> >>> If this doesn't help you may have to bisect to find the offending commit.
> >>
> >> We had tried fallback driver to a few previous commits as following,
> >> but with no luck.
> >>
> >> 9675931e6b65 r8169: re-enable MSI-X on RTL8168g (v4.19)
> >> 098b01ad9837 r8169: don't include asm headers directly (v4.19-rc1)
> >> a2965f12fde6 r8169: remove rtl8169_set_speed_xmii (v4.19-rc1)
> >> 6fcf9b1d4d6c r8169: fix runtime suspend (v4.19-rc1)
> >> e397286b8e89 r8169: remove TBI 1000BaseX support (v4.19-rc1)
> >>
> >> Thanks,
> >> David Chang
> >>
> >>>
> >>> Heiner
> >>>
> >>>
> >>> On 30.01.2019 10:59, Peter Ceiley wrote:
> >>>> Hi Heiner,
> >>>>
> >>>> I tried disabling the ASPM using the pcie_aspm=off kernel parameter
> >>>> and this made no difference.
> >>>>
> >>>> I tried compiling the 4.18.16 r8169.c with the 4.19.18 source and
> >>>> subsequently loaded the module in the running 4.19.18 kernel. I can
> >>>> confirm that this immediately resolved the issue and access to the NFS
> >>>> shares operated as expected.
> >>>>
> >>>> I presume this means it is an issue with the r8169 driver included in
> >>>> 4.19 onwards?
> >>>>
> >>>> To answer your last questions:
> >>>>
> >>>> Base Board Information
> >>>> Manufacturer: Alienware
> >>>> Product Name: 0PGRP5
> >>>> Version: A02
> >>>>
> >>>> ... and yes, the RTL8168 is the onboard network chip.
> >>>>
> >>>> Regards,
> >>>>
> >>>> Peter.
> >>>>
> >>>> On Tue, 29 Jan 2019 at 17:44, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>>>>
> >>>>> Hi Peter,
> >>>>>
> >>>>> I think the vendor driver doesn't enable ASPM per default.
> >>>>> So it's worth a try to disable ASPM in the BIOS or via sysfs.
> >>>>> Few older systems seem to have issues with ASPM, what kind of
> >>>>> system / mainboard are you using? The RTL8168 is the onboard
> >>>>> network chip?
> >>>>>
> >>>>> Rgds, Heiner
> >>>>>
> >>>>>
> >>>>> On 29.01.2019 07:20, Peter Ceiley wrote:
> >>>>>> Hi Heiner,
> >>>>>>
> >>>>>> Thanks, I'll do some more testing. It might not be the driver - I
> >>>>>> assumed it was due to the fact that using the r8168 driver 'resolves'
> >>>>>> the issue. I'll see if I can test the r8169.c on top of 4.19 - this is
> >>>>>> a good idea.
> >>>>>>
> >>>>>> Cheers,
> >>>>>>
> >>>>>> Peter.
> >>>>>>
> >>>>>> On Tue, 29 Jan 2019 at 17:16, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>>>>>>
> >>>>>>> Hi Peter,
> >>>>>>>
> >>>>>>> at a first glance it doesn't look like a typical driver issue.
> >>>>>>> What you could do:
> >>>>>>>
> >>>>>>> - Test the r8169.c from 4.18 on top of 4.19.
> >>>>>>>
> >>>>>>> - Check whether disabling ASPM (/sys/module/pcie_aspm) has an effect.
> >>>>>>>
> >>>>>>> - Bisect between 4.18 and 4.19 to find the offending commit.
> >>>>>>>
> >>>>>>> Any specific reason why you think root cause is in the driver and not
> >>>>>>> elsewhere in the network subsystem?
> >>>>>>>
> >>>>>>> Heiner
> >>>>>>>
> >>>>>>>
> >>>>>>> On 28.01.2019 23:10, Peter Ceiley wrote:
> >>>>>>>> Hi Heiner,
> >>>>>>>>
> >>>>>>>> Thanks for getting back to me.
> >>>>>>>>
> >>>>>>>> No, I don't use jumbo packets.
> >>>>>>>>
> >>>>>>>> Bandwidth is *generally* good, and iperf results to my NAS provide
> >>>>>>>> over 900 Mbits/s in both circumstances. The issue seems to appear when
> >>>>>>>> establishing a connection and is most notable, for example, on my
> >>>>>>>> mounted NFS shares where it takes seconds (up to 10's of seconds on
> >>>>>>>> larger directories) to list the contents of each directory. Once a
> >>>>>>>> transfer begins on a file, I appear to get good bandwidth.
> >>>>>>>>
> >>>>>>>> I'm unsure of the best scientific data to provide you in order to
> >>>>>>>> troubleshoot this issue. Running the following
> >>>>>>>>
> >>>>>>>> netstat -s |grep retransmitted
> >>>>>>>>
> >>>>>>>> shows a steady increase in retransmitted segments each time I list the
> >>>>>>>> contents of a remote directory, for example, running 'ls' on a
> >>>>>>>> directory containing 345 media files did the following using kernel
> >>>>>>>> 4.19.18:
> >>>>>>>>
> >>>>>>>> increased retransmitted segments by 21 and the 'time' command showed
> >>>>>>>> the following:
> >>>>>>>> real 0m19.867s
> >>>>>>>> user 0m0.012s
> >>>>>>>> sys 0m0.036s
> >>>>>>>>
> >>>>>>>> The same command shows no retransmitted segments running kernel
> >>>>>>>> 4.18.16 and 'time' showed:
> >>>>>>>> real 0m0.300s
> >>>>>>>> user 0m0.004s
> >>>>>>>> sys 0m0.007s
> >>>>>>>>
> >>>>>>>> ifconfig does not show any RX/TX errors nor dropped packets in either case.
> >>>>>>>>
> >>>>>>>> dmesg XID:
> >>>>>>>> [ 2.979984] r8169 0000:03:00.0 eth0: RTL8168g/8111g,
> >>>>>>>> f8:b1:56:fe:67:e0, XID 4c000800, IRQ 32
> >>>>>>>>
> >>>>>>>> # lspci -vv
> >>>>>>>> 03:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
> >>>>>>>> RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 0c)
> >>>>>>>> Subsystem: Dell RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
> >>>>>>>> Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
> >>>>>>>> ParErr- Stepping- SERR- FastB2B- DisINTx+
> >>>>>>>> Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
> >>>>>>>> <TAbort- <MAbort- >SERR- <PERR- INTx-
> >>>>>>>> Latency: 0, Cache Line Size: 64 bytes
> >>>>>>>> Interrupt: pin A routed to IRQ 19
> >>>>>>>> Region 0: I/O ports at d000 [size=256]
> >>>>>>>> Region 2: Memory at f7b00000 (64-bit, non-prefetchable) [size=4K]
> >>>>>>>> Region 4: Memory at f2100000 (64-bit, prefetchable) [size=16K]
> >>>>>>>> Capabilities: [40] Power Management version 3
> >>>>>>>> Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA
> >>>>>>>> PME(D0+,D1+,D2+,D3hot+,D3cold+)
> >>>>>>>> Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
> >>>>>>>> Capabilities: [50] MSI: Enable- Count=1/1 Maskable- 64bit+
> >>>>>>>> Address: 0000000000000000 Data: 0000
> >>>>>>>> Capabilities: [70] Express (v2) Endpoint, MSI 01
> >>>>>>>> DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
> >>>>>>>> <512ns, L1 <64us
> >>>>>>>> ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
> >>>>>>>> SlotPowerLimit 10.000W
> >>>>>>>> DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq-
> >>>>>>>> RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
> >>>>>>>> MaxPayload 128 bytes, MaxReadReq 4096 bytes
> >>>>>>>> DevSta: CorrErr+ NonFatalErr- FatalErr- UnsupReq- AuxPwr+ TransPend-
> >>>>>>>> LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit
> >>>>>>>> Latency L0s unlimited, L1 <64us
> >>>>>>>> ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp+
> >>>>>>>> LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk+
> >>>>>>>> ExtSynch- ClockPM+ AutWidDis- BWInt- AutBWInt-
> >>>>>>>> LnkSta: Speed 2.5GT/s (ok), Width x1 (ok)
> >>>>>>>> TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
> >>>>>>>> DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR+,
> >>>>>>>> OBFF Via message/WAKE#
> >>>>>>>> AtomicOpsCap: 32bit- 64bit- 128bitCAS-
> >>>>>>>> DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR+,
> >>>>>>>> OBFF Disabled
> >>>>>>>> AtomicOpsCtl: ReqEn-
> >>>>>>>> LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-
> >>>>>>>> Transmit Margin: Normal Operating Range,
> >>>>>>>> EnterModifiedCompliance- ComplianceSOS-
> >>>>>>>> Compliance De-emphasis: -6dB
> >>>>>>>> LnkSta2: Current De-emphasis Level: -6dB,
> >>>>>>>> EqualizationComplete-, EqualizationPhase1-
> >>>>>>>> EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
> >>>>>>>> Capabilities: [b0] MSI-X: Enable+ Count=4 Masked-
> >>>>>>>> Vector table: BAR=4 offset=00000000
> >>>>>>>> PBA: BAR=4 offset=00000800
> >>>>>>>> Capabilities: [d0] Vital Product Data
> >>>>>>>> pcilib: sysfs_read_vpd: read failed: Input/output error
> >>>>>>>> Not readable
> >>>>>>>> Capabilities: [100 v1] Advanced Error Reporting
> >>>>>>>> UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt-
> >>>>>>>> RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
> >>>>>>>> UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt-
> >>>>>>>> RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
> >>>>>>>> UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt-
> >>>>>>>> RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
> >>>>>>>> CESta: RxErr+ BadTLP+ BadDLLP+ Rollover- Timeout+ AdvNonFatalErr-
> >>>>>>>> CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr+
> >>>>>>>> AERCap: First Error Pointer: 00, ECRCGenCap+ ECRCGenEn-
> >>>>>>>> ECRCChkCap+ ECRCChkEn-
> >>>>>>>> MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap-
> >>>>>>>> HeaderLog: 00000000 00000000 00000000 00000000
> >>>>>>>> Capabilities: [140 v1] Virtual Channel
> >>>>>>>> Caps: LPEVC=0 RefClk=100ns PATEntryBits=1
> >>>>>>>> Arb: Fixed- WRR32- WRR64- WRR128-
> >>>>>>>> Ctrl: ArbSelect=Fixed
> >>>>>>>> Status: InProgress-
> >>>>>>>> VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
> >>>>>>>> Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
> >>>>>>>> Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=01
> >>>>>>>> Status: NegoPending- InProgress-
> >>>>>>>> Capabilities: [160 v1] Device Serial Number 01-00-00-00-68-4c-e0-00
> >>>>>>>> Capabilities: [170 v1] Latency Tolerance Reporting
> >>>>>>>> Max snoop latency: 71680ns
> >>>>>>>> Max no snoop latency: 71680ns
> >>>>>>>> Kernel driver in use: r8169
> >>>>>>>> Kernel modules: r8169
> >>>>>>>>
> >>>>>>>> Please let me know if you have any other ideas in terms of testing.
> >>>>>>>>
> >>>>>>>> Thanks!
> >>>>>>>>
> >>>>>>>> Peter.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Tue, 29 Jan 2019 at 05:28, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>>>>>>>>
> >>>>>>>>> On 28.01.2019 12:13, Peter Ceiley wrote:
> >>>>>>>>>> Hi,
> >>>>>>>>>>
> >>>>>>>>>> I have been experiencing very poor network performance since Kernel
> >>>>>>>>>> 4.19 and I'm confident it's related to the r8169 driver.
> >>>>>>>>>>
> >>>>>>>>>> I have no issue with kernel versions 4.18 and prior. I am experiencing
> >>>>>>>>>> this issue in kernels 4.19 and 4.20 (currently running/testing with
> >>>>>>>>>> 4.20.4 & 4.19.18).
> >>>>>>>>>>
> >>>>>>>>>> If someone could guide me in the right direction, I'm happy to help
> >>>>>>>>>> troubleshoot this issue. Note that I have been keeping an eye on one
> >>>>>>>>>> issue related to loading of the PHY driver, however, my symptoms
> >>>>>>>>>> differ in that I still have a network connection. I have attempted to
> >>>>>>>>>> reload the driver on a running system, but this does not improve the
> >>>>>>>>>> situation.
> >>>>>>>>>>
> >>>>>>>>>> Using the proprietary r8168 driver returns my device to proper working order.
> >>>>>>>>>>
> >>>>>>>>>> lshw shows:
> >>>>>>>>>> description: Ethernet interface
> >>>>>>>>>> product: RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
> >>>>>>>>>> vendor: Realtek Semiconductor Co., Ltd.
> >>>>>>>>>> physical id: 0
> >>>>>>>>>> bus info: pci@0000:03:00.0
> >>>>>>>>>> logical name: enp3s0
> >>>>>>>>>> version: 0c
> >>>>>>>>>> serial:
> >>>>>>>>>> size: 1Gbit/s
> >>>>>>>>>> capacity: 1Gbit/s
> >>>>>>>>>> width: 64 bits
> >>>>>>>>>> clock: 33MHz
> >>>>>>>>>> capabilities: pm msi pciexpress msix vpd bus_master cap_list
> >>>>>>>>>> ethernet physical tp aui bnc mii fibre 10bt 10bt-fd 100bt 100bt-fd
> >>>>>>>>>> 1000bt-fd autonegotiation
> >>>>>>>>>> configuration: autonegotiation=on broadcast=yes driver=r8169
> >>>>>>>>>> duplex=full firmware=rtl8168g-2_0.0.1 02/06/13 ip=192.168.1.25
> >>>>>>>>>> latency=0 link=yes multicast=yes port=MII speed=1Gbit/s
> >>>>>>>>>> resources: irq:19 ioport:d000(size=256)
> >>>>>>>>>> memory:f7b00000-f7b00fff memory:f2100000-f2103fff
> >>>>>>>>>>
> >>>>>>>>>> Kind Regards,
> >>>>>>>>>>
> >>>>>>>>>> Peter.
> >>>>>>>>>>
> >>>>>>>>> Hi Peter,
> >>>>>>>>>
> >>>>>>>>> the description "poor network performance" is quite vague, therefore:
> >>>>>>>>>
> >>>>>>>>> - Can you provide any measurements?
> >>>>>>>>> - iperf results before and after
> >>>>>>>>> - statistics about dropped packets (rx and/or tx)
> >>>>>>>>> - Do you use jumbo packets?
> >>>>>>>>>
> >>>>>>>>> Also help would be a "lspci -vv" output for the network card and
> >>>>>>>>> the dmesg output line with the chip XID.
> >>>>>>>>>
> >>>>>>>>> Heiner
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
>
>
^ permalink raw reply
* Re: [PATCH] net: esp4: Fix double free on esp4 functions
From: Steffen Klassert @ 2019-01-31 7:27 UTC (permalink / raw)
To: Ramin Farajpour Cami; +Cc: Eric Dumazet, davem, Herbert Xu, netdev
In-Reply-To: <CAKEzZ8zyhvJ1Fr6UDOxVqBnUz73xPSLwfnoM4FFGf6wHSTU=Jg@mail.gmail.com>
On Thu, Jan 31, 2019 at 06:32:07AM +0000, Ramin Farajpour Cami wrote:
> Hi Eric,
>
> I going to for avoid double free of resource identifiers we should set
> variables initialized in "tmp/key" to NULL if an error occurred int the
> "esp_init_authenc()" and "esp_output_tail()" attempts to free the memory
> again.my means like this patch c7055fd15ff46d92eb0dd1c16a4fe010d58224c8
> <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c7055fd15ff46d92eb0dd1c16a4fe010d58224c8>
There is a big difference between the above-noted patch and
your patch. You really have to understand the code you are
trying to change.
^ permalink raw reply
* Re: [PATCH] can: flexcan: fix timeout when set small bitrate
From: Marc Kleine-Budde @ 2019-01-31 7:34 UTC (permalink / raw)
To: Joakim Zhang, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, dl-linux-imx, Aisheng Dong
In-Reply-To: <20190131065619.7298-1-qiangqing.zhang@nxp.com>
[-- Attachment #1.1: Type: text/plain, Size: 2059 bytes --]
On 1/31/19 7:58 AM, Joakim Zhang wrote:
> From: Dong Aisheng <aisheng.dong@nxp.com>
>
> Current we can meet timeout issue when setting a small bitrate
> like 10000 as follows:
> root@imx6qdlsolo:~# ip link set can0 up type can bitrate 10000
> A link change request failed with some changes committed already.
> Interface can0 may have been left with an inconsistent configuration,
> please check.
> RTNETLINK answers: Connection timed out
Which SoC are you using? Which clock rate has the flexcan IP core?
> It is caused by calling of flexcan_chip_unfreeze() timeout.
>
> Originally the code is using usleep_range(10, 20) for unfreeze operation,
> but the patch (8badd65 can: flexcan: avoid calling usleep_range from
> interrupt context) changed it into udelay(10) which is only a half delay
> of before, there're also some other delay changes.
>
> After only changed unfreeze delay back to udelay(20), the issue is gone.
> So other timeout values are kept the same as 8badd65 changed.
Can you change FLEXCAN_TIMEOUT_US instead?
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
> drivers/net/can/flexcan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 2bca867bcfaa..1d3a9053bbeb 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -530,7 +530,7 @@ static int flexcan_chip_unfreeze(struct flexcan_priv *priv)
> priv->write(reg, ®s->mcr);
>
> while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK))
> - udelay(10);
> + udelay(20);
>
> if (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)
> return -ETIMEDOUT;
>
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 net-next v3 1/8] devlink: add device information API
From: Jiri Pirko @ 2019-01-31 7:40 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon
In-Reply-To: <20190130234133.4298-2-jakub.kicinski@netronome.com>
Thu, Jan 31, 2019 at 12:41:26AM CET, jakub.kicinski@netronome.com wrote:
>ethtool -i has served us well for a long time, but its showing
>its limitations more and more. The device information should
>also be reported per device not per-netdev.
>
>Lay foundation for a simple devlink-based way of reading device
>info. Add driver name and device serial number as initial pieces
>of information exposed via this new API.
>
>v3:
> - rename helpers (Jiri);
> - rename driver name attr (Jiri);
> - remove double spacing in commit message (Jiri).
>RFC v2:
> - wrap the skb into an opaque structure (Jiri);
> - allow the serial number of be any length (Jiri & Andrew);
> - add driver name (Jonathan).
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v2 01/12] net: bridge: multicast: Propagate br_mc_disabled_update() return
From: Ido Schimmel @ 2019-01-31 7:50 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, andrew@lunn.ch, vivien.didelot@gmail.com,
davem@davemloft.net, Jiri Pirko, ilias.apalodimas@linaro.org,
ivan.khoronzhuk@linaro.org, roopa@cumulusnetworks.com,
nikolay@cumulusnetworks.com, Petr Machata
In-Reply-To: <63b446c5-8104-d9f9-3924-aaa481ee3b8c@gmail.com>
On Wed, Jan 30, 2019 at 05:00:57PM -0800, Florian Fainelli wrote:
> On 1/29/19 11:36 PM, Ido Schimmel wrote:
> > On Tue, Jan 29, 2019 at 04:55:37PM -0800, Florian Fainelli wrote:
> >> -static void br_mc_disabled_update(struct net_device *dev, bool value)
> >> +static int br_mc_disabled_update(struct net_device *dev, bool value)
> >> {
> >> struct switchdev_attr attr = {
> >> .orig_dev = dev,
> >> .id = SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED,
> >> - .flags = SWITCHDEV_F_DEFER,
> >> + .flags = SWITCHDEV_F_DEFER | SWITCHDEV_F_SKIP_EOPNOTSUPP,
> >
> > Actually, since the operation is deferred I don't think the return value
> > from the driver is ever checked. Can you test it?
>
> You are right, you get a WARN() from switchdev_attr_port_set_now(), but
> this does not propagate back to the caller, so you can still create a
> bridge device and enslave a device successfully despite getting warnings
> on the console.
>
> >
> > I think it would be good to convert the attributes to use the switchdev
> > notifier like commit d17d9f5e5143 ("switchdev: Replace port obj add/del
> > SDO with a notification") did for objects. Then you can have your
> > listener veto the operation in the same context it is happening.
>
> Alright, working on it. Would you do that just for the attr_set, or for
> attr_get as well (to be symmetrical)?
Yes, then we can get rid of switchdev_ops completely.
^ permalink raw reply
* Re: [PATCH net-next v3 2/8] devlink: add version reporting to devlink info API
From: Jiri Pirko @ 2019-01-31 7:57 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon
In-Reply-To: <20190130234133.4298-3-jakub.kicinski@netronome.com>
Thu, Jan 31, 2019 at 12:41:27AM CET, jakub.kicinski@netronome.com wrote:
>ethtool -i has a few fixed-size fields which can be used to report
>firmware version and expansion ROM version. Unfortunately, modern
>hardware has more firmware components. There is usually some
>datapath microcode, management controller, PXE drivers, and a
>CPLD load. Running ethtool -i on modern controllers reveals the
>fact that vendors cram multiple values into firmware version field.
>
>Here are some examples from systems I could lay my hands on quickly:
>
>tg3: "FFV20.2.17 bc 5720-v1.39"
>i40e: "6.01 0x800034a4 1.1747.0"
>nfp: "0.0.3.5 0.25 sriov-2.1.16 nic"
>
>Add a new devlink API to allow retrieving multiple versions, and
>provide user-readable name for those versions.
>
>While at it break down the versions into three categories:
> - fixed - this is the board/fixed component version, usually vendors
> report information like the board version in the PCI VPD,
> but it will benefit from naming and common API as well;
> - running - this is the running firmware version;
> - stored - this is firmware in the flash, after firmware update
> this value will reflect the flashed version, while the
> running version may only be updated after reboot.
>
>v3:
> - add per-type helpers instead of using the special argument (Jiri).
>RFCv2:
> - remove the nesting in attr DEVLINK_ATTR_INFO_VERSIONS (now
> versions are mixed with other info attrs)l
> - have the driver report versions from the same callback as
> other info.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v3 3/8] devlink: add generic info version names
From: Jiri Pirko @ 2019-01-31 7:58 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon
In-Reply-To: <20190130234133.4298-4-jakub.kicinski@netronome.com>
Thu, Jan 31, 2019 at 12:41:28AM CET, jakub.kicinski@netronome.com wrote:
>Add defines and docs for generic info versions.
>
>v3:
> - add docs;
> - separate patch (Jiri).
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v3 4/8] nfp: devlink: report driver name and serial number
From: Jiri Pirko @ 2019-01-31 7:59 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon
In-Reply-To: <20190130234133.4298-5-jakub.kicinski@netronome.com>
Thu, Jan 31, 2019 at 12:41:29AM CET, jakub.kicinski@netronome.com wrote:
>Report the basic info through new devlink info API.
>
>RFCv2:
> - add driver name;
> - align serial to core changes.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: Latitude 5495's tg3 hangs under heavy load
From: Kai-Heng Feng @ 2019-01-31 8:08 UTC (permalink / raw)
To: Siva Reddy Kallam
Cc: Prashant Sreedharan, Michael Chan, Linux Netdev List,
Chih-Hsyuan Ho
In-Reply-To: <7E14D23E-92C4-4D9E-B891-4F08075DF382@canonical.com>
Hi tg3 folks,
> On Jan 9, 2019, at 13:42, Kai Heng Feng <kai.heng.feng@canonical.com> wrote:
>
>
>
>> On Jan 7, 2019, at 5:12 PM, Siva Reddy Kallam <siva.kallam@broadcom.com> wrote:
>>
>> On Mon, Jan 7, 2019 at 11:34 AM Kai Heng Feng
>> <kai.heng.feng@canonical.com> wrote:
>>>
>>> Hi tg3 folks,
>>>
>>> Any idea how to solve the bug?
>>>
>>> Kai-Heng
>> Hi,
>> Can you share Register dump(ethtool -d)?
>
> `ethtool -d` before freeze:
> https://pastebin.com/MSkJzhcv
>
> `ethtool -d` after freeze:
> https://pastebin.com/dQj8mLsN
>
>> Is ifconfig down/up bringing back interface?
>
> Yes. And seems like network works fine afterward.
>
> `ethtool -d` after down/up:
> https://pastebin.com/vL1gCC2n
Wondering if there’s any update?
Are these info sufficient?
Kai-Heng
>
> Kai-Heng
>
>>
>>>
>>>> On Dec 7, 2018, at 17:27, Kai Heng Feng <kai.heng.feng@canonical.com> wrote:
>>>>
>>>> Hi tg3 maintainers,
>>>>
>>>> I’ve encountered network freeze when using tg3 in gigabits net.
>>>>
>>>> The issue can be easily reproduced when using scp to transfer files in local network.
>>>>
>>>> The symptom is pretty similar to what this commit is trying to solve:
>>>> commit 3a498606bb04af603a46ebde8296040b2de350d1
>>>> Author: Sanjeev Bansal <sanjeevb.bansal@broadcom.com>
>>>> Date: Mon Jul 16 11:13:32 2018 +0530
>>>>
>>>> tg3: Add higher cpu clock for 5762.
>>>>
>>>> This patch has fix for TX timeout while running bi-directional
>>>> traffic with 100 Mbps using 5762.
>>>>
>>>> But reverting this commit doesn’t help.
>>>>
>>>> Latitude 5495 is a AMD Raven Ridge platform, not sure if this matters.
>>>>
>>>> Here’s the lspci for this device:
>>>> 03:00.0 Ethernet controller [0200]: Broadcom Inc. and subsidiaries NetXtreme BCM5762 Gigabit Ethernet PCIe [14e4:1687] (rev 10)
>>>> Subsystem: Dell NetXtreme BCM5762 Gigabit Ethernet PCIe [1028:0814]
>>>> Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
>>>> Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>>> Latency: 0
>>>> Interrupt: pin A routed to IRQ 16
>>>> Region 0: Memory at e0220000 (64-bit, prefetchable) [size=64K]
>>>> Region 2: Memory at e0210000 (64-bit, prefetchable) [size=64K]
>>>> Region 4: Memory at e0200000 (64-bit, prefetchable) [size=64K]
>>>> Capabilities: [48] Power Management version 3
>>>> Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
>>>> Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=1 PME-
>>>> Capabilities: [50] Vital Product Data
>>>> Product Name: Broadcom NetXtreme Gigabit Ethernet Controller
>>>> Read-only fields:
>>>> [PN] Part number: BCM95762
>>>> [EC] Engineering changes: 106679-15
>>>> [SN] Serial number: 0123456789
>>>> [MN] Manufacture ID: 31 34 65 34
>>>> [RV] Reserved: checksum good, 28 byte(s) reserved
>>>> Read/write fields:
>>>> [YA] Asset tag: XYZ01234567
>>>> [RW] Read-write area: 107 byte(s) free
>>>> End
>>>> Capabilities: [58] MSI: Enable- Count=1/8 Maskable- 64bit+
>>>> Address: 0000000000000000 Data: 0000
>>>> Capabilities: [a0] MSI-X: Enable+ Count=6 Masked-
>>>> Vector table: BAR=4 offset=00000000
>>>> PBA: BAR=2 offset=00000120
>>>> Capabilities: [ac] Express (v2) Endpoint, MSI 00
>>>> DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s <4us, L1 <64us
>>>> ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset+ SlotPowerLimit 0.000W
>>>> DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
>>>> RlxdOrd- ExtTag- PhantFunc- AuxPwr+ NoSnoop- FLReset-
>>>> MaxPayload 128 bytes, MaxReadReq 4096 bytes
>>>> DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
>>>> LnkCap: Port #0, Speed 5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s <2us, L1 <64us
>>>> ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp+
>>>> LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+
>>>> ExtSynch- ClockPM+ AutWidDis- BWInt- AutBWInt-
>>>> LnkSta: Speed 5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
>>>> DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR+, OBFF Via WAKE#
>>>> DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR+, OBFF Disabled
>>>> LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-
>>>> Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS-
>>>> Compliance De-emphasis: -6dB
>>>> LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-, EqualizationPhase1-
>>>> EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
>>>> Capabilities: [100 v1] Advanced Error Reporting
>>>> UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
>>>> UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
>>>> UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
>>>> CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
>>>> CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
>>>> AERCap: First Error Pointer: 00, GenCap+ CGenEn- ChkCap+ ChkEn-
>>>> Capabilities: [13c v1] Device Serial Number 00-00-a4-4c-c8-5b-65-74
>>>> Capabilities: [150 v1] Power Budgeting <?>
>>>> Capabilities: [160 v1] Virtual Channel
>>>> Caps: LPEVC=0 RefClk=100ns PATEntryBits=1
>>>> Arb: Fixed- WRR32- WRR64- WRR128-
>>>> Ctrl: ArbSelect=Fixed
>>>> Status: InProgress-
>>>> VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
>>>> Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
>>>> Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=ff
>>>> Status: NegoPending- InProgress-
>>>> Capabilities: [1b0 v1] Latency Tolerance Reporting
>>>> Max snoop latency: 1048576ns
>>>> Max no snoop latency: 1048576ns
>>>> Capabilities: [230 v1] Transaction Processing Hints
>>>> Interrupt vector mode supported
>>>> Steering table in MSI-X table
>>>> Kernel driver in use: tg3
>>>> Kernel modules: tg3
>>>>
>>>> Please let me know if you need more information.
>>>>
>>>> Kai-Heng
>>>
>
^ permalink raw reply
* Re: [PATCH net-next v3 5/8] nfp: devlink: report fixed versions
From: Jiri Pirko @ 2019-01-31 8:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon
In-Reply-To: <20190130234133.4298-6-jakub.kicinski@netronome.com>
Thu, Jan 31, 2019 at 12:41:30AM CET, jakub.kicinski@netronome.com wrote:
>Report information about the hardware.
>
>RFCv2:
> - add defines for board IDs which are likely to be reusable for
> other drivers (Jiri).
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v3 7/8] nfp: devlink: report the running and flashed versions
From: Jiri Pirko @ 2019-01-31 8:03 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon
In-Reply-To: <20190130234133.4298-8-jakub.kicinski@netronome.com>
Thu, Jan 31, 2019 at 12:41:32AM CET, jakub.kicinski@netronome.com wrote:
>Report versions of firmware components using the new NSP command.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net] l2tp: copy 4 more bytes to linear part if necessary
From: Guillaume Nault @ 2019-01-31 8:43 UTC (permalink / raw)
To: Jacob Wen; +Cc: netdev
In-Reply-To: <20190131071856.22120-1-jian.w.wen@oracle.com>
On Thu, Jan 31, 2019 at 03:18:56PM +0800, Jacob Wen wrote:
> The size of L2TPv2 header with all optional fields is 14 bytes.
> l2tp_udp_recv_core only moves 10 bytes to the linear part of a
> skb. This may lead to l2tp_recv_common read data outside of a skb.
>
Looks good, thanks for taking care of this!
Acked-by: Guillaume Nault <gnault@redhat.com>
^ permalink raw reply
* Re: [PATCH net-next v8 0/8] devlink: Add configuration parameters support for devlink_port
From: Vasundhara Volam @ 2019-01-31 8:44 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, michael.chan@broadcom.com, Jiri Pirko,
Michal Kubecek, Netdev
In-Reply-To: <20190130155758.3a6be144@cakuba.hsd1.ca.comcast.net>
On Thu, Jan 31, 2019 at 5:28 AM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Mon, 28 Jan 2019 18:00:19 +0530, Vasundhara Volam wrote:
> > This patchset adds support for configuration parameters setting through
> > devlink_port. Each device registers supported configuration parameters
> > table.
> >
> > The user can retrieve data on these parameters by
> > "devlink port param show" command and can set new value to a
> > parameter by "devlink port param set" command.
> > All configuration modes supported by devlink_dev are supported
> > by devlink_port also.
>
> Hm, I think we were kind of going somewhere with the ethtool/nl
> attribute encapsulation idea. You seem to have ignored those comments
> on v7 and reposted v8 a day after.
Jakub, I have added the idea of future expansion of WOL in my v8 cover letter
mentioning the same. I will work on this as a future patchset.
>
> I think we should explore the nesting further. The only obstacle is
> that ethtool netlink conversion is not yet finished, but that's just
> a simple matter of programming. Do you disagree with that direction?
> Please comment.
No, I agree with you about ethtool netlink encapsulation.
^ permalink raw reply
* RE: [PATCH] can: flexcan: fix timeout when set small bitrate
From: Joakim Zhang @ 2019-01-31 8:48 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, dl-linux-imx, Aisheng Dong
In-Reply-To: <1902c81b-86fa-469e-b495-c37dca744cb9@pengutronix.de>
Hi Marc,
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2019年1月31日 15:34
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org
> Cc: wg@grandegger.com; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Aisheng
> Dong <aisheng.dong@nxp.com>
> Subject: Re: [PATCH] can: flexcan: fix timeout when set small bitrate
>
> On 1/31/19 7:58 AM, Joakim Zhang wrote:
> > From: Dong Aisheng <aisheng.dong@nxp.com>
> >
> > Current we can meet timeout issue when setting a small bitrate like
> > 10000 as follows:
> > root@imx6qdlsolo:~# ip link set can0 up type can bitrate 10000 A link
> > change request failed with some changes committed already.
> > Interface can0 may have been left with an inconsistent configuration,
> > please check.
> > RTNETLINK answers: Connection timed out
>
> Which SoC are you using? Which clock rate has the flexcan IP core?
We tested on i.MX6 series boards and all met this issue. And ipg clock rate is 66MHZ, per clock rate is 30MHZ.
> > It is caused by calling of flexcan_chip_unfreeze() timeout.
> >
> > Originally the code is using usleep_range(10, 20) for unfreeze
> > operation, but the patch (8badd65 can: flexcan: avoid calling
> > usleep_range from interrupt context) changed it into udelay(10) which
> > is only a half delay of before, there're also some other delay changes.
> >
> > After only changed unfreeze delay back to udelay(20), the issue is gone.
> > So other timeout values are kept the same as 8badd65 changed.
>
> Can you change FLEXCAN_TIMEOUT_US instead?
Of course, we can change FLEXCAN_TIMEOUT_US to 100, but this will extend the time of enable/disable/softreset.
Which method do you think is better?
Best Regards,
Joakim Zhang
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> > drivers/net/can/flexcan.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 2bca867bcfaa..1d3a9053bbeb 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -530,7 +530,7 @@ static int flexcan_chip_unfreeze(struct flexcan_priv
> *priv)
> > priv->write(reg, ®s->mcr);
> >
> > while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK))
> > - udelay(10);
> > + udelay(20);
> >
> > if (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)
> > return -ETIMEDOUT;
> >
>
> 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 |
^ permalink raw reply
* Re: [PATCH v5 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Peter Zijlstra @ 2019-01-31 8:49 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, davem, daniel, jannh, paulmck, will.deacon,
mingo, netdev, kernel-team
In-Reply-To: <20190130213418.gxbyfbmuiohn7vj4@ast-mbp.dhcp.thefacebook.com>
On Wed, Jan 30, 2019 at 01:34:19PM -0800, Alexei Starovoitov wrote:
> On Wed, Jan 30, 2019 at 10:05:29PM +0100, Peter Zijlstra wrote:
> >
> > Would something like the below work for you instead?
> >
> > I find it easier to read, and the additional CONFIG symbol would give
> > architectures (say ARM) an easy way to force the issue.
> >
> >
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -221,6 +221,72 @@ const struct bpf_func_proto bpf_get_curr
> > .arg2_type = ARG_CONST_SIZE,
> > };
> >
> > +#if defined(CONFIG_QUEUED_SPINLOCKS) || defined(CONFIG_BPF_ARCH_SPINLOCK)
> > +
> > +static inline void __bpf_spin_lock(struct bpf_spin_lock *lock)
> > +{
> > + arch_spinlock_t *l = (void *)lock;
> > + BUILD_BUG_ON(sizeof(*l) != sizeof(__u32));
> > + if (1) {
> > + union {
> > + __u32 val;
> > + arch_spinlock_t lock;
> > + } u = { .lock = __ARCH_SPIN_LOCK_UNLOCKED };
> > + compiletime_assert(u.val == 0, "__ARCH_SPIN_LOCK_UNLOCKED not 0");
> > + }
> > + arch_spin_lock(l);
>
> And archs can select CONFIG_BPF_ARCH_SPINLOCK when they don't
> use qspinlock and their arch_spinlock_t is compatible ?
> Nice. I like the idea!
Exactly, took me a little while to come up with that test for
__ARCH_SPIN_LOCK_UNLOCKED, but it now checks for both assumptions, so no
surprises when people get it wrong by accident.
> > +}
> > +
> > +static inline void __bpf_spin_unlock(struct bpf_spin_lock *lock)
> > +{
> > + arch_spinlock_t *l = (void *)lock;
> > + arch_spin_unlock(l);
> > +}
> > +
> > +#else
> > +
> > +static inline void __bpf_spin_lock(struct bpf_spin_lock *lock)
> > +{
> > + atomic_t *l = (void *)lock;
> > + do {
> > + atomic_cond_read_relaxed(l, !VAL);
>
> wow. that's quite a macro magic.
Yeah, C sucks for not having lambdas, this was the best we could come up
with.
This basically allows architectures to optimize the
wait-for-variable-to-change thing. Currently only ARM64 does that, I
have a horrible horrible patch that makes x86 use MONITOR/MWAIT for
this, and I suppose POWER should use it but doesn't.
> Should it be
> atomic_cond_read_relaxed(l, (!VAL));
> like qspinlock.c does ?
Extra parens doesn't hurt of course, but I don't think it's strictly
needed, the atomic_cond_read_*() wrappers already add extra parent
before passing it on to smp_cond_load_*().
^ permalink raw reply
* Re: general protection fault in __xfrm_policy_bysel_ctx
From: Dmitry Vyukov @ 2019-01-31 8:49 UTC (permalink / raw)
To: Florian Westphal
Cc: syzbot, David Miller, Herbert Xu, LKML, netdev, Steffen Klassert,
syzkaller-bugs
In-Reply-To: <CACT4Y+ZWafAqKCt0ubJREcuHx4EiKhNKL0X-et_Y9YZBr8adbg@mail.gmail.com>
On Wed, Jan 30, 2019 at 3:30 PM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Wed, Jan 30, 2019 at 3:20 PM Florian Westphal <fw@strlen.de> wrote:
> >
> > Dmitry Vyukov <dvyukov@google.com> wrote:
> > > > syzbot <syzbot+e6e1fe9148cffa18cf97@syzkaller.appspotmail.com> wrote:
> > > > > Hello,
> > > > >
> > > > > syzbot found the following crash on:
> > > > >
> > > > > HEAD commit: 085c4c7dd2b6 net: lmc: remove -I. header search path
> > > > > git tree: net-next
> > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=12347128c00000
> > > > > kernel config: https://syzkaller.appspot.com/x/.config?x=505743eba4e4f68
> > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=e6e1fe9148cffa18cf97
> > > > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > > > >
> > > > > Unfortunately, I don't have any reproducer for this crash yet.
> > > >
> > > > net-next doesn't contain the fixes for the rbtree fallout yet, so
> > > > this might already be fixed (fingers crossed).
> > >
> > > Hi Florian,
> > >
> > > What is that fix for the record?
> >
> > I don't know. I managed to add every bug class imagineable in that series 8-(
> >
> > The last (most recent) fix from the 'fallout cleanup' is:
> > 12750abad517a991c4568969bc748db302ab52cd
> > ("xfrm: policy: fix infinite loop when merging src-nodes")
> >
> > so if syzkaller can generate a splat with that change present
> > something is still broken.
> >
> > > We will need to close this later. Or perhaps we can already mark this
> > > as fixed by that patch with "#syz fix:" command?
> >
> > There are a lot of open xfrm related splats that could all be explained
> > by the rbtree bugs (one had a reproducer, the fix has appropriate
> > reported-by tag).
> >
> > It would be great if there was a way to tell syzkaller to report those
> > again if they still appear.
>
> That's exactly what "#syz fix:" will do.
> syzbot will wait until the fixing commit appears in all builds/trees
> it tests, then close this bug, and then any new similarly looking
> crash will produce a new bug report. So if the patch indeed fixes the
> bug, then the bug will be closed and we are done. If it does not fix
> this bug, then we will get another report but at that time on a tree
> that includes the commit.
>
> > I could pretend and claim above commit as "sys-fix", but it seems fishy.
> >
> > Let me know and I can tag all of them.
>
> It's "safe" to mark these crashes as fixed when we are not 100% sure
> in the sense that we won't lose the bug (it will be reported again
> later if it's not fixed).
> It's also useful to keep the list of open/active bugs shorter and more
> precise (don't leave too many obsoleted open bugs). What happened
> multiple times is that a bug was fixed but left open, and then a
> similarly looking crashes started happening again (a new bug), but it
> wasn't reported by syzbot because for syzbot it looked like the old
> still unfixed bug.
Thanks for cleaning it all up!
(Florian updated 17 other bugs)
^ permalink raw reply
* Re: [PATCH v5 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Peter Zijlstra @ 2019-01-31 8:51 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, davem, daniel, jannh, paulmck, will.deacon,
mingo, netdev, kernel-team, Nicholas Piggin
In-Reply-To: <20190131084952.GG2296@hirez.programming.kicks-ass.net>
On Thu, Jan 31, 2019 at 09:49:52AM +0100, Peter Zijlstra wrote:
> On Wed, Jan 30, 2019 at 01:34:19PM -0800, Alexei Starovoitov wrote:
> > On Wed, Jan 30, 2019 at 10:05:29PM +0100, Peter Zijlstra wrote:
> > > +static inline void __bpf_spin_lock(struct bpf_spin_lock *lock)
> > > +{
> > > + atomic_t *l = (void *)lock;
> > > + do {
> > > + atomic_cond_read_relaxed(l, !VAL);
> >
> > wow. that's quite a macro magic.
>
> Yeah, C sucks for not having lambdas, this was the best we could come up
> with.
>
> This basically allows architectures to optimize the
> wait-for-variable-to-change thing. Currently only ARM64 does that, I
> have a horrible horrible patch that makes x86 use MONITOR/MWAIT for
> this, and I suppose POWER should use it but doesn't.
Nick, do you guys want something like this?
---
diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index fbe8df433019..111984c5670d 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -99,6 +99,20 @@ do { \
#define barrier_nospec()
#endif /* CONFIG_PPC_BARRIER_NOSPEC */
+#define smp_cond_load_relaxed(ptr, cond_expr) ({ \
+ typeof(ptr) __PTR = (ptr); \
+ typeof(*ptr) VAL = READ_ONCE(*__PTR); \
+ if (unlikely(!(cond_expr))) { \
+ spin_begin(); \
+ do { \
+ spin_cpu_relax(); \
+ VAL = READ_ONCE(*__PTR); \
+ } while (!(cond_expr)); \
+ spin_end(); \
+ } \
+ VAL; \
+})
+
#include <asm-generic/barrier.h>
#endif /* _ASM_POWERPC_BARRIER_H */
^ permalink raw reply related
* Re: [PATCH] bpf: selftests: handle sparse CPU allocations
From: Martynas @ 2019-01-31 8:57 UTC (permalink / raw)
To: Y Song; +Cc: netdev, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <CAH3MdRUKvPpxmOL_BGjsKeiWTEWCgKNiDKGsXsVzDLbY5F+Dzw@mail.gmail.com>
Oops, thanks for noticing the issue with strlen in the for loop. Changing to "int len = strlen(buff); for (i = 0; i < len; i++) <..>" should fix the issue. I'm going to re-submit the patch.
Also, I put some testing results: https://gist.github.com/brb/5369b5cfd08babb80cf2c4081dc19762
On Thu, Jan 31, 2019, at 7:25 AM, Y Song wrote:
> [ My reply somehow rejected by netdev, this is to send it again. ]
>
> On Wed, Jan 30, 2019 at 1:19 AM Martynas Pumputis <m@lambda.lt> wrote:
> >
> > Previously, bpf_num_possible_cpus() had a bug when calculating a
> > number of possible CPUs in the case of sparse CPU allocations, as
> > it was considering only the first range or element of
> > /sys/devices/system/cpu/possible.
> >
> > E.g. in the case of "0,2-3" (CPU 1 is not available), the function
> > returned 1 instead of 3.
> >
> > This patch fixes the function by making it parse all CPU ranges and
> > elements.
> >
> > Signed-off-by: Martynas Pumputis <m@lambda.lt>
> > ---
> > tools/testing/selftests/bpf/bpf_util.h | 29 +++++++++++++++++---------
> > 1 file changed, 19 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_util.h b/tools/testing/selftests/bpf/bpf_util.h
> > index 315a44fa32af..8cab50408204 100644
> > --- a/tools/testing/selftests/bpf/bpf_util.h
> > +++ b/tools/testing/selftests/bpf/bpf_util.h
> > @@ -13,7 +13,7 @@ static inline unsigned int bpf_num_possible_cpus(void)
> > unsigned int start, end, possible_cpus = 0;
> > char buff[128];
> > FILE *fp;
> > - int n;
> > + int n, i, j = 0;
> >
> > fp = fopen(fcpu, "r");
> > if (!fp) {
> > @@ -21,17 +21,26 @@ static inline unsigned int bpf_num_possible_cpus(void)
> > exit(1);
> > }
> >
> > - while (fgets(buff, sizeof(buff), fp)) {
> > - n = sscanf(buff, "%u-%u", &start, &end);
> > - if (n == 0) {
> > - printf("Failed to retrieve # possible CPUs!\n");
> > - exit(1);
> > - } else if (n == 1) {
> > - end = start;
> > + if (!fgets(buff, sizeof(buff), fp)) {
> > + printf("Failed to read %s!\n", fcpu);
> > + exit(1);
> > + }
> > +
> > + for (i = 0; i <= strlen(buff); i++) {
> > + if (buff[i] == ',' || buff[i] == '\0') {
> > + buff[i] = '\0';
>
> This does not sound right. For example, the cpu list "0,2-3",
> you will change "," to '\0" so buffer becomes "0\02-3".
> The next iteration you will get strlen(buff) = 1.
> The "2-3" will be skipped.
>
> > + n = sscanf(&buff[j], "%u-%u", &start, &end);
> > + if (n <= 0) {
> > + printf("Failed to retrieve # possible CPUs!\n");
> > + exit(1);
> > + } else if (n == 1) {
> > + end = start;
> > + }
> > + possible_cpus += end - start + 1;
> > + j = i + 1;
> > }
> > - possible_cpus = start == 0 ? end + 1 : 0;
> > - break;
> > }
> > +
> > fclose(fp);
> >
> > return possible_cpus;
> > --
> > 2.20.1
> >
^ permalink raw reply
* RE: [PATCH] can: flexcan: fix timeout when set small bitrate
From: Aisheng Dong @ 2019-01-31 9:09 UTC (permalink / raw)
To: Joakim Zhang, Marc Kleine-Budde, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, dl-linux-imx
In-Reply-To: <DB7PR04MB46189FE45531AA8FD65D5327E6910@DB7PR04MB4618.eurprd04.prod.outlook.com>
> From: Joakim Zhang
> Sent: Thursday, January 31, 2019 4:49 PM
[...]
> > > Current we can meet timeout issue when setting a small bitrate like
> > > 10000 as follows:
> > > root@imx6qdlsolo:~# ip link set can0 up type can bitrate 10000 A
> > > link change request failed with some changes committed already.
> > > Interface can0 may have been left with an inconsistent
> > > configuration, please check.
> > > RTNETLINK answers: Connection timed out
> >
> > Which SoC are you using? Which clock rate has the flexcan IP core?
>
> We tested on i.MX6 series boards and all met this issue. And ipg clock rate is
> 66MHZ, per clock rate is 30MHZ.
>
> > > It is caused by calling of flexcan_chip_unfreeze() timeout.
> > >
> > > Originally the code is using usleep_range(10, 20) for unfreeze
> > > operation, but the patch (8badd65 can: flexcan: avoid calling
> > > usleep_range from interrupt context) changed it into udelay(10)
> > > which is only a half delay of before, there're also some other delay
> changes.
> > >
> > > After only changed unfreeze delay back to udelay(20), the issue is gone.
> > > So other timeout values are kept the same as 8badd65 changed.
> >
> > Can you change FLEXCAN_TIMEOUT_US instead?
>
> Of course, we can change FLEXCAN_TIMEOUT_US to 100, but this will
> extend the time of enable/disable/softreset.
> Which method do you think is better?
>
That seems not a big deal for an error case.
So change FLEXCAN_TIMEOUT_US seems like a good suggestion to me.
You can cook a patch with commit message updated. No need keep my author name
as it's a different solution.
Regards
Dong Aisheng
> Best Regards,
> Joakim Zhang
>
> > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > > ---
> > > drivers/net/can/flexcan.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > > index 2bca867bcfaa..1d3a9053bbeb 100644
> > > --- a/drivers/net/can/flexcan.c
> > > +++ b/drivers/net/can/flexcan.c
> > > @@ -530,7 +530,7 @@ static int flexcan_chip_unfreeze(struct
> > > flexcan_priv
> > *priv)
> > > priv->write(reg, ®s->mcr);
> > >
> > > while (timeout-- && (priv->read(®s->mcr) &
> FLEXCAN_MCR_FRZ_ACK))
> > > - udelay(10);
> > > + udelay(20);
> > >
> > > if (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)
> > > return -ETIMEDOUT;
> > >
> >
> > 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 |
^ permalink raw reply
* Re: [PATCH] can: flexcan: fix timeout when set small bitrate
From: Marc Kleine-Budde @ 2019-01-31 9:11 UTC (permalink / raw)
To: Joakim Zhang, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, dl-linux-imx, Aisheng Dong
In-Reply-To: <DB7PR04MB46189FE45531AA8FD65D5327E6910@DB7PR04MB4618.eurprd04.prod.outlook.com>
[-- Attachment #1.1: Type: text/plain, Size: 1801 bytes --]
On 1/31/19 9:48 AM, Joakim Zhang wrote:
>> Which SoC are you using? Which clock rate has the flexcan IP core?
>
> We tested on i.MX6 series boards and all met this issue. And ipg clock rate is 66MHZ, per clock rate is 30MHZ.
ok
>
>>> It is caused by calling of flexcan_chip_unfreeze() timeout.
>>>
>>> Originally the code is using usleep_range(10, 20) for unfreeze
>>> operation, but the patch (8badd65 can: flexcan: avoid calling
>>> usleep_range from interrupt context) changed it into udelay(10) which
>>> is only a half delay of before, there're also some other delay changes.
>>>
>>> After only changed unfreeze delay back to udelay(20), the issue is gone.
>>> So other timeout values are kept the same as 8badd65 changed.
>>
>> Can you change FLEXCAN_TIMEOUT_US instead?
>
> Of course, we can change FLEXCAN_TIMEOUT_US to 100, but this will extend the time of enable/disable/softreset.
> Which method do you think is better?
If you double to FLEXCAN_TIMEOUT_US to 100, the loops in question will
spin at maximum the double time. But the loops are left as soon as the
condition is satisfied.
It will fix your problem with the 10 kbit/s bitrate. But if there is
some kind of problem with the IP core it will still fail, it just takes
double amount of time (100 µs + overhead) until the function returns.
I don't see any harm in looping longer:
- The previous good case is unchanged.
- The error case takes double amount of time.
- Your problem is hopefully fixed.
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
* [PATCH v2 bpf] bpf: selftests: handle sparse CPU allocations
From: Martynas Pumputis @ 2019-01-31 9:19 UTC (permalink / raw)
To: netdev; +Cc: ys114321, ast, daniel, m
Previously, bpf_num_possible_cpus() had a bug when calculating a
number of possible CPUs in the case of sparse CPU allocations, as
it was considering only the first range or element of
/sys/devices/system/cpu/possible.
E.g. in the case of "0,2-3" (CPU 1 is not available), the function
returned 1 instead of 3.
This patch fixes the function by making it parse all CPU ranges and
elements.
Signed-off-by: Martynas Pumputis <m@lambda.lt>
---
Testing of the patch: https://gist.github.com/brb/5369b5cfd08babb80cf2c4081dc19762
---
tools/testing/selftests/bpf/bpf_util.h | 31 +++++++++++++++++---------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_util.h b/tools/testing/selftests/bpf/bpf_util.h
index 315a44fa32af..442c3ab2d688 100644
--- a/tools/testing/selftests/bpf/bpf_util.h
+++ b/tools/testing/selftests/bpf/bpf_util.h
@@ -9,11 +9,12 @@
static inline unsigned int bpf_num_possible_cpus(void)
{
+
static const char *fcpu = "/sys/devices/system/cpu/possible";
unsigned int start, end, possible_cpus = 0;
char buff[128];
FILE *fp;
- int n;
+ int len, n, i, j = 0;
fp = fopen(fcpu, "r");
if (!fp) {
@@ -21,17 +22,27 @@ static inline unsigned int bpf_num_possible_cpus(void)
exit(1);
}
- while (fgets(buff, sizeof(buff), fp)) {
- n = sscanf(buff, "%u-%u", &start, &end);
- if (n == 0) {
- printf("Failed to retrieve # possible CPUs!\n");
- exit(1);
- } else if (n == 1) {
- end = start;
+ if (!fgets(buff, sizeof(buff), fp)) {
+ printf("Failed to read %s!\n", fcpu);
+ exit(1);
+ }
+
+ len = strlen(buff);
+ for (i = 0; i <= len; i++) {
+ if (buff[i] == ',' || buff[i] == '\0') {
+ buff[i] = '\0';
+ n = sscanf(&buff[j], "%u-%u", &start, &end);
+ if (n <= 0) {
+ printf("Failed to retrieve # possible CPUs!\n");
+ exit(1);
+ } else if (n == 1) {
+ end = start;
+ }
+ possible_cpus += end - start + 1;
+ j = i + 1;
}
- possible_cpus = start == 0 ? end + 1 : 0;
- break;
}
+
fclose(fp);
return possible_cpus;
--
2.20.1
^ permalink raw reply related
* RE: [PATCH] can: flexcan: fix timeout when set small bitrate
From: Joakim Zhang @ 2019-01-31 9:18 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, dl-linux-imx, Aisheng Dong
In-Reply-To: <0b660fc9-0b02-0b3f-46f5-1d194a52628c@pengutronix.de>
Hi Marc,
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2019年1月31日 17:12
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org
> Cc: wg@grandegger.com; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Aisheng
> Dong <aisheng.dong@nxp.com>
> Subject: Re: [PATCH] can: flexcan: fix timeout when set small bitrate
>
> On 1/31/19 9:48 AM, Joakim Zhang wrote:
> >> Which SoC are you using? Which clock rate has the flexcan IP core?
> >
> > We tested on i.MX6 series boards and all met this issue. And ipg clock rate
> is 66MHZ, per clock rate is 30MHZ.
>
> ok
>
> >
> >>> It is caused by calling of flexcan_chip_unfreeze() timeout.
> >>>
> >>> Originally the code is using usleep_range(10, 20) for unfreeze
> >>> operation, but the patch (8badd65 can: flexcan: avoid calling
> >>> usleep_range from interrupt context) changed it into udelay(10)
> >>> which is only a half delay of before, there're also some other delay
> changes.
> >>>
> >>> After only changed unfreeze delay back to udelay(20), the issue is gone.
> >>> So other timeout values are kept the same as 8badd65 changed.
> >>
> >> Can you change FLEXCAN_TIMEOUT_US instead?
> >
> > Of course, we can change FLEXCAN_TIMEOUT_US to 100, but this will
> extend the time of enable/disable/softreset.
> > Which method do you think is better?
>
> If you double to FLEXCAN_TIMEOUT_US to 100, the loops in question will spin
> at maximum the double time. But the loops are left as soon as the condition is
> satisfied.
>
> It will fix your problem with the 10 kbit/s bitrate. But if there is some kind of
> problem with the IP core it will still fail, it just takes double amount of time
> (100 µs + overhead) until the function returns.
>
> I don't see any harm in looping longer:
> - The previous good case is unchanged.
> - The error case takes double amount of time.
> - Your problem is hopefully fixed.
Thanks for your explanation, I will cook a patch then resend.
Best Regards,
Joakim Zhang
> 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 |
^ 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