* Re: brcmfmac: drop unused fields from struct brcmf_pub
From: Kalle Valo @ 2016-09-26 17:51 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rafał Miłecki, Arend van Spriel, Franky Lin,
Hante Meuleman, Pieter-Paul Giesberts, Franky (Zhenhui) Lin,
Colin Ian King,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:NETWORKING DRIVERS, open list
In-Reply-To: <20160923132759.21875-1-zajec5@gmail.com>
Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> They seem to be there from the first day. We calculate these values but
> never use them.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Patch applied to wireless-drivers-next.git, thanks.
2df86ad959c9 brcmfmac: drop unused fields from struct brcmf_pub
--
https://patchwork.kernel.org/patch/9348103/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: brcmfmac: fix memory leak in brcmf_fill_bss_param
From: Kalle Valo @ 2016-09-26 17:49 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rafał Miłecki, Arend van Spriel, Franky Lin,
Hante Meuleman, Pieter-Paul Giesberts, Franky (Zhenhui) Lin,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:NETWORKING DRIVERS, open list
In-Reply-To: <20160921062327.28729-1-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>
> This function is called from get_station callback which means that every
> time user space was getting/dumping station(s) we were leaking 2 KiB.
>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> Fixes: 1f0dc59a6de ("brcmfmac: rework .get_station() callback")
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # 4.2+
> Acked-by: Arend van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Patch applied to wireless-drivers-next.git, thanks.
23e9c128adb2 brcmfmac: fix memory leak in brcmf_fill_bss_param
--
https://patchwork.kernel.org/patch/9342879/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCH v2 5/5] ISDN-Gigaset: Enclose two expressions for the sizeof operator by parentheses
From: SF Markus Elfring @ 2016-09-26 17:44 UTC (permalink / raw)
To: Joe Perches, David Laight,
gigaset307x-common@lists.sourceforge.net, netdev@vger.kernel.org,
Karsten Keil, Paul Bolle
Cc: LKML, kernel-janitors@vger.kernel.org, Julia Lawall
In-Reply-To: <1474907003.2238.13.camel@perches.com>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 26 Sep 2016 19:34:27 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The script "checkpatch.pl" can point information out like the following.
WARNING: sizeof … should be sizeof(…)
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
v2: Position the desired closing parenthesis behind the variable name for
a character buffer.
drivers/isdn/gigaset/common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
index 2e9382f..dba44e1 100644
--- a/drivers/isdn/gigaset/common.c
+++ b/drivers/isdn/gigaset/common.c
@@ -53,7 +53,7 @@ void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
{
unsigned char outbuf[80];
unsigned char c;
- size_t space = sizeof outbuf - 1;
+ size_t space = sizeof(outbuf) - 1;
unsigned char *out = outbuf;
size_t numin = len;
@@ -1079,7 +1079,7 @@ struct gigaset_driver *gigaset_initdriver(unsigned minor, unsigned minors,
unsigned long flags;
unsigned i;
- drv = kmalloc(sizeof *drv, GFP_KERNEL);
+ drv = kmalloc(sizeof(*drv), GFP_KERNEL);
if (!drv)
return NULL;
--
2.10.0
^ permalink raw reply related
* Re: [PATCH 5/5] ISDN-Gigaset: Enclose two expressions for the sizeof operator by parentheses
From: Sergei Shtylyov @ 2016-09-26 17:38 UTC (permalink / raw)
To: SF Markus Elfring, gigaset307x-common, netdev, Karsten Keil,
Paul Bolle
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <e97b38b6-8c02-6733-8b57-9786bfb911a6@users.sourceforge.net>
Hello.
On 09/26/2016 06:44 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 26 Sep 2016 17:03:56 +0200
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> The script "checkpatch.pl" can point information out like the following.
>
> WARNING: sizeof … should be sizeof(…)
>
> Thus fix the affected source code places.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/isdn/gigaset/common.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
> index 2e9382f..d901ed7 100644
> --- a/drivers/isdn/gigaset/common.c
> +++ b/drivers/isdn/gigaset/common.c
> @@ -53,7 +53,7 @@ void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
> {
> unsigned char outbuf[80];
> unsigned char c;
> - size_t space = sizeof outbuf - 1;
> + size_t space = sizeof(outbuf - 1);
What?! Does that compile?
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v5 02/16] IB/pvrdma: Add user-level shared functions
From: Adit Ranadive @ 2016-09-26 17:33 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
asarwade-pghWNbHTmq7QT0dZR+AlfA,
georgezhang-pghWNbHTmq7QT0dZR+AlfA,
bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <20160926061338.GH4088-2ukJVAZIZ/Y@public.gmane.org>
On Sun, Sep 25, 2016 at 23:13:38PM -0700, Leon Romanovsky wrote:
> On Sun, Sep 25, 2016 at 09:22:11PM -0700, Adit Ranadive wrote:
>> On Sun, Sep 25 2016 at 10:26:24AM +0300, Leon Romanovsky wrote:
> > > > On Sat, Sep 24, 2016 at 04:21:26PM -0700, Adit Ranadive wrote:
> > > > We share some common structures with the user-level driver. This patch adds
> > > > those structures and shared functions to traverse the QP/CQ rings.
> >
> > <...>
> >
> > > > +
> > > > +#include <linux/types.h>
> > > > +
> > > > +#define PVRDMA_UVERBS_ABI_VERSION 3
> > > > +#define PVRDMA_BOARD_ID 1
> > > > +#define PVRDMA_REV_ID 1
> > > >
> > > > Please don't add defines which you are not using in the library and the
> > > > two above are not in use.
> > > >
> >
> > I'll move these to the pvrdma.h file.
> >
> > <...>
> >
> > > > diff --git a/include/uapi/rdma/pvrdma-uapi.h b/include/uapi/rdma/pvrdma-uapi.h
> > > > new file mode 100644
> > > > index 0000000..430d8a5
> >
> > <...>
> >
> > > > +
> > > > +#ifndef __PVRDMA_UAPI_H__
> > > > +#define __PVRDMA_UAPI_H__
> > > > +
> > > > +#include <linux/types.h>
> > > > +
> > > > +#define PVRDMA_VERSION 17
> > > >
> > > > What do you plan to do with this VERSION?
> > > > How is it related to ABI?
> > > >
> >
> > Not related. This is only for the driver to know which APIs to support.
> > For example, an older driver would still be able to work with a newer
> > device. I can move this to pvrdma.h as well.
> >
> > To be honest, I thought I can move this file into the uapi folder since
> > the structures here are shared with the user-level library. Based on
> > your comments in this thread and the other ones, I think it makes sense
> > to move this file back to the pvrdma driver folder and rename it
> > (pvrdma_wqe.h?) to avoid confusion. There might still be some duplicate
> > code (especially the UAR offsets and WQE structs) here and in our
> > user-level library.
> >
> > Let me know if that makes sense.
> >
> > > > +
> > > > +#define PVRDMA_UAR_HANDLE_MASK 0x00FFFFFF /* Bottom 24 bits. */
> > > > +#define PVRDMA_UAR_QP_OFFSET 0 /* Offset of QP doorbell. */
> > > > +#define PVRDMA_UAR_QP_SEND BIT(30) /* Send bit. */
> > > > +#define PVRDMA_UAR_QP_RECV BIT(31) /* Recv bit. */
> > > > +#define PVRDMA_UAR_CQ_OFFSET 4 /* Offset of CQ doorbell. */
> > > > +#define PVRDMA_UAR_CQ_ARM_SOL BIT(29) /* Arm solicited bit. */
> > > > +#define PVRDMA_UAR_CQ_ARM BIT(30) /* Arm bit. */
> > > > +#define PVRDMA_UAR_CQ_POLL BIT(31) /* Poll bit. */
> > > > +#define PVRDMA_INVALID_IDX -1 /* Invalid index. */
> > > >
> > > > +
> > > > +/* PVRDMA atomic compare and swap */
> > > > +struct pvrdma_exp_cmp_swap {
> > > >
> > > > _EXP_ looks very similar to MLNX_OFED naming convention.
> > > >
> >
> > Yes, the operation was based on that. Any concerns?
> > I can rename this and the one below.
>
> Yes, please.
> The common practice in IB subsystem is to use _ex_ notation for such
> extended structures.
>
Ok.
> >
> > > > + __u64 swap_val;
> > > > + __u64 compare_val;
> > > > + __u64 swap_mask;
> > > > + __u64 compare_mask;
> > > > +};
> > > > +
> > > > +/* PVRDMA atomic fetch and add */
> > > > +struct pvrdma_exp_fetch_add {
> > > >
> > > > The same as above.
> > > >
> > > > + __u64 add_val;
> > > > + __u64 field_boundary;
> > > > +};
> > > > +
> > > > +/* PVRDMA address vector. */
> > > > +struct pvrdma_av {
> > > > + __u32 port_pd;
> > > > + __u32 sl_tclass_flowlabel;
> > > > + __u8 dgid[16];
> > > > + __u8 src_path_bits;
> > > > + __u8 gid_index;
> > > > + __u8 stat_rate;
> > > > + __u8 hop_limit;
> > > > + __u8 dmac[6];
> > > > + __u8 reserved[6];
> > > > +};
> > > > +
> > > > +/* PVRDMA scatter/gather entry */
> > > > +struct pvrdma_sge {
> > > > + __u64 addr;
> > > > + __u32 length;
> > > > + __u32 lkey;
> > > > +};
> > > > +
> > > > +/* PVRDMA receive queue work request */
> > > > +struct pvrdma_rq_wqe_hdr {
> > > > + __u64 wr_id; /* wr id */
> > > > + __u32 num_sge; /* size of s/g array */
> > > > + __u32 total_len; /* reserved */
> > > > +};
> > > > +/* Use pvrdma_sge (ib_sge) for receive queue s/g array elements. */
> > > > +
> > > > +/* PVRDMA send queue work request */
> > > > +struct pvrdma_sq_wqe_hdr {
> > > > + __u64 wr_id; /* wr id */
> > > > + __u32 num_sge; /* size of s/g array */
> > > > + __u32 total_len; /* reserved */
> > > > + __u32 opcode; /* operation type */
> > > > + __u32 send_flags; /* wr flags */
> > > > + union {
> > > > + __u32 imm_data;
> > > > + __u32 invalidate_rkey;
> > > > + } ex;
> > > > + __u32 reserved;
> > > > + union {
> > > > + struct {
> > > > + __u64 remote_addr;
> > > > + __u32 rkey;
> > > > + __u8 reserved[4];
> > > > + } rdma;
> > > > + struct {
> > > > + __u64 remote_addr;
> > > > + __u64 compare_add;
> > > > + __u64 swap;
> > > > + __u32 rkey;
> > > > + __u32 reserved;
> > > > + } atomic;
> > > > + struct {
> > > > + __u64 remote_addr;
> > > > + __u32 log_arg_sz;
> > > > + __u32 rkey;
> > > > + union {
> > > > + struct pvrdma_exp_cmp_swap cmp_swap;
> > > > + struct pvrdma_exp_fetch_add fetch_add;
> > > > + } wr_data;
> > > > + } masked_atomics;
> > > > + struct {
> > > > + __u64 iova_start;
> > > > + __u64 pl_pdir_dma;
> > > > + __u32 page_shift;
> > > > + __u32 page_list_len;
> > > > + __u32 length;
> > > > + __u32 access_flags;
> > > > + __u32 rkey;
> > > > + } fast_reg;
> > > > + struct {
> > > > + __u32 remote_qpn;
> > > > + __u32 remote_qkey;
> > > > + struct pvrdma_av av;
> > > > + } ud;
> > > > + } wr;
> > > > +};
> > > >
> > > > No, I have half-baked patch series which refactors this structure in kernel.
> > > > There is no need to put this structure in UAPI.
> > > >
> >
> > This is specific to our device.. We do need to enqueue the WQE in this format
> > for the device to recognize it. This is the same format that the user-level
> > library will put the WQE in. As I said above, we can move this to the main
> > pvrdma driver directory if you prefer.
>
> This is different implementations between kernel and user space.
> We don't want to bring user space limitations to kernel.
> Take a look here:
> http://lxr.free-electrons.com/source/include/rdma/ib_verbs.h#L1192
>
We anyway convert the WR structs defined there to our own device-specific
format. Similarly, in the user-level library we would convert from the
user-space WR structure to a device-specific structure. The struct above
defines the device-specific format.
There might be some overlap in these structures with userspace and kernel,
since some of the same opcodes would be supported between the two.
As long as OFED hides those limitations shouldnt it be okay to have similar
structures between the two?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 net-next 2/3] net: mpls: Fixups for GSO
From: Jiri Benc @ 2016-09-26 17:02 UTC (permalink / raw)
To: David Ahern
Cc: pravin shelar, Simon Horman, Pravin B Shelar,
Linux Kernel Network Developers, David S. Miller, buytenh,
Eric W. Biederman, rshearma, tom, Thomas Graf, olivier.dugeon,
Alexander Duyck, roopa
In-Reply-To: <20160926175622.3b00d478@griffin>
On Mon, 26 Sep 2016 17:56:22 +0200, Jiri Benc wrote:
> After push_mpls, network_header points to the start of MPLS headers.
> Which I understand was the point of this patch. However, push_mpls also
> calls invalidate_flow_key. Meaning that, depending on actions, we may
> end up calling key_extract soon after. And key_extract sets the network
> header *after* the MPLS headers.
>
> That means that on output, for otherwise identical packet,
> network_header can point before or after MPLS headers based on what
> actions happened to be executed (recirculation, mainly).
>
> If I'm not misreading the code or missing something, this can't be
> right.
>
> mpls_gso_segment does not care, it resets the network_header anyway.
> What about drivers? What is the correct behavior?
Answering to myself: it breaks skb_mac_gso_segment. Seems we need to
fix key_extract to set network_header to the beginning of MPLS headers.
I'll prepare a patch.
Jiri
^ permalink raw reply
* Re: [PATCH] net: tg3: use new api ethtool_{get|set}_link_ksettings
From: Michael Chan @ 2016-09-26 16:54 UTC (permalink / raw)
To: Philippe Reynes
Cc: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan,
Florian Fainelli, David Miller, Netdev, open list
In-Reply-To: <1474839084-10883-1-git-send-email-tremyfr@gmail.com>
On Sun, Sep 25, 2016 at 2:31 PM, Philippe Reynes <tremyfr@gmail.com> wrote:
> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
>
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
Looks good to me. Thanks.
Acked-by: Michael Chan <michael.chan@broadcom.com>
^ permalink raw reply
* Re: [PATCH] ath10k: fix system hang at qca99x0 probe on x86 platform (DMA32 issue)
From: Ben Greear @ 2016-09-26 16:53 UTC (permalink / raw)
To: Adrian Chadd, Michal Kazior
Cc: Manoharan, Rajkumar, Rajkumar Manoharan, Sebastian Gottschall,
Valo, Kalle, ath10k@lists.infradead.org, nbd@nbd.name,
linux-wireless@vger.kernel.org, netdev
In-Reply-To: <CAJ-VmomcD8FgA34z3OFKZvc5yq8EsHOTqP=Lg-6gFOAcWys8og@mail.gmail.com>
On 07/20/2016 10:02 AM, Adrian Chadd wrote:
> Hi,
>
> The "right" way for the target CPU to interact with host CPU memory
> (and vice versa, for mostly what it's worth) is to have the copy
> engine copy (ie, "DMA") the pieces between them. This may be for
> diagnostic purposes, but it's not supposed to be used like this for
> doing wifi data exchange, right? :-P
>
> Now, there /may/ be some alignment hilarity in various bits of code
> and/or hardware. Eg, Merlin (AR9280) requires its descriptors to be
> within a 4k block - the code to iterate through the descriptor
> physical address space didn't do a "val = val + offset", it did
> something in verilog like "val = (val & 0xffffc000) | (offset &
> 0x3fff)". This meant if you allocated a descriptor that started just
> before the end of a 4k physmem aligned block, you'd end up with
> exciting results. I don't know if there are any situations like this
> in the ath10k hardware, but I'm sure there will be some gotchas
> somewhere.
>
> In any case, if ath10k is consuming too much bounce buffers, the calls
> to allocate memory aren't working right and should be restricted to 32
> bit addresses. Whether that's by using the DMA memory API (before it's
> mapped) or passing in GFP_DMA32 is a fun debate.
>
> (My test hardware arrived, so I'll test this all out today on
> Peregrine-v2 and see if the driver works.)
I have been running this patch for a while:
ath10k: Use GPF_DMA32 for firmware swap memory.
This fixes OS crash when using QCA 9984 NIC on x86-64 system
without vt-d enabled.
Also tested on ea8500 with 9980, and x86-64 with 9980 and 9880.
All tests were with CT firmware.
Signed-off-by: Ben Greear <greearb@candelatech.com>
-------------------- drivers/net/wireless/ath/ath10k/wmi.c --------------------
index e20aa39..727b3aa 100644
@@ -4491,7 +4491,7 @@ static int ath10k_wmi_alloc_chunk(struct ath10k *ar, u32 req_id,
if (!pool_size)
return -EINVAL;
- vaddr = kzalloc(pool_size, GFP_KERNEL | __GFP_NOWARN);
+ vaddr = kzalloc(pool_size, GFP_KERNEL | __GFP_NOWARN | GFP_DMA32);
if (!vaddr)
num_units /= 2;
}
It mostly seems to work, but then sometimes I get a splat like this below. It appears
it is invalid to actually do kzalloc with GFP_DMA32 (based on that BUG_ON that
hit in the new_slab method)??
Any idea for a more proper way to do this?
gfp: 4
------------[ cut here ]------------
kernel BUG at /home/greearb/git/linux-4.7.dev.y/mm/slub.c:1508!
invalid opcode: 0000 [#1] PREEMPT SMP
Modules linked in: coretemp hwmon ath9k intel_rapl ath10k_pci x86_pkg_temp_thermal ath9k_common ath10k_core intel_powerclamp ath9k_hw ath kvm iTCO_wdt mac80211
iTCO_vendor_support irqbypass snd_hda_codec_hdmi 6
CPU: 2 PID: 268 Comm: kworker/u8:5 Not tainted 4.7.2+ #16
Hardware name: To be filled by O.E.M. To be filled by O.E.M./ChiefRiver, BIOS 4.6.5 06/07/2013
Workqueue: ath10k_aux_wq ath10k_wmi_event_service_ready_work [ath10k_core]
task: ffff880036433a00 ti: ffff880036440000 task.ti: ffff880036440000
RIP: 0010:[<ffffffff8124592a>] [<ffffffff8124592a>] new_slab+0x39a/0x410
RSP: 0018:ffff880036443b58 EFLAGS: 00010092
RAX: 0000000000000006 RBX: 00000000024082c4 RCX: 0000000000000000
RDX: 0000000000000006 RSI: ffff88021e30dd08 RDI: ffff88021e30dd08
RBP: ffff880036443b90 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000372 R12: ffff88021dc01200
R13: ffff88021dc00cc0 R14: ffff88021dc01200 R15: 0000000000000001
FS: 0000000000000000(0000) GS:ffff88021e300000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f3e65c1c730 CR3: 0000000001e06000 CR4: 00000000001406e0
Stack:
ffffffff8127a4fc ffff0a01ffffff10 00000000024082c4 ffff88021dc01200
ffff88021dc00cc0 ffff88021dc01200 0000000000000001 ffff880036443c58
ffffffff81247ac6 ffff88021e31b360 ffff880036433a00 ffff880036433a00
Call Trace:
[<ffffffff8127a4fc>] ? __d_lookup+0x9c/0x160
[<ffffffff81247ac6>] ___slab_alloc+0x396/0x4a0
[<ffffffffa0f8e14d>] ? ath10k_wmi_event_service_ready_work+0x5ad/0x800 [ath10k_core]
[<ffffffff811f5279>] ? alloc_kmem_pages+0x9/0x10
[<ffffffff8120f203>] ? kmalloc_order+0x13/0x40
[<ffffffffa0f8e14d>] ? ath10k_wmi_event_service_ready_work+0x5ad/0x800 [ath10k_core]
[<ffffffff81247bf6>] __slab_alloc.isra.72+0x26/0x40
[<ffffffff81248767>] __kmalloc+0x147/0x1b0
[<ffffffffa0f8e14d>] ath10k_wmi_event_service_ready_work+0x5ad/0x800 [ath10k_core]
[<ffffffff811370a1>] ? dequeue_entity+0x261/0xac0
[<ffffffff8111c2d8>] process_one_work+0x148/0x420
[<ffffffff8111c929>] worker_thread+0x49/0x480
[<ffffffff8111c8e0>] ? rescuer_thread+0x330/0x330
[<ffffffff81121984>] kthread+0xc4/0xe0
[<ffffffff8184d75f>] ret_from_fork+0x1f/0x40
[<ffffffff811218c0>] ? kthread_create_on_node+0x170/0x170
Code: e9 65 fd ff ff 49 8b 57 20 48 8d 42 ff 83 e2 01 49 0f 44 c7 f0 80 08 40 e9 6f fd ff ff 89 c6 48 c7 c7 01 36 c7 81 e8 e8 40 fa ff <0f> 0b ba 00 10 00 00 be
5a 00 00 00 48 89 c7 48 d3 e2 e8 bf 18
RIP [<ffffffff8124592a>] new_slab+0x39a/0x410
RSP <ffff880036443b58>
---[ end trace ea3b0043b2911d93 ]---
static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
{
if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
pr_emerg("gfp: %u\n", flags & GFP_SLAB_BUG_MASK);
BUG();
}
return allocate_slab(s,
flags & (GFP_RECLAIM_MASK | GFP_CONSTRAINT_MASK), node);
}
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [ovs-dev] [PATCH net-next v11 5/6] openvswitch: add layer 3 flow/port support
From: Jiri Benc @ 2016-09-26 16:53 UTC (permalink / raw)
To: pravin shelar; +Cc: Simon Horman, Linux Kernel Network Developers, ovs dev
In-Reply-To: <CAOrHB_A1ri6fouFH0H4KpEqg1icCmz+U1tcgn-5BoryX5pb1gw@mail.gmail.com>
Reviving a very old thread, sorry. Simon handed this over to me, I'm
preparing v12.
On Fri, 15 Jul 2016 14:07:37 -0700, pravin shelar wrote:
> I am not sure if you can use only mac_len to detect L3 packet. This
> does not work with MPLS packets, mac_len is used to account MPLS
> headers pushed on skb. Therefore in case of a MPLS header on L3
> packet, mac_len would be non zero and we have to look at either
> mac_header or some other metadata like is_layer3 flag from key to
> check for L3 packet.
I went through the relevant code paths and I don't see any problem in
using mac_len for that. MPLS GSO seems to work correctly. The kernel
MPLS code expects mac_len to be just the L2 header len, excluding MPLS.
The same is the case for openvswitch (you're not correct that "mac_len
is used to account MPLS headers pushed on skb", at least not with the
current code). In no place I see any problem with mac_len being 0, the
calculations just nicely work.
What was your concern with that, Pravin?
In another mail in this thread you mentioned skb_mpls_header. That one
works correctly with mac_len == 0 if mac_header points to the beginning
of the packet.
You also wrote:
> I was thinking in overall networking stack rather than just ovs
> datapath. I think we should have consistent method of detecting L3
> packet. As commented in previous mail it could be achieved using
> skb-protocol and device type.
Again, mac_len == 0 works correctly and consistently, provided that
both mac_header and network_header point to the same place. In case of
a MPLS packet it would be the beginning of MPLS headers.
> > --- a/include/net/mpls.h
> > +++ b/include/net/mpls.h
> > @@ -34,6 +34,8 @@ static inline bool eth_p_mpls(__be16 eth_type)
> > */
> > static inline unsigned char *skb_mpls_header(struct sk_buff *skb)
> > {
> > - return skb_mac_header(skb) + skb->mac_len;
> > + return skb_mac_header_was_set(skb) ?
> > + skb_mac_header(skb) + skb->mac_len :
> > + skb->data;
> > }
>
> This function is also called from GSO layer.
I don't see it used anywhere outside of openvswitch. Not even when
grepping git history. I may be missing something, though.
> issue is in GSO layer, it
> does reset mac header and mac length and then calls mpls-gso-handler.
> So all subsequent check for L3 packet fails.
> So far we have explored three different ways to detect L3 packet but
> each has its own issue.
> 1. skb mac header : GSO can reset mac header.
> 2. skb mac length : MPLS uses mac_len to account for MPLS header
> length along with L2 header
It does not appear to be the case. Or at least not anymore.
> 3. skb protocol: ETH_P_TEB is not set for all L2 frames, networking
> stack is not ready to handle this type for given skb.
>
> So none of them works consistently. I think the only option to detect
> L3 packet reliably (and without adding field to skb) is to use
> skb-protocol along with ARPHRD_NONE device type. If ARPHRD_NONE type
> device generates L2 packet it needs to set protocol to ETH_P_TEB. Some
> networking stack function also needs to be fixed to handle this
> protocol type, e.g. vlan_get_protocol(), br_dev_queue_push_xmit(),
> etc.
All of this said, I'm not opposed to using the skb_eth_header_present
helper and checking the device type, it works. I just want to understand
whether I missed some problem with mac_len. Seems to make some things
simpler if we could use mac_len.
Thanks,
Jiri
^ permalink raw reply
* Re: [PATCH v5 00/16] Add Paravirtual RDMA Driver
From: Jason Gunthorpe @ 2016-09-26 16:51 UTC (permalink / raw)
To: Adit Ranadive
Cc: Leon Romanovsky, dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
asarwade-pghWNbHTmq7QT0dZR+AlfA,
georgezhang-pghWNbHTmq7QT0dZR+AlfA,
bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <9f65ab5c-d8c2-e8e3-9334-5d1865a20dc9-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
On Sun, Sep 25, 2016 at 10:25:12PM -0700, Adit Ranadive wrote:
> > As Jason said, you need a very good reason to split and create number of
> > files per-driver in UAPI folder.
>
> I can move the pvrdma-uapi.h back to the pvrdma driver folder.
Just don't copy any of the content into your user space provider.
Thanks
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 5/5] ISDN-Gigaset: Enclose two expressions for the sizeof operator by parentheses
From: Julia Lawall @ 2016-09-26 16:50 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Joe Perches, David Laight,
gigaset307x-common@lists.sourceforge.net, netdev@vger.kernel.org,
Karsten Keil, Paul Bolle, LKML, kernel-janitors@vger.kernel.org,
Julia Lawall
In-Reply-To: <d325c0ab-9f1b-e00d-6b4c-30f39d939ab4@users.sourceforge.net>
On Mon, 26 Sep 2016, SF Markus Elfring wrote:
> >>> @@ -53,7 +53,7 @@ void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
> >>> {
> >>> unsigned char outbuf[80];
> >>> unsigned char c;
> >>> - size_t space = sizeof outbuf - 1;
> >>> + size_t space = sizeof(outbuf - 1);
> >
> >> wrong ...
> >> think that is 7 instead of 79.
> >
> >
> > Well, it's sizeof(pointer) so either 4 or 8, but still,
> > what Markus proposes here is _wrong_.
>
> You are right at this place.
>
>
> > Markus, do you know the script probably has a lower
> > style conversion defect introduction rate than you do
> > when doing these mechanical things.
>
> Would it be nice if the script "checkpatch.pl" could avoid to point
> such a false positive out anyhow?
checkpatch reports:
WARNING: sizeof outbuf should be sizeof(outbuf)
#56: FILE: drivers/isdn/gigaset/common.c:56:
+ size_t space = sizeof outbuf - 1;
There is nothing wrong with the report,
julia
>
>
> > Please use it to verify what you are doing before
> > submitting more defective patches.
>
> Do you expect a resend for this update step so that an other statement
> in the function "gigaset_initdriver" would eventually be adjusted?
>
> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 5/5] ISDN-Gigaset: Enclose two expressions for the sizeof operator by parentheses
From: SF Markus Elfring @ 2016-09-26 16:45 UTC (permalink / raw)
To: Joe Perches, David Laight,
gigaset307x-common@lists.sourceforge.net, netdev@vger.kernel.org,
Karsten Keil, Paul Bolle
Cc: LKML, kernel-janitors@vger.kernel.org, Julia Lawall
In-Reply-To: <1474907003.2238.13.camel@perches.com>
>>> @@ -53,7 +53,7 @@ void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
>>> {
>>> unsigned char outbuf[80];
>>> unsigned char c;
>>> - size_t space = sizeof outbuf - 1;
>>> + size_t space = sizeof(outbuf - 1);
>
>> wrong ...
>> think that is 7 instead of 79.
>
>
> Well, it's sizeof(pointer) so either 4 or 8, but still,
> what Markus proposes here is _wrong_.
You are right at this place.
> Markus, do you know the script probably has a lower
> style conversion defect introduction rate than you do
> when doing these mechanical things.
Would it be nice if the script "checkpatch.pl" could avoid to point
such a false positive out anyhow?
> Please use it to verify what you are doing before
> submitting more defective patches.
Do you expect a resend for this update step so that an other statement
in the function "gigaset_initdriver" would eventually be adjusted?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH nf-next 2/2] nf_set_hooks_head: acommodate different kconfig
From: Aaron Conole @ 2016-09-26 16:43 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, netdev, Pablo Neira Ayuso
In-Reply-To: <20160926163920.GB17426@breakpoint.cc>
Florian Westphal <fw@strlen.de> writes:
> Aaron Conole <aconole@bytheb.org> wrote:
>> When CONFIG_NETFILTER_INGRESS is unset (or no), we need to handle
>> the request for registration properly by dropping the hook. This
>> releases the entry during the set.
>>
>> Signed-off-by: Aaron Conole <aconole@bytheb.org>
>> ---
>> net/netfilter/core.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
>> index e58e420..1d0a4c9 100644
>> --- a/net/netfilter/core.c
>> +++ b/net/netfilter/core.c
>> @@ -90,10 +90,14 @@ static void nf_set_hooks_head(struct net *net, const struct nf_hook_ops *reg,
>> {
>> switch (reg->pf) {
>> case NFPROTO_NETDEV:
>> +#ifdef CONFIG_NETFILTER_INGRESS
>> /* We already checked in nf_register_net_hook() that this is
>> * used from ingress.
>> */
>> rcu_assign_pointer(reg->dev->nf_hooks_ingress, entry);
>> +#else
>> + kfree(entry);
>> +#endif
>> break;
>
> This looks dodgy (its correct though).
>
> I'd propose to add a test to nf_register_net_hook()
> to bail with -EOPNOSTUPP instead of this "#else kfree()" if we get
> NFPROTO_NETDEV pf with CONFIG_NETFILTER_INGRESS=n build instead.
Okay, I'll spin a new version.
Thanks for the review, Florian!
-Aaron
^ permalink raw reply
* Re: [PATCH nf-next 2/2] nf_set_hooks_head: acommodate different kconfig
From: Florian Westphal @ 2016-09-26 16:39 UTC (permalink / raw)
To: Aaron Conole; +Cc: netfilter-devel, netdev, Florian Westphal, Pablo Neira Ayuso
In-Reply-To: <1474907071-13591-3-git-send-email-aconole@bytheb.org>
Aaron Conole <aconole@bytheb.org> wrote:
> When CONFIG_NETFILTER_INGRESS is unset (or no), we need to handle
> the request for registration properly by dropping the hook. This
> releases the entry during the set.
>
> Signed-off-by: Aaron Conole <aconole@bytheb.org>
> ---
> net/netfilter/core.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
> index e58e420..1d0a4c9 100644
> --- a/net/netfilter/core.c
> +++ b/net/netfilter/core.c
> @@ -90,10 +90,14 @@ static void nf_set_hooks_head(struct net *net, const struct nf_hook_ops *reg,
> {
> switch (reg->pf) {
> case NFPROTO_NETDEV:
> +#ifdef CONFIG_NETFILTER_INGRESS
> /* We already checked in nf_register_net_hook() that this is
> * used from ingress.
> */
> rcu_assign_pointer(reg->dev->nf_hooks_ingress, entry);
> +#else
> + kfree(entry);
> +#endif
> break;
This looks dodgy (its correct though).
I'd propose to add a test to nf_register_net_hook()
to bail with -EOPNOSTUPP instead of this "#else kfree()" if we get
NFPROTO_NETDEV pf with CONFIG_NETFILTER_INGRESS=n build instead.
^ permalink raw reply
* [PATCH nf-next 2/2] nf_set_hooks_head: acommodate different kconfig
From: Aaron Conole @ 2016-09-26 16:24 UTC (permalink / raw)
To: netfilter-devel, netdev; +Cc: Florian Westphal, Pablo Neira Ayuso
In-Reply-To: <1474907071-13591-1-git-send-email-aconole@bytheb.org>
When CONFIG_NETFILTER_INGRESS is unset (or no), we need to handle
the request for registration properly by dropping the hook. This
releases the entry during the set.
Signed-off-by: Aaron Conole <aconole@bytheb.org>
---
net/netfilter/core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index e58e420..1d0a4c9 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -90,10 +90,14 @@ static void nf_set_hooks_head(struct net *net, const struct nf_hook_ops *reg,
{
switch (reg->pf) {
case NFPROTO_NETDEV:
+#ifdef CONFIG_NETFILTER_INGRESS
/* We already checked in nf_register_net_hook() that this is
* used from ingress.
*/
rcu_assign_pointer(reg->dev->nf_hooks_ingress, entry);
+#else
+ kfree(entry);
+#endif
break;
default:
rcu_assign_pointer(net->nf.hooks[reg->pf][reg->hooknum],
--
2.5.5
^ permalink raw reply related
* [PATCH nf-next 1/2] netfilter: Fix potential null pointer dereference
From: Aaron Conole @ 2016-09-26 16:24 UTC (permalink / raw)
To: netfilter-devel, netdev; +Cc: Florian Westphal, Pablo Neira Ayuso
In-Reply-To: <1474907071-13591-1-git-send-email-aconole@bytheb.org>
It's possible for nf_hook_entry_head to return NULL if two
nf_unregister_net_hook calls happen simultaneously with a single hook
entry in the list. This fix ensures that no null pointer dereference
could occur when such a race happens.
Signed-off-by: Aaron Conole <aconole@bytheb.org>
---
net/netfilter/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 360c63d..e58e420 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -160,7 +160,7 @@ void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
mutex_lock(&nf_hook_mutex);
hooks_entry = nf_hook_entry_head(net, reg);
- if (hooks_entry->orig_ops == reg) {
+ if (hooks_entry && hooks_entry->orig_ops == reg) {
nf_set_hooks_head(net, reg,
nf_entry_dereference(hooks_entry->next));
goto unlock;
--
2.5.5
^ permalink raw reply related
* [PATCH nf-next 0/2] fixes for recent nf_compact hooks
From: Aaron Conole @ 2016-09-26 16:24 UTC (permalink / raw)
To: netfilter-devel, netdev; +Cc: Florian Westphal, Pablo Neira Ayuso
Two possible error conditions were caught during an extended testing
session, and by a build robot. These patches fix the two issues (a
missing handler when config is changed, and a potential NULL
dereference).
Aaron Conole (2):
netfilter: Fix potential null pointer dereference
nf_set_hooks_head: acommodate different kconfig
net/netfilter/core.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
--
2.5.5
^ permalink raw reply
* Re: [PATCH 5/5] ISDN-Gigaset: Enclose two expressions for the sizeof operator by parentheses
From: Joe Perches @ 2016-09-26 16:23 UTC (permalink / raw)
To: David Laight, 'SF Markus Elfring',
gigaset307x-common@lists.sourceforge.net, netdev@vger.kernel.org,
Karsten Keil, Paul Bolle
Cc: LKML, kernel-janitors@vger.kernel.org, Julia Lawall
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0109E63@AcuExch.aculab.com>
On Mon, 2016-09-26 at 16:00 +0000, David Laight wrote:
> From: SF Markus Elfring Sent: 26 September 2016 16:45
> > The script "checkpatch.pl" can point information out like the following.
> > WARNING: sizeof … should be sizeof(…)
> []
> > diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
[]
> > @@ -53,7 +53,7 @@ void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
> > {
> > unsigned char outbuf[80];
> > unsigned char c;
> > - size_t space = sizeof outbuf - 1;
> > + size_t space = sizeof(outbuf - 1);
> wrong ...
> think that is 7 instead of 79.
Well, it's sizeof(pointer) so either 4 or 8, but still,
what Markus proposes here is _wrong_.
Markus, do you know the script probably has a lower
style conversion defect introduction rate than you do
when doing these mechanical things.
Please use it to verify what you are doing before
submitting more defective patches.
Excuses and apologies aren't good enough at this point.
^ permalink raw reply
* RE: [PATCH 5/5] ISDN-Gigaset: Enclose two expressions for the sizeof operator by parentheses
From: David Laight @ 2016-09-26 16:00 UTC (permalink / raw)
To: 'SF Markus Elfring',
gigaset307x-common@lists.sourceforge.net, netdev@vger.kernel.org,
Karsten Keil, Paul Bolle
Cc: LKML, kernel-janitors@vger.kernel.org, Julia Lawall
In-Reply-To: <e97b38b6-8c02-6733-8b57-9786bfb911a6@users.sourceforge.net>
From: SF Markus Elfring
> Sent: 26 September 2016 16:45
...
> The script "checkpatch.pl" can point information out like the following.
>
> WARNING: sizeof … should be sizeof(…)
...
> ---
> drivers/isdn/gigaset/common.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
> index 2e9382f..d901ed7 100644
> --- a/drivers/isdn/gigaset/common.c
> +++ b/drivers/isdn/gigaset/common.c
> @@ -53,7 +53,7 @@ void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
> {
> unsigned char outbuf[80];
> unsigned char c;
> - size_t space = sizeof outbuf - 1;
> + size_t space = sizeof(outbuf - 1);
wrong ...
think that is 7 instead of 79.
David
^ permalink raw reply
* Re: hostap: Delete an unnecessary jump label in prism2_ioctl_priv_hostapd()
From: SF Markus Elfring @ 2016-09-26 16:03 UTC (permalink / raw)
To: Kalle Valo
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Jouni Malinen, LKML,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA, Julia Lawall
In-Reply-To: <20160926150656.213D961568-4h6buKAYkuurB/BPivuO70B+6BGkLq7r@public.gmane.org>
> 9291771 [2/3] hostap: Delete an unnecessary jump label in prism2_ioctl_priv_hostapd()
> 9291775 [3/3] hostap: Delete unnecessary initialisations for the variable "ret"
>
> Reason: The benefit is not clear.
How do you think about to reduce the source code a bit at these places?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: mpls: Fixups for GSO
From: Jiri Benc @ 2016-09-26 15:56 UTC (permalink / raw)
To: David Ahern
Cc: pravin shelar, Simon Horman, Pravin B Shelar,
Linux Kernel Network Developers, David S. Miller, buytenh,
Eric W. Biederman, rshearma, tom, Thomas Graf, olivier.dugeon,
Alexander Duyck, roopa
In-Reply-To: <586321df-72c6-0dee-4ce6-22ca2a0860fb@cumulusnetworks.com>
On Wed, 24 Aug 2016 10:37:51 -0600, David Ahern wrote:
> Something like this should be able to handle multiple labels. The
> inner network header is set once and the outer one pointing to MPLS
> is adjusted each time a label is pushed:
>
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 1ecbd7715f6d..0f37b17e3a73 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -162,10 +162,16 @@ static int push_mpls(struct sk_buff *skb,
> struct sw_flow_key *key, if (skb_cow_head(skb, MPLS_HLEN) < 0)
> return -ENOMEM;
>
> + if (!skb->inner_protocol) {
> + skb_set_inner_network_header(skb, skb->mac_len);
> + skb_set_inner_protocol(skb, skb->protocol);
> + }
> +
> skb_push(skb, MPLS_HLEN);
> memmove(skb_mac_header(skb) - MPLS_HLEN, skb_mac_header(skb),
> skb->mac_len);
> skb_reset_mac_header(skb);
> + skb_set_network_header(skb, skb->mac_len);
Sorry for chiming in after a month. The code above got in
(48d2ab609b6bb), I'm currently looking at this and it looks very
suspicious to me.
After push_mpls, network_header points to the start of MPLS headers.
Which I understand was the point of this patch. However, push_mpls also
calls invalidate_flow_key. Meaning that, depending on actions, we may
end up calling key_extract soon after. And key_extract sets the network
header *after* the MPLS headers.
That means that on output, for otherwise identical packet,
network_header can point before or after MPLS headers based on what
actions happened to be executed (recirculation, mainly).
If I'm not misreading the code or missing something, this can't be
right.
mpls_gso_segment does not care, it resets the network_header anyway.
What about drivers? What is the correct behavior?
Jiri
^ permalink raw reply
* Re: [PATCH net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Daniel Borkmann @ 2016-09-26 15:53 UTC (permalink / raw)
To: Hannes Frederic Sowa, Florian Westphal, Jamal Hadi Salim
Cc: Shmulik Ladkani, David S. Miller, WANG Cong, Eric Dumazet, netdev
In-Reply-To: <c8d67662-1eaf-7f7e-5a99-17ec0a4521f7@stressinduktion.org>
On 09/26/2016 05:12 PM, Hannes Frederic Sowa wrote:
[...]
> I would just care that we sometimes reschedule and don't do everything
> in one stack so we don't corrupt the machine and an admin has still a
> chance to solve the problem.
Sounds reasonable to me, and which is what dev_forward_skb() is doing
implicitly as well.
^ permalink raw reply
* RE: [RFC v3 00/11] QLogic RDMA Driver (qedr) RFC
From: Amrani, Ram @ 2016-09-26 15:45 UTC (permalink / raw)
To: davem@davemloft.net, dledford@redhat.com
Cc: Elior, Ariel, Kalderon, Michal, Mintz, Yuval, Borundia, Rajesh,
linux-rdma@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <1474899407-12635-1-git-send-email-Ram.Amrani@cavium.com>
> The series adds on top of RFC v2:
> * fix licensing header to dual license
> * remove 'debug' module paramter and make use of pr_debug
> * relocation of qedr user API to include/rdma/uapi/
> * use the __u32/64 in uapi and include types.h
> * advance ABI version (since shifting to __u32/64 changed the ABI)
> * remove the check for all drivers that IB_ACCESS_MW_BIND isn't set
> in ib_get_dma_mr. It will be sent in a dedicated patch.
> * misc.: fixed typos, removed redundant includes
>
> For RFC v2 visit http://marc.info/?l=linux-rdma&m=147436779328618&w=2
I see that the following e-mails were sent with "PATCH" in the subject rather than "RFC v3".
This is a mistake. All e-mails are intended to be "RFC v3".
I don't think it should matter much for reading and reviewing purposes, but don't hesitate if
you would like me to re-send with fixed headers.
Ram
^ permalink raw reply
* [PATCH 5/5] ISDN-Gigaset: Enclose two expressions for the sizeof operator by parentheses
From: SF Markus Elfring @ 2016-09-26 15:44 UTC (permalink / raw)
To: gigaset307x-common, netdev, Karsten Keil, Paul Bolle
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <6d4bbb77-914f-19b8-a1a9-2731d1158612@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 26 Sep 2016 17:03:56 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The script "checkpatch.pl" can point information out like the following.
WARNING: sizeof … should be sizeof(…)
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/isdn/gigaset/common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
index 2e9382f..d901ed7 100644
--- a/drivers/isdn/gigaset/common.c
+++ b/drivers/isdn/gigaset/common.c
@@ -53,7 +53,7 @@ void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
{
unsigned char outbuf[80];
unsigned char c;
- size_t space = sizeof outbuf - 1;
+ size_t space = sizeof(outbuf - 1);
unsigned char *out = outbuf;
size_t numin = len;
@@ -1079,7 +1079,7 @@ struct gigaset_driver *gigaset_initdriver(unsigned minor, unsigned minors,
unsigned long flags;
unsigned i;
- drv = kmalloc(sizeof *drv, GFP_KERNEL);
+ drv = kmalloc(sizeof(*drv), GFP_KERNEL);
if (!drv)
return NULL;
--
2.10.0
^ permalink raw reply related
* [PATCH 4/5] ISDN-Gigaset: Release memory in gigaset_initcs() after an allocation failure
From: SF Markus Elfring @ 2016-09-26 15:43 UTC (permalink / raw)
To: gigaset307x-common, netdev, Karsten Keil, Paul Bolle
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <6d4bbb77-914f-19b8-a1a9-2731d1158612@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 26 Sep 2016 16:30:50 +0200
Memory was not released (as it would be expected) when one call
of further resource reservations failed.
* Split a condition check for memory allocation failures so that
each pointer from these function calls will be checked immediately.
See also background information:
Topic "CWE-754: Improper check for unusual or exceptional conditions"
Link: https://cwe.mitre.org/data/definitions/754.html
* Adjust jump targets according to the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/isdn/gigaset/common.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
index c05a2a4..2e9382f 100644
--- a/drivers/isdn/gigaset/common.c
+++ b/drivers/isdn/gigaset/common.c
@@ -710,10 +710,13 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
cs->mode = M_UNKNOWN;
cs->mstate = MS_UNINITIALIZED;
cs->bcs = kmalloc_array(channels, sizeof(*cs->bcs), GFP_KERNEL);
+ if (!cs->bcs)
+ goto report_failure;
+
cs->inbuf = kmalloc(sizeof(*cs->inbuf), GFP_KERNEL);
- if (!cs->bcs || !cs->inbuf) {
- goto error;
- }
+ if (!cs->inbuf)
+ goto free_bcs;
+
++cs->cs_init;
gig_dbg(DEBUG_INIT, "setting up at_state");
@@ -737,14 +740,14 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
gig_dbg(DEBUG_INIT, "setting up iif");
if (gigaset_isdn_regdev(cs, modulename) < 0) {
pr_err("error registering ISDN device\n");
- goto error;
+ goto free_bcs;
}
make_valid(cs, VALID_ID);
++cs->cs_init;
gig_dbg(DEBUG_INIT, "setting up hw");
if (cs->ops->initcshw(cs) < 0)
- goto error;
+ goto free_bcs;
++cs->cs_init;
@@ -759,7 +762,7 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
gig_dbg(DEBUG_INIT, "setting up bcs[%d]", i);
if (gigaset_initbcs(cs->bcs + i, cs, i) < 0) {
pr_err("could not allocate channel %d data\n", i);
- goto error;
+ goto free_bcs;
}
}
@@ -772,8 +775,9 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
gig_dbg(DEBUG_INIT, "cs initialized");
return cs;
-
-error:
+free_bcs:
+ kfree(cs->bcs);
+report_failure:
gig_dbg(DEBUG_INIT, "failed");
gigaset_freecs(cs);
return NULL;
--
2.10.0
^ permalink raw reply related
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