* Aw: Re: dvb usb issues since kernel 4.9
From: Josef Griebichler @ 2018-01-08 16:26 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Alan Stern, Greg Kroah-Hartman, linux-usb, Eric Dumazet,
Rik van Riel, Paolo Abeni, Hannes Frederic Sowa,
Jesper Dangaard Brouer, linux-kernel, netdev, Jonathan Corbet,
LMML, Peter Zijlstra, David Miller, torvalds
In-Reply-To: <20180108074324.3c153189@vento.lan>
Hi Maro,
I tried your mentioned patch but unfortunately no real improvement for me.
dmesg http://ix.io/DOg
tvheadend service log http://ix.io/DOi
Errors during recording are still there.
Errors increase if there is additional tcp load on raspberry.
Unfortunately there's no usbmon or tshark on libreelec so I can't provide further logs.
Regards,
Josef
> On Sun, 7 Jan 2018, Mauro Carvalho Chehab wrote:
>
> > > > It seems that the original patch were designed to solve some IRQ issues
> > > > with network cards with causes data losses on high traffic. However,
> > > > it is also causing bad effects on sustained high bandwidth demands
> > > > required by DVB cards, at least on some USB host drivers.
> > > >
> > > > Alan/Greg/Eric/David:
> > > >
> > > > Any ideas about how to fix it without causing regressions to
> > > > network?
> > >
> > > It would be good to know what hardware was involved on the x86 system
> > > and to have some timing data. Can we see the output from lsusb and
> > > usbmon, running on a vanilla kernel that gets plenty of video glitches?
> >
> > From Josef's report, and from the BZ, the affected hardware seems
> > to be based on Montage Technology M88DS3103/M88TS2022 chipset.
>
> What type of USB host controller does the x86_64 system use? EHCI or
> xHCI?
I'll let Josef answer this.
>
> > The driver it uses is at drivers/media/usb/dvb-usb-v2/dvbsky.c,
> > with shares a USB implementation that is used by a lot more drivers.
> > The URB handling code is at:
> >
> > drivers/media/usb/dvb-usb-v2/usb_urb.c
> >
> > This particular driver allocates 8 buffers with 4096 bytes each
> > for bulk transfers, using transfer_flags = URB_NO_TRANSFER_DMA_MAP.
> >
> > This become a popular USB hardware nowadays. I have one S960c
> > myself, so I can send you the lsusb from it. You should notice, however,
> > that a DVB-C/DVB-S2 channel can easily provide very high sustained bit
> > rates. Here, on my DVB-S2 provider, a typical transponder produces 58 Mpps
> > of payload after removing URB headers.
>
> You mentioned earlier that the driver uses bulk transfers. In USB-2.0,
> the maximum possible payload data transfer rate using bulk transfers is
> 53248 bytes/ms, which is 53.248 MB/s (i.e., lower than 58 MB/s). And
> even this is possible only if almost nothing else is using the bus at
> the same time.
No, I said 58 Mbits/s (not bytes).
On DVB-C and DVB-S2 specs, AFAIKT, there's no hard limit for the maximum
payload data rate, although industry seems to limit it to be around
60 Mbits/s. On those standards, the maximal bit rate is defined by the
modulation type and by the channel symbol rate.
To give you a practical example, my DVB-S2 provider modulates each
transponder with 8/PSK (3 bits/symbol), and define channels with a
symbol rate of 30 Mbauds/s. So, it could, theoretically, transport
a MPEG-TS stream up to 90 Mbits/s (minus headers and guard intervals).
In practice, the streams there are transmitted with 58,026.5 Kbits/s.
> > A 10 minutes record with the
> > entire data (with typically contains 5-10 channels) can easily go
> > above 4 GB, just to reproduce 1-2 glitches. So, I'm not sure if
> > a usbmon dump would be useful.
>
> It might not be helpful at all. However, I'm not interested in the
> payload data (which would be unintelligible to me anyway) but rather
> the timing of URB submissions and completions. A usbmon trace which
> didn't keep much of the payload data would only require on the order of
> 50 MB per minute -- and Josef said that glitches usually would show up
> within a minute or so.
Yeah, this could help.
Josef,
You can get it with wireshark/tshark or tcpdump. See:
https://technolinchpin.wordpress.com/2015/10/23/usb-bus-sniffers-for-linux-system/
https://wiki.wireshark.org/CaptureSetup/USB
> > I'm enclosing the lsusb from a S960C device, with is based on those
> > Montage chipsets:
>
> What I wanted to see was the output from "lsusb" on the affected
> system, not the output from "lsusb -v -s B:D" on your system.
>
> > > Overall, this may be a very difficult problem to solve. The
> > > 4cd13c21b207 commit was intended to improve throughput at the cost of
> > > increased latency. But then what do you do when the latency becomes
> > > too high for the video subsystem to handle?
> >
> > Latency can't be too high, otherwise frames will be dropped.
>
> Yes, that's the whole point.
>
> > Even if the Kernel itself doesn't drop, if the delay goes higher
> > than a certain threshold, userspace will need to drop, as it
> > should be presenting audio and video on real time. Yet, typically,
> > userspace will delay it by one or two seconds, with would mean
> > 1500-3500 buffers, with I suspect it is a lot more than the hardware
> > limits. So I suspect that the hardware starves free buffers a way
> > before userspace, as media hardware don't have unlimited buffers
> > inside them, as they assume that the Kernel/userspace will be fast
> > enough to sustain bit rates up to 66 Mbps of payload.
>
> The timing information would tell us how large the latency is.
>
> In any case, you might be able to attack the problem simply by using
> more than 8 buffers. With just eight 4096-byte buffers, the total
> pipeline capacity is only about 0.62 ms (at the maximum possible
> transfer rate). Increasing the number of buffers to 65 would give a
> capacity of 5 ms, which is probably a lot better suited for situations
> where completions are handled by the ksoftirqd thread.
Increasing it to 65 shouldn't be hard. Not sure, however, if the hardware
will actually fill the 65 buffers, but it is worth to try.
> > Perhaps media drivers could pass some quirk similar to URB_ISO_ASAP,
> > in order to revert the kernel logic to prioritize latency instead of
> > throughput.
>
> It can't be done without pervasive changes to the USB subsystem, which
> I would greatly prefer to avoid. Besides, this wouldn't really solve
> the problem. Decreasing the latency for one device will cause it to be
> increased for others.
If there is a TV streaming traffic at a USB bus, it means that the
user wants to either watch and/or record a TV program. On such
usecase scenario, a low latency is highly desired for the TV capture
(and display, if the GPU is USB), even it means a higher latency for
other traffic.
Josef,
Could you please try the following patch on Kernel 4.14.10 (without
reverting any changesets), and see if it fixes the issue?
media: dvbsky: Increase the number of buffers
Right now, This driver expects a 0.62 ms delay with 8 buffers on an USB 2.0
high speed bus. Increase it to 65 buffers, in order to give more time for
the top half of the USB transfer handler to complete its task.
Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c b/drivers/media/usb/dvb-usb-v2/dvbsky.c
index 131b6c08e199..d3f5ffc54b25 100644
--- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
+++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
@@ -740,7 +740,7 @@ static struct dvb_usb_device_properties dvbsky_s960_props = {
.num_adapters = 1,
.adapter = {
{
- .stream = DVB_USB_STREAM_BULK(0x82, 8, 4096),
+ .stream = DVB_USB_STREAM_BULK(0x82, 65, 4096),
}
}
};
>
Thanks,
Mauro
^ permalink raw reply related
* Re: [PATCH 02/18] Documentation: document nospec helpers
From: Jonathan Corbet @ 2018-01-08 16:29 UTC (permalink / raw)
To: Dan Williams
Cc: linux-kernel, Mark Rutland, linux-arch, peterz, netdev,
Will Deacon, gregkh, tglx, torvalds, alan
In-Reply-To: <151520100323.32271.8384226462583945132.stgit@dwillia2-desk3.amr.corp.intel.com>
On Fri, 05 Jan 2018 17:10:03 -0800
Dan Williams <dan.j.williams@intel.com> wrote:
> Document the rationale and usage of the new nospec*() helpers.
I have just a couple of overall comments.
- It would be nice if the document were done in RST and placed in the
core-API manual, perhaps using kerneldoc comments for the macros
themselves. It's already 99.9% RST now, so the changes required would
be minimal.
- More importantly: is there any way at all to give guidance to
developers wondering *when* they should use these primitives? I think
it would be easy to create a situation where they don't get used where
they are really needed; meanwhile, there may well be a flood of
"helpful" patches adding them where they make no sense at all.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH v5 1/3] clocksource/drivers/atcpit100: Add andestech atcpit100 timer
From: Arnd Bergmann @ 2018-01-08 16:30 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Greentime Hu, Greentime, Rick Chen, Rick Chen,
Linux Kernel Mailing List, Linus Walleij, linux-arch,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, netdev,
Vincent Chen, DTML, Al Viro, David Howells, Will Deacon,
linux-serial, John Stultz
In-Reply-To: <78dc7813-524d-c108-0e3d-516f8f4dabfe@linaro.org>
On Mon, Jan 8, 2018 at 5:08 PM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> On 08/01/2018 16:26, Arnd Bergmann wrote:
>> On Fri, Jan 5, 2018 at 10:31 AM, Daniel Lezcano
>> <daniel.lezcano@linaro.org> wrote:
>>>
>>> etc ...
>>
>> I'd actually prefer to not do it for ARM either: Most other subsystems
>> don't do that, and I don't see a strong reason why clocksource should
>> be special here.
>
> The majority doing the opposite does not mean it is right.
>
> Do you know which clock belongs to which board ? Who will unselect a
> clock ? I'm pretty sure nobody. Everyone relies on the platform Kconfig
> and expect it to select the right drivers.
The point is that there is no platform specific Kconfig that could select
it, as the concept doesn't make a lot of sense here.
If you require the driver to always be selected by the
architecture code, it adds a little bit of bloat on systems
that don't need it. This is possible, but I think it's preferable
to give users a way of tuning a kernel for a particular chip.
> We don't expect the hackers to have a deep knowledge of the hardware and
> the driver dependencies. It is very convenient to not care about that
> and let the platform's Kconfig to select the right drivers.
>
> And that is the behavior I would like to keep.
I'm not worried about the driver bloating the kernel here when it
isn't disabled, this is no different from enabling the USB controller
by default for a board that doesn't have a USB connector, or
enabling ten different pinctrl drivers for all members of a chip
family even though you know which particular chip you are
running on.
>> Selecting 'TIMER_OF' from the individual drivers that need it (as you
>> suggest) makes sense, but I think for ARM we treat SoC families
>> as a bit too special, in the end they are for the most part collections
>> of individual hardware blocks that may or may not be present on
>> some chip.
>>
>> In case of risc-v and nds32, I expect that the separation will be
>> even less visibile in the hardware, as a typical model here is
>> that one company designs SoCs for multiple customers that each
>> have different requirements. Some of them may have one
>> timer and some have another timer or multiple timers, but there
>> is no strict separation between SoC families as I understand.
>> Here we'd be better off not having a per-SoC Kconfig option at
>> all, just a generic defconfig that enables all the drivers that might
>> be used, and integrators can have a defconfig file that only
>> enables the stuff they actually use on a given chip.
>
> Yes, the result is the same, the option is not showed in the menu.
>
> However, I can understand it could be interesting to have the ability to
> unselect a driver for experts.
>
> I'm wondering if we can create a bool_expert which shows up only when
> CONFIG_EXPERT is set.
Having a dependency on CONFIG_EXPERT means that a more
users will end up having to set that for a shipping system. It's
something we can do (even without a special Kconfig keyword),
but it should be used carefully for stuff that 99% of the users want
to enable.
Why not just:
config CLKSRC_ATCPIT100
bool "Clocksource for AE3XX platform"
depends on NDS32 || COMPILE_TEST
depends on HAS_IOMEM
default NDS32
help
This option enables support for the Andestech AE3XX platform timers.
That way, it's simply enabled on NDS32 by default, but just as easy
to disable for systems that don't need it.
Arnd
^ permalink raw reply
* Re: Aw: Re: dvb usb issues since kernel 4.9
From: Alan Stern @ 2018-01-08 16:31 UTC (permalink / raw)
To: Josef Griebichler
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-usb,
Eric Dumazet, Rik van Riel, Paolo Abeni, Hannes Frederic Sowa,
Jesper Dangaard Brouer, linux-kernel, netdev, Jonathan Corbet,
LMML, Peter Zijlstra, David Miller, torvalds
In-Reply-To: <trinity-c7ec7cbd-a186-4a2a-bcb6-cce8993d6a90-1515428770628@3c-app-gmx-bs32>
On Mon, 8 Jan 2018, Josef Griebichler wrote:
> Hi Maro,
>
> I tried your mentioned patch but unfortunately no real improvement for me.
> dmesg http://ix.io/DOg
> tvheadend service log http://ix.io/DOi
> Errors during recording are still there.
> Errors increase if there is additional tcp load on raspberry.
>
> Unfortunately there's no usbmon or tshark on libreelec so I can't provide further logs.
Can you try running the same test on an x86_64 system?
Alan Stern
^ permalink raw reply
* Re: b43: Replace mdelay with msleep in b43_radio_2057_init_post
From: Larry Finger @ 2018-01-08 16:31 UTC (permalink / raw)
To: Kalle Valo, Jia-Ju Bai
Cc: kstewart, johannes.berg, tiwai, gregkh, linux-wireless,
linux-kernel, andrew.zaborowski, b43-dev, netdev, colin.king
In-Reply-To: <20180108162128.6D37560B1D@smtp.codeaurora.org>
On 01/08/2018 10:21 AM, Kalle Valo wrote:
> Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>
>> b43_radio_2057_init_post is not called in an interrupt handler
>> nor holding a spinlock.
>> The function mdelay in it can be replaced with msleep, to reduce busy wait.
>>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>
> You submitted an identical patch a week earlier:
>
> https://patchwork.kernel.org/patch/10137671/
>
> How is this different? Also always add version number to the patch so that the
> maintainers can follow the changes easily:
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#patch_version_missing
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#changelog_missing
I had negative comments on one of those due to the possibility of msleep(2)
extending as long as 20 msec. Until the author, or someone else, can test that
this is OK, then the mdelay(2) can only be replaced with usleep_range(2000, 3000).
NACK for both.
Larry
^ permalink raw reply
* Re: [PATCH bpf] selftests/bpf: fix test_align
From: Edward Cree @ 2018-01-08 16:38 UTC (permalink / raw)
To: Alexei Starovoitov, David S . Miller; +Cc: Daniel Borkmann, netdev
In-Reply-To: <20180105230200.2183754-1-ast@fb.com>
On 05/01/18 23:02, Alexei Starovoitov wrote:
> since commit 82abbf8d2fc4 the verifier rejects the bit-wise
> arithmetic on pointers earlier.
> The test 'dubious pointer arithmetic' now has less output to match on.
> Adjust it.
>
> Fixes: 82abbf8d2fc4 ("bpf: do not allow root to mangle valid pointers")
> Reported-by: kernel test robot <xiaolong.ye@intel.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> tools/testing/selftests/bpf/test_align.c | 22 +---------------------
> 1 file changed, 1 insertion(+), 21 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c
> index 8591c89c0828..471bbbdb94db 100644
> --- a/tools/testing/selftests/bpf/test_align.c
> +++ b/tools/testing/selftests/bpf/test_align.c
> @@ -474,27 +474,7 @@ static struct bpf_align_test tests[] = {
> .result = REJECT,
> .matches = {
> {4, "R5=pkt(id=0,off=0,r=0,imm=0)"},
> - /* ptr & 0x40 == either 0 or 0x40 */
> - {5, "R5=inv(id=0,umax_value=64,var_off=(0x0; 0x40))"},
> - /* ptr << 2 == unknown, (4n) */
> - {7, "R5=inv(id=0,smax_value=9223372036854775804,umax_value=18446744073709551612,var_off=(0x0; 0xfffffffffffffffc))"},
> - /* (4n) + 14 == (4n+2). We blow our bounds, because
> - * the add could overflow.
> - */
> - {8, "R5=inv(id=0,var_off=(0x2; 0xfffffffffffffffc))"},
> - /* Checked s>=0 */
> - {10, "R5=inv(id=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
> - /* packet pointer + nonnegative (4n+2) */
> - {12, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
> - {14, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
> - /* NET_IP_ALIGN + (4n+2) == (4n), alignment is fine.
> - * We checked the bounds, but it might have been able
> - * to overflow if the packet pointer started in the
> - * upper half of the address space.
> - * So we did not get a 'range' on R6, and the access
> - * attempt will fail.
> - */
> - {16, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
> + /* R5 bitwise operator &= on pointer prohibited */
> }
> },
> {
Rather than neutering this test, we should change it to keep the part where
it tests that a large pkt_ptr offset prevents us getting a reg->range.
Specifically, in this test we have
r2 = pkt
r5 = large unknown scalar
r6 = r2 + r5
r4 = r6 + 4
Then we check r4 < pkt_end, which normally would give r6->range = 4, but in
this case must not do so since r6 could be (u64)(-2) in which case r4 = 2
< pkt_end despite r6 not pointing into the packet.
AFAICT there is not other coverage of this case in test_align, and I don't
recall such a test being in test_verifier either. So please instead replace
the insns that do prohibited ops on pointers with some other way of creating
a large unknown scalar, and keep the rest of the test case intact.
-Ed
^ permalink raw reply
* Re: b53 tags on bpi-r1 (bcm53125)
From: Jochen Friedrich @ 2018-01-08 16:51 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev
In-Reply-To: <08f78158-3072-6929-995f-eda622833256@gmail.com>
Hi Florian,
Am 04.01.2018 um 05:49 schrieb Florian Fainelli:
> On 12/29/2017 07:22 PM, Florian Fainelli wrote:
>> Le 12/29/17 à 13:56, Florian Fainelli a écrit :
>>> Le 12/29/17 à 12:21, Florian Fainelli a écrit :
>>>> Hi Jochen,
>>>>
>>>> Le 12/18/17 à 02:44, Jochen Friedrich a écrit :
>>>>> Hi Florian,
>>>>>
>>>>> unfortunately, this doesn't make any difference.
>>>>>
>>>>> Just out of curiosity, BPI-R1 has pull-down resistors on LED6 and 7
>>>>> (MII_MODE0/1). However, the public available 53125U sheet doesn't
>>>>> document these pins:
>>>>>
>>>>> LED[6] IMP_MODE[0] Pull-up Active low (since IMP Mode is not used)
>>>>> LED[7] IMP_MODE[1] Pull-up Active low (since IMP Mode is not used)
>>>>>
>>>>> Is this MII mode maybe incompatible with Broadcom tags?
>>>> AFAICT, it should not, this is largely independent from enabling
>>>> Broadcom tags.
>>>>
>>>> I have now reproduced this on my BPI-R1 as well and am looking at what
>>>> might be going wrong.
>>> OK, so I have been able to find out what was going on. On BCM53125 and
>>> earlier switches, we need to do a couple of things for Broadcom tags to
>>> be usable:
>>>
>>> - turn on managed mode (SM_SW_FWD_MODE)
>>> - configure Port 8 for IMP mode (B53_GLOBAL_CONFIG, setting bit
>>> GC_FRM_MGMT_PORT_MII)
>>>
>>> After doing that, I can now see the correct outgoing packets on my host,
>>> however, the replies don't seem to be delivered to the per-port DSA
>>> network devices, and I suspect it's because of stmmac, so I am
>>> investigating this now.
>>>
>> So stmmac was indeed part of the problem. I had to clear the
>> GMAC_CONTROL_ACS bit in GMAC_CORE_INIT in order to allow stmmac to
>> properly receive packets, otherwise, packets were truncated to 8 bytes
>> on reception which I assume is because the Broadcom tags may look like
>> some sort of weird LLC/SNAP packet which was confusing the hardware.
>> This is all working correctly now after a bunch of changes that I will
>> submit in the next few days.
>>
>> Preliminary patches available here:
>>
>> https://github.com/ffainelli/linux/commits/stmmac-fixes
>>
>> Thank you very much for your patience!
> Turning on managed mode has some other effects, like how multicast
> traffic needs to be handled and how to resolve ARL misses, which means
> that the changes are too invasive for the upcoming release since we
> would be essentially adding multicast support through a number of code
> paths (including core DSA).
>
> I will submit a simple change for now which does not turn on Broadcom
> tags on switches that require managed mode (5395, 97, 98, 53125 and
> friends) and target enabling Broadcom tags for these models for net-next.
>
> Thanks
Thanks. That's the best approach for now :-)
^ permalink raw reply
* [PATCH iproute2 v3 0/7] ip6/tunnel: Unify tclass, flowlabel and encap_limit output
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
To: netdev
With this series I want to improve printing of tclass,
flowlabel and encapsulation limit (encap_limit) options.
Everything within this series is open for your comments,
suggestions and criticism.
See individual patch description message for details.
Thanks,
Serhii
Serhey Popovych (7):
ip6/tunnel: Fix tclass output
ip6tnl/tunnel: Do not print obscure flowinfo
ip6/tunnel: Unify tclass printing
ip6/tunnel: Unify flowlabel printing
ip6/tunnel: Unify encap_limit printing
gre6/tunnel: Output flowlabel after tclass
ip6tnl/tunnel: Output hoplimit before encapsulation limit
ip/link_gre6.c | 54 ++++++++++++++++------------------------------
ip/link_ip6tnl.c | 63 ++++++++++++++++++------------------------------------
2 files changed, 40 insertions(+), 77 deletions(-)
--
1.7.10.4
^ permalink raw reply
* [PATCH iproute2 v3 1/7] ip6/tunnel: Fix tclass output
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>
In link_gre6.c it seems copy paste error: tclass is 8 bits,
not 20 as flowlabel.
In link_iptnl.c rename "flowinfo_tclass" to "tclass" as it
correct name since flowinfo is implementation internal name
used to label combined within u32 attribute tclass and
flowlabel.
Fixes: 1facc1c61c07 ("ip: link_ip6tnl.c: add json output support")
Fixes: 2e706e12d9b0 ("Merge branch 'master' into net-next")
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre6.c | 2 +-
ip/link_ip6tnl.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 7ae4b49..87c313c 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -475,7 +475,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
if (is_json_context()) {
SPRINT_BUF(b1);
- snprintf(b1, sizeof(b1), "0x%05x",
+ snprintf(b1, sizeof(b1), "0x%02x",
ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
print_string(PRINT_JSON, "tclass", NULL, b1);
} else {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 84205b1..8e84ed0 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -416,7 +416,7 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
SPRINT_BUF(b1);
snprintf(b1, sizeof(b1), "0x%02x", (__u8)(val >> 20));
- print_string(PRINT_JSON, "flowinfo_tclass", NULL, b1);
+ print_string(PRINT_JSON, "tclass", NULL, b1);
} else {
printf("tclass 0x%02x ", (__u8)(val >> 20));
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 v3 2/7] ip6tnl/tunnel: Do not print obscure flowinfo
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>
It is implementation internal and main purpose
of printing it seems debugging.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_ip6tnl.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 8e84ed0..e084975 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -440,16 +440,6 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
}
}
- if (is_json_context()) {
- SPRINT_BUF(flwinfo);
-
- snprintf(flwinfo, sizeof(flwinfo), "0x%08x", ntohl(flowinfo));
- print_string(PRINT_JSON, "flowinfo", NULL, flwinfo);
- } else {
- printf("(flowinfo 0x%08x) ", ntohl(flowinfo));
-
- }
-
if (flags & IP6_TNL_F_RCV_DSCP_COPY)
print_bool(PRINT_ANY,
"ip6_tnl_f_rcv_dscp_copy",
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 v3 3/7] ip6/tunnel: Unify tclass printing
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>
Use @s2 buffer to store string representation of
tclass and get rid of extra SPRINT_BUF(): no
need to preserve @s2 contents for later.
Use print_string(PRINT_ANY, ...) with prepared by
snprintf() string for both PRINT_JSON and PRINT_FP
cases.
While there use __u32 for flowinfo in link_gre6.c
and check for IFLA_GRE_FLOWINFO attribute presense.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre6.c | 16 +++++-----------
ip/link_ip6tnl.c | 16 +++++-----------
2 files changed, 10 insertions(+), 22 deletions(-)
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 87c313c..a02dd4a 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -380,7 +380,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
unsigned int iflags = 0;
unsigned int oflags = 0;
unsigned int flags = 0;
- unsigned int flowinfo = 0;
+ __u32 flowinfo = 0;
struct in6_addr in6_addr_any = IN6ADDR_ANY_INIT;
if (!tb)
@@ -471,17 +471,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
"ip6_tnl_f_use_orig_tclass",
"tclass inherit ",
true);
- } else {
- if (is_json_context()) {
- SPRINT_BUF(b1);
+ } else if (tb[IFLA_GRE_FLOWINFO]) {
+ __u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20;
- snprintf(b1, sizeof(b1), "0x%02x",
- ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
- print_string(PRINT_JSON, "tclass", NULL, b1);
- } else {
- fprintf(f, "tclass 0x%02x ",
- ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
- }
+ snprintf(s2, sizeof(s2), "0x%02x", val);
+ print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
}
if (flags & IP6_TNL_F_RCV_DSCP_COPY)
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index e084975..05322fd 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -404,22 +404,16 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
"hoplimit %u ",
rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
- if (flags & IP6_TNL_F_USE_ORIG_TCLASS)
+ if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
print_bool(PRINT_ANY,
"ip6_tnl_f_use_orig_tclass",
"tclass inherit ",
true);
- else if (tb[IFLA_IPTUN_FLOWINFO]) {
- __u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS);
+ } else if (tb[IFLA_IPTUN_FLOWINFO]) {
+ __u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20;
- if (is_json_context()) {
- SPRINT_BUF(b1);
-
- snprintf(b1, sizeof(b1), "0x%02x", (__u8)(val >> 20));
- print_string(PRINT_JSON, "tclass", NULL, b1);
- } else {
- printf("tclass 0x%02x ", (__u8)(val >> 20));
- }
+ snprintf(s2, sizeof(s2), "0x%02x", val);
+ print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
}
if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 v3 4/7] ip6/tunnel: Unify flowlabel printing
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>
Use @s2 buffer to store string representation of
flowlabel and get rid of extra SPRINT_BUF(): no
need to preserve @s2 contents for later.
Use print_string(PRINT_ANY, ...) with prepared by
snprintf() string for both PRINT_JSON and PRINT_FP
cases.
Omit flowlabel from output if no flowinfo attribute
is given and IP6_TNL_F_USE_ORIG_FLOWLABEL isn't set.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre6.c | 15 ++++-----------
ip/link_ip6tnl.c | 14 ++++----------
2 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index a02dd4a..1205946 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -452,18 +452,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
"ip6_tnl_f_use_orig_flowlabel",
"flowlabel inherit ",
true);
- } else {
- if (is_json_context()) {
- SPRINT_BUF(b1);
-
- snprintf(b1, sizeof(b1), "0x%05x",
- ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
- print_string(PRINT_JSON, "flowlabel", NULL, b1);
+ } else if (tb[IFLA_GRE_FLOWINFO]) {
+ __u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
- } else {
- fprintf(f, "flowlabel 0x%05x ",
- ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
- }
+ snprintf(s2, sizeof(s2), "0x%05x", val);
+ print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
}
if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 05322fd..7000056 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -421,17 +421,11 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
"ip6_tnl_f_use_orig_flowlabel",
"flowlabel inherit ",
true);
- } else {
- if (is_json_context()) {
- SPRINT_BUF(b1);
+ } else if (tb[IFLA_IPTUN_FLOWINFO]) {
+ __u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
- snprintf(b1, sizeof(b1), "0x%05x",
- ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
- print_string(PRINT_JSON, "flowlabel", NULL, b1);
- } else {
- printf("flowlabel 0x%05x ",
- ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
- }
+ snprintf(s2, sizeof(s2), "0x%05x", val);
+ print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
}
if (flags & IP6_TNL_F_RCV_DSCP_COPY)
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 v3 5/7] ip6/tunnel: Unify encap_limit printing
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>
Use %u format specifier to print it in link_gre6.c and
make code more readable.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre6.c | 11 ++++-------
ip/link_ip6tnl.c | 12 ++++++------
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 1205946..8014207 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -433,18 +433,15 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
print_int(PRINT_JSON, "ttl", NULL, ttl);
}
- if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
+ if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
print_bool(PRINT_ANY,
"ip6_tnl_f_ign_encap_limit",
"encaplimit none ",
true);
- else if (tb[IFLA_GRE_ENCAP_LIMIT]) {
- int encap_limit = rta_getattr_u8(tb[IFLA_GRE_ENCAP_LIMIT]);
+ } else if (tb[IFLA_GRE_ENCAP_LIMIT]) {
+ __u8 val = rta_getattr_u8(tb[IFLA_GRE_ENCAP_LIMIT]);
- print_int(PRINT_ANY,
- "encap_limit",
- "encaplimit %d ",
- encap_limit);
+ print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
}
if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 7000056..379eb33 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -387,16 +387,16 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
print_uint(PRINT_ANY, "link_index", "dev %u ", link);
}
- if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
+ if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
print_bool(PRINT_ANY,
"ip6_tnl_f_ign_encap_limit",
"encaplimit none ",
true);
- else if (tb[IFLA_IPTUN_ENCAP_LIMIT])
- print_uint(PRINT_ANY,
- "encap_limit",
- "encaplimit %u ",
- rta_getattr_u8(tb[IFLA_IPTUN_ENCAP_LIMIT]));
+ } else if (tb[IFLA_IPTUN_ENCAP_LIMIT]) {
+ __u8 val = rta_getattr_u8(tb[IFLA_IPTUN_ENCAP_LIMIT]);
+
+ print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
+ }
if (tb[IFLA_IPTUN_TTL])
print_uint(PRINT_ANY,
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 v3 6/7] gre6/tunnel: Output flowlabel after tclass
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>
To follow ip6tnl output print flowlabel after tclass
in link_gre6.c.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre6.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 8014207..55bd1fb 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -444,18 +444,6 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
}
- if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
- print_bool(PRINT_ANY,
- "ip6_tnl_f_use_orig_flowlabel",
- "flowlabel inherit ",
- true);
- } else if (tb[IFLA_GRE_FLOWINFO]) {
- __u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
-
- snprintf(s2, sizeof(s2), "0x%05x", val);
- print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
- }
-
if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
print_bool(PRINT_ANY,
"ip6_tnl_f_use_orig_tclass",
@@ -468,6 +456,18 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
}
+ if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
+ print_bool(PRINT_ANY,
+ "ip6_tnl_f_use_orig_flowlabel",
+ "flowlabel inherit ",
+ true);
+ } else if (tb[IFLA_GRE_FLOWINFO]) {
+ __u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
+
+ snprintf(s2, sizeof(s2), "0x%05x", val);
+ print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
+ }
+
if (flags & IP6_TNL_F_RCV_DSCP_COPY)
print_bool(PRINT_ANY,
"ip6_tnl_f_rcv_dscp_copy",
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH bpf] bpf: prevent out-of-bounds speculation
From: Mark Rutland @ 2018-01-08 17:05 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S . Miller, Daniel Borkmann, Jann Horn, Linus Torvalds,
Dan Williams, Peter Zijlstra, Elena Reshetova, Alan Cox, netdev,
kernel-team, will.deacon
In-Reply-To: <20180105042811.1590965-1-ast@fb.com>
Hi Alexei,
On Thu, Jan 04, 2018 at 08:28:11PM -0800, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> Under speculation, CPUs may mis-predict branches in bounds checks. Thus,
> memory accesses under a bounds check may be speculated even if the
> bounds check fails, providing a primitive for building a side channel.
>
> To avoid leaking kernel data round up array-based maps and mask the index
> after bounds check, so speculated load with out of bounds index will load
> either valid value from the array or zero from the padded area.
Thanks for putting this together, this certainly looks neat.
I'm a little worried that in the presence of some CPU/compiler
optimisations, the masking may effectively be skipped under speculation.
So I'm not sure how robust this is going to be.
More on that below.
> To avoid duplicating map_lookup functions for root/unpriv always generate
> a sequence of bpf instructions equivalent to map_lookup function for
> array and array_of_maps map types when map was created by unpriv user.
> And unconditionally mask index for percpu_array, since it's fast enough,
> even when max_entries are not rounded to power of 2 for root user,
> since percpu_array doesn't have map_gen_lookup callback yet.
Is there a noticeable slowdown from the masking? Can't we always have
that in place?
> @@ -157,7 +175,7 @@ static void *percpu_array_map_lookup_elem(struct bpf_map *map, void *key)
> if (unlikely(index >= array->map.max_entries))
> return NULL;
>
> - return this_cpu_ptr(array->pptrs[index]);
> + return this_cpu_ptr(array->pptrs[index & array->index_mask]);
As above, I think this isn't necessarily robust, as CPU/compiler
optimisations can break the dependency on the index_mask, allowing
speculation without a mask.
e.g. a compiler could re-write this as:
if (array->index_mask != 0xffffffff)
index &= array->index_mask;
return this_cpu_ptr(array->pptrs[index]);
... which would allow an unmasked index to be used in speculated paths.
Similar cases could occur with some CPU implementations. For example, HW
value-prediction could result in the use of an all-ones mask under
speculation.
I think that we may need to be able to provide an arch-specific
pointer sanitization sequence (though we could certainly have masking as
the default).
I have a rough idea as to how that could be plumbed into the JIT. First
I need to verify the sequence I have in mind for arm/arm64 is
sufficient.
Thanks,
Mark.
^ permalink raw reply
* Re: b43: Replace mdelay with msleep in b43_radio_2057_init_post
From: Kalle Valo @ 2018-01-08 17:06 UTC (permalink / raw)
To: Larry Finger
Cc: kstewart-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
johannes.berg-ral2JQCrhuEAvxtiuMwx3w, tiwai-l3A5Bk7waGM,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
andrew.zaborowski-ral2JQCrhuEAvxtiuMwx3w, Jia-Ju Bai,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
netdev-u79uwXL29TY76Z2rM5mHXA, colin.king-Z7WLFzj8eWMS+FvcfC7Uqw
In-Reply-To: <fc3e2558-c31f-289d-8413-33fef4f5ca64-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org> writes:
> On 01/08/2018 10:21 AM, Kalle Valo wrote:
>> Jia-Ju Bai <baijiaju1990-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>>> b43_radio_2057_init_post is not called in an interrupt handler
>>> nor holding a spinlock.
>>> The function mdelay in it can be replaced with msleep, to reduce busy wait.
>>>
>>> Signed-off-by: Jia-Ju Bai <baijiaju1990-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> You submitted an identical patch a week earlier:
>>
>> https://patchwork.kernel.org/patch/10137671/
>>
>> How is this different? Also always add version number to the patch so that the
>> maintainers can follow the changes easily:
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#patch_version_missing
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#changelog_missing
>
> I had negative comments on one of those due to the possibility of
> msleep(2) extending as long as 20 msec. Until the author, or someone
> else, can test that this is OK, then the mdelay(2) can only be
> replaced with usleep_range(2000, 3000).
>
> NACK for both.
Ok, patches dropped.
--
Kalle Valo
^ permalink raw reply
* [PATCH iproute2 0/7] ip6/tunnel: Unify tclass, flowlabel and encap_limit output
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
To: netdev
With this series I want to improve printing of tclass,
flowlabel and encapsulation limit (encap_limit) options.
Everything within this series is open for your comments,
suggestions and criticism.
See individual patch description message for details.
Thanks,
Serhii
Serhey Popovych (7):
ip6/tunnel: Fix tclass output
ip6tnl/tunnel: Do not print obscure flowinfo
ip6/tunnel: Unify tclass printing
ip6/tunnel: Unify flowlabel printing
ip6/tunnel: Unify encap_limit printing
gre6/tunnel: Output flowlabel after tclass
ip6tnl/tunnel: Output hoplimit before encapsulation limit
ip/link_gre6.c | 54 ++++++++++++++++------------------------------
ip/link_ip6tnl.c | 63 ++++++++++++++++++------------------------------------
2 files changed, 40 insertions(+), 77 deletions(-)
--
1.7.10.4
^ permalink raw reply
* [PATCH iproute2 1/7] ip6/tunnel: Fix tclass output
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>
In link_gre6.c it seems copy paste error: tclass is 8 bits,
not 20 as flowlabel.
In link_iptnl.c rename "flowinfo_tclass" to "tclass" as it
correct name since flowinfo is implementation internal name
used to label combined within u32 attribute tclass and
flowlabel.
Fixes: 1facc1c61c07 ("ip: link_ip6tnl.c: add json output support")
Fixes: 2e706e12d9b0 ("Merge branch 'master' into net-next")
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre6.c | 2 +-
ip/link_ip6tnl.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 7ae4b49..87c313c 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -475,7 +475,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
if (is_json_context()) {
SPRINT_BUF(b1);
- snprintf(b1, sizeof(b1), "0x%05x",
+ snprintf(b1, sizeof(b1), "0x%02x",
ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
print_string(PRINT_JSON, "tclass", NULL, b1);
} else {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 84205b1..8e84ed0 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -416,7 +416,7 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
SPRINT_BUF(b1);
snprintf(b1, sizeof(b1), "0x%02x", (__u8)(val >> 20));
- print_string(PRINT_JSON, "flowinfo_tclass", NULL, b1);
+ print_string(PRINT_JSON, "tclass", NULL, b1);
} else {
printf("tclass 0x%02x ", (__u8)(val >> 20));
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 2/7] ip6tnl/tunnel: Do not print obscure flowinfo
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>
It is implementation internal and main purpose
of printing it seems debugging.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_ip6tnl.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 8e84ed0..e084975 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -440,16 +440,6 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
}
}
- if (is_json_context()) {
- SPRINT_BUF(flwinfo);
-
- snprintf(flwinfo, sizeof(flwinfo), "0x%08x", ntohl(flowinfo));
- print_string(PRINT_JSON, "flowinfo", NULL, flwinfo);
- } else {
- printf("(flowinfo 0x%08x) ", ntohl(flowinfo));
-
- }
-
if (flags & IP6_TNL_F_RCV_DSCP_COPY)
print_bool(PRINT_ANY,
"ip6_tnl_f_rcv_dscp_copy",
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 3/7] ip6/tunnel: Unify tclass printing
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>
Use @s2 buffer to store string representation of
tclass and get rid of extra SPRINT_BUF(): no
need to preserve @s2 contents for later.
Use print_string(PRINT_ANY, ...) with prepared by
snprintf() string for both PRINT_JSON and PRINT_FP
cases.
While there use __u32 for flowinfo in link_gre6.c
and check for IFLA_GRE_FLOWINFO attribute presense.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre6.c | 16 +++++-----------
ip/link_ip6tnl.c | 16 +++++-----------
2 files changed, 10 insertions(+), 22 deletions(-)
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 87c313c..a02dd4a 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -380,7 +380,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
unsigned int iflags = 0;
unsigned int oflags = 0;
unsigned int flags = 0;
- unsigned int flowinfo = 0;
+ __u32 flowinfo = 0;
struct in6_addr in6_addr_any = IN6ADDR_ANY_INIT;
if (!tb)
@@ -471,17 +471,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
"ip6_tnl_f_use_orig_tclass",
"tclass inherit ",
true);
- } else {
- if (is_json_context()) {
- SPRINT_BUF(b1);
+ } else if (tb[IFLA_GRE_FLOWINFO]) {
+ __u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20;
- snprintf(b1, sizeof(b1), "0x%02x",
- ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
- print_string(PRINT_JSON, "tclass", NULL, b1);
- } else {
- fprintf(f, "tclass 0x%02x ",
- ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
- }
+ snprintf(s2, sizeof(s2), "0x%02x", val);
+ print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
}
if (flags & IP6_TNL_F_RCV_DSCP_COPY)
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index e084975..05322fd 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -404,22 +404,16 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
"hoplimit %u ",
rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
- if (flags & IP6_TNL_F_USE_ORIG_TCLASS)
+ if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
print_bool(PRINT_ANY,
"ip6_tnl_f_use_orig_tclass",
"tclass inherit ",
true);
- else if (tb[IFLA_IPTUN_FLOWINFO]) {
- __u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS);
+ } else if (tb[IFLA_IPTUN_FLOWINFO]) {
+ __u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20;
- if (is_json_context()) {
- SPRINT_BUF(b1);
-
- snprintf(b1, sizeof(b1), "0x%02x", (__u8)(val >> 20));
- print_string(PRINT_JSON, "tclass", NULL, b1);
- } else {
- printf("tclass 0x%02x ", (__u8)(val >> 20));
- }
+ snprintf(s2, sizeof(s2), "0x%02x", val);
+ print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
}
if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 4/7] ip6/tunnel: Unify flowlabel printing
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>
Use @s2 buffer to store string representation of
flowlabel and get rid of extra SPRINT_BUF(): no
need to preserve @s2 contents for later.
Use print_string(PRINT_ANY, ...) with prepared by
snprintf() string for both PRINT_JSON and PRINT_FP
cases.
Omit flowlabel from output if no flowinfo attribute
is given and IP6_TNL_F_USE_ORIG_FLOWLABEL isn't set.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre6.c | 15 ++++-----------
ip/link_ip6tnl.c | 14 ++++----------
2 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index a02dd4a..1205946 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -452,18 +452,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
"ip6_tnl_f_use_orig_flowlabel",
"flowlabel inherit ",
true);
- } else {
- if (is_json_context()) {
- SPRINT_BUF(b1);
-
- snprintf(b1, sizeof(b1), "0x%05x",
- ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
- print_string(PRINT_JSON, "flowlabel", NULL, b1);
+ } else if (tb[IFLA_GRE_FLOWINFO]) {
+ __u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
- } else {
- fprintf(f, "flowlabel 0x%05x ",
- ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
- }
+ snprintf(s2, sizeof(s2), "0x%05x", val);
+ print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
}
if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 05322fd..7000056 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -421,17 +421,11 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
"ip6_tnl_f_use_orig_flowlabel",
"flowlabel inherit ",
true);
- } else {
- if (is_json_context()) {
- SPRINT_BUF(b1);
+ } else if (tb[IFLA_IPTUN_FLOWINFO]) {
+ __u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
- snprintf(b1, sizeof(b1), "0x%05x",
- ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
- print_string(PRINT_JSON, "flowlabel", NULL, b1);
- } else {
- printf("flowlabel 0x%05x ",
- ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
- }
+ snprintf(s2, sizeof(s2), "0x%05x", val);
+ print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
}
if (flags & IP6_TNL_F_RCV_DSCP_COPY)
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 5/7] ip6/tunnel: Unify encap_limit printing
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>
Use %u format specifier to print it in link_gre6.c and
make code more readable.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre6.c | 11 ++++-------
ip/link_ip6tnl.c | 12 ++++++------
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 1205946..8014207 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -433,18 +433,15 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
print_int(PRINT_JSON, "ttl", NULL, ttl);
}
- if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
+ if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
print_bool(PRINT_ANY,
"ip6_tnl_f_ign_encap_limit",
"encaplimit none ",
true);
- else if (tb[IFLA_GRE_ENCAP_LIMIT]) {
- int encap_limit = rta_getattr_u8(tb[IFLA_GRE_ENCAP_LIMIT]);
+ } else if (tb[IFLA_GRE_ENCAP_LIMIT]) {
+ __u8 val = rta_getattr_u8(tb[IFLA_GRE_ENCAP_LIMIT]);
- print_int(PRINT_ANY,
- "encap_limit",
- "encaplimit %d ",
- encap_limit);
+ print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
}
if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 7000056..379eb33 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -387,16 +387,16 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
print_uint(PRINT_ANY, "link_index", "dev %u ", link);
}
- if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
+ if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
print_bool(PRINT_ANY,
"ip6_tnl_f_ign_encap_limit",
"encaplimit none ",
true);
- else if (tb[IFLA_IPTUN_ENCAP_LIMIT])
- print_uint(PRINT_ANY,
- "encap_limit",
- "encaplimit %u ",
- rta_getattr_u8(tb[IFLA_IPTUN_ENCAP_LIMIT]));
+ } else if (tb[IFLA_IPTUN_ENCAP_LIMIT]) {
+ __u8 val = rta_getattr_u8(tb[IFLA_IPTUN_ENCAP_LIMIT]);
+
+ print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
+ }
if (tb[IFLA_IPTUN_TTL])
print_uint(PRINT_ANY,
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 6/7] gre6/tunnel: Output flowlabel after tclass
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>
To follow ip6tnl output print flowlabel after tclass
in link_gre6.c.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre6.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 8014207..55bd1fb 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -444,18 +444,6 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
}
- if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
- print_bool(PRINT_ANY,
- "ip6_tnl_f_use_orig_flowlabel",
- "flowlabel inherit ",
- true);
- } else if (tb[IFLA_GRE_FLOWINFO]) {
- __u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
-
- snprintf(s2, sizeof(s2), "0x%05x", val);
- print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
- }
-
if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
print_bool(PRINT_ANY,
"ip6_tnl_f_use_orig_tclass",
@@ -468,6 +456,18 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
}
+ if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
+ print_bool(PRINT_ANY,
+ "ip6_tnl_f_use_orig_flowlabel",
+ "flowlabel inherit ",
+ true);
+ } else if (tb[IFLA_GRE_FLOWINFO]) {
+ __u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
+
+ snprintf(s2, sizeof(s2), "0x%05x", val);
+ print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
+ }
+
if (flags & IP6_TNL_F_RCV_DSCP_COPY)
print_bool(PRINT_ANY,
"ip6_tnl_f_rcv_dscp_copy",
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 7/7] ip6tnl/tunnel: Output hoplimit before encapsulation limit
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>
To follow gre6 output print hoplimit before encapsulation
limit in link_ip6tnl.c.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_ip6tnl.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 379eb33..bbc7878 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -387,6 +387,13 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
print_uint(PRINT_ANY, "link_index", "dev %u ", link);
}
+ if (tb[IFLA_IPTUN_TTL]) {
+ print_uint(PRINT_ANY,
+ "ttl",
+ "hoplimit %u ",
+ rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
+ }
+
if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
print_bool(PRINT_ANY,
"ip6_tnl_f_ign_encap_limit",
@@ -398,12 +405,6 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
}
- if (tb[IFLA_IPTUN_TTL])
- print_uint(PRINT_ANY,
- "ttl",
- "hoplimit %u ",
- rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
-
if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
print_bool(PRINT_ANY,
"ip6_tnl_f_use_orig_tclass",
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH iproute2 v3 0/7] ip6/tunnel: Unify tclass, flowlabel and encap_limit output
From: Serhey Popovych @ 2018-01-08 17:07 UTC (permalink / raw)
To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 944 bytes --]
Please ignore series: wrong series number (3).
Sorry for noise.
> With this series I want to improve printing of tclass,
> flowlabel and encapsulation limit (encap_limit) options.
>
> Everything within this series is open for your comments,
> suggestions and criticism.
>
> See individual patch description message for details.
>
> Thanks,
> Serhii
>
> Serhey Popovych (7):
> ip6/tunnel: Fix tclass output
> ip6tnl/tunnel: Do not print obscure flowinfo
> ip6/tunnel: Unify tclass printing
> ip6/tunnel: Unify flowlabel printing
> ip6/tunnel: Unify encap_limit printing
> gre6/tunnel: Output flowlabel after tclass
> ip6tnl/tunnel: Output hoplimit before encapsulation limit
>
> ip/link_gre6.c | 54 ++++++++++++++++------------------------------
> ip/link_ip6tnl.c | 63 ++++++++++++++++++------------------------------------
> 2 files changed, 40 insertions(+), 77 deletions(-)
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ 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