* [RFC net-next 0/5] bonding: get rid of bond->lock
From: Nikolay Aleksandrov @ 2014-09-05 18:16 UTC (permalink / raw)
To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
Hi all,
This is only a raw version of the idea, I've only compile-tested this
patch-set but I think it's possible to remove bond->lock completely as
its current users are only few and can be either removed or converted.
I'm sending it early as RFC to see if you have any suggestions, comments
or objections, and I'll do stress-testing and lockdep next week and
resubmit it with any changes/fixes that come up in the process.
If this gets accepted in some form, a re-work of curr_slave_lock will be
following as it can be dropped in a lot of places and only kept to sync the
few users that actually need it (e.g. 3ad wq handler and slave release, alb
wq handler and active change etc).
The goal of these patchsets is to simplify bond locking as it's getting
more convoluted and unnecessarily complex, in most of the cases we should
be able to rely on RTNL or rcu alone, and if the case is more complex we
should explain/document well why we need another lock.
Best regards,
Nikolay Aleksandrov
Nikolay Aleksandrov (5):
bonding: 3ad: use curr_slave_lock instead of bond->lock
bonding: alb: clean bond->lock
bonding: procfs: clean bond->lock usage and use RTNL
bonding: options: remove bond->lock usage
bonding: remove last users of bond->lock and bond->lock itself
drivers/net/bonding/bond_3ad.c | 9 +++----
drivers/net/bonding/bond_alb.c | 11 ++------
drivers/net/bonding/bond_main.c | 51 ++++++--------------------------------
drivers/net/bonding/bond_options.c | 19 +-------------
drivers/net/bonding/bond_procfs.c | 8 ++----
drivers/net/bonding/bonding.h | 8 ++----
6 files changed, 18 insertions(+), 88 deletions(-)
--
1.9.3
^ permalink raw reply
* [RFC net-next 1/5] bonding: 3ad: use curr_slave_lock instead of bond->lock
From: Nikolay Aleksandrov @ 2014-09-05 18:16 UTC (permalink / raw)
To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
In-Reply-To: <1409941011-5494-1-git-send-email-nikolay@redhat.com>
In 3ad mode the only syncing needed by bond->lock is for the wq
and the recv handler, so change them to use curr_slave_lock.
There're no locking dependencies here as 3ad doesn't use
curr_slave_lock at all.
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_3ad.c | 9 ++++-----
drivers/net/bonding/bond_main.c | 12 +++++++-----
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index ee2c73a9de39..5d27a6207384 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2057,7 +2057,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
struct port *port;
bool should_notify_rtnl = BOND_SLAVE_NOTIFY_LATER;
- read_lock(&bond->lock);
+ read_lock(&bond->curr_slave_lock);
rcu_read_lock();
/* check if there are any slaves */
@@ -2120,7 +2120,7 @@ re_arm:
}
}
rcu_read_unlock();
- read_unlock(&bond->lock);
+ read_unlock(&bond->curr_slave_lock);
if (should_notify_rtnl && rtnl_trylock()) {
bond_slave_state_notify(bond);
@@ -2395,7 +2395,6 @@ int __bond_3ad_get_active_agg_info(struct bonding *bond,
return 0;
}
-/* Wrapper used to hold bond->lock so no slave manipulation can occur */
int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
{
int ret;
@@ -2487,9 +2486,9 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
if (!lacpdu)
return ret;
- read_lock(&bond->lock);
+ read_lock(&bond->curr_slave_lock);
ret = bond_3ad_rx_indication(lacpdu, slave, skb->len);
- read_unlock(&bond->lock);
+ read_unlock(&bond->curr_slave_lock);
return ret;
}
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f0f5eab0fab1..dcd331bd0c17 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1687,13 +1687,15 @@ static int __bond_release_one(struct net_device *bond_dev,
* for this slave anymore.
*/
netdev_rx_handler_unregister(slave_dev);
- write_lock_bh(&bond->lock);
- /* Inform AD package of unbinding of slave. */
- if (BOND_MODE(bond) == BOND_MODE_8023AD)
+ if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+ /* Sync against bond_3ad_rx_indication and
+ * bond_3ad_state_machine_handler
+ */
+ write_lock_bh(&bond->curr_slave_lock);
bond_3ad_unbind_slave(slave);
-
- write_unlock_bh(&bond->lock);
+ write_unlock_bh(&bond->curr_slave_lock);
+ }
netdev_info(bond_dev, "Releasing %s interface %s\n",
bond_is_active_slave(slave) ? "active" : "backup",
--
1.9.3
^ permalink raw reply related
* Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM
From: Luis R. Rodriguez @ 2014-09-05 18:12 UTC (permalink / raw)
To: Tejun Heo
Cc: Dmitry Torokhov, Greg Kroah-Hartman, Wu Zhangjin, Takashi Iwai,
Arjan van de Ven, linux-kernel@vger.kernel.org, Oleg Nesterov,
hare, Andrew Morton, Tetsuo Handa, Joseph Salisbury,
Benjamin Poirier, Santosh Rastapur, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit
In-Reply-To: <20140905174925.GA12991@mtj.dyndns.org>
On Fri, Sep 5, 2014 at 10:49 AM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On Fri, Sep 05, 2014 at 09:44:05AM -0700, Dmitry Torokhov wrote:
>> Which problem are we talking about here though? It does solve the slow device
>> stalling the rest if the kernel booting (non-module case) for me.
>
> The other one. The one with timeout. Neither cxgb4 or pata_marvell
> has slow probing stalling boot problem.
>
>> I also reject the notion that anyone should be relying on drivers to be fully
>> bound on module loading. It is not nineties anymore. We have hot pluggable
>> buses, deferred probing, and even for not hot-pluggable ones the module
>> providing the device itself might not be yet loaded. Any scripts that expect to
>> find device 100% ready after module loading are simply broken.
>
> We've been treating loading + probing as a single operation when
> loading drivers and the assumption has always been that the existing
> devices at the time of loading finished probing by the time insmod
> finishes. We now need to split loading and probing and wait for each
> of them differently. The *only* thing we can do is somehow making the
> issuer specify that it's gonna wait for probing separately. I'm not
> sure this can even be up for discussion. We're talking about a major
> userland visible behavior change. We simply can't change it
> underneath the existing users.
Meanwhile we are allowing a major design consideration such as a 30
second timeout for both init + probe all of a sudden become a hard
requirement for device drivers. I see your point but can't also be
introducing major design changes willy nilly either. We *need* a
solution for the affected drivers.
Also what stops drivers from going ahead and just implementing their
own async probe? Would that now be frowned upon as it strives away
from the original design? The bool would let those drivers do this
easily, and we would still need to identify these drivers, although
this particular change can be NAK'd Oleg's suggestion on
WARN_ON(fatal_signal_pending() at the end of load_module() seems to me
at least needed. And if its not async probe... what do those with
failed drivers do?
Luis
^ permalink raw reply
* Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM
From: Dmitry Torokhov @ 2014-09-05 18:10 UTC (permalink / raw)
To: Tejun Heo
Cc: Luis R. Rodriguez, Greg Kroah-Hartman, Wu Zhangjin, Takashi Iwai,
Arjan van de Ven, linux-kernel@vger.kernel.org, Oleg Nesterov,
hare, Andrew Morton, Tetsuo Handa, Joseph Salisbury,
Benjamin Poirier, Santosh Rastapur, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit
In-Reply-To: <20140905174925.GA12991@mtj.dyndns.org>
On Sat, Sep 06, 2014 at 02:49:25AM +0900, Tejun Heo wrote:
> Hello,
>
> On Fri, Sep 05, 2014 at 09:44:05AM -0700, Dmitry Torokhov wrote:
> > Which problem are we talking about here though? It does solve the slow device
> > stalling the rest if the kernel booting (non-module case) for me.
>
> The other one. The one with timeout. Neither cxgb4 or pata_marvell
> has slow probing stalling boot problem.
>
> > I also reject the notion that anyone should be relying on drivers to be fully
> > bound on module loading. It is not nineties anymore. We have hot pluggable
> > buses, deferred probing, and even for not hot-pluggable ones the module
> > providing the device itself might not be yet loaded. Any scripts that expect to
> > find device 100% ready after module loading are simply broken.
>
> We've been treating loading + probing as a single operation when
> loading drivers and the assumption has always been that the existing
> devices at the time of loading finished probing by the time insmod
> finishes. We now need to split loading and probing and wait for each
> of them differently. The *only* thing we can do is somehow making the
> issuer specify that it's gonna wait for probing separately. I'm not
> sure this can even be up for discussion. We're talking about a major
> userland visible behavior change.
I do not agree that it is actually user-visible change: generally speaking you
do not really know if device is there or not. They come and go. Like I said,
consider all permutations, with hot-pluggable buses, deferred probing, etc,
etc.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM
From: Tejun Heo @ 2014-09-05 17:49 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Luis R. Rodriguez, Greg Kroah-Hartman, Wu Zhangjin, Takashi Iwai,
Arjan van de Ven, linux-kernel@vger.kernel.org, Oleg Nesterov,
hare, Andrew Morton, Tetsuo Handa, Joseph Salisbury,
Benjamin Poirier, Santosh Rastapur, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit
In-Reply-To: <20140905164405.GA28964@core.coreip.homeip.net>
Hello,
On Fri, Sep 05, 2014 at 09:44:05AM -0700, Dmitry Torokhov wrote:
> Which problem are we talking about here though? It does solve the slow device
> stalling the rest if the kernel booting (non-module case) for me.
The other one. The one with timeout. Neither cxgb4 or pata_marvell
has slow probing stalling boot problem.
> I also reject the notion that anyone should be relying on drivers to be fully
> bound on module loading. It is not nineties anymore. We have hot pluggable
> buses, deferred probing, and even for not hot-pluggable ones the module
> providing the device itself might not be yet loaded. Any scripts that expect to
> find device 100% ready after module loading are simply broken.
We've been treating loading + probing as a single operation when
loading drivers and the assumption has always been that the existing
devices at the time of loading finished probing by the time insmod
finishes. We now need to split loading and probing and wait for each
of them differently. The *only* thing we can do is somehow making the
issuer specify that it's gonna wait for probing separately. I'm not
sure this can even be up for discussion. We're talking about a major
userland visible behavior change. We simply can't change it
underneath the existing users.
Thanks.
--
tejun
^ permalink raw reply
* Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM
From: Luis R. Rodriguez @ 2014-09-05 17:35 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Luis R. Rodriguez, gregkh, dmitry.torokhov, falcon, tiwai, tj,
arjan, linux-kernel, hare, akpm, penguin-kernel, joseph.salisbury,
bpoirier, santosh, Kay Sievers, One Thousand Gnomes, Tim Gardner,
Pierre Fersing, Nagalakshmi Nandigama, Praveen Krishnamoorthy,
Sreekanth Reddy, Abhijit Mahajan, Casey Leedom, Hariprasad S,
MPT-FusionLinux.pdl, linux-scsi, netd
In-Reply-To: <20140905105949.GA22778@redhat.com>
On Fri, Sep 05, 2014 at 12:59:49PM +0200, Oleg Nesterov wrote:
> On 09/04, Luis R. Rodriguez wrote:
> >
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > The new umh kill option has allowed kthreads to receive
> > kill signals but they are generally accepting all sources
> > of kill signals
>
> And I think this is right,
>
> > while the original motivation was to enable
> > through the OOM from sending the kill.
>
> even if the main concern was OOM.
>
> > Users can provide a log output and it should be clear on
> > the trace what probe / driver got the kill signal.
>
> Well, if you need a WARN output, perhaps you could just add
> WARN_ON(fatal_signal_pending()) at the end of load_module() ?
We could and that's a good idea, thanks! This however would
at least allow the device to be functional in the case the
kill was received during kthread usage, but it would certainly
also set precedents for doing similar things in the kernel
which I do agree with is hacky. If we had upstream at
least WARN_ON(fatal_signal_pending()) as you note then
I think it would at least be a reasonable compromise.
> Not only kthread_create() can fail if systemd sends SIGKILL.
Sure, although its currently the only source found and debugged.
> > Although Oleg had rejected a
> > similar change a while ago
>
> And honestly, I still dislike this change.
Don't blame you. The code is sensitive and hacky.
Luis
^ permalink raw reply
* Re: [PATCH] bpf: fix a false positive kmemcheck warning
From: Daniel Borkmann @ 2014-09-05 17:34 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Mikulas Patocka, Hannes Frederic Sowa, Pablo Neira Ayuso,
David S. Miller, LKML, Network Development
In-Reply-To: <CAMEtUuyX8Oak48vS+EDLmHY6p0yjrYNr9YXLNEYJsS7HsuooRg@mail.gmail.com>
On 09/05/2014 07:21 PM, Alexei Starovoitov wrote:
...
> imo it's cleaner to convert to bool unconditionally instead
> of annotating things everywhere.
Will do, fine as well.
^ permalink raw reply
* Re: [RFC v2 2/6] driver-core: add driver async_probe support
From: Luis R. Rodriguez @ 2014-09-05 17:25 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Luis R. Rodriguez, gregkh, dmitry.torokhov, falcon, tiwai, tj,
arjan, linux-kernel, hare, akpm, penguin-kernel, joseph.salisbury,
bpoirier, santosh, Kay Sievers, One Thousand Gnomes, Tim Gardner,
Pierre Fersing, Nagalakshmi Nandigama, Praveen Krishnamoorthy,
Sreekanth Reddy, Abhijit Mahajan, Casey Leedom, Hariprasad S,
MPT-FusionLinux.pdl, linux-scsi, netd
In-Reply-To: <20140905112417.GB22778@redhat.com>
On Fri, Sep 05, 2014 at 01:24:17PM +0200, Oleg Nesterov wrote:
> On 09/04, Luis R. Rodriguez wrote:
> >
> > struct driver_private {
> > struct kobject kobj;
> > struct klist klist_devices;
> > struct klist_node knode_bus;
> > struct module_kobject *mkobj;
> > + struct driver_attach_work *attach_work;
> > struct device_driver *driver;
>
> I am not arguing, just curious...
>
> Are you trying to shrink sizeof(driver_private) ?
Yeap.
> The code can be simpler
> if you just embedd "struct work_struct attach_work" into driver_private,
> and you do not need "struct driver_attach_work" or another ->driver pointer
> this way.
Agreed, I considered it and figured it wouldn't make much sense
to push onto folks more bytes if this feature was optional and
likely only used by a few drivers, so a pointer / kzalloc seemed
better to deal with. This saves us 24 bytes. I even tried to
implement a container_of_p() for pointers but that obviosly
didn't work well fast as a pointer can have any address and is
not relative to the parent, and if its on stack the address
can vary depending on implementation. For example the first member
should always have the same address as the struct but if the
first member is a pointer it would be off for me by 12 bytes.
I am not sure if this is standarized or not.
Luis
^ permalink raw reply
* Re: [PATCH] rose: use %*ph specifier
From: Sergei Shtylyov @ 2014-09-05 17:23 UTC (permalink / raw)
To: Andy Shevchenko, David S. Miller, linux-hams, netdev
In-Reply-To: <1409929311-3225-1-git-send-email-andriy.shevchenko@linux.intel.com>
On 09/05/2014 07:01 PM, Andy Shevchenko wrote:
> Instead of dereference each byte let's use %*ph specifier in the printk()
> calls.
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> net/rose/rose_link.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/net/rose/rose_link.c b/net/rose/rose_link.c
> index bc55142..ce60771 100644
> --- a/net/rose/rose_link.c
> +++ b/net/rose/rose_link.c
> @@ -160,7 +160,7 @@ void rose_link_rx_restart(struct sk_buff *skb, struct rose_neigh *neigh, unsigne
> break;
>
> case ROSE_DIAGNOSTIC:
> - printk(KERN_WARNING "ROSE: received diagnostic #%d - %02X %02X %02X\n", skb->data[3], skb->data[4], skb->data[5], skb->data[6]);
> + pr_warn("ROSE: received diagnostic #%d - %4ph\n", skb->data + 3);
Er, this doesn't seem right. The equivalent code would be:
pr_warn("ROSE: received diagnostic #%d - %3ph\n", skb->data[3], skb->data + 4);
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] bpf: fix a false positive kmemcheck warning
From: Alexei Starovoitov @ 2014-09-05 17:21 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Mikulas Patocka, Hannes Frederic Sowa, Pablo Neira Ayuso,
David S. Miller, LKML, Network Development
In-Reply-To: <5409F032.50101@redhat.com>
On Fri, Sep 5, 2014 at 10:17 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 09/05/2014 07:13 PM, Mikulas Patocka wrote:
>>
>> On Fri, 5 Sep 2014, Daniel Borkmann wrote:
>>>
>>> On 09/05/2014 07:00 PM, Hannes Frederic Sowa wrote:
>>>>
>>>> On Fr, 2014-09-05 at 18:20 +0200, Daniel Borkmann wrote:
>>>>>
>>>>> Hi Mikulas,
>>>>>
>>>>> On 09/05/2014 06:01 PM, Mikulas Patocka wrote:
>>>>>>
>>>>>> This patch fixes false positive kmemcheck warning in bpf.
>>>>>>
>>>>>> When we try to write the variable len, the compiler generates a code
>>>>>> that
>>>>>> reads the 32-bit word, modifies the bits belonging to "len" and writes
>>>>>> the
>>>>>> 32-bit word back. The reading of the word results in kmemcheck warning
>>>>>> due
>>>>>> to reading uninitialized memory. This patch fixes it by avoiding using
>>>>>> bit
>>>>>> fields when kmemcheck is enabled.
>>>>>>
>>>>>> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>>>>>
>>>>>
>>>>> You need to submit this patch to netdev (Cc'ed).
>>>>>
>>>>>> ---
>>>>>> include/linux/filter.h | 5 +++++
>>>>>> 1 file changed, 5 insertions(+)
>>>>>>
>>>>>> Index: linux-2.6/include/linux/filter.h
>>>>>> ===================================================================
>>>>>> --- linux-2.6.orig/include/linux/filter.h 2014-09-04
>>>>>> 23:04:26.000000000
>>>>>> +0200
>>>>>> +++ linux-2.6/include/linux/filter.h 2014-09-04 23:43:05.000000000
>>>>>> +0200
>>>>>> @@ -325,8 +325,13 @@ struct sock;
>>>>>> struct seccomp_data;
>>>>>>
>>>>>> struct bpf_prog {
>>>>>> +#ifdef CONFIG_KMEMCHECK
>>>>>> + bool jited;
>>>>>> + u32 len;
>>>>>> +#else
>>>>>> u32 jited:1, /* Is our filter
>>>>>> JIT'ed? */
>>>>>> len:31; /* Number of filter
>>>>>> blocks */
>>>>>> +#endif
>>>>>> struct sock_fprog_kern *orig_prog; /* Original BPF
>>>>>> program */
>>>>>> unsigned int (*bpf_func)(const struct sk_buff *skb,
>>>>>> const struct bpf_insn
>>>>>> *filter);
>>>>>
>>>>>
>>>>> I don't really like this if-def. If you really want to fix it, can't
>>>>> you just use :
>>>>>
>>>>> kmemcheck_bitfield_begin(bpf_anc_data)
>>>>> ...
>>>>> kmemcheck_bitfield_end(bpf_anc_data)
>>>>
>>>>
>>>> you also need to annotate the bitfield after allocation:
>>>> struct bpf_prog *prog = kalloc(...);
>>>> kmemcheck_annotate_bitfield(prog, bpf_anc_data);
>>>
>>>
>>> Yes, sure, sorry if that was not clear from my side, that was what I
>>> intended to say with kmemcheck /infrastructure/. :)
>>
>>
>> So, change it to use these markings. I'm not an expert in this area, so I
>> don't know all the places where this structure could be allocated. If you
>> know them all, mark it in this way.
>
>
> Ok, fine by me. I have some pending items, so I'll put it
> on top of them.
imo it's cleaner to convert to bool unconditionally instead
of annotating things everywhere.
^ permalink raw reply
* Re: [PATCH] irda: vlsi_ir: use %*ph specifier
From: Sergei Shtylyov @ 2014-09-05 17:21 UTC (permalink / raw)
To: Andy Shevchenko, Samuel Ortiz, netdev
In-Reply-To: <1409927840-28791-1-git-send-email-andriy.shevchenko@linux.intel.com>
Hello.
On 09/05/2014 06:37 PM, Andy Shevchenko wrote:
> Instead of looping in the code let's use kernel extension to dump small
> buffers.
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/net/irda/vlsi_ir.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
> diff --git a/drivers/net/irda/vlsi_ir.c b/drivers/net/irda/vlsi_ir.c
> index a04af9d..0319eba 100644
> --- a/drivers/net/irda/vlsi_ir.c
> +++ b/drivers/net/irda/vlsi_ir.c
> @@ -323,14 +323,9 @@ static void vlsi_proc_ring(struct seq_file *seq, struct vlsi_ring *r)
> j = (unsigned) rd_get_count(rd);
> seq_printf(seq, "current: rd = %d / status = %02x / len = %u\n",
> h, (unsigned)rd_get_status(rd), j);
> - if (j > 0) {
> - seq_printf(seq, " data:");
> - if (j > 20)
> - j = 20;
> - for (i = 0; i < j; i++)
> - seq_printf(seq, " %02x", (unsigned)((unsigned char *)rd->buf)[i]);
> - seq_printf(seq, "\n");
> - }
> + if (j > 0)
> + seq_printf(seq, " data: %*ph\n", j > 20 ? 20 : j,
Why not min(j, 20)?
> + rd->buf);
> }
WBR, Sergei
^ permalink raw reply
* [PATCH v2] iproute2 tests: Fix problem with test running
From: Vadim Kochan @ 2014-09-05 17:10 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
Tests were not allowed to be ran, the following
issues were fixed:
- creating the results folder before test running
- sudo $PREFIX moved before variables definition which
allow to pass them through the sudo to test script.
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
testsuite/Makefile | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/testsuite/Makefile b/testsuite/Makefile
index 2a4e0ba..648c793 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -1,6 +1,7 @@
## -- Config --
DEV := lo
PREFIX := sudo
+RESULTS_DIR := results
## -- End Config --
TESTS := $(patsubst tests/%,%,$(wildcard tests/*.t))
@@ -23,23 +24,24 @@ listtests:
alltests: $(TESTS)
clean:
- @rm -rf results/*
+ @rm -rf $(RESULTS_DIR)
distclean: clean
echo "Entering iproute2" && cd iproute2 && $(MAKE) distclean && cd ..;
$(TESTS):
+ @mkdir -p $(RESULTS_DIR)
@for i in $(IPVERS); do \
o=`echo $$i | sed -e 's/iproute2\///'`; \
echo -n "Running $@ [$$o/`uname -r`]: "; \
- TC="$$i/tc/tc" IP="$$i/ip/ip" DEV="$(DEV)" IPVER="$@" SNAME="$$i" \
- ERRF="results/$@.$$o.err" $(KENV) $(PREFIX) tests/$@ > results/$@.$$o.out; \
+ $(PREFIX) TC="$$i/tc/tc" IP="$$i/ip/ip" DEV="$(DEV)" IPVER="$@" SNAME="$$i" \
+ ERRF="$(RESULTS_DIR)/$@.$$o.err" $(KENV) tests/$@ > $(RESULTS_DIR)/$@.$$o.out; \
if [ "$$?" = "127" ]; then \
echo "SKIPPED"; \
- elif [ -e "results/$@.$$o.err" ]; then \
+ elif [ -e "$(RESULTS_DIR)/$@.$$o.err" ]; then \
echo "FAILED"; \
else \
echo "PASS"; \
fi; \
- dmesg > results/$@.$$o.dmesg; \
+ dmesg > $(RESULTS_DIR)/$@.$$o.dmesg; \
done
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] bpf: fix a false positive kmemcheck warning
From: Daniel Borkmann @ 2014-09-05 17:17 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Hannes Frederic Sowa, Alexei Starovoitov, Pablo Neira Ayuso,
David S. Miller, linux-kernel, netdev
In-Reply-To: <alpine.LRH.2.02.1409051313350.5269@file01.intranet.prod.int.rdu2.redhat.com>
On 09/05/2014 07:13 PM, Mikulas Patocka wrote:
> On Fri, 5 Sep 2014, Daniel Borkmann wrote:
>> On 09/05/2014 07:00 PM, Hannes Frederic Sowa wrote:
>>> On Fr, 2014-09-05 at 18:20 +0200, Daniel Borkmann wrote:
>>>> Hi Mikulas,
>>>>
>>>> On 09/05/2014 06:01 PM, Mikulas Patocka wrote:
>>>>> This patch fixes false positive kmemcheck warning in bpf.
>>>>>
>>>>> When we try to write the variable len, the compiler generates a code
>>>>> that
>>>>> reads the 32-bit word, modifies the bits belonging to "len" and writes
>>>>> the
>>>>> 32-bit word back. The reading of the word results in kmemcheck warning
>>>>> due
>>>>> to reading uninitialized memory. This patch fixes it by avoiding using
>>>>> bit
>>>>> fields when kmemcheck is enabled.
>>>>>
>>>>> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>>>>
>>>> You need to submit this patch to netdev (Cc'ed).
>>>>
>>>>> ---
>>>>> include/linux/filter.h | 5 +++++
>>>>> 1 file changed, 5 insertions(+)
>>>>>
>>>>> Index: linux-2.6/include/linux/filter.h
>>>>> ===================================================================
>>>>> --- linux-2.6.orig/include/linux/filter.h 2014-09-04 23:04:26.000000000
>>>>> +0200
>>>>> +++ linux-2.6/include/linux/filter.h 2014-09-04 23:43:05.000000000
>>>>> +0200
>>>>> @@ -325,8 +325,13 @@ struct sock;
>>>>> struct seccomp_data;
>>>>>
>>>>> struct bpf_prog {
>>>>> +#ifdef CONFIG_KMEMCHECK
>>>>> + bool jited;
>>>>> + u32 len;
>>>>> +#else
>>>>> u32 jited:1, /* Is our filter
>>>>> JIT'ed? */
>>>>> len:31; /* Number of filter
>>>>> blocks */
>>>>> +#endif
>>>>> struct sock_fprog_kern *orig_prog; /* Original BPF
>>>>> program */
>>>>> unsigned int (*bpf_func)(const struct sk_buff *skb,
>>>>> const struct bpf_insn
>>>>> *filter);
>>>>
>>>> I don't really like this if-def. If you really want to fix it, can't
>>>> you just use :
>>>>
>>>> kmemcheck_bitfield_begin(bpf_anc_data)
>>>> ...
>>>> kmemcheck_bitfield_end(bpf_anc_data)
>>>
>>> you also need to annotate the bitfield after allocation:
>>> struct bpf_prog *prog = kalloc(...);
>>> kmemcheck_annotate_bitfield(prog, bpf_anc_data);
>>
>> Yes, sure, sorry if that was not clear from my side, that was what I
>> intended to say with kmemcheck /infrastructure/. :)
>
> So, change it to use these markings. I'm not an expert in this area, so I
> don't know all the places where this structure could be allocated. If you
> know them all, mark it in this way.
Ok, fine by me. I have some pending items, so I'll put it
on top of them.
^ permalink raw reply
* Re: [PATCH] bpf: fix a false positive kmemcheck warning
From: Mikulas Patocka @ 2014-09-05 17:13 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Hannes Frederic Sowa, Alexei Starovoitov, Pablo Neira Ayuso,
David S. Miller, linux-kernel, netdev
In-Reply-To: <5409EE70.9020304@redhat.com>
On Fri, 5 Sep 2014, Daniel Borkmann wrote:
> On 09/05/2014 07:00 PM, Hannes Frederic Sowa wrote:
> > On Fr, 2014-09-05 at 18:20 +0200, Daniel Borkmann wrote:
> > > Hi Mikulas,
> > >
> > > On 09/05/2014 06:01 PM, Mikulas Patocka wrote:
> > > > This patch fixes false positive kmemcheck warning in bpf.
> > > >
> > > > When we try to write the variable len, the compiler generates a code
> > > > that
> > > > reads the 32-bit word, modifies the bits belonging to "len" and writes
> > > > the
> > > > 32-bit word back. The reading of the word results in kmemcheck warning
> > > > due
> > > > to reading uninitialized memory. This patch fixes it by avoiding using
> > > > bit
> > > > fields when kmemcheck is enabled.
> > > >
> > > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > >
> > > You need to submit this patch to netdev (Cc'ed).
> > >
> > > > ---
> > > > include/linux/filter.h | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > Index: linux-2.6/include/linux/filter.h
> > > > ===================================================================
> > > > --- linux-2.6.orig/include/linux/filter.h 2014-09-04 23:04:26.000000000
> > > > +0200
> > > > +++ linux-2.6/include/linux/filter.h 2014-09-04 23:43:05.000000000
> > > > +0200
> > > > @@ -325,8 +325,13 @@ struct sock;
> > > > struct seccomp_data;
> > > >
> > > > struct bpf_prog {
> > > > +#ifdef CONFIG_KMEMCHECK
> > > > + bool jited;
> > > > + u32 len;
> > > > +#else
> > > > u32 jited:1, /* Is our filter
> > > > JIT'ed? */
> > > > len:31; /* Number of filter
> > > > blocks */
> > > > +#endif
> > > > struct sock_fprog_kern *orig_prog; /* Original BPF
> > > > program */
> > > > unsigned int (*bpf_func)(const struct sk_buff *skb,
> > > > const struct bpf_insn
> > > > *filter);
> > >
> > > I don't really like this if-def. If you really want to fix it, can't
> > > you just use :
> > >
> > > kmemcheck_bitfield_begin(bpf_anc_data)
> > > ...
> > > kmemcheck_bitfield_end(bpf_anc_data)
> >
> > you also need to annotate the bitfield after allocation:
> > struct bpf_prog *prog = kalloc(...);
> > kmemcheck_annotate_bitfield(prog, bpf_anc_data);
>
> Yes, sure, sorry if that was not clear from my side, that was what I
> intended to say with kmemcheck /infrastructure/. :)
So, change it to use these markings. I'm not an expert in this area, so I
don't know all the places where this structure could be allocated. If you
know them all, mark it in this way.
Mikulas
^ permalink raw reply
* Re: [PATCH] bpf: fix a false positive kmemcheck warning
From: Daniel Borkmann @ 2014-09-05 17:10 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Mikulas Patocka, Alexei Starovoitov, Pablo Neira Ayuso,
David S. Miller, linux-kernel, netdev
In-Reply-To: <1409936457.5306.2.camel@localhost>
On 09/05/2014 07:00 PM, Hannes Frederic Sowa wrote:
> On Fr, 2014-09-05 at 18:20 +0200, Daniel Borkmann wrote:
>> Hi Mikulas,
>>
>> On 09/05/2014 06:01 PM, Mikulas Patocka wrote:
>>> This patch fixes false positive kmemcheck warning in bpf.
>>>
>>> When we try to write the variable len, the compiler generates a code that
>>> reads the 32-bit word, modifies the bits belonging to "len" and writes the
>>> 32-bit word back. The reading of the word results in kmemcheck warning due
>>> to reading uninitialized memory. This patch fixes it by avoiding using bit
>>> fields when kmemcheck is enabled.
>>>
>>> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>>
>> You need to submit this patch to netdev (Cc'ed).
>>
>>> ---
>>> include/linux/filter.h | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> Index: linux-2.6/include/linux/filter.h
>>> ===================================================================
>>> --- linux-2.6.orig/include/linux/filter.h 2014-09-04 23:04:26.000000000 +0200
>>> +++ linux-2.6/include/linux/filter.h 2014-09-04 23:43:05.000000000 +0200
>>> @@ -325,8 +325,13 @@ struct sock;
>>> struct seccomp_data;
>>>
>>> struct bpf_prog {
>>> +#ifdef CONFIG_KMEMCHECK
>>> + bool jited;
>>> + u32 len;
>>> +#else
>>> u32 jited:1, /* Is our filter JIT'ed? */
>>> len:31; /* Number of filter blocks */
>>> +#endif
>>> struct sock_fprog_kern *orig_prog; /* Original BPF program */
>>> unsigned int (*bpf_func)(const struct sk_buff *skb,
>>> const struct bpf_insn *filter);
>>
>> I don't really like this if-def. If you really want to fix it, can't
>> you just use :
>>
>> kmemcheck_bitfield_begin(bpf_anc_data)
>> ...
>> kmemcheck_bitfield_end(bpf_anc_data)
>
> you also need to annotate the bitfield after allocation:
> struct bpf_prog *prog = kalloc(...);
> kmemcheck_annotate_bitfield(prog, bpf_anc_data);
Yes, sure, sorry if that was not clear from my side, that was what I
intended to say with kmemcheck /infrastructure/. :)
> Bye,
> Hannes
^ permalink raw reply
* Re: [PATCH] iproute2 tests: Fix problem with test running
From: Vadim Kochan @ 2014-09-05 17:09 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20140905100707.16c063e2@urahara>
Eff, mistake ...
On Fri, Sep 5, 2014 at 8:07 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Fri, 5 Sep 2014 19:46:31 +0300
> Vadim Kochan <vadim4j@gmail.com> wrote:
>
>> index 2a4e0ba..6f2ea85 100644
>> --- a/testsuite/Makefile
>> +++ b/testsuite/Makefile
>> @@ -1,6 +1,7 @@
>> ## -- Config --
>> DEV := lo
>> PREFIX := sudo
>> +RESULTS_DIR := "results"
>
> Why the quotes?
^ permalink raw reply
* Re: [PATCH] iproute2 tests: Fix problem with test running
From: Stephen Hemminger @ 2014-09-05 17:07 UTC (permalink / raw)
To: Vadim Kochan; +Cc: netdev
In-Reply-To: <1409935591-30145-1-git-send-email-vadim4j@gmail.com>
On Fri, 5 Sep 2014 19:46:31 +0300
Vadim Kochan <vadim4j@gmail.com> wrote:
> index 2a4e0ba..6f2ea85 100644
> --- a/testsuite/Makefile
> +++ b/testsuite/Makefile
> @@ -1,6 +1,7 @@
> ## -- Config --
> DEV := lo
> PREFIX := sudo
> +RESULTS_DIR := "results"
Why the quotes?
^ permalink raw reply
* Re: [patch net-next 0/5] bridge: implement rtnl_link options for getting and setting bridge options
From: Stephen Hemminger @ 2014-09-05 17:06 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, sfeldma, arvid.brodin, sucheta.chakraborty
In-Reply-To: <1409925092-17808-1-git-send-email-jiri@resnulli.us>
On Fri, 5 Sep 2014 15:51:27 +0200
Jiri Pirko <jiri@resnulli.us> wrote:
> So far, only sysfs is complete interface for getting and setting bridge
> options. This patchset follows-up on the similar bonding code and
> allows userspace to get/set bridge master/port options using Netlink
> IFLA_INFO_DATA/IFLA_INFO_SLAVE_DATA attr.
>
> Jiri Pirko (5):
> bridge: switch order of rx_handler reg and upper dev link
> bridge: implement rtnl_link_ops->get_slave_size and
> rtnl_link_ops->fill_slave_info
> bridge: implement rtnl_link_ops->slave_changelink
> bridge: implement rtnl_link_ops->get_size and rtnl_link_ops->fill_info
> bridge: implement rtnl_link_ops->changelink
>
> include/uapi/linux/if_link.h | 12 +++++
> net/bridge/br_if.c | 15 +++---
> net/bridge/br_netlink.c | 109 +++++++++++++++++++++++++++++++++++++++----
> 3 files changed, 121 insertions(+), 15 deletions(-)
>
Looks good, I will try and go over in more detail and look for
any specific issues.
^ permalink raw reply
* Re: [PATCH] bpf: fix a false positive kmemcheck warning
From: Hannes Frederic Sowa @ 2014-09-05 17:00 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Mikulas Patocka, Alexei Starovoitov, Pablo Neira Ayuso,
David S. Miller, linux-kernel, netdev
In-Reply-To: <5409E2C8.2080200@redhat.com>
On Fr, 2014-09-05 at 18:20 +0200, Daniel Borkmann wrote:
> Hi Mikulas,
>
> On 09/05/2014 06:01 PM, Mikulas Patocka wrote:
> > This patch fixes false positive kmemcheck warning in bpf.
> >
> > When we try to write the variable len, the compiler generates a code that
> > reads the 32-bit word, modifies the bits belonging to "len" and writes the
> > 32-bit word back. The reading of the word results in kmemcheck warning due
> > to reading uninitialized memory. This patch fixes it by avoiding using bit
> > fields when kmemcheck is enabled.
> >
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>
> You need to submit this patch to netdev (Cc'ed).
>
> > ---
> > include/linux/filter.h | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > Index: linux-2.6/include/linux/filter.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/filter.h 2014-09-04 23:04:26.000000000 +0200
> > +++ linux-2.6/include/linux/filter.h 2014-09-04 23:43:05.000000000 +0200
> > @@ -325,8 +325,13 @@ struct sock;
> > struct seccomp_data;
> >
> > struct bpf_prog {
> > +#ifdef CONFIG_KMEMCHECK
> > + bool jited;
> > + u32 len;
> > +#else
> > u32 jited:1, /* Is our filter JIT'ed? */
> > len:31; /* Number of filter blocks */
> > +#endif
> > struct sock_fprog_kern *orig_prog; /* Original BPF program */
> > unsigned int (*bpf_func)(const struct sk_buff *skb,
> > const struct bpf_insn *filter);
>
> I don't really like this if-def. If you really want to fix it, can't
> you just use :
>
> kmemcheck_bitfield_begin(bpf_anc_data)
> ...
> kmemcheck_bitfield_end(bpf_anc_data)
you also need to annotate the bitfield after allocation:
struct bpf_prog *prog = kalloc(...);
kmemcheck_annotate_bitfield(prog, bpf_anc_data);
Bye,
Hannes
^ permalink raw reply
* [PATCH] iproute2 tests: Fix problem with test running
From: Vadim Kochan @ 2014-09-05 16:46 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
Tests were not allowed to be ran, the following
issues were fixed:
- creating the results folder before test running
- sudo $PREFIX moved before variables definition which
allow to pass them through the sudo to test script.
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
testsuite/Makefile | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/testsuite/Makefile b/testsuite/Makefile
index 2a4e0ba..6f2ea85 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -1,6 +1,7 @@
## -- Config --
DEV := lo
PREFIX := sudo
+RESULTS_DIR := "results"
## -- End Config --
TESTS := $(patsubst tests/%,%,$(wildcard tests/*.t))
@@ -23,23 +24,24 @@ listtests:
alltests: $(TESTS)
clean:
- @rm -rf results/*
+ @rm -rf $(RESULTS_DIR)
distclean: clean
echo "Entering iproute2" && cd iproute2 && $(MAKE) distclean && cd ..;
$(TESTS):
+ @mkdir -p $(RESULTS_DIR)
@for i in $(IPVERS); do \
o=`echo $$i | sed -e 's/iproute2\///'`; \
echo -n "Running $@ [$$o/`uname -r`]: "; \
- TC="$$i/tc/tc" IP="$$i/ip/ip" DEV="$(DEV)" IPVER="$@" SNAME="$$i" \
- ERRF="results/$@.$$o.err" $(KENV) $(PREFIX) tests/$@ > results/$@.$$o.out; \
+ $(PREFIX) TC="$$i/tc/tc" IP="$$i/ip/ip" DEV="$(DEV)" IPVER="$@" SNAME="$$i" \
+ ERRF="$(RESULTS_DIR)/$@.$$o.err" $(KENV) tests/$@ > $(RESULTS_DIR)/$@.$$o.out; \
if [ "$$?" = "127" ]; then \
echo "SKIPPED"; \
- elif [ -e "results/$@.$$o.err" ]; then \
+ elif [ -e "$(RESULTS_DIR)/$@.$$o.err" ]; then \
echo "FAILED"; \
else \
echo "PASS"; \
fi; \
- dmesg > results/$@.$$o.dmesg; \
+ dmesg > $(RESULTS_DIR)/$@.$$o.dmesg; \
done
--
2.1.0
^ permalink raw reply related
* Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM
From: Dmitry Torokhov @ 2014-09-05 16:44 UTC (permalink / raw)
To: Tejun Heo
Cc: Luis R. Rodriguez, Greg Kroah-Hartman, Wu Zhangjin, Takashi Iwai,
Arjan van de Ven, linux-kernel@vger.kernel.org, Oleg Nesterov,
hare, Andrew Morton, Tetsuo Handa, Joseph Salisbury,
Benjamin Poirier, Santosh Rastapur, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit
In-Reply-To: <20140905141241.GC10455@mtj.dyndns.org>
On Friday, September 05, 2014 11:12:41 PM Tejun Heo wrote:
> On Fri, Sep 05, 2014 at 12:47:16AM -0700, Luis R. Rodriguez wrote:
> > Ah -- well without it the way we "find" drivers that need this new
> > "async feature" is by a bug report and folks saying their system can't
> > boot, or they say their device doesn't come up. That's all. Tracing
> > this to systemd and a timeout was one of the most ugliest things ever.
> > There two insane bug reports you can go check:
> >
> > mptsas was the first:
> >
> > http://article.gmane.org/gmane.linux.kernel/1669550
> > https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1297248
> >
> > Then cxgb4:
> >
> > https://bugzilla.novell.com/show_bug.cgi?id=877622
> >
> > I only had Cc'd you on the newest gem pata_marvell :
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=59581
> >
> > We can't seriously expect to be doing all this work for every driver.
> > a WARN_ONCE() would enable us to find the drivers that need this new
> > async probe "feature".
>
> This whole approach of trying to mark specific drivers as needing
> "async probing" is completely broken for the problem at hand. It
> can't address the problem adequately while breaking backward
> compatibility. I don't think this makes much sense.
>
Which problem are we talking about here though? It does solve the slow device
stalling the rest if the kernel booting (non-module case) for me.
I also reject the notion that anyone should be relying on drivers to be fully
bound on module loading. It is not nineties anymore. We have hot pluggable
buses, deferred probing, and even for not hot-pluggable ones the module
providing the device itself might not be yet loaded. Any scripts that expect to
find device 100% ready after module loading are simply broken.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] bpf: fix a false positive kmemcheck warning
From: Daniel Borkmann @ 2014-09-05 16:20 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Alexei Starovoitov, Pablo Neira Ayuso, David S. Miller,
linux-kernel, netdev
In-Reply-To: <alpine.LRH.2.02.1409051157440.5269@file01.intranet.prod.int.rdu2.redhat.com>
Hi Mikulas,
On 09/05/2014 06:01 PM, Mikulas Patocka wrote:
> This patch fixes false positive kmemcheck warning in bpf.
>
> When we try to write the variable len, the compiler generates a code that
> reads the 32-bit word, modifies the bits belonging to "len" and writes the
> 32-bit word back. The reading of the word results in kmemcheck warning due
> to reading uninitialized memory. This patch fixes it by avoiding using bit
> fields when kmemcheck is enabled.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
You need to submit this patch to netdev (Cc'ed).
> ---
> include/linux/filter.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> Index: linux-2.6/include/linux/filter.h
> ===================================================================
> --- linux-2.6.orig/include/linux/filter.h 2014-09-04 23:04:26.000000000 +0200
> +++ linux-2.6/include/linux/filter.h 2014-09-04 23:43:05.000000000 +0200
> @@ -325,8 +325,13 @@ struct sock;
> struct seccomp_data;
>
> struct bpf_prog {
> +#ifdef CONFIG_KMEMCHECK
> + bool jited;
> + u32 len;
> +#else
> u32 jited:1, /* Is our filter JIT'ed? */
> len:31; /* Number of filter blocks */
> +#endif
> struct sock_fprog_kern *orig_prog; /* Original BPF program */
> unsigned int (*bpf_func)(const struct sk_buff *skb,
> const struct bpf_insn *filter);
I don't really like this if-def. If you really want to fix it, can't
you just use :
kmemcheck_bitfield_begin(bpf_anc_data)
...
kmemcheck_bitfield_end(bpf_anc_data)
et al infrastructure as container (in case in future we will add some more
bit flags, since len doesn't really need all 31 bit universe)?
Note, there are currently some patches pending in patchwork that also fill
the u32 hole thus the extra bool would introduce a new hole after that.
Therefore, I think above would be better.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH v2] rose: use %*ph specifier
From: Eric Dumazet @ 2014-09-05 15:52 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: David S . Miller, linux-hams, netdev
In-Reply-To: <1409931138-25447-1-git-send-email-andriy.shevchenko@linux.intel.com>
On Fri, 2014-09-05 at 18:32 +0300, Andy Shevchenko wrote:
> Instead of dereference each byte let's use %*ph specifier in the printk()
> calls.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> - Since v1:
> - compile tested
> - fix a wrong argument list
> net/rose/rose_link.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/rose/rose_link.c b/net/rose/rose_link.c
> index bc55142..e873d7d 100644
> --- a/net/rose/rose_link.c
> +++ b/net/rose/rose_link.c
> @@ -160,7 +160,8 @@ void rose_link_rx_restart(struct sk_buff *skb, struct rose_neigh *neigh, unsigne
> break;
>
> case ROSE_DIAGNOSTIC:
> - printk(KERN_WARNING "ROSE: received diagnostic #%d - %02X %02X %02X\n", skb->data[3], skb->data[4], skb->data[5], skb->data[6]);
> + pr_warn("ROSE: received diagnostic #%d - %3ph\n", skb->data[3],
> + skb->data + 4);
> break;
>
> default:
This looks better ;)
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH v2] rose: use %*ph specifier
From: Andy Shevchenko @ 2014-09-05 15:32 UTC (permalink / raw)
To: David S . Miller, linux-hams, netdev, Eric Dumazet; +Cc: Andy Shevchenko
Instead of dereference each byte let's use %*ph specifier in the printk()
calls.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
- Since v1:
- compile tested
- fix a wrong argument list
net/rose/rose_link.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/rose/rose_link.c b/net/rose/rose_link.c
index bc55142..e873d7d 100644
--- a/net/rose/rose_link.c
+++ b/net/rose/rose_link.c
@@ -160,7 +160,8 @@ void rose_link_rx_restart(struct sk_buff *skb, struct rose_neigh *neigh, unsigne
break;
case ROSE_DIAGNOSTIC:
- printk(KERN_WARNING "ROSE: received diagnostic #%d - %02X %02X %02X\n", skb->data[3], skb->data[4], skb->data[5], skb->data[6]);
+ pr_warn("ROSE: received diagnostic #%d - %3ph\n", skb->data[3],
+ skb->data + 4);
break;
default:
--
2.1.0
^ permalink raw reply related
* Solved: Regression: TCP connections fail over wireless: bad checksum
From: Ted Percival @ 2014-09-05 15:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Tom Herbert, Linux Netdev List
In-Reply-To: <1409891901.26422.133.camel@edumazet-glaptop2.roam.corp.google.com>
On 09/04/2014 10:38 PM, Eric Dumazet wrote:
> Most probably this is already fixed in net-next tree
>
> http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=1f59533f9ca5634e7b8914252e48aee9d9cbe501
Yep. 50cbe9ab5f8d was the regressing commit and cherry-picking that
commit on top of it solves the problem.
^ 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