* [PATCH] orinoco: Delete an unnecessary check before the function call "kfree"
From: SF Markus Elfring @ 2015-02-04 18:56 UTC (permalink / raw)
To: Kalle Valo, linux-wireless, netdev; +Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 4 Feb 2015 19:53:11 +0100
The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/wireless/orinoco/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/orinoco/main.c b/drivers/net/wireless/orinoco/main.c
index 38ec8d1..c410180 100644
--- a/drivers/net/wireless/orinoco/main.c
+++ b/drivers/net/wireless/orinoco/main.c
@@ -2342,7 +2342,7 @@ void free_orinocodev(struct orinoco_private *priv)
list_for_each_entry_safe(sd, sdtemp, &priv->scan_list, list) {
list_del(&sd->list);
- if ((sd->len > 0) && sd->buf)
+ if (sd->len > 0)
kfree(sd->buf);
kfree(sd);
}
--
2.2.2
^ permalink raw reply related
* Re: [PATCH net-next v2 1/2] pkt_sched: fq: avoid artificial bursts for clocked flows
From: Eric Dumazet @ 2015-02-04 18:47 UTC (permalink / raw)
To: Kenneth Klette Jonassen; +Cc: netdev@vger.kernel.org, David Miller
In-Reply-To: <2E34EE83-0F64-4CDE-BDE2-4AB59B4D5CF8@ifi.uio.no>
On Wed, 2015-02-04 at 17:56 +0000, Kenneth Klette Jonassen wrote:
>
> If it is a code issue, please refactor it or point out some specifics.
>
> Thanks for cc’ing me on your related patch (“do not pace pure ack
> packets”). Note that your patch does not address the core issue that
> this patch attempts to fix: fq throttling is just broken for small,
> clocked flows. This not only concerns TCP/SCTP/ARQ acks, but also
> thin-streams like VoIP and gaming.
I already told you this patch was not wanted.
We want a reasonably efficient packet scheduler, allowing TCP pacing,
at the scale of million of flows.
You can reduce quantum to whatever you need.
We have an internal patch doing something really different,
not adding 8 bytes overhead in the flow structure.
We will upstream it when fully tested.
commit 799735e5d08b3a7fc19176eee316ca647c9bad32
Author: Eric Dumazet <edumazet@google.com>
Date: Mon Aug 11 14:35:36 2014 -0700
net-sched-fq: special case low rate flows
Effort: net-sched-fq
Change-Id: Ibee17453702cccf729154f6b4db7aeb028fc555a
^ permalink raw reply
* [PATCH] ath10k: Delete unnecessary checks before the function call "release_firmware"
From: SF Markus Elfring @ 2015-02-04 18:33 UTC (permalink / raw)
To: Kalle Valo, ath10k, linux-wireless, netdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 4 Feb 2015 19:30:23 +0100
The release_firmware() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/wireless/ath/ath10k/core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 2d0671e..45171ad 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -393,16 +393,16 @@ static int ath10k_download_fw(struct ath10k *ar, enum ath10k_firmware_mode mode)
static void ath10k_core_free_firmware_files(struct ath10k *ar)
{
- if (ar->board && !IS_ERR(ar->board))
+ if (!IS_ERR(ar->board))
release_firmware(ar->board);
- if (ar->otp && !IS_ERR(ar->otp))
+ if (!IS_ERR(ar->otp))
release_firmware(ar->otp);
- if (ar->firmware && !IS_ERR(ar->firmware))
+ if (!IS_ERR(ar->firmware))
release_firmware(ar->firmware);
- if (ar->cal_file && !IS_ERR(ar->cal_file))
+ if (!IS_ERR(ar->cal_file))
release_firmware(ar->cal_file);
ar->board = NULL;
--
2.2.2
^ permalink raw reply related
* Re: [PATCH net-next v2 1/2] pkt_sched: fq: avoid artificial bursts for clocked flows
From: Kenneth Klette Jonassen @ 2015-02-04 17:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev@vger.kernel.org, David Miller
In-Reply-To: <1423001164.907.43.camel@edumazet-glaptop2.roam.corp.google.com>
>> - if (tb[TCA_FQ_INITIAL_QUANTUM])
>> - q->initial_quantum = nla_get_u32(tb[TCA_FQ_INITIAL_QUANTUM]);
>> + if (tb[TCA_FQ_INITIAL_QUANTUM]) {
>> + u32 initial_quantum = nla_get_u32(tb[TCA_FQ_INITIAL_QUANTUM]);
>> +
>> + if (initial_quantum > q->quantum)
>> + q->initial_quantum = initial_quantum - q->quantum;
>> + else
>> + q->initial_quantum = 0;
>> + }
>
> Please do not mix patches. This part already have been Acked by me.
The hang fix touches another part of fq. This part is specific to this patch.
>
> Quite frankly I do not like this patch.
If it is a code issue, please refactor it or point out some specifics.
Thanks for cc’ing me on your related patch (“do not pace pure ack packets”). Note that your patch does not address the core issue that this patch attempts to fix: fq throttling is just broken for small, clocked flows. This not only concerns TCP/SCTP/ARQ acks, but also thin-streams like VoIP and gaming.
^ permalink raw reply
* [PATCH] ath9k: Delete an unnecessary check before the function call "relay_close"
From: SF Markus Elfring @ 2015-02-04 17:54 UTC (permalink / raw)
To: Kalle Valo, ath9k-devel, linux-wireless, QCA ath9k Development,
netdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 4 Feb 2015 18:48:28 +0100
The relay_close() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/wireless/ath/ath9k/common-spectral.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath9k/common-spectral.c b/drivers/net/wireless/ath/ath9k/common-spectral.c
index ec93ddf..5cee231 100644
--- a/drivers/net/wireless/ath/ath9k/common-spectral.c
+++ b/drivers/net/wireless/ath/ath9k/common-spectral.c
@@ -582,7 +582,7 @@ static struct rchan_callbacks rfs_spec_scan_cb = {
void ath9k_cmn_spectral_deinit_debug(struct ath_spec_scan_priv *spec_priv)
{
- if (config_enabled(CONFIG_ATH9K_DEBUGFS) && spec_priv->rfs_chan_spec_scan) {
+ if (config_enabled(CONFIG_ATH9K_DEBUGFS)) {
relay_close(spec_priv->rfs_chan_spec_scan);
spec_priv->rfs_chan_spec_scan = NULL;
}
--
2.2.2
^ permalink raw reply related
* Re: [PATCH net-next] i40e: mark constant as ULL
From: Jeff Kirsher @ 2015-02-04 17:49 UTC (permalink / raw)
To: Stefan Assmann; +Cc: netdev, davem, e1000-devel
In-Reply-To: <1423050064-25087-1-git-send-email-sassmann@kpanic.de>
[-- Attachment #1: Type: text/plain, Size: 519 bytes --]
On Wed, 2015-02-04 at 12:41 +0100, Stefan Assmann wrote:
> This avoids a compile error on 32bit.
>
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
> drivers/net/ethernet/intel/i40e/i40e_lan_hmc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Come to find out, I already have a fix in my queue for this. So I will
be dropping your patch.
The patch in my queue fixes the issue by doing the folling change:
- mask = 0xFFFFFFFFFFFFFFFF;
+ mask = ~(u64)0;
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] bridge: Remove BR_PROXYARP flooding check code
From: Jouni Malinen @ 2015-02-04 17:36 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev, Kyeyoon Park
In-Reply-To: <20141209142158.7e513dbf@urahara>
On Tue, Dec 09, 2014 at 02:21:58PM -0800, Stephen Hemminger wrote:
> On Mon, 8 Dec 2014 17:27:40 +0200
> > From: Kyeyoon Park <kyeyoonp@codeaurora.org>
> > Because dropping broadcast packets for IEEE 802.11 Proxy ARP is more
> > selective than previously thought, it is better to remove the direct
> > dropping logic in the bridge code in favor of using the netfilter
> > infrastructure to provide more control on which frames get dropped. This
> > code was added in commit 958501163ddd ("bridge: Add support for IEEE
> > 802.11 Proxy ARP").
> Aren't you at risk of duplicate ARP responses in some cases.
> You can't assume user will run netfilter.
The 'bridge: Selectively prevent bridge port flooding for proxy ARP'
patch that I posted earlier today addresses this by keeping record of
whether the proxyarp functionality has replied to a packet and skipping
flooding conditionally on that. In other words, this 'bridge: Remove
BR_PROXYARP flooding check code' can be dropped.
--
Jouni Malinen PGP id EFC895FA
^ permalink raw reply
* Re: skb_try_coalesce and fraglists (bug?)
From: Eric Dumazet @ 2015-02-04 17:28 UTC (permalink / raw)
To: Erik Hugne; +Cc: netdev, mkubecek, ying.xue
In-Reply-To: <20150204163343.GA857@haze>
On Wed, 2015-02-04 at 17:33 +0100, Erik Hugne wrote:
> skb_try_coalesce should bail out for a number of reasons, if the target skb is
> cloned, doesn't have enough room, or if either source or target skb have
> fraglists.
>
> However, it seems that the skb_has_frag_list check is done too late, and a small
> skb may be copied into the tailroom of the head, even if it has a fraglist.
>
> Wouldn't this be more correct?
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 56db472..8d02140 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4056,6 +4056,9 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
> if (skb_cloned(to))
> return false;
>
> + if (skb_has_frag_list(to) || skb_has_frag_list(from))
> + return false;
> +
> if (len <= skb_tailroom(to)) {
> if (len)
> BUG_ON(skb_copy_bits(from, 0, skb_put(to, len), len));
> @@ -4063,9 +4066,6 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
> return true;
> }
>
> - if (skb_has_frag_list(to) || skb_has_frag_list(from))
> - return false;
> -
> if (skb_headlen(from) != 0) {
> struct page *page;
> unsigned int offset;
>
Patch is not needed.
If to had a fraglist, skb_tailroom(to) would be 0
^ permalink raw reply
* (unknown),
From: Hines, Aaron @ 2015-02-04 5:07 UTC (permalink / raw)
I am Patrick Chan from Hong Kong i have a lucrative biz deal worth $16.7M to relate to you if interested contact me: mrpatrickchan42@yahoo.com.hk
^ permalink raw reply
* [PATCH 2/2] CW1200: Less function calls in cw1200_load_firmware_cw1200() after error detection
From: SF Markus Elfring @ 2015-02-04 16:48 UTC (permalink / raw)
To: Kalle Valo, Solomon Peachy, netdev, linux-wireless
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54D24CC3.9010006@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 4 Feb 2015 17:28:41 +0100
The functions kfree() and release_firmware() were called in a few cases
by the cw1200_load_firmware_cw1200() function during error handling even if
the passed variables contained still a null pointer.
Corresponding implementation details could be improved by adjustments for
jump targets.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/wireless/cw1200/fwio.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/cw1200/fwio.c b/drivers/net/wireless/cw1200/fwio.c
index 581dfde..30e7646 100644
--- a/drivers/net/wireless/cw1200/fwio.c
+++ b/drivers/net/wireless/cw1200/fwio.c
@@ -66,25 +66,31 @@ static int cw1200_load_firmware_cw1200(struct cw1200_common *priv)
do { \
ret = cw1200_apb_write_32(priv, CW1200_APB(reg), (val)); \
if (ret < 0) \
- goto error; \
+ goto exit; \
+ } while (0)
+#define APB_WRITE2(reg, val) \
+ do { \
+ ret = cw1200_apb_write_32(priv, CW1200_APB(reg), (val)); \
+ if (ret < 0) \
+ goto free_buffer; \
} while (0)
#define APB_READ(reg, val) \
do { \
ret = cw1200_apb_read_32(priv, CW1200_APB(reg), &(val)); \
if (ret < 0) \
- goto error; \
+ goto free_buffer; \
} while (0)
#define REG_WRITE(reg, val) \
do { \
ret = cw1200_reg_write_32(priv, (reg), (val)); \
if (ret < 0) \
- goto error; \
+ goto exit; \
} while (0)
#define REG_READ(reg, val) \
do { \
ret = cw1200_reg_read_32(priv, (reg), &(val)); \
if (ret < 0) \
- goto error; \
+ goto exit; \
} while (0)
switch (priv->hw_revision) {
@@ -142,14 +148,14 @@ static int cw1200_load_firmware_cw1200(struct cw1200_common *priv)
ret = request_firmware(&firmware, fw_path, priv->pdev);
if (ret) {
pr_err("Can't load firmware file %s.\n", fw_path);
- goto error;
+ goto exit;
}
buf = kmalloc(DOWNLOAD_BLOCK_SIZE, GFP_KERNEL | GFP_DMA);
if (!buf) {
pr_err("Can't allocate firmware load buffer.\n");
ret = -ENOMEM;
- goto error;
+ goto firmware_release;
}
/* Check if the bootloader is ready */
@@ -163,7 +169,7 @@ static int cw1200_load_firmware_cw1200(struct cw1200_common *priv)
if (val32 != DOWNLOAD_I_AM_HERE) {
pr_err("Bootloader is not ready.\n");
ret = -ETIMEDOUT;
- goto error;
+ goto free_buffer;
}
/* Calculcate number of download blocks */
@@ -171,7 +177,7 @@ static int cw1200_load_firmware_cw1200(struct cw1200_common *priv)
/* Updating the length in Download Ctrl Area */
val32 = firmware->size; /* Explicit cast from size_t to u32 */
- APB_WRITE(DOWNLOAD_IMAGE_SIZE_REG, val32);
+ APB_WRITE2(DOWNLOAD_IMAGE_SIZE_REG, val32);
/* Firmware downloading loop */
for (block = 0; block < num_blocks; block++) {
@@ -183,7 +189,7 @@ static int cw1200_load_firmware_cw1200(struct cw1200_common *priv)
if (val32 != DOWNLOAD_PENDING) {
pr_err("Bootloader reported error %d.\n", val32);
ret = -EIO;
- goto error;
+ goto free_buffer;
}
/* loop until put - get <= 24K */
@@ -198,7 +204,7 @@ static int cw1200_load_firmware_cw1200(struct cw1200_common *priv)
if ((put - get) > (DOWNLOAD_FIFO_SIZE - DOWNLOAD_BLOCK_SIZE)) {
pr_err("Timeout waiting for FIFO.\n");
ret = -ETIMEDOUT;
- goto error;
+ goto free_buffer;
}
/* calculate the block size */
@@ -220,12 +226,12 @@ static int cw1200_load_firmware_cw1200(struct cw1200_common *priv)
if (ret < 0) {
pr_err("Can't write firmware block @ %d!\n",
put & (DOWNLOAD_FIFO_SIZE - 1));
- goto error;
+ goto free_buffer;
}
/* update the put register */
put += block_size;
- APB_WRITE(DOWNLOAD_PUT_REG, put);
+ APB_WRITE2(DOWNLOAD_PUT_REG, put);
} /* End of firmware download loop */
/* Wait for the download completion */
@@ -238,18 +244,21 @@ static int cw1200_load_firmware_cw1200(struct cw1200_common *priv)
if (val32 != DOWNLOAD_SUCCESS) {
pr_err("Wait for download completion failed: 0x%.8X\n", val32);
ret = -ETIMEDOUT;
- goto error;
+ goto free_buffer;
} else {
pr_info("Firmware download completed.\n");
ret = 0;
}
-error:
+free_buffer:
kfree(buf);
+firmware_release:
release_firmware(firmware);
+exit:
return ret;
#undef APB_WRITE
+#undef APB_WRITE2
#undef APB_READ
#undef REG_WRITE
#undef REG_READ
--
2.2.2
^ permalink raw reply related
* [PATCH 1/2] CW1200: Delete an unnecessary check before the function call "release_firmware"
From: SF Markus Elfring @ 2015-02-04 16:47 UTC (permalink / raw)
To: Kalle Valo, Solomon Peachy, netdev, linux-wireless
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54D24CC3.9010006@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 4 Feb 2015 16:32:15 +0100
The release_firmware() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/wireless/cw1200/fwio.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/cw1200/fwio.c b/drivers/net/wireless/cw1200/fwio.c
index 6f1b9aa..581dfde 100644
--- a/drivers/net/wireless/cw1200/fwio.c
+++ b/drivers/net/wireless/cw1200/fwio.c
@@ -246,8 +246,7 @@ static int cw1200_load_firmware_cw1200(struct cw1200_common *priv)
error:
kfree(buf);
- if (firmware)
- release_firmware(firmware);
+ release_firmware(firmware);
return ret;
#undef APB_WRITE
--
2.2.2
^ permalink raw reply related
* [PATCH 0/2] CW1200: Deletion of an unnecessary check
From: SF Markus Elfring @ 2015-02-04 16:45 UTC (permalink / raw)
To: Kalle Valo, Solomon Peachy, netdev, linux-wireless
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 4 Feb 2015 17:31:00 +0100
Another update suggestion was taken into account after a patch was applied
from static source code analysis.
Markus Elfring (2):
Delete an unnecessary check before the function call "release_firmware"
Less function calls in cw1200_load_firmware_cw1200() after error detection
drivers/net/wireless/cw1200/fwio.c | 40 +++++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 16 deletions(-)
--
2.2.2
^ permalink raw reply
* skb_try_coalesce and fraglists (bug?)
From: Erik Hugne @ 2015-02-04 16:33 UTC (permalink / raw)
To: netdev; +Cc: mkubecek, ying.xue
skb_try_coalesce should bail out for a number of reasons, if the target skb is
cloned, doesn't have enough room, or if either source or target skb have
fraglists.
However, it seems that the skb_has_frag_list check is done too late, and a small
skb may be copied into the tailroom of the head, even if it has a fraglist.
Wouldn't this be more correct?
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 56db472..8d02140 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4056,6 +4056,9 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
if (skb_cloned(to))
return false;
+ if (skb_has_frag_list(to) || skb_has_frag_list(from))
+ return false;
+
if (len <= skb_tailroom(to)) {
if (len)
BUG_ON(skb_copy_bits(from, 0, skb_put(to, len), len));
@@ -4063,9 +4066,6 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
return true;
}
- if (skb_has_frag_list(to) || skb_has_frag_list(from))
- return false;
-
if (skb_headlen(from) != 0) {
struct page *page;
unsigned int offset;
^ permalink raw reply related
* Re: [PATCH net-next] bridge: Let bridge not age 'externally' learnt FDB entries, they are removed when 'external' entity notifies the aging
From: roopa @ 2015-02-04 16:19 UTC (permalink / raw)
To: Siva Mannem; +Cc: Netdev, Scott Feldman, Jiri Pirko
In-Reply-To: <CA+CtxLSziWmx94_V9S-uW9=6VYg4WXsU4CERx-uovvrFOtO1dA@mail.gmail.com>
On 2/4/15, 12:02 AM, Siva Mannem wrote:
> On Tue, Feb 3, 2015 at 8:41 PM, roopa <roopa@cumulusnetworks.com> wrote:
>> On 2/2/15, 9:21 AM, Siva Mannem wrote:
>>> When 'learned_sync' flag is turned on, the offloaded switch
>>> port syncs learned MAC addresses to bridge's FDB via switchdev notifier
>>> (NETDEV_SWITCH_FDB_ADD). Currently, FDB entries learnt via this
>>> mechanism are
>>> wrongly being deleted by bridge aging logic. This patch ensures that FDB
>>> entries synced from offloaded switch ports are not deleted by bridging
>>> logic.
>>> Such entries can only be deleted via switchdev notifier
>>> (NETDEV_SWITCH_FDB_DEL).
>>
>> Your patch seems right and maintains symmetry for fdb add/del of externally
>> learnt entries.
>> However, this could be made configurable. I think some drivers may rely on
>> bridge driver aging these entries (The default setting needs more thought).
>> I am not sure what rocker does (CC'ed rocker maintainers). But, our driver
>> does rely on the bridge driver aging these entries by default.
> added_by_external_learn flag is only set for entries learned via
> switchdev notifier
> (NETDEV_SWITCH_FDB_ADD) and rocker is the only driver using these notifiers.
> I see that rocker is deleting the entries via switchdev notifier
> (NETDEV_SWITCH_FDB_DEL).
> This mechanism is only used by drivers when learned_sync is turned on by user.
>
> $ sudo bridge link set dev swp1 learning_sync on self
>
> Am I missing something here?
I know that its enabled by an external flag. I wasn't sure rocker was
doing a del
or was relying on the bridge driver to age those entries (hence the CC
to rocker maintainers).
And, my only point was its valid in some cases for the switch driver to
rely on bridge driver ageing those entries.
For symmetry, your patch seems right.
^ permalink raw reply
* Re: [PATCH v3 2/3] genetlink: disallow subscribing to unknown mcast groups
From: Johannes Berg @ 2015-02-04 16:16 UTC (permalink / raw)
To: Bjørn Mork; +Cc: netdev, Jeff Layton, Sedat Dilek
In-Reply-To: <874mr1isch.fsf@nemi.mork.no>
On Wed, 2015-02-04 at 16:55 +0100, Bjørn Mork wrote:
> This was never noticed before because there are only two users,
> netfilter and generic netlink, and both were willing to accept the
> off-by-one values without much fuzz.
Generic netlink didn't even have it until my patchset.
I think audit also uses the callback, but only for permissions checks
and doesn't care about the group at all.
johannes
^ permalink raw reply
* Re: [PATCH v3 2/3] genetlink: disallow subscribing to unknown mcast groups
From: Bjørn Mork @ 2015-02-04 16:15 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, Jeff Layton, Sedat Dilek
In-Reply-To: <874mr1isch.fsf@nemi.mork.no>
Bjørn Mork <bjorn@mork.no> writes:
> So I am pretty sure Pablo's patch fixes the problem.
Confirmed now.
Running acpid on v3.19-rc7 + commit 8b7c36d810c6 ("netlink: fix
wrong subscription bitmask to group mapping in") works fine:
nemi:/tmp# strace -s 128 -e trace=socket,bind,sendmsg,recvmsg -f acpid -l -d
socket(PF_LOCAL, SOCK_DGRAM|SOCK_CLOEXEC, 0) = 3
Deprecated /proc/acpi/event was not found. Trying netlink and the input layer...
input layer /dev/input/event0 (AT Translated Set 2 keyboard) opened successfully, fd 4
input layer /dev/input/event1 (Lid Switch) opened successfully, fd 5
input layer /dev/input/event10 (HDA Intel Headphone) opened successfully, fd 6
input layer /dev/input/event2 (Sleep Button) opened successfully, fd 7
input layer /dev/input/event3 (Power Button) opened successfully, fd 8
input layer /dev/input/event4 (Video Bus) opened successfully, fd 9
input layer /dev/input/event5 (ThinkPad Extra Buttons) opened successfully, fd 10
input layer /dev/input/event7 (HDA Intel Mic) opened successfully, fd 11
input layer /dev/input/event8 (HDA Intel Dock Mic) opened successfully, fd 12
input layer /dev/input/event9 (HDA Intel Dock Headphone) opened successfully, fd 13
inotify fd: 14
inotify wd: 1
socket(PF_NETLINK, SOCK_RAW|SOCK_CLOEXEC, NETLINK_GENERIC) = 15
bind(15, {sa_family=AF_NETLINK, pid=0, groups=00000000}, 12) = 0
sendmsg(15, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=00000000}, msg_iov(1)=[{"$\0\0\0\20\0\5\0oD\322T\0\0\0\0\3\0\0\0\17\0\2\0acpi_event\0\0", 36}], msg_controllen=0, msg_flags=0}, 0) = 36
recvmsg(15, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=00000000}, msg_iov(1)=[{"h\0\0\0\20\0\0\0oD\322T@\20\0\0\1\2\0\0\17\0\2\0acpi_event\0\0\6\0\1\0\23\0\0\0\10\0\3\0\1\0\0\0\10\0\4\0\0\0\0\0\10\0\5\0\1\0\0\0$\0\7\0 \0\1\0\10\0\2\0\2\0\0\0\22\0\1\0acpi_mc_group\0\0\0", 16384}], msg_controllen=0, msg_flags=MSG_CMSG_CLOEXEC}, MSG_CMSG_CLOEXEC) = 104
socket(PF_NETLINK, SOCK_RAW|SOCK_CLOEXEC, NETLINK_GENERIC) = 15
bind(15, {sa_family=AF_NETLINK, pid=0, groups=00000002}, 12) = 0
netlink opened successfully
socket(PF_LOCAL, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0) = 16
bind(16, {sa_family=AF_LOCAL, sun_path="/var/run/acpid.socket"}, 110) = 0
acpid: starting up with netlink and the input layer
parsing conf file /etc/acpi/events/powerbtn-acpi-support
acpid: skipping non-file /etc/acpi/events/CVS
parsing conf file /etc/acpi/events/any
parsing conf file /etc/acpi/events/generic-hibernatebtn
parsing conf file /etc/acpi/events/low_battery
parsing conf file /etc/acpi/events/lidbtn
parsing conf file /etc/acpi/events/sleepbtn
acpid: 6 rules loaded
acpid: waiting for events: event logging is on
And the acpi events are of course received as expected. Thanks again
for making the kernel more robust.
Bjørn
^ permalink raw reply
* [PATCH net-next] vxlan: Only set has-GBP bit in header if any other bits would be set
From: Thomas Graf @ 2015-02-04 16:00 UTC (permalink / raw)
To: davem; +Cc: netdev
This allows for a VXLAN-GBP socket to talk to a Linux VXLAN socket by
not setting any of the bits.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
drivers/net/vxlan.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 31bac2a..fd68dc2 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1681,6 +1681,9 @@ static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
{
struct vxlanhdr_gbp *gbp;
+ if (!md->gbp)
+ return;
+
gbp = (struct vxlanhdr_gbp *)vxh;
vxh->vx_flags |= htonl(VXLAN_HF_GBP);
--
1.9.3
^ permalink raw reply related
* RE: [PATCH] net: ep93xx_eth: Delete unnecessary checks before the function call "kfree"
From: Hartley Sweeten @ 2015-02-04 15:59 UTC (permalink / raw)
To: SF Markus Elfring, netdev@vger.kernel.org
Cc: LKML, kernel-janitors@vger.kernel.org, Julia Lawall
In-Reply-To: <54D23415.6070005@users.sourceforge.net>
On Wednesday, February 04, 2015 8:01 AM, Markus Elfring wrote:
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/net/ethernet/cirrus/ep93xx_eth.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/cirrus/ep93xx_eth.c b/drivers/net/ethernet/cirrus/ep93xx_eth.c
> index 3a12c09..de9f7c9 100644
> --- a/drivers/net/ethernet/cirrus/ep93xx_eth.c
> +++ b/drivers/net/ethernet/cirrus/ep93xx_eth.c
> @@ -475,8 +475,7 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
> if (d)
> dma_unmap_single(dev, d, PKT_BUF_SIZE, DMA_FROM_DEVICE);
>
> - if (ep->rx_buf[i] != NULL)
> - kfree(ep->rx_buf[i]);
> + kfree(ep->rx_buf[i]);
> }
>
> for (i = 0; i < TX_QUEUE_ENTRIES; i++) {
> @@ -486,8 +485,7 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
> if (d)
> dma_unmap_single(dev, d, PKT_BUF_SIZE, DMA_TO_DEVICE);
>
> - if (ep->tx_buf[i] != NULL)
> - kfree(ep->tx_buf[i]);
> + kfree(ep->tx_buf[i]);
> }
>
> dma_free_coherent(dev, sizeof(struct ep93xx_descs), ep->descs,
Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Thanks
^ permalink raw reply
* Re: [PATCH v3 2/3] genetlink: disallow subscribing to unknown mcast groups
From: Bjørn Mork @ 2015-02-04 15:55 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, Jeff Layton, Sedat Dilek
In-Reply-To: <1423064608.4741.6.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> On Wed, 2015-02-04 at 16:36 +0100, Bjørn Mork wrote:
>
>> >> - int i, err = 0;
>> >> + int i, err = -ENOENT;
>> >>
>> >> down_read(&cb_lock);
>> >> for (i = 0; i < GENL_FAM_TAB_SIZE; i++) {
>> >
>> > This change cause serious problems for acpid, as reported on
>> > https://bugzilla.kernel.org/show_bug.cgi?id=92121
>>
>> Ah, I see this bug is already fixed by commit 8b7c36d810c6 ("netlink:
>> fix wrong subscription bitmask to group mapping in"). Your change was
>> obviously correct, and found the long standing off by one bug. Thanks.
>>
>> Sorry about the noise. I should have checked the current "net" first.
>
> Interesting. I was completely willing to entertain the notion that some
> userspace might be broken and be attempting to subscribe to a (static
> through the hacks we had to put in or "I think I know it already")
> group.
Me to. So I went through the whole loop, checking that acpid did
everything by the book, adding a debug message to genl_bind() only to
see that it was called with '1' instead of the expected '2'. Then
looking at af_netlink.c and its history. Etc.
> Have you checked acpid with the bitmap fix?
No, not yet. But I went down far enough into this that I actually wrote
the exact same patch as Pablo. Had to scratch my head when I couldn't
cherry-pick it into net because it was already there :-)
So I am pretty sure Pablo's patch fixes the problem.
This was never noticed before because there are only two users,
netfilter and generic netlink, and both were willing to accept the
off-by-one values without much fuzz.
Bjørn
^ permalink raw reply
* Re: [PATCH net] ip6_gre: fix endianness errors in ip6gre_err
From: Eric Dumazet @ 2015-02-04 15:50 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: davem, netdev
In-Reply-To: <1423064676.907.137.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, 2015-02-04 at 07:44 -0800, Eric Dumazet wrote:
> On Wed, 2015-02-04 at 15:25 +0100, Sabrina Dubroca wrote:
> > info is in network byte order, change it back to host byte order
> > before use. In particular, the current code sets the MTU of the tunnel
> > to a wrong (too big) value.
> >
> > Fixes: c12b395a4664 ("gre: Support GRE over IPv6")
> > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> > ---
>
> Acked-by: Eric Dumazet <edumazet@google.com>
>
BTW, "fl6.flowi6_proto = skb->protocol; " in ip6gre_xmit_other()
looks suspicious...
^ permalink raw reply
* Re: [PATCH net] ip6_gre: fix endianness errors in ip6gre_err
From: Eric Dumazet @ 2015-02-04 15:44 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: davem, netdev
In-Reply-To: <1423059909-15606-1-git-send-email-sd@queasysnail.net>
On Wed, 2015-02-04 at 15:25 +0100, Sabrina Dubroca wrote:
> info is in network byte order, change it back to host byte order
> before use. In particular, the current code sets the MTU of the tunnel
> to a wrong (too big) value.
>
> Fixes: c12b395a4664 ("gre: Support GRE over IPv6")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH v3 2/3] genetlink: disallow subscribing to unknown mcast groups
From: Johannes Berg @ 2015-02-04 15:43 UTC (permalink / raw)
To: Bjørn Mork; +Cc: netdev, Jeff Layton, Sedat Dilek
In-Reply-To: <87d25pit90.fsf@nemi.mork.no>
On Wed, 2015-02-04 at 16:36 +0100, Bjørn Mork wrote:
> >> - int i, err = 0;
> >> + int i, err = -ENOENT;
> >>
> >> down_read(&cb_lock);
> >> for (i = 0; i < GENL_FAM_TAB_SIZE; i++) {
> >
> > This change cause serious problems for acpid, as reported on
> > https://bugzilla.kernel.org/show_bug.cgi?id=92121
>
> Ah, I see this bug is already fixed by commit 8b7c36d810c6 ("netlink:
> fix wrong subscription bitmask to group mapping in"). Your change was
> obviously correct, and found the long standing off by one bug. Thanks.
>
> Sorry about the noise. I should have checked the current "net" first.
Interesting. I was completely willing to entertain the notion that some
userspace might be broken and be attempting to subscribe to a (static
through the hacks we had to put in or "I think I know it already")
group.
Have you checked acpid with the bitmap fix?
johannes
^ permalink raw reply
* Re: [PATCH v3 2/3] genetlink: disallow subscribing to unknown mcast groups
From: Bjørn Mork @ 2015-02-04 15:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, Jeff Layton, Sedat Dilek, Johannes Berg
In-Reply-To: <87egq5uc07.fsf@nemi.mork.no>
Bjørn Mork <bjorn@mork.no> writes:
> Johannes Berg <johannes@sipsolutions.net> writes:
>
>> diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
>> index 2e11061ef885..c18d3f5624b2 100644
>> --- a/net/netlink/genetlink.c
>> +++ b/net/netlink/genetlink.c
>> @@ -985,7 +985,7 @@ static struct genl_multicast_group genl_ctrl_groups[] = {
>>
>> static int genl_bind(struct net *net, int group)
>> {
>> - int i, err = 0;
>> + int i, err = -ENOENT;
>>
>> down_read(&cb_lock);
>> for (i = 0; i < GENL_FAM_TAB_SIZE; i++) {
>
> This change cause serious problems for acpid, as reported on
> https://bugzilla.kernel.org/show_bug.cgi?id=92121
Ah, I see this bug is already fixed by commit 8b7c36d810c6 ("netlink:
fix wrong subscription bitmask to group mapping in"). Your change was
obviously correct, and found the long standing off by one bug. Thanks.
Sorry about the noise. I should have checked the current "net" first.
Bjørn
^ permalink raw reply
* [PATCH] net: ep93xx_eth: Delete unnecessary checks before the function call "kfree"
From: SF Markus Elfring @ 2015-02-04 15:00 UTC (permalink / raw)
To: Hartley Sweeten, netdev; +Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 4 Feb 2015 15:56:58 +0100
The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/cirrus/ep93xx_eth.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cirrus/ep93xx_eth.c b/drivers/net/ethernet/cirrus/ep93xx_eth.c
index 3a12c09..de9f7c9 100644
--- a/drivers/net/ethernet/cirrus/ep93xx_eth.c
+++ b/drivers/net/ethernet/cirrus/ep93xx_eth.c
@@ -475,8 +475,7 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
if (d)
dma_unmap_single(dev, d, PKT_BUF_SIZE, DMA_FROM_DEVICE);
- if (ep->rx_buf[i] != NULL)
- kfree(ep->rx_buf[i]);
+ kfree(ep->rx_buf[i]);
}
for (i = 0; i < TX_QUEUE_ENTRIES; i++) {
@@ -486,8 +485,7 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
if (d)
dma_unmap_single(dev, d, PKT_BUF_SIZE, DMA_TO_DEVICE);
- if (ep->tx_buf[i] != NULL)
- kfree(ep->tx_buf[i]);
+ kfree(ep->tx_buf[i]);
}
dma_free_coherent(dev, sizeof(struct ep93xx_descs), ep->descs,
--
2.2.2
^ permalink raw reply related
* Re: [PATCH] net: Mellanox: Delete unnecessary checks before the function call "vunmap"
From: Eli Cohen @ 2015-02-04 14:59 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: netdev, linux-rdma, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54D22B29.3020200@users.sourceforge.net>
Acked-by: Eli Cohen <eli@mellanox.com>
On Wed, Feb 04, 2015 at 03:22:33PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 4 Feb 2015 15:17:00 +0100
>
> The vunmap() function performs also input parameter validation.
> Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.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