* Re: [PATCH v3 3/3] lib/string_helpers.c: Change semantics of string_escape_mem
From: Rasmus Villemoes @ 2015-02-10 13:02 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andrew Morton, Trond Myklebust, J. Bruce Fields, David S. Miller,
linux-kernel, linux-nfs, netdev
In-Reply-To: <1423571552.31903.468.camel@linux.intel.com>
On Tue, Feb 10 2015, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Tue, 2015-02-10 at 00:44 +0100, Rasmus Villemoes wrote:
>> The current semantics of string_escape_mem are inadequate for one of
>> its current users, vsnprintf(). If that is to honour its contract, it
>> must know how much space would be needed for the entire escaped
>> buffer, and string_escape_mem provides no way of obtaining that (short
>> of allocating a large enough buffer (~4 times input string) to let it
>> play with, and that's definitely a big no-no inside vsnprintf).
>>
>> So change the semantics for string_escape_mem to be more
>> snprintf-like: Return the size of the output that would be generated
>> if the destination buffer was big enough, but of course still only
>> write to the part of dst it is allowed to, and don't do
>> '\0'-termination. It is then up to the caller to detect whether output
>> was truncated and to append a '\0' if desired. Also, we must output
>> partial escape sequences, otherwise a call such as snprintf(buf, 3,
>> "%1pE", "\123") would cause printf to write a \0 to buf[2] but leaving
>> buf[0] and buf[1] with whatever they previously contained.
>>
>> This also fixes a bug in the escaped_string() helper function, which
>> used to unconditionally pass a length of "end-buf" to
>> string_escape_mem(); since the latter doesn't check osz for being
>> insanely large, it would happily write to dst. For example,
>> kasprintf(GFP_KERNEL, "something and then %pE", ...); is an easy way
>> to trigger an oops.
>>
>> In test-string_helpers.c, I removed the now meaningless -ENOMEM test,
>> and replaced it with testing for getting the expected return value
>> even if the buffer is too small. Also ensure that nothing is written
>> when osz == 0.
>>
>> In net/sunrpc/cache.c, I think qword_add still has the same
>> semantics. Someone should definitely double-check this.
>
> Thanks for an update. My comments below.
> After addressing 'em, wrt changes to patch 2/3, take my
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> for all parts except net/sunrpc/cache.c.
>
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> ---
>> index ab0d30e1e18f..5f759c3c2f60 100644
>> --- a/lib/test-string_helpers.c
>> +++ b/lib/test-string_helpers.c
>> @@ -264,12 +264,12 @@ static __init void test_string_escape(const char *name,
>> const struct test_string_2 *s2,
>> unsigned int flags, const char *esc)
>> {
>> - int q_real = 512;
>> - char *out_test = kmalloc(q_real, GFP_KERNEL);
>> - char *out_real = kmalloc(q_real, GFP_KERNEL);
>> + size_t out_size = 512;
>> + char *out_test = kmalloc(out_size, GFP_KERNEL);
>> + char *out_real = kmalloc(out_size, GFP_KERNEL);
>> char *in = kmalloc(256, GFP_KERNEL);
>> - char *buf = out_real;
>> int p = 0, q_test = 0;
>> + int q_real;
>>
>> if (!out_test || !out_real || !in)
>> goto out;
>> @@ -301,29 +301,26 @@ static __init void test_string_escape(const char *name,
>> q_test += len;
>> }
>>
>> - q_real = string_escape_mem(in, p, &buf, q_real, flags, esc);
>> + q_real = string_escape_mem(in, p, out_real, out_size, flags, esc);
>>
>> test_string_check_buf(name, flags, in, p, out_real, q_real, out_test,
>> q_test);
>> +
>> + memset(out_real, 'Z', out_size);
>> + q_real = string_escape_mem(in, p, out_real, 0, flags, esc);
>> + if (q_real != q_test)
>> + pr_warn("Test '%s' failed: flags = %u, osz = 0, expected %d, got %d\n",
>> + name, flags, q_test, q_real);
>> + if (memchr_inv(out_real, 'Z', out_size))
>> + pr_warn("Test '%s' failed: osz = 0 but string_escape_mem wrote to the buffer\n",
>> + name);
>> +
>
> So, why couldn't we split this to separate test case? It seems I already
> pointed this out.
>
This actually provides better coverage since we do this for all the
"positive" test cases, instead of just the single ad hoc case done previously. Of
course the added lines could be factored into a separate helper, but
there's quite a lot of state to pass, so I thought this would actually
be simpler - note how the two string_escape_mem calls are easily seen to
be identical except for the outsize argument.
It may already be too late for the merge window, but I didn't want to
spend too much time on these mostly cosmetic details (that also goes for
the 3- versus 2-line issue).
Rasmus
^ permalink raw reply
* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Eric Dumazet @ 2015-02-10 13:05 UTC (permalink / raw)
To: Michal Kazior
Cc: Neal Cardwell, linux-wireless, Network Development, Eyal Perry
In-Reply-To: <1423572896.28434.13.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, 2015-02-10 at 04:54 -0800, Eric Dumazet wrote:
> Hi Michal
>
> This is almost it ;)
>
> As I said you must do this using u64 arithmetics, we still support 32bit
> kernels.
>
> Also, >> 20 instead of / 1000000 introduces a 5% error, I would use a
> plain divide, as the compiler will use a reciprocal divide (ie : a
> multiply)
>
> We use >> 10 instead of /1000 because a 2.4 % error is probably okay.
>
> ewma_add(&ar->tx_delay_us,
> ktime_to_ns(ktime_sub(ktime_get(),
> skb_cb->stamp)) /
> NSEC_PER_USEC);
btw I suspect this wont compile on 32 bit kernel
You need to use do_div() as well :
u64 val = ktime_to_ns(ktime_sub(ktime_get(),
skb_cb->stamp));
do_div(val, NSEC_PER_USEC);
ewma_add(&ar->tx_delay_us, val);
^ permalink raw reply
* Re: [PATCH v3 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Steven Rostedt @ 2015-02-10 13:05 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Ingo Molnar, Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa,
Masami Hiramatsu, Linux API, Network Development, LKML,
Linus Torvalds
In-Reply-To: <CAMEtUuztTca2jC9Su0YkTHEUHsD4p2PBRyukabrjGO0WfFzdfA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, 9 Feb 2015 22:10:45 -0800
Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> One can argue that current TP_printk format is already an ABI,
> because somebody might be parsing the text output.
If somebody does, then it is an ABI. Luckily, it's not that useful to
parse, thus it hasn't been an issue. As Linus has stated in the past,
it's not that we can't change ABI interfaces, its just that we can not
change them if there's a user space application that depends on it.
The harder we make it for interface changes to break user space, the
better. The field layouts is a user interface. In fact, some of those
fields must always be there. This is because tools do parse the layout
and expect some events to have specific fields. Now we can add new
fields, or even remove fields that no user space tool is using. This is
because today, tools use libtraceevent to parse the event data.
This is why I'm nervous about exporting the parameters of the trace
event call. Right now, those parameters can always change, because the
only way to know they exist is by looking at the code. And currently,
there's no way to interact with those parameters. Once we have eBPF in
mainline, we now have a way to interact with the parameters and if
those parameters change, then the eBPF program will break, and if eBPF
can be part of a user space tool, that will break that tool and
whatever change in the trace point that caused this breakage would have
to be reverted. IOW, this can limit development in the kernel.
Al Viro currently does not let any tracepoint in VFS because he doesn't
want the internals of that code locked to an ABI. He's right to be
worried.
> so in some cases we cannot change tracepoints without
> somebody complaining that his tool broke.
> In other cases tracepoints are used for debugging only
> and no one will notice when they change...
> It was and still a grey area.
Not really. If a tool uses the tracepoint, it can lock that tracepoint
down. This is exactly what latencytop did. It happened, it's not a
hypothetical situation.
> bpf doesn't change any of that.
> It actually makes addition of new tracepoints easier.
I totally disagree. It adds more ways to see inside the kernel, and if
user space depends on this, it adds more ways the kernel can not change.
It comes down to how robust eBPF is with the internals of the kernel
changing. If we limit eBPF to system call tracepoints only, that's
fine because those have the same ABI as the system call itself. I'm
worried about the internal tracepoints for scheduling, irqs, file
systems, etc.
> In the future we might add a tracepoint and pass a single
> pointer to interesting data struct to it. bpf programs will walk
> data structures 'as safe modules' via bpf_fetch*() methods
> without exposing it as ABI.
Will this work if that structure changes? When the field we are looking
for no longer exists?
> whereas today we pass a lot of fields to tracepoints and
> make all of these fields immutable.
The parameters passed to the tracepoint are not shown to userspace and
can change at will. Now, we present the final parsing of the parameters
that convert to fields. As all currently known tools uses
libtraceevent.a, and parse the format files, those fields can move
around and even change in size. The structures are not immutable. The
fields are locked down if user space relies on them. But they can move
about within the tracepoint, because the parsing allows for it.
Remember, these are processed fields. The result of TP_fast_assign()
and what gets put into the ring buffer. Now what is passed to the
actual tracepoint is not visible by userspace, and in lots of cases, it
is just a pointer to some structure. What eBPF brings to the table is a
way to access this structure from user space. What keeps a structured
passed to a tracepoint from becoming immutable if there's a eBPF
program that expects it to have a specific field?
>
> To me tracepoints are like gdb breakpoints.
Unfortunately, it doesn't matter what you think they are. It doesn't
matter what I think they are. What matters is what Linus thinks they
are and if user space depends on it and Linus decides to revert what
ever change broke that user space program, no matter what we think, we
just screwed ourselves.
I'm being stubborn on this because this is exactly what happened in the
past, which caused every trace point to hold 4 bytes of padding. 4
bytes may not sound like a lot, but when you have 1 million
tracepoints, that's 4 megs of wasted space.
> and bpf programs like live debugger that examine things.
If bpf programs only dealt with kprobes, I may agree. But tracepoints
have already been proven to be a type of ABI. If we open another window
into the kernel, this can screw us later. It's better to solve this now
than when we are fighting with Linus over user space breakage.
>
> the next step is to be able to write bpf scripts on the fly
> without leaving debugger. Something like perf probe +
> editor + live execution. Truly like gdb for kernel.
> while kernel is running.
What we need is to know if eBPF programs are modules or a user space
interface. If they are a user interface then we need to be extremely
careful here. If they are treated the same as modules, then it would
not add any API. But that hasn't been settled yet, even if we have a
comment in the kernel.
Maybe what we should do is to make eBPF pass the kernel version it was
made for (with all the mod version checks). If it doesn't match, fail
to load it. Perhaps the more eBPF is limited like modules are, the
better chance we have that no eBPF program creates a new ABI.
-- Steve
^ permalink raw reply
* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Eric Dumazet @ 2015-02-10 13:14 UTC (permalink / raw)
To: Michal Kazior
Cc: Neal Cardwell, linux-wireless, Network Development, Eyal Perry
In-Reply-To: <CA+BoTQ=u_xPuqTVOVaFTQNRrJ+UTXe89SY+=+7Y1LpxxrkRDfg@mail.gmail.com>
On Tue, 2015-02-10 at 11:33 +0100, Michal Kazior wrote:
> ath10k_core_napi_dummy_poll, 64);
> + ewma_init(&ar->tx_delay_us, 16384, 8);
1) 16384 factor might be too big.
2) a weight of 8 seems too low given aggregation values used in wifi.
On 32bit arches, the max range for ewma value would be 262144 usec,
a quarter of a second...
You could use a factor of 64 instead, and a weight of 16.
^ permalink raw reply
* Re: [PATCH] rtlwifi: ratelimit skb allocation failure message
From: Eric Dumazet @ 2015-02-10 13:17 UTC (permalink / raw)
To: Colin King
Cc: Larry Finger, Chaoming Li, Kalle Valo, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1423558471-27278-1-git-send-email-colin.king@canonical.com>
On Tue, 2015-02-10 at 08:54 +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> when running low on memory I noticed rtlwifi was producing a large
> quantity of repeated skb allocation failures messages. This should
> be ratelimited to reduce the noise.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/wireless/rtlwifi/pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
> index c70efb9..ca0fd50 100644
> --- a/drivers/net/wireless/rtlwifi/pci.c
> +++ b/drivers/net/wireless/rtlwifi/pci.c
> @@ -817,7 +817,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
> /* get a new skb - if fail, old one will be reused */
> new_skb = dev_alloc_skb(rtlpci->rxbuffersize);
> if (unlikely(!new_skb)) {
> - pr_err("Allocation of new skb failed in %s\n",
> + pr_err_ratelimited("Allocation of new skb failed in %s\n",
> __func__);
Or even better, remove the message.
^ permalink raw reply
* [PATCH v4 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
From: Rojhalat Ibrahim @ 2015-02-10 13:40 UTC (permalink / raw)
To: linux-gpio@vger.kernel.org
Cc: Alexandre Courbot, Alexandre Courbot, Linus Walleij, David Miller,
netdev, Mika Westerberg, Rafael J. Wysocki
Use the new gpiod_get_array and gpiod_put_array functions for obtaining and
disposing of GPIO descriptors.
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
---
Change log:
v4: use shorter names for members of struct gpio_descs
v3: no change
v2: use the new interface
drivers/net/phy/mdio-mux-gpio.c | 60 ++++++++++++-----------------------------
1 file changed, 17 insertions(+), 43 deletions(-)
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index 320eb15..c49ad09 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -12,33 +12,30 @@
#include <linux/module.h>
#include <linux/phy.h>
#include <linux/mdio-mux.h>
-#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
#define DRV_VERSION "1.1"
#define DRV_DESCRIPTION "GPIO controlled MDIO bus multiplexer driver"
-#define MDIO_MUX_GPIO_MAX_BITS 8
-
struct mdio_mux_gpio_state {
- struct gpio_desc *gpio[MDIO_MUX_GPIO_MAX_BITS];
- unsigned int num_gpios;
+ struct gpio_descs *gpios;
void *mux_handle;
};
static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
void *data)
{
- int values[MDIO_MUX_GPIO_MAX_BITS];
- unsigned int n;
struct mdio_mux_gpio_state *s = data;
+ int values[s->gpios->ndescs];
+ unsigned int n;
if (current_child == desired_child)
return 0;
- for (n = 0; n < s->num_gpios; n++) {
+ for (n = 0; n < s->gpios->ndescs; n++)
values[n] = (desired_child >> n) & 1;
- }
- gpiod_set_array_cansleep(s->num_gpios, s->gpio, values);
+
+ gpiod_set_array_cansleep(s->gpios->ndescs, s->gpios->desc, values);
return 0;
}
@@ -46,56 +43,33 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
static int mdio_mux_gpio_probe(struct platform_device *pdev)
{
struct mdio_mux_gpio_state *s;
- int num_gpios;
- unsigned int n;
int r;
- if (!pdev->dev.of_node)
- return -ENODEV;
-
- num_gpios = of_gpio_count(pdev->dev.of_node);
- if (num_gpios <= 0 || num_gpios > MDIO_MUX_GPIO_MAX_BITS)
- return -ENODEV;
-
s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
if (!s)
return -ENOMEM;
- s->num_gpios = num_gpios;
-
- for (n = 0; n < num_gpios; ) {
- struct gpio_desc *gpio = gpiod_get_index(&pdev->dev, NULL, n,
- GPIOD_OUT_LOW);
- if (IS_ERR(gpio)) {
- r = PTR_ERR(gpio);
- goto err;
- }
- s->gpio[n] = gpio;
- n++;
- }
+ s->gpios = gpiod_get_array(&pdev->dev, NULL, GPIOD_OUT_LOW);
+ if (IS_ERR(s->gpios))
+ return PTR_ERR(s->gpios);
r = mdio_mux_init(&pdev->dev,
mdio_mux_gpio_switch_fn, &s->mux_handle, s);
- if (r == 0) {
- pdev->dev.platform_data = s;
- return 0;
- }
-err:
- while (n) {
- n--;
- gpiod_put(s->gpio[n]);
+ if (r != 0) {
+ gpiod_put_array(s->gpios);
+ return r;
}
- return r;
+
+ pdev->dev.platform_data = s;
+ return 0;
}
static int mdio_mux_gpio_remove(struct platform_device *pdev)
{
- unsigned int n;
struct mdio_mux_gpio_state *s = dev_get_platdata(&pdev->dev);
mdio_mux_uninit(s->mux_handle);
- for (n = 0; n < s->num_gpios; n++)
- gpiod_put(s->gpio[n]);
+ gpiod_put_array(s->gpios);
return 0;
}
--
2.0.5
^ permalink raw reply related
* Re: [PATCH] rtlwifi: ratelimit skb allocation failure message
From: Kalle Valo @ 2015-02-10 13:48 UTC (permalink / raw)
To: Eric Dumazet
Cc: Colin King, Larry Finger, Chaoming Li, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1423574228.28434.22.camel@edumazet-glaptop2.roam.corp.google.com>
Eric Dumazet <eric.dumazet@gmail.com> writes:
> On Tue, 2015-02-10 at 08:54 +0000, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> when running low on memory I noticed rtlwifi was producing a large
>> quantity of repeated skb allocation failures messages. This should
>> be ratelimited to reduce the noise.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/net/wireless/rtlwifi/pci.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
>> index c70efb9..ca0fd50 100644
>> --- a/drivers/net/wireless/rtlwifi/pci.c
>> +++ b/drivers/net/wireless/rtlwifi/pci.c
>> @@ -817,7 +817,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
>> /* get a new skb - if fail, old one will be reused */
>> new_skb = dev_alloc_skb(rtlpci->rxbuffersize);
>> if (unlikely(!new_skb)) {
>> - pr_err("Allocation of new skb failed in %s\n",
>> + pr_err_ratelimited("Allocation of new skb failed in %s\n",
>> __func__);
>
> Or even better, remove the message.
There's actually a pending patch for that, I'll send it to Dave ASAP:
https://patchwork.kernel.org/patch/5671121/
--
Kalle Valo
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2015-02-07
From: Kalle Valo @ 2015-02-10 13:49 UTC (permalink / raw)
To: David Miller
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150209.121342.1535841700027616341.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:
> From: Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Date: Sat, 07 Feb 2015 13:40:51 +0200
>
>> There's a small conflict in drivers/net/wireless/rtlwifi/pci.c, the fix
>> is to leave the two labels like this:
>>
>> schedule_work(&rtlpriv->works.lps_change_work);
>> }
>> end:
>> skb = new_skb;
>> no_new:
>> if (rtlpriv->use_new_trx_flow) {
>>
>>
>
> That can't be the correct resolution:
>
> drivers/net/wireless/rtlwifi/pci.c: In function ‘_rtl_pci_rx_interrupt’:
> drivers/net/wireless/rtlwifi/pci.c:934:1: warning: label ‘end’ defined but not used [-Wunused-label]
>
> So I've removed that label in the merge commit.
Sorry about that, but great that you catched my mistake.
--
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] rtlwifi: ratelimit skb allocation failure message
From: Colin Ian King @ 2015-02-10 13:52 UTC (permalink / raw)
To: Kalle Valo, Eric Dumazet
Cc: Larry Finger, Chaoming Li, linux-wireless, netdev, linux-kernel
In-Reply-To: <87a90lop1x.fsf@kamboji.qca.qualcomm.com>
On 10/02/15 13:48, Kalle Valo wrote:
> Eric Dumazet <eric.dumazet@gmail.com> writes:
>
>> On Tue, 2015-02-10 at 08:54 +0000, Colin King wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> when running low on memory I noticed rtlwifi was producing a large
>>> quantity of repeated skb allocation failures messages. This should
>>> be ratelimited to reduce the noise.
>>>
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>> ---
>>> drivers/net/wireless/rtlwifi/pci.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
>>> index c70efb9..ca0fd50 100644
>>> --- a/drivers/net/wireless/rtlwifi/pci.c
>>> +++ b/drivers/net/wireless/rtlwifi/pci.c
>>> @@ -817,7 +817,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
>>> /* get a new skb - if fail, old one will be reused */
>>> new_skb = dev_alloc_skb(rtlpci->rxbuffersize);
>>> if (unlikely(!new_skb)) {
>>> - pr_err("Allocation of new skb failed in %s\n",
>>> + pr_err_ratelimited("Allocation of new skb failed in %s\n",
>>> __func__);
>>
>> Or even better, remove the message.
>
> There's actually a pending patch for that, I'll send it to Dave ASAP:
>
> https://patchwork.kernel.org/patch/5671121/
>
Thanks!
^ permalink raw reply
* Re: bride: IPv6 multicast snooping enhancements
From: Vasily Averin @ 2015-02-10 13:59 UTC (permalink / raw)
To: Linus Lüssing
Cc: Herbert Xu, netdev, bridge, linux-kernel, David S. Miller,
Cong Wang
In-Reply-To: <20150210114428.GK2489@odroid>
On 10.02.2015 14:44, Linus Lüssing wrote:
> Hi Vasily,
>
> On Tue, Feb 10, 2015 at 11:44:29AM +0300, Vasily Averin wrote:
>> This patch prevent forwarding of ICMPv6 in bridges,
>> so containers/VMs with virtual eth adapters connected in local bridge cannot ping each other via ipv6 (but can do it via ipv4)
>
> If a host wants to receive packets, then it needs to signalize
> that via MLD. If your host does not do that, then it is expected
> to not receive ICMPv6 echo requests to multicast addresses. An
> exception is ff02::1, that should always work.
Thank you for explanation, seems now I understand finally how it should work.
I'm trying to fix ICMPv6 processing broken in OpenVZ after rebase to last RHEL6u6 kernel.
After some unclear manipulation bridge begins to forward icmp6 NS (fe02::1) into wrong port,
and at present I do not found the reason of this failure.
Thank you,
Vasily Averin
^ permalink raw reply
* Re: [PATCH net-next 6/6] ipv6: Allow for partial checksums on non-ufo packets
From: Sabrina Dubroca @ 2015-02-10 14:07 UTC (permalink / raw)
To: Vladislav Yasevich; +Cc: netdev, herbert, hannes, Vladislav Yasevich
In-Reply-To: <1422718818-21093-7-git-send-email-vyasevic@redhat.com>
2015-01-31, 10:40:18 -0500, Vladislav Yasevich wrote:
> Currntly, if we are not doing UFO on the packet, all UDP
> packets will start with CHECKSUM_NONE and thus perform full
> checksum computations in software even if device support
> IPv6 checksum offloading.
>
> Let's start start with CHECKSUM_PARTIAL if the device
> supports it and we are sending only a single packet at
> or below mtu size.
>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
This patch causes ICMPv6 checksumming issues for me.
On my tg3 device and on a qemu VM with e1000 emulation, outgoing pings
have a bad checksum. Router solicitations also have a bad checksum,
so autoconf fails. When I revert this patch, or when I disable
tx-checksumming with ethtool, everything looks okay again.
On tg3, replies to ping seem always good.
On e1000, replies to ping work (more or less). Sometimes the checksum
is bad, sometimes it's good.
% ping6 fec0::3456
PING fec0::3456(fec0::3456) 56 data bytes
64 bytes from fec0::3456: icmp_seq=1 ttl=64 time=0.433 ms
64 bytes from fec0::3456: icmp_seq=2 ttl=64 time=0.457 ms
64 bytes from fec0::3456: icmp_seq=10 ttl=64 time=0.448 ms
64 bytes from fec0::3456: icmp_seq=11 ttl=64 time=0.451 ms
64 bytes from fec0::3456: icmp_seq=18 ttl=64 time=0.485 ms
64 bytes from fec0::3456: icmp_seq=20 ttl=64 time=0.476 ms
64 bytes from fec0::3456: icmp_seq=22 ttl=64 time=0.448 ms
64 bytes from fec0::3456: icmp_seq=26 ttl=64 time=0.438 ms
64 bytes from fec0::3456: icmp_seq=27 ttl=64 time=0.413 ms
64 bytes from fec0::3456: icmp_seq=28 ttl=64 time=0.452 ms
64 bytes from fec0::3456: icmp_seq=29 ttl=64 time=0.440 ms
64 bytes from fec0::3456: icmp_seq=30 ttl=64 time=0.485 ms
64 bytes from fec0::3456: icmp_seq=32 ttl=64 time=0.473 ms
64 bytes from fec0::3456: icmp_seq=33 ttl=64 time=0.472 ms
64 bytes from fec0::3456: icmp_seq=34 ttl=64 time=0.395 ms
64 bytes from fec0::3456: icmp_seq=35 ttl=64 time=0.456 ms
64 bytes from fec0::3456: icmp_seq=36 ttl=64 time=0.409 ms
^C
--- fec0::3456 ping statistics ---
36 packets transmitted, 17 received, 52% packet loss, time 34998ms
rtt min/avg/max/mdev = 0.395/0.448/0.485/0.037 ms
I've seen a few strange source addresses, but I don't know if it's
related.
% ping6 fec0::3456
PING fec0::3456(fec0::3456) 56 data bytes
64 bytes from fec0::ff:ff00:0:3456: icmp_seq=1 ttl=64 time=0.423 ms <---
64 bytes from fec0::3456: icmp_seq=4 ttl=64 time=0.396 ms
64 bytes from fec0::3456: icmp_seq=5 ttl=64 time=0.400 ms
This could be a driver issue, or just exposing another problem
somewhere else, I don't know.
Any idea?
Thanks
--
Sabrina
^ permalink raw reply
* [3.16.y-ckt stable] Patch "lib/checksum.c: fix carry in csum_tcpudp_nofold" has been added to staging queue
From: Luis Henriques @ 2015-02-10 14:09 UTC (permalink / raw)
To: karl beldan
Cc: Karl Beldan, Al Viro, Eric Dumazet, Arnd Bergmann, Mike Frysinger,
netdev, linux-kernel, Eric Dumazet, David S. Miller,
Luis Henriques, kernel-team
This is a note to let you know that I have just added a patch titled
lib/checksum.c: fix carry in csum_tcpudp_nofold
to the linux-3.16.y-queue branch of the 3.16.y-ckt extended stable tree
which can be found at:
http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.16.y-queue
This patch is scheduled to be released in version 3.16.7-ckt7.
If you, or anyone else, feels it should not be added to this tree, please
reply to this email.
For more information about the 3.16.y-ckt tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
Thanks.
-Luis
------
>From 0a7d96f7a48c2bc383d413bb9ca999704eace496 Mon Sep 17 00:00:00 2001
From: karl beldan <karl.beldan@gmail.com>
Date: Wed, 28 Jan 2015 10:58:11 +0100
Subject: lib/checksum.c: fix carry in csum_tcpudp_nofold
commit 150ae0e94634714b23919f0c333fee28a5b199d5 upstream.
The carry from the 64->32bits folding was dropped, e.g with:
saddr=0xFFFFFFFF daddr=0xFF0000FF len=0xFFFF proto=0 sum=1,
csum_tcpudp_nofold returned 0 instead of 1.
Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
lib/checksum.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/lib/checksum.c b/lib/checksum.c
index 129775eb6de6..fcf38943132c 100644
--- a/lib/checksum.c
+++ b/lib/checksum.c
@@ -47,6 +47,15 @@ static inline unsigned short from32to16(unsigned int x)
return x;
}
+static inline u32 from64to32(u64 x)
+{
+ /* add up 32-bit and 32-bit for 32+c bit */
+ x = (x & 0xffffffff) + (x >> 32);
+ /* add up carry.. */
+ x = (x & 0xffffffff) + (x >> 32);
+ return (u32)x;
+}
+
static unsigned int do_csum(const unsigned char *buff, int len)
{
int odd;
@@ -195,8 +204,7 @@ __wsum csum_tcpudp_nofold(__be32 saddr, __be32 daddr,
#else
s += (proto + len) << 8;
#endif
- s += (s >> 32);
- return (__force __wsum)s;
+ return (__force __wsum)from64to32(s);
}
EXPORT_SYMBOL(csum_tcpudp_nofold);
#endif
--
2.1.4
^ permalink raw reply related
* Re: [V2, for, 3.19, 1/7] rtlwifi: Remove logging statement that is no longer needed
From: Kalle Valo @ 2015-02-10 14:10 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, Larry Finger, netdev, Stable
In-Reply-To: <1421773286-1039-2-git-send-email-Larry.Finger@lwfinger.net>
> In commit e9538cf4f907 ("rtlwifi: Fix error when accessing unmapped memory
> in skb"), a printk was included to indicate that the condition had been
> reached. There is now enough evidence from other users that the fix is
> working. That logging statement can now be removed.
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Stable <stable@vger.kernel.org> [V3.18]
Thanks, applied to wireless-drivers.git.
Kalle Valo
^ permalink raw reply
* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Johannes Berg @ 2015-02-10 14:19 UTC (permalink / raw)
To: Michal Kazior
Cc: Eric Dumazet, Neal Cardwell, linux-wireless, Network Development,
Eyal Perry
In-Reply-To: <CA+BoTQ=u_xPuqTVOVaFTQNRrJ+UTXe89SY+=+7Y1LpxxrkRDfg@mail.gmail.com>
On Tue, 2015-02-10 at 11:33 +0100, Michal Kazior wrote:
> + if (msdu->sk) {
> + ewma_add(&ar->tx_delay_us,
> + ktime_to_ns(ktime_sub(ktime_get(), skb_cb->stamp)) /
> + NSEC_PER_USEC);
> +
> + ACCESS_ONCE(msdu->sk->sk_tx_completion_delay_cushion) =
> + (ewma_read(&ar->tx_delay_us) *
> + msdu->sk->sk_pacing_rate) >> 20;
> + }
To some extent, every wifi driver is going to have this problem. Perhaps
we should do this in mac80211?
johannes
^ permalink raw reply
* [PATCH] rds: rds_cong_queue_updates needs to defer the congestion update transmission
From: Sowmini Varadhan @ 2015-02-10 14:22 UTC (permalink / raw)
To: chien.yen, davem
Cc: rds-devel, netdev, linux-kernel, sowmini.varadhan, chuck.lever
This patch fixes a sock_lock deadlock in the rds_cong_queue_update path.
We cannot inline the call to rds_send_xmit from rds_cong_queue_update
because
(a) we are already holding the sock_lock in the recv path, and
will deadlock when tcp_setsockopt/tcp_sendmsg try to get the sock
lock
(b) cong_queue_update does an irqsave on the rds_cong_lock, and this
will trigger warnings (for a good reason) from functions called
out of sock_lock.
Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
net/rds/cong.c | 16 +++++++++++++++-
1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/net/rds/cong.c b/net/rds/cong.c
index e5b65ac..765d18f 100644
--- a/net/rds/cong.c
+++ b/net/rds/cong.c
@@ -221,7 +221,21 @@ void rds_cong_queue_updates(struct rds_cong_map *map)
list_for_each_entry(conn, &map->m_conn_list, c_map_item) {
if (!test_and_set_bit(0, &conn->c_map_queued)) {
rds_stats_inc(s_cong_update_queued);
- rds_send_xmit(conn);
+ /* We cannot inline the call to rds_send_xmit() here
+ * for two reasons:
+ * 1. When we get here from the receive path, we
+ * are already holding the sock_lock (held by
+ * tcp_v4_rcv()). So inlining calls to
+ * tcp_setsockopt and/or tcp_sendmsg will deadlock
+ * when it tries to get the sock_lock())
+ * 2. Interrupts are masked so that we can mark the
+ * the port congested from both send and recv paths.
+ * (See comment around declaration of rds_cong_lock).
+ * An attempt to get the sock_lock() here will
+ * therefore trigger warnings.
+ * Defer the xmit to rds_send_worker() instead.
+ */
+ queue_delayed_work(rds_wq, &conn->c_send_w, 0);
}
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v3 3/3] lib/string_helpers.c: Change semantics of string_escape_mem
From: Andy Shevchenko @ 2015-02-10 14:22 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Andrew Morton, Trond Myklebust, J. Bruce Fields, David S. Miller,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87386dj4x0.fsf-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
On Tue, 2015-02-10 at 14:02 +0100, Rasmus Villemoes wrote:
> On Tue, Feb 10 2015, Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>
> > On Tue, 2015-02-10 at 00:44 +0100, Rasmus Villemoes wrote:
> >> The current semantics of string_escape_mem are inadequate for one of
> >> its current users, vsnprintf(). If that is to honour its contract, it
> >> must know how much space would be needed for the entire escaped
> >> buffer, and string_escape_mem provides no way of obtaining that (short
> >> of allocating a large enough buffer (~4 times input string) to let it
> >> play with, and that's definitely a big no-no inside vsnprintf).
> >>
> >> So change the semantics for string_escape_mem to be more
> >> snprintf-like: Return the size of the output that would be generated
> >> if the destination buffer was big enough, but of course still only
> >> write to the part of dst it is allowed to, and don't do
> >> '\0'-termination. It is then up to the caller to detect whether output
> >> was truncated and to append a '\0' if desired. Also, we must output
> >> partial escape sequences, otherwise a call such as snprintf(buf, 3,
> >> "%1pE", "\123") would cause printf to write a \0 to buf[2] but leaving
> >> buf[0] and buf[1] with whatever they previously contained.
> >>
> >> This also fixes a bug in the escaped_string() helper function, which
> >> used to unconditionally pass a length of "end-buf" to
> >> string_escape_mem(); since the latter doesn't check osz for being
> >> insanely large, it would happily write to dst. For example,
> >> kasprintf(GFP_KERNEL, "something and then %pE", ...); is an easy way
> >> to trigger an oops.
> >>
> >> In test-string_helpers.c, I removed the now meaningless -ENOMEM test,
> >> and replaced it with testing for getting the expected return value
> >> even if the buffer is too small. Also ensure that nothing is written
> >> when osz == 0.
> >>
> >> In net/sunrpc/cache.c, I think qword_add still has the same
> >> semantics. Someone should definitely double-check this.
> >
> > Thanks for an update. My comments below.
> > After addressing 'em, wrt changes to patch 2/3, take my
> > Acked-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >
> > for all parts except net/sunrpc/cache.c.
> >
> >>
> >> Signed-off-by: Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
> >> ---
> >> index ab0d30e1e18f..5f759c3c2f60 100644
> >> --- a/lib/test-string_helpers.c
> >> +++ b/lib/test-string_helpers.c
> >> @@ -264,12 +264,12 @@ static __init void test_string_escape(const char *name,
> >> const struct test_string_2 *s2,
> >> unsigned int flags, const char *esc)
> >> {
> >> - int q_real = 512;
> >> - char *out_test = kmalloc(q_real, GFP_KERNEL);
> >> - char *out_real = kmalloc(q_real, GFP_KERNEL);
> >> + size_t out_size = 512;
> >> + char *out_test = kmalloc(out_size, GFP_KERNEL);
> >> + char *out_real = kmalloc(out_size, GFP_KERNEL);
> >> char *in = kmalloc(256, GFP_KERNEL);
> >> - char *buf = out_real;
> >> int p = 0, q_test = 0;
> >> + int q_real;
> >>
> >> if (!out_test || !out_real || !in)
> >> goto out;
> >> @@ -301,29 +301,26 @@ static __init void test_string_escape(const char *name,
> >> q_test += len;
> >> }
> >>
> >> - q_real = string_escape_mem(in, p, &buf, q_real, flags, esc);
> >> + q_real = string_escape_mem(in, p, out_real, out_size, flags, esc);
> >>
> >> test_string_check_buf(name, flags, in, p, out_real, q_real, out_test,
> >> q_test);
> >> +
> >> + memset(out_real, 'Z', out_size);
> >> + q_real = string_escape_mem(in, p, out_real, 0, flags, esc);
> >> + if (q_real != q_test)
> >> + pr_warn("Test '%s' failed: flags = %u, osz = 0, expected %d, got %d\n",
> >> + name, flags, q_test, q_real);
> >> + if (memchr_inv(out_real, 'Z', out_size))
> >> + pr_warn("Test '%s' failed: osz = 0 but string_escape_mem wrote to the buffer\n",
> >> + name);
> >> +
> >
> > So, why couldn't we split this to separate test case? It seems I already
> > pointed this out.
> >
>
> This actually provides better coverage
I do not see much advantage of doing so. You may create a loop with
random number for in-size and check. So, I prefer to see separate case
for that.
> since we do this for all the
> "positive" test cases, instead of just the single ad hoc case done previously. Of
> course the added lines could be factored into a separate helper, but
> there's quite a lot of state to pass, so I thought this would actually
> be simpler - note how the two string_escape_mem calls are easily seen to
> be identical except for the outsize argument.
>
> It may already be too late for the merge window, but I didn't want to
> spend too much time on these mostly cosmetic details (that also goes for
> the 3- versus 2-line issue).
Yes, too late, thus it's enough time to address my comments :-)
On the other hand I actually don't know if it's a good idea to push this
series to stable. I guess you may just put Fixes: tags in the patches
1/3, 3/3 w/o Cc'ing to stable since we have no issues with current
users.
--
Andy Shevchenko <andriy.shevchenko-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 linux-trace 0/8] tracing: attach eBPF programs to tracepoints/syscalls/kprobe
From: Steven Rostedt @ 2015-02-10 14:55 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Ingo Molnar, Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa,
Masami Hiramatsu, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1423539961-21792-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
On Mon, 9 Feb 2015 19:45:53 -0800
Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> So the overhead of realistic bpf program is 5.05963/4.80074 = ~5%
> which is faster than perf_event filtering: 5.69732/4.80074 = ~18%
> or ftrace filtering: 6.50091/4.80074 = ~35%
Come to think of it, this is comparing apples to oranges, as you move
the filtering before the recording. It would be interesting to see the
ftrace speed up, if it were to use eBPF instead of its own filtering.
Maybe that 35% is the filter part, and not the discard part.
I just tried the dd test with count==1234 and count!=1234 and the one
that drops events is only slightly slower. In this case it does seem
that the most overhead is in the filter logic.
But by moving it before the recording, we can not use the fields
defined in the format files, as the parameters and the fields do not
match in most trace points. And to use the parameters, as I have
stated, there's no interface to know what those parameters are, then
filtering on them is a one shot deal. Might as well write a module and
hook directly to the tracepoint and do the filtering natively. That
would be faster than BPF too.
My point is, what's the use case? If you filter before recording, you
can not use the fields of the tracepoint. That limits you to filtering
only syscalls, and perhaps kprobes.
-- Steve
^ permalink raw reply
* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Eric Dumazet @ 2015-02-10 15:09 UTC (permalink / raw)
To: Johannes Berg
Cc: Michal Kazior, Neal Cardwell, linux-wireless, Network Development,
Eyal Perry
In-Reply-To: <1423577962.2215.2.camel@sipsolutions.net>
On Tue, 2015-02-10 at 15:19 +0100, Johannes Berg wrote:
> On Tue, 2015-02-10 at 11:33 +0100, Michal Kazior wrote:
>
> > + if (msdu->sk) {
> > + ewma_add(&ar->tx_delay_us,
> > + ktime_to_ns(ktime_sub(ktime_get(), skb_cb->stamp)) /
> > + NSEC_PER_USEC);
> > +
> > + ACCESS_ONCE(msdu->sk->sk_tx_completion_delay_cushion) =
> > + (ewma_read(&ar->tx_delay_us) *
> > + msdu->sk->sk_pacing_rate) >> 20;
> > + }
>
> To some extent, every wifi driver is going to have this problem. Perhaps
> we should do this in mac80211?
I'll provide the TCP patch.
sk->sk_tx_completion_delay_cushion is probably a wrong name, as the
units here are in bytes, since it is really number of bytes in the
network driver that accommodate for tx completions delays.
tx_completion_delay * pacing_rate
sk_tx_completion_cushion maybe.
^ permalink raw reply
* NetDev 0.1 online registration deadline Feb 12 (Thursday)
From: Richard Guy Briggs @ 2015-02-10 15:21 UTC (permalink / raw)
To: netdev, linux-wireless, lwn, netdev01, lartc, netfilter,
netfilter-devel, swan-dev, swan, users, dev
Cc: info, speakers, attendees
Fellow netheads:
NetDev 0.1 starts in 4 days.
Registration:
==========================
Online registration ends Thursday the 12th at midnight. Registration is
possible on site, but may incur a late registration fee.
https://onlineregistrations.ca/netdev01/
$100/day, or $350 for 4 days (Cdn dollars).
There have been additions made to the "travel" section of the web site
concerning local transit details, weather and clothing. Please check it
before travelling so you know what to expect.
Travel/hotel/weather/clothing: https://www.netdev01.org/travel
Hotel:
==========================
The Westin Hotel may still have rooms for Netdev01 available.
There are lots of big and small hotels in Ottawa, but they will fill up
fast as Winterlude approaches, so book something soon. There may be
some possibilities to billet locally, but don't leave it last minute.
Make your reservations at:
https://www.starwoodmeeting.com/StarGroupsWeb/res?id=1412035802&key=1AC9C1F8
Schedule:
==========================
The final schedule is out coded with session links.
https://www.netdev01.org/schedule
Sponsors:
=========
NetDev 0.1 would like to gratefully acknowledge all our sponsors:
https://netdev01.org/sponsors
Intel http://www.intel.com/
CENGN http://www.cengn.ca/
Google https://www.google.ca
Qualcomm https://www.qualcomm.com/
Verizon http://www.verizon.com/
Cumulus Networks http://cumulusnetworks.com/
Mojatatu Networks http://mojatatu.com/
-------------------------------------------------------------------------------
THE Technical Conference on Linux Networking, February 14-17, 2015, Ottawa, Canada
https://netdev01.org/
Contact: info@netdev01.info
Main site: https://www.netdev01.org/
Travel/hotel/weather/clothing: https://www.netdev01.org/travel
RSS feed: https://netdev01.org/atom
Follow us on Twitter: @netdev01 https://twitter.com/netdev01
slainte mhath, RGB
--
Richard Guy Briggs -- ~\ -- ~\ <hpv.tricolour.ca>
<www.TriColour.ca> -- \___ o \@ @ Ride yer bike!
Ottawa, ON, CANADA -- Lo_>__M__\\/\%__\\/\%
Vote! -- <greenparty.ca>_____GTVS6#790__(*)__(*)________(*)(*)_________________
^ permalink raw reply
* Kernel sends ICMP unreachable for GRE packets even if there is a listening socket
From: Steinar H. Gunderson @ 2015-02-10 15:24 UTC (permalink / raw)
To: netdev
[Resent from Bugzilla]
Hi,
I have a userspace GRE listener, which opens a raw socket (error handling removed):
int gresock = socket(AF_INET6, SOCK_RAW, IPPROTO_GRE);
bind(gresock, (sockaddr *)&my_addr, sizeof(my_addr));
and then select()s and recvfrom()s on it, as well as sendto().
This works great. I can send and receive GRE packets. However, Linux _also_
sends ICMPv6 unreachables when the other end sends to me. I assume this is
because I don't have a corresponding kernel GRE tunnel interface. (I don't use
the kernel GRE because I want, among others, stronger reordering functionality
and possibly error correction, which is not in any standard.)
Can this be suppressed when I have such a socket giong? ip6tables -A OUTPUT can
stop them, but it seems very hacky and kludgy.
/* Steinar */
--
Homepage: http://www.sesse.net/
^ permalink raw reply
* Re: [PATCH net-next 6/6] ipv6: Allow for partial checksums on non-ufo packets
From: Vlad Yasevich @ 2015-02-10 15:34 UTC (permalink / raw)
To: Sabrina Dubroca, Vladislav Yasevich; +Cc: netdev, herbert, hannes
In-Reply-To: <20150210140704.GA3372@kria>
On 02/10/2015 09:07 AM, Sabrina Dubroca wrote:
> 2015-01-31, 10:40:18 -0500, Vladislav Yasevich wrote:
>> Currntly, if we are not doing UFO on the packet, all UDP
>> packets will start with CHECKSUM_NONE and thus perform full
>> checksum computations in software even if device support
>> IPv6 checksum offloading.
>>
>> Let's start start with CHECKSUM_PARTIAL if the device
>> supports it and we are sending only a single packet at
>> or below mtu size.
>>
>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>> ---
>
> This patch causes ICMPv6 checksumming issues for me.
>
> On my tg3 device and on a qemu VM with e1000 emulation, outgoing pings
> have a bad checksum. Router solicitations also have a bad checksum,
> so autoconf fails. When I revert this patch, or when I disable
> tx-checksumming with ethtool, everything looks okay again.
>
> On tg3, replies to ping seem always good.
>
> On e1000, replies to ping work (more or less). Sometimes the checksum
> is bad, sometimes it's good.
>
>
> % ping6 fec0::3456
> PING fec0::3456(fec0::3456) 56 data bytes
> 64 bytes from fec0::3456: icmp_seq=1 ttl=64 time=0.433 ms
> 64 bytes from fec0::3456: icmp_seq=2 ttl=64 time=0.457 ms
> 64 bytes from fec0::3456: icmp_seq=10 ttl=64 time=0.448 ms
> 64 bytes from fec0::3456: icmp_seq=11 ttl=64 time=0.451 ms
> 64 bytes from fec0::3456: icmp_seq=18 ttl=64 time=0.485 ms
> 64 bytes from fec0::3456: icmp_seq=20 ttl=64 time=0.476 ms
> 64 bytes from fec0::3456: icmp_seq=22 ttl=64 time=0.448 ms
> 64 bytes from fec0::3456: icmp_seq=26 ttl=64 time=0.438 ms
> 64 bytes from fec0::3456: icmp_seq=27 ttl=64 time=0.413 ms
> 64 bytes from fec0::3456: icmp_seq=28 ttl=64 time=0.452 ms
> 64 bytes from fec0::3456: icmp_seq=29 ttl=64 time=0.440 ms
> 64 bytes from fec0::3456: icmp_seq=30 ttl=64 time=0.485 ms
> 64 bytes from fec0::3456: icmp_seq=32 ttl=64 time=0.473 ms
> 64 bytes from fec0::3456: icmp_seq=33 ttl=64 time=0.472 ms
> 64 bytes from fec0::3456: icmp_seq=34 ttl=64 time=0.395 ms
> 64 bytes from fec0::3456: icmp_seq=35 ttl=64 time=0.456 ms
> 64 bytes from fec0::3456: icmp_seq=36 ttl=64 time=0.409 ms
> ^C
> --- fec0::3456 ping statistics ---
> 36 packets transmitted, 17 received, 52% packet loss, time 34998ms
> rtt min/avg/max/mdev = 0.395/0.448/0.485/0.037 ms
>
>
> I've seen a few strange source addresses, but I don't know if it's
> related.
>
> % ping6 fec0::3456
> PING fec0::3456(fec0::3456) 56 data bytes
> 64 bytes from fec0::ff:ff00:0:3456: icmp_seq=1 ttl=64 time=0.423 ms <---
> 64 bytes from fec0::3456: icmp_seq=4 ttl=64 time=0.396 ms
> 64 bytes from fec0::3456: icmp_seq=5 ttl=64 time=0.400 ms
>
>
> This could be a driver issue, or just exposing another problem
> somewhere else, I don't know.
>
> Any idea?
Hi Sabrina
Thanks for reporting this. I'll take a look.
-vlad
>
>
> Thanks
>
^ permalink raw reply
* Re: [PATCH 1/3] rhashtable: require max_shift definition
From: Josh Hunt @ 2015-02-10 15:56 UTC (permalink / raw)
To: Daniel Borkmann, Thomas Graf
Cc: Pablo Neira Ayuso, Patrick McHardy, netfilter-devel, netdev
In-Reply-To: <54D9C18B.5090208@iogearbox.net>
On 02/10/2015 02:30 AM, Daniel Borkmann wrote:
> On 02/10/2015 01:58 AM, Thomas Graf wrote:
>> On 02/09/15 at 07:48pm, Josh Hunt wrote:
>>> if ((params->key_len && !params->hashfn) ||
>>> - (!params->key_len && !params->obj_hashfn))
>>> + (!params->key_len && !params->obj_hashfn) ||
>>> + (!params->max_shift))
>>> return -EINVAL;
>>
>> You can drop the parenthesis around the new max_shift check.
>
> Also, I think the test should be expanded to check if there's
> a grow_decision given and only in that case require max_shift
> to be non-zero, otherwise we would require users who don't
> want to expand their table to give a upper expansion limit.
This is a good point. I'll make this change.
max_shift restricts the # of buckets, but should there be an optional
parameter, maxelems, to set a ceiling on the # of elements in a table
also? If not, I believe users will be able to add an "unlimited" # of
entries to the existing buckets, whether or not a grow_decision fn is
defined.
Josh
^ permalink raw reply
* [PATCH] ipv6: Partial checksum only UDP packets
From: Vladislav Yasevich @ 2015-02-10 15:55 UTC (permalink / raw)
To: sd; +Cc: netdev, Vladislav Yasevich
In-Reply-To: <20150210140704.GA3372@kria>
ip6_append_data is used by other protocols and some of them can't
be partially checksummed. Only partially checksum UDP protocol.
Fixes: 32dce968dd987a (ipv6: Allow for partial checksums on non-ufo packets)
Reported-by: Sabrian Dubroca <sd@queasysnail.net>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
Hi Sabrina
Can you try this patch. Thanks.
net/ipv6/ip6_output.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index d33df4c..8f2d558 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1273,7 +1273,7 @@ emsgsize:
/* If this is the first and only packet and device
* supports checksum offloading, let's use it.
*/
- if (!skb &&
+ if (!skb && sk->protocol == IPPROTO_UDP &&
length + fragheaderlen < mtu &&
rt->dst.dev->features & NETIF_F_V6_CSUM &&
!exthdrlen)
--
1.9.3
^ permalink raw reply related
* Re: Kernel sends ICMP unreachable for GRE packets even if there is a listening socket
From: Eric Dumazet @ 2015-02-10 16:09 UTC (permalink / raw)
To: Steinar H. Gunderson; +Cc: netdev
In-Reply-To: <20150210152410.GA9225@sesse.net>
On Tue, 2015-02-10 at 16:24 +0100, Steinar H. Gunderson wrote:
> [Resent from Bugzilla]
>
> Hi,
>
> I have a userspace GRE listener, which opens a raw socket (error handling removed):
>
> int gresock = socket(AF_INET6, SOCK_RAW, IPPROTO_GRE);
> bind(gresock, (sockaddr *)&my_addr, sizeof(my_addr));
>
> and then select()s and recvfrom()s on it, as well as sendto().
>
> This works great. I can send and receive GRE packets. However, Linux _also_
> sends ICMPv6 unreachables when the other end sends to me. I assume this is
> because I don't have a corresponding kernel GRE tunnel interface. (I don't use
> the kernel GRE because I want, among others, stronger reordering functionality
> and possibly error correction, which is not in any standard.)
>
> Can this be suppressed when I have such a socket giong? ip6tables -A OUTPUT can
> stop them, but it seems very hacky and kludgy.
Thats because you loaded ip6_gre maybe ?
^ permalink raw reply
* Re: Kernel sends ICMP unreachable for GRE packets even if there is a listening socket
From: Steinar H. Gunderson @ 2015-02-10 16:16 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1423584596.28434.30.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Feb 10, 2015 at 08:09:56AM -0800, Eric Dumazet wrote:
>> Can this be suppressed when I have such a socket giong? ip6tables -A OUTPUT can
>> stop them, but it seems very hacky and kludgy.
> Thats because you loaded ip6_gre maybe ?
I have loaded ip6_gre, yes. But I might need the kernel variant for other
tunnels, so unloading it is a suboptimal workaround.
/* Steinar */
--
Homepage: http://www.sesse.net/
^ 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