* Re: [PATCH] mrp: add periodictimer to retry lost packets
From: David Miller @ 2013-09-18 17:54 UTC (permalink / raw)
To: noel; +Cc: netdev
In-Reply-To: <CAB5uXS7b2CGjF=1OTzCmEeduGHSO_SOmMDv6=-bZU2VzaEQNfA@mail.gmail.com>
From: Noel Burton-Krahn <noel@burton-krahn.com>
Date: Wed, 18 Sep 2013 10:09:25 -0700
> MRP doesn't implement the periodictimer in 802.1Q, so it never retries
> if packets get lost. I ran into this problem when MRP sent a MVRP
> JoinIn before the interface was fully up. The JoinIn was lost, MRP
> didn't retry, and MVRP registration failed.
>
> (Sorry for the repost, my earlier message wasn't formatted correctly)
Still corrupted by your email client, it turned TAB characters
into spaces.
Still missing a signoff.
^ permalink raw reply
* Re: [PATCH ] ipv6: handle the update of the NDISC_REDIRECT error code in icmpv6_err_convert
From: Hannes Frederic Sowa @ 2013-09-18 18:00 UTC (permalink / raw)
To: David Miller; +Cc: duanj.fnst, netdev
In-Reply-To: <20130918.123613.61436246160249555.davem@davemloft.net>
On Wed, Sep 18, 2013 at 12:36:13PM -0400, David Miller wrote:
> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> Date: Wed, 18 Sep 2013 20:04:48 +0800
>
> > From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> >
> > when dealing with redirect message in udpv6_err() and
> > rawv6_err() the err shoud be assigned to 0, not EPROTO.
> >
> > Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>
> No, you should fix this the same way you handled DCCP, but skipping
> the whole socket error setting path for NDISC_REDIRECT.
>
> Clearing the socket error to zero is not correct, it means you will
> lose any existing error setting.
My reasoning when I proposed this change was that we should not stop
appending ipv6 redirects to the raw and udp sockets error queues because
userland could want to deal with them (and probably already does). Also,
we need to notify poll of the appended error, hence we still needed a
call to sk_err_report to fire up a POLL_ERR event. Because we don't know
if the router sending the redirect actually dropped the packet I thought
taht '0' would be the only sane choice. We currently report back EPROTO,
IPv4 does report (rightfully) EHOSTUNREACH.
Perhaps we should stay with EPROTO?
I agree, all other error functions should return early after updating
the routing table without reporting any error. Either they should not
call into this function or ignore err == 0 (I prefer the first).
I know, it is a bit hacky but this was the most sane idea I came up with.
(I still have not reviewed this patch, just wanted to provide a bit of
background that lead to this change.)
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH] mrp: add periodictimer to retry lost packets
From: Joe Perches @ 2013-09-18 18:20 UTC (permalink / raw)
To: Noel Burton-Krahn; +Cc: netdev
In-Reply-To: <CAB5uXS7b2CGjF=1OTzCmEeduGHSO_SOmMDv6=-bZU2VzaEQNfA@mail.gmail.com>
On Wed, 2013-09-18 at 10:09 -0700, Noel Burton-Krahn wrote:
> MRP doesn't implement the periodictimer in 802.1Q, so it never retries
> if packets get lost. I ran into this problem when MRP sent a MVRP
> JoinIn before the interface was fully up. The JoinIn was lost, MRP
> didn't retry, and MVRP registration failed.
Beyond the whitespace errors that make this not apply:
> diff --git a/net/802/mrp.c b/net/802/mrp.c
[]
> @@ -595,6 +600,26 @@ static void mrp_join_timer(unsigned long data)
> mrp_join_timer_arm(app);
> }
>
> +static void mrp_periodic_timer_arm(struct mrp_applicant *app)
> +{
> + unsigned long delay;
> +
> + delay = (u64)msecs_to_jiffies(mrp_periodic_time);
Useless cast to u64
> + mod_timer(&app->periodic_timer, jiffies + delay);
This might also be neater without the temporary
static void mrp_periodic_timer_arm(struct mrp_applicant *app)
{
mod_timer(&app->periodic_timer,
jiffies + msecs_to_jiffies(mrp_periodic_timer));
}
^ permalink raw reply
* Re: [PATCH] USBNET: fix handling padding packet
From: Bjørn Mork @ 2013-09-18 18:33 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Oliver Neukum,
Network Development, linux-usb, Oliver Neukum
In-Reply-To: <CACVXFVNp5Qj-EV+L=FezwFqRnFa7uRFca7Ju9=iTqU+DS1r-fg@mail.gmail.com>
Ming Lei <ming.lei@canonical.com> writes:
> On Wed, Sep 18, 2013 at 11:52 PM, Bjørn Mork <bjorn@mork.no> wrote:
>
>> Why don't you test it on the device you tested the SG patch with? I am
>> pretty sure it works just fine using proper ZLP transfer termination.
>
> I should have planned to test it, but didn't know how to build TCP TSO
> to make the whole frame length plus 8 dividable by 1024.
You could test this without enabling TSO and just send IP packets of the
appriopriate size, taking any additional framing into account
Bjørn
^ permalink raw reply
* Re: [RFC PATCH 0/4] cpsw: support for control module register
From: Tony Lindgren @ 2013-09-18 18:49 UTC (permalink / raw)
To: Daniel Mack
Cc: Mugunthan V N, netdev, davem, bcousson, devicetree-discuss,
linux-omap
In-Reply-To: <522D8DB8.5050305@gmail.com>
* Daniel Mack <zonque@gmail.com> [130909 02:06]:
> On 08.09.2013 13:23, Mugunthan V N wrote:
> > This patch series adds the support for configuring GMII_SEL register
> > of control module to select the phy mode type and also to configure
> > the clock source for RMII phy mode whether to use internal clock or
> > the external clock from the phy itself.
> >
> > Till now CPSW works as this configuration is done in U-Boot and carried
> > over to the kernel. But during suspend/resume Control module tends to
> > lose its configured value for GMII_SEL register in AM33xx PG1.0, so
> > if CPSW is used in RMII or RGMII mode, on resume cpsw is not working
> > as GMII_SEL register lost its configuration values.
> >
> > The initial version of the patch is done by Daniel Mack but as per
> > Tony's comment he wants it as a seperate driver as it is done in USB
> > control module. I have created a seperate driver for the same and as
> > the merge window is open now and no feature request is accepted I am
> > submitting it as RFC for reviews.
>
> Thanks for doing this. It's a somehow expensive approach of writing a
> single 32bit register, but I agree it's cleaner to not have this code in
> the cpsw driver directly.
The main reason for doing that is because this register is in a
separate omap SCM (System Control Module) and may need to be
eventually a child of a SCM core driver. This is to avoid invalid
PM dependencies between separate hardware modules as they can
be clocked separately.
Most of the SCM registers can be already handled by using
pinctrl-single driver, but it's not intended for using for
transceivers etc that are not strictly pinctrl related.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] USBNET: fix handling padding packet
From: Bjørn Mork @ 2013-09-18 18:56 UTC (permalink / raw)
To: Oliver Neukum
Cc: Ming Lei, David S. Miller, Greg Kroah-Hartman,
Network Development, linux-usb
In-Reply-To: <1379523984.2430.3.camel@linux-fkkt.site>
Oliver Neukum <oneukum@suse.de> wrote:
>On Wed, 2013-09-18 at 17:52 +0200, Bjørn Mork wrote:
>
>> No modern device should need the padding. No old device will be able
>to
>> use the SG feature as implemented. You only enable it on USB3, don't
>
>On XHCI.
>
>> you? If this feature is restricted to USB3 capable devices, then it
>most
>> certainly can be restricted to ZLP capable devices with absolutely no
>> difference in the resulting set of supported devices.
>
>No, USB 3.0 uses no companion controllers, so you can have devices
>of any speed connected to it.
>
Ah, right. I don't own such modern hardware, but I should have known this anyway.
This still doesn't change the fact that the driver is brand new for brand new devices. I believe we should assume such devices will support ZLPs unless we have documentation stating anything else.
>> Anyway, if you want to keep the padding for SG then maybe this will
>work
>> and allow you to drop the extra struct usbnet field and allocation:
>>
>> if (skb_tailroom(skb) && !dev->can_dma_sg) {
>> skb->data[skb->len] = 0;
>> __skb_put(skb, 1);
>> } else if (dev->can_dma_sg) {
>> sg_set_buf(&urb->sg[urb->num_sgs++],
>skb->data, 1);
>> }
>>
>> I.e. cheat and use the skb->data buffer twice, if that is allowed?
>The
>> actual value of the padding byte should not matter, I believe?
>
>That makes me immediately suspect a violation of the DMA rules.
Sounds likely. And it's an ugly hack in any case. Probably not a good idea. Just one of the many random thoughts I should have kept to myself :-)
Bjørn
^ permalink raw reply
* [PATCH 1/1] mrp: add periodictimer to allow retries when packets get lost
From: Noel Burton-Krahn @ 2013-09-18 19:24 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Joe Perches, Noel Burton-Krahn
In-Reply-To: <1379528419.1787.50.camel@joe-AO722>
MRP doesn't implement the periodictimer in 802.1Q, so it never retries
if packets get lost. I ran into this problem when MRP sent a MVRP
JoinIn before the interface was fully up. The JoinIn was lost, MRP
didn't retry, and MVRP registration failed.
Tested against Juniper QFabric switches
Signed-off-by: Noel Burton-Krahn <noel@burton-krahn.com>
---
include/net/mrp.h | 1 +
net/802/mrp.c | 27 +++++++++++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/include/net/mrp.h b/include/net/mrp.h
index 4fbf02a..0f7558b 100644
--- a/include/net/mrp.h
+++ b/include/net/mrp.h
@@ -112,6 +112,7 @@ struct mrp_applicant {
struct mrp_application *app;
struct net_device *dev;
struct timer_list join_timer;
+ struct timer_list periodic_timer;
spinlock_t lock;
struct sk_buff_head queue;
diff --git a/net/802/mrp.c b/net/802/mrp.c
index 1eb05d8..3ed6162 100644
--- a/net/802/mrp.c
+++ b/net/802/mrp.c
@@ -24,6 +24,11 @@
static unsigned int mrp_join_time __read_mostly = 200;
module_param(mrp_join_time, uint, 0644);
MODULE_PARM_DESC(mrp_join_time, "Join time in ms (default 200ms)");
+
+static unsigned int mrp_periodic_time __read_mostly = 1000;
+module_param(mrp_periodic_time, uint, 0644);
+MODULE_PARM_DESC(mrp_periodic_time, "Periodic time in ms (default 1s)");
+
MODULE_LICENSE("GPL");
static const u8
@@ -595,6 +600,24 @@ static void mrp_join_timer(unsigned long data)
mrp_join_timer_arm(app);
}
+static void mrp_periodic_timer_arm(struct mrp_applicant *app)
+{
+ mod_timer(&app->periodic_timer,
+ jiffies + msecs_to_jiffies(mrp_periodic_time));
+}
+
+static void mrp_periodic_timer(unsigned long data)
+{
+ struct mrp_applicant *app = (struct mrp_applicant *)data;
+
+ spin_lock(&app->lock);
+ mrp_mad_event(app, MRP_EVENT_PERIODIC);
+ mrp_pdu_queue(app);
+ spin_unlock(&app->lock);
+
+ mrp_periodic_timer_arm(app);
+}
+
static int mrp_pdu_parse_end_mark(struct sk_buff *skb, int *offset)
{
__be16 endmark;
@@ -845,6 +868,9 @@ int mrp_init_applicant(struct net_device *dev, struct mrp_application *appl)
rcu_assign_pointer(dev->mrp_port->applicants[appl->type], app);
setup_timer(&app->join_timer, mrp_join_timer, (unsigned long)app);
mrp_join_timer_arm(app);
+ setup_timer(&app->periodic_timer, mrp_periodic_timer,
+ (unsigned long)app);
+ mrp_periodic_timer_arm(app);
return 0;
err3:
@@ -870,6 +896,7 @@ void mrp_uninit_applicant(struct net_device *dev, struct mrp_application *appl)
* all pending messages before the applicant is gone.
*/
del_timer_sync(&app->join_timer);
+ del_timer_sync(&app->periodic_timer);
spin_lock_bh(&app->lock);
mrp_mad_event(app, MRP_EVENT_TX);
--
1.7.9.5
^ permalink raw reply related
* [GIT PULL 0/5] IPVS fixes for v3.12
From: Simon Horman @ 2013-09-18 19:45 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Simon Horman
Hi Pablo,
please consider the following fixes for IPVS for v3.12.
The following changes since commit 61c5923a2f2d8ab98a1e3c76f17e0f4a871ec75b:
Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf (2013-09-17 20:22:53 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git tags/ipvs-fixes-for-v3.12
for you to fetch changes up to d1ee4fea0b6946dd8bc61b46db35ea80af7af34b:
ipvs: stats should not depend on CPU 0 (2013-09-18 14:40:20 -0500)
----------------------------------------------------------------
Fixes for IPVS for v3.12
* Fix overflow in LBLC and LBLCR schedulers.
This resolves a regression introduced by commit b552f7e3a9524abcbcdf
("ipvs: unify the formula to estimate the overhead of processing
connections").
This problem was introduced in v2.6.38
* Correct stats calculation in the absence of CPU 0 in the possible mask
This resolves a regression introduced by commit 17fc9963f837ef1
("IPVS: netns, ip_vs_stats and its procfs").
This problem was introduced in v2.6.38
* Make the service replacement more robust
This resolves a regression introduced by commit 578bc3ef1e473a
("ipvs: reorganize dest trash")
This problem was introduced in 3.9
* Do not use dest after ip_vs_dest_put in LBLCR
This resolves a regression introduced by commit c5549571f975ab
("ipvs: convert lblcr scheduler to rcu")
This problem was introduced in 3.9
* Do not use dest after ip_vs_dest_put in LBLC
This resolves a regression introduced by commit 2a4ffb70eef39
("ipvs: convert lblc scheduler to rcu")
This problem was introduced in 3.9
----------------------------------------------------------------
Julian Anastasov (4):
ipvs: make the service replacement more robust
ipvs: do not use dest after ip_vs_dest_put in LBLC
ipvs: do not use dest after ip_vs_dest_put in LBLCR
ipvs: stats should not depend on CPU 0
Simon Kirby (1):
ipvs: fix overflow on dest weight multiply
include/net/ip_vs.h | 9 ++---
net/netfilter/ipvs/ip_vs_core.c | 12 +++++-
net/netfilter/ipvs/ip_vs_ctl.c | 86 ++++++++++++++++------------------------
net/netfilter/ipvs/ip_vs_est.c | 4 +-
net/netfilter/ipvs/ip_vs_lblc.c | 72 +++++++++++++++------------------
net/netfilter/ipvs/ip_vs_lblcr.c | 62 ++++++++++++-----------------
net/netfilter/ipvs/ip_vs_nq.c | 8 ++--
net/netfilter/ipvs/ip_vs_sed.c | 8 ++--
net/netfilter/ipvs/ip_vs_wlc.c | 6 +--
9 files changed, 121 insertions(+), 146 deletions(-)
^ permalink raw reply
* [PATCH 2/5] ipvs: make the service replacement more robust
From: Simon Horman @ 2013-09-18 19:45 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Simon Horman
In-Reply-To: <1379533542-9891-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
commit 578bc3ef1e473a ("ipvs: reorganize dest trash") added
IP_VS_DEST_STATE_REMOVING flag and RCU callback named
ip_vs_dest_wait_readers() to keep dests and services after
removal for at least a RCU grace period. But we have the
following corner cases:
- we can not reuse the same dest if its service is removed
while IP_VS_DEST_STATE_REMOVING is still set because another dest
removal in the first grace period can not extend this period.
It can happen when ipvsadm -C && ipvsadm -R is used.
- dest->svc can be replaced but ip_vs_in_stats() and
ip_vs_out_stats() have no explicit read memory barriers
when accessing dest->svc. It can happen that dest->svc
was just freed (replaced) while we use it to update
the stats.
We solve the problems as follows:
- IP_VS_DEST_STATE_REMOVING is removed and we ensure a fixed
idle period for the dest (IP_VS_DEST_TRASH_PERIOD). idle_start
will remember when for first time after deletion we noticed
dest->refcnt=0. Later, the connections can grab a reference
while in RCU grace period but if refcnt becomes 0 we can
safely free the dest and its svc.
- dest->svc becomes RCU pointer. As result, we add explicit
RCU locking in ip_vs_in_stats() and ip_vs_out_stats().
- __ip_vs_unbind_svc is renamed to __ip_vs_svc_put(), it
now can free the service immediately or after a RCU grace
period. dest->svc is not set to NULL anymore.
As result, unlinked dests and their services are
freed always after IP_VS_DEST_TRASH_PERIOD period, unused
services are freed after a RCU grace period.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
include/net/ip_vs.h | 7 +---
net/netfilter/ipvs/ip_vs_core.c | 12 +++++-
net/netfilter/ipvs/ip_vs_ctl.c | 86 +++++++++++++++++------------------------
3 files changed, 47 insertions(+), 58 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index fe782ed..9c4d37e 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -723,8 +723,6 @@ struct ip_vs_dest_dst {
struct rcu_head rcu_head;
};
-/* In grace period after removing */
-#define IP_VS_DEST_STATE_REMOVING 0x01
/*
* The real server destination forwarding entry
* with ip address, port number, and so on.
@@ -742,7 +740,7 @@ struct ip_vs_dest {
atomic_t refcnt; /* reference counter */
struct ip_vs_stats stats; /* statistics */
- unsigned long state; /* state flags */
+ unsigned long idle_start; /* start time, jiffies */
/* connection counters and thresholds */
atomic_t activeconns; /* active connections */
@@ -756,14 +754,13 @@ struct ip_vs_dest {
struct ip_vs_dest_dst __rcu *dest_dst; /* cached dst info */
/* for virtual service */
- struct ip_vs_service *svc; /* service it belongs to */
+ struct ip_vs_service __rcu *svc; /* service it belongs to */
__u16 protocol; /* which protocol (TCP/UDP) */
__be16 vport; /* virtual port number */
union nf_inet_addr vaddr; /* virtual IP address */
__u32 vfwmark; /* firewall mark of service */
struct list_head t_list; /* in dest_trash */
- struct rcu_head rcu_head;
unsigned int in_rs_table:1; /* we are in rs_table */
};
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 4f69e83..74fd00c 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -116,6 +116,7 @@ ip_vs_in_stats(struct ip_vs_conn *cp, struct sk_buff *skb)
if (dest && (dest->flags & IP_VS_DEST_F_AVAILABLE)) {
struct ip_vs_cpu_stats *s;
+ struct ip_vs_service *svc;
s = this_cpu_ptr(dest->stats.cpustats);
s->ustats.inpkts++;
@@ -123,11 +124,14 @@ ip_vs_in_stats(struct ip_vs_conn *cp, struct sk_buff *skb)
s->ustats.inbytes += skb->len;
u64_stats_update_end(&s->syncp);
- s = this_cpu_ptr(dest->svc->stats.cpustats);
+ rcu_read_lock();
+ svc = rcu_dereference(dest->svc);
+ s = this_cpu_ptr(svc->stats.cpustats);
s->ustats.inpkts++;
u64_stats_update_begin(&s->syncp);
s->ustats.inbytes += skb->len;
u64_stats_update_end(&s->syncp);
+ rcu_read_unlock();
s = this_cpu_ptr(ipvs->tot_stats.cpustats);
s->ustats.inpkts++;
@@ -146,6 +150,7 @@ ip_vs_out_stats(struct ip_vs_conn *cp, struct sk_buff *skb)
if (dest && (dest->flags & IP_VS_DEST_F_AVAILABLE)) {
struct ip_vs_cpu_stats *s;
+ struct ip_vs_service *svc;
s = this_cpu_ptr(dest->stats.cpustats);
s->ustats.outpkts++;
@@ -153,11 +158,14 @@ ip_vs_out_stats(struct ip_vs_conn *cp, struct sk_buff *skb)
s->ustats.outbytes += skb->len;
u64_stats_update_end(&s->syncp);
- s = this_cpu_ptr(dest->svc->stats.cpustats);
+ rcu_read_lock();
+ svc = rcu_dereference(dest->svc);
+ s = this_cpu_ptr(svc->stats.cpustats);
s->ustats.outpkts++;
u64_stats_update_begin(&s->syncp);
s->ustats.outbytes += skb->len;
u64_stats_update_end(&s->syncp);
+ rcu_read_unlock();
s = this_cpu_ptr(ipvs->tot_stats.cpustats);
s->ustats.outpkts++;
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index c8148e4..a3df9bd 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -460,7 +460,7 @@ static inline void
__ip_vs_bind_svc(struct ip_vs_dest *dest, struct ip_vs_service *svc)
{
atomic_inc(&svc->refcnt);
- dest->svc = svc;
+ rcu_assign_pointer(dest->svc, svc);
}
static void ip_vs_service_free(struct ip_vs_service *svc)
@@ -470,18 +470,25 @@ static void ip_vs_service_free(struct ip_vs_service *svc)
kfree(svc);
}
-static void
-__ip_vs_unbind_svc(struct ip_vs_dest *dest)
+static void ip_vs_service_rcu_free(struct rcu_head *head)
{
- struct ip_vs_service *svc = dest->svc;
+ struct ip_vs_service *svc;
+
+ svc = container_of(head, struct ip_vs_service, rcu_head);
+ ip_vs_service_free(svc);
+}
- dest->svc = NULL;
+static void __ip_vs_svc_put(struct ip_vs_service *svc, bool do_delay)
+{
if (atomic_dec_and_test(&svc->refcnt)) {
IP_VS_DBG_BUF(3, "Removing service %u/%s:%u\n",
svc->fwmark,
IP_VS_DBG_ADDR(svc->af, &svc->addr),
ntohs(svc->port));
- ip_vs_service_free(svc);
+ if (do_delay)
+ call_rcu(&svc->rcu_head, ip_vs_service_rcu_free);
+ else
+ ip_vs_service_free(svc);
}
}
@@ -667,11 +674,6 @@ ip_vs_trash_get_dest(struct ip_vs_service *svc, const union nf_inet_addr *daddr,
IP_VS_DBG_ADDR(svc->af, &dest->addr),
ntohs(dest->port),
atomic_read(&dest->refcnt));
- /* We can not reuse dest while in grace period
- * because conns still can use dest->svc
- */
- if (test_bit(IP_VS_DEST_STATE_REMOVING, &dest->state))
- continue;
if (dest->af == svc->af &&
ip_vs_addr_equal(svc->af, &dest->addr, daddr) &&
dest->port == dport &&
@@ -697,8 +699,10 @@ out:
static void ip_vs_dest_free(struct ip_vs_dest *dest)
{
+ struct ip_vs_service *svc = rcu_dereference_protected(dest->svc, 1);
+
__ip_vs_dst_cache_reset(dest);
- __ip_vs_unbind_svc(dest);
+ __ip_vs_svc_put(svc, false);
free_percpu(dest->stats.cpustats);
kfree(dest);
}
@@ -771,6 +775,7 @@ __ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
struct ip_vs_dest_user_kern *udest, int add)
{
struct netns_ipvs *ipvs = net_ipvs(svc->net);
+ struct ip_vs_service *old_svc;
struct ip_vs_scheduler *sched;
int conn_flags;
@@ -792,13 +797,14 @@ __ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
atomic_set(&dest->conn_flags, conn_flags);
/* bind the service */
- if (!dest->svc) {
+ old_svc = rcu_dereference_protected(dest->svc, 1);
+ if (!old_svc) {
__ip_vs_bind_svc(dest, svc);
} else {
- if (dest->svc != svc) {
- __ip_vs_unbind_svc(dest);
+ if (old_svc != svc) {
ip_vs_zero_stats(&dest->stats);
__ip_vs_bind_svc(dest, svc);
+ __ip_vs_svc_put(old_svc, true);
}
}
@@ -998,16 +1004,6 @@ ip_vs_edit_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
return 0;
}
-static void ip_vs_dest_wait_readers(struct rcu_head *head)
-{
- struct ip_vs_dest *dest = container_of(head, struct ip_vs_dest,
- rcu_head);
-
- /* End of grace period after unlinking */
- clear_bit(IP_VS_DEST_STATE_REMOVING, &dest->state);
-}
-
-
/*
* Delete a destination (must be already unlinked from the service)
*/
@@ -1023,20 +1019,16 @@ static void __ip_vs_del_dest(struct net *net, struct ip_vs_dest *dest,
*/
ip_vs_rs_unhash(dest);
- if (!cleanup) {
- set_bit(IP_VS_DEST_STATE_REMOVING, &dest->state);
- call_rcu(&dest->rcu_head, ip_vs_dest_wait_readers);
- }
-
spin_lock_bh(&ipvs->dest_trash_lock);
IP_VS_DBG_BUF(3, "Moving dest %s:%u into trash, dest->refcnt=%d\n",
IP_VS_DBG_ADDR(dest->af, &dest->addr), ntohs(dest->port),
atomic_read(&dest->refcnt));
if (list_empty(&ipvs->dest_trash) && !cleanup)
mod_timer(&ipvs->dest_trash_timer,
- jiffies + IP_VS_DEST_TRASH_PERIOD);
+ jiffies + (IP_VS_DEST_TRASH_PERIOD >> 1));
/* dest lives in trash without reference */
list_add(&dest->t_list, &ipvs->dest_trash);
+ dest->idle_start = 0;
spin_unlock_bh(&ipvs->dest_trash_lock);
ip_vs_dest_put(dest);
}
@@ -1108,24 +1100,30 @@ static void ip_vs_dest_trash_expire(unsigned long data)
struct net *net = (struct net *) data;
struct netns_ipvs *ipvs = net_ipvs(net);
struct ip_vs_dest *dest, *next;
+ unsigned long now = jiffies;
spin_lock(&ipvs->dest_trash_lock);
list_for_each_entry_safe(dest, next, &ipvs->dest_trash, t_list) {
- /* Skip if dest is in grace period */
- if (test_bit(IP_VS_DEST_STATE_REMOVING, &dest->state))
- continue;
if (atomic_read(&dest->refcnt) > 0)
continue;
+ if (dest->idle_start) {
+ if (time_before(now, dest->idle_start +
+ IP_VS_DEST_TRASH_PERIOD))
+ continue;
+ } else {
+ dest->idle_start = max(1UL, now);
+ continue;
+ }
IP_VS_DBG_BUF(3, "Removing destination %u/%s:%u from trash\n",
dest->vfwmark,
- IP_VS_DBG_ADDR(dest->svc->af, &dest->addr),
+ IP_VS_DBG_ADDR(dest->af, &dest->addr),
ntohs(dest->port));
list_del(&dest->t_list);
ip_vs_dest_free(dest);
}
if (!list_empty(&ipvs->dest_trash))
mod_timer(&ipvs->dest_trash_timer,
- jiffies + IP_VS_DEST_TRASH_PERIOD);
+ jiffies + (IP_VS_DEST_TRASH_PERIOD >> 1));
spin_unlock(&ipvs->dest_trash_lock);
}
@@ -1320,14 +1318,6 @@ out:
return ret;
}
-static void ip_vs_service_rcu_free(struct rcu_head *head)
-{
- struct ip_vs_service *svc;
-
- svc = container_of(head, struct ip_vs_service, rcu_head);
- ip_vs_service_free(svc);
-}
^ permalink raw reply related
* [PATCH 5/5] ipvs: stats should not depend on CPU 0
From: Simon Horman @ 2013-09-18 19:45 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Simon Horman
In-Reply-To: <1379533542-9891-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
When reading percpu stats we need to properly reset
the sum when CPU 0 is not present in the possible mask.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_est.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c
index 6bee6d0..1425e9a 100644
--- a/net/netfilter/ipvs/ip_vs_est.c
+++ b/net/netfilter/ipvs/ip_vs_est.c
@@ -59,12 +59,13 @@ static void ip_vs_read_cpu_stats(struct ip_vs_stats_user *sum,
struct ip_vs_cpu_stats __percpu *stats)
{
int i;
+ bool add = false;
for_each_possible_cpu(i) {
struct ip_vs_cpu_stats *s = per_cpu_ptr(stats, i);
unsigned int start;
__u64 inbytes, outbytes;
- if (i) {
+ if (add) {
sum->conns += s->ustats.conns;
sum->inpkts += s->ustats.inpkts;
sum->outpkts += s->ustats.outpkts;
@@ -76,6 +77,7 @@ static void ip_vs_read_cpu_stats(struct ip_vs_stats_user *sum,
sum->inbytes += inbytes;
sum->outbytes += outbytes;
} else {
+ add = true;
sum->conns = s->ustats.conns;
sum->inpkts = s->ustats.inpkts;
sum->outpkts = s->ustats.outpkts;
--
1.8.4
^ permalink raw reply related
* [PATCH 4/5] ipvs: do not use dest after ip_vs_dest_put in LBLCR
From: Simon Horman @ 2013-09-18 19:45 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Simon Horman
In-Reply-To: <1379533542-9891-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
commit c5549571f975ab ("ipvs: convert lblcr scheduler to rcu")
allows RCU readers to use dest after calling ip_vs_dest_put().
In the corner case it can race with ip_vs_dest_trash_expire()
which can release the dest while it is being returned to the
RCU readers as scheduling result.
To fix the problem do not allow e->dest to be replaced and
defer the ip_vs_dest_put() call by using RCU callback. Now
e->dest does not need to be RCU pointer.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_lblcr.c | 50 ++++++++++++++++------------------------
1 file changed, 20 insertions(+), 30 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index e65f7c5..0b85500 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -89,7 +89,7 @@
*/
struct ip_vs_dest_set_elem {
struct list_head list; /* list link */
- struct ip_vs_dest __rcu *dest; /* destination server */
+ struct ip_vs_dest *dest; /* destination server */
struct rcu_head rcu_head;
};
@@ -107,11 +107,7 @@ static void ip_vs_dest_set_insert(struct ip_vs_dest_set *set,
if (check) {
list_for_each_entry(e, &set->list, list) {
- struct ip_vs_dest *d;
-
- d = rcu_dereference_protected(e->dest, 1);
- if (d == dest)
- /* already existed */
+ if (e->dest == dest)
return;
}
}
@@ -121,7 +117,7 @@ static void ip_vs_dest_set_insert(struct ip_vs_dest_set *set,
return;
ip_vs_dest_hold(dest);
- RCU_INIT_POINTER(e->dest, dest);
+ e->dest = dest;
list_add_rcu(&e->list, &set->list);
atomic_inc(&set->size);
@@ -129,22 +125,27 @@ static void ip_vs_dest_set_insert(struct ip_vs_dest_set *set,
set->lastmod = jiffies;
}
+static void ip_vs_lblcr_elem_rcu_free(struct rcu_head *head)
+{
+ struct ip_vs_dest_set_elem *e;
+
+ e = container_of(head, struct ip_vs_dest_set_elem, rcu_head);
+ ip_vs_dest_put(e->dest);
+ kfree(e);
+}
+
static void
ip_vs_dest_set_erase(struct ip_vs_dest_set *set, struct ip_vs_dest *dest)
{
struct ip_vs_dest_set_elem *e;
list_for_each_entry(e, &set->list, list) {
- struct ip_vs_dest *d;
-
- d = rcu_dereference_protected(e->dest, 1);
- if (d == dest) {
+ if (e->dest == dest) {
/* HIT */
atomic_dec(&set->size);
set->lastmod = jiffies;
- ip_vs_dest_put(dest);
list_del_rcu(&e->list);
- kfree_rcu(e, rcu_head);
+ call_rcu(&e->rcu_head, ip_vs_lblcr_elem_rcu_free);
break;
}
}
@@ -155,16 +156,8 @@ static void ip_vs_dest_set_eraseall(struct ip_vs_dest_set *set)
struct ip_vs_dest_set_elem *e, *ep;
list_for_each_entry_safe(e, ep, &set->list, list) {
- struct ip_vs_dest *d;
-
- d = rcu_dereference_protected(e->dest, 1);
- /*
- * We don't kfree dest because it is referred either
- * by its service or by the trash dest list.
- */
- ip_vs_dest_put(d);
list_del_rcu(&e->list);
- kfree_rcu(e, rcu_head);
+ call_rcu(&e->rcu_head, ip_vs_lblcr_elem_rcu_free);
}
}
@@ -175,12 +168,9 @@ static inline struct ip_vs_dest *ip_vs_dest_set_min(struct ip_vs_dest_set *set)
struct ip_vs_dest *dest, *least;
int loh, doh;
- if (set == NULL)
- return NULL;
-
/* select the first destination server, whose weight > 0 */
list_for_each_entry_rcu(e, &set->list, list) {
- least = rcu_dereference(e->dest);
+ least = e->dest;
if (least->flags & IP_VS_DEST_F_OVERLOAD)
continue;
@@ -195,7 +185,7 @@ static inline struct ip_vs_dest *ip_vs_dest_set_min(struct ip_vs_dest_set *set)
/* find the destination with the weighted least load */
nextstage:
list_for_each_entry_continue_rcu(e, &set->list, list) {
- dest = rcu_dereference(e->dest);
+ dest = e->dest;
if (dest->flags & IP_VS_DEST_F_OVERLOAD)
continue;
@@ -232,7 +222,7 @@ static inline struct ip_vs_dest *ip_vs_dest_set_max(struct ip_vs_dest_set *set)
/* select the first destination server, whose weight > 0 */
list_for_each_entry(e, &set->list, list) {
- most = rcu_dereference_protected(e->dest, 1);
+ most = e->dest;
if (atomic_read(&most->weight) > 0) {
moh = ip_vs_dest_conn_overhead(most);
goto nextstage;
@@ -243,7 +233,7 @@ static inline struct ip_vs_dest *ip_vs_dest_set_max(struct ip_vs_dest_set *set)
/* find the destination with the weighted most load */
nextstage:
list_for_each_entry_continue(e, &set->list, list) {
- dest = rcu_dereference_protected(e->dest, 1);
+ dest = e->dest;
doh = ip_vs_dest_conn_overhead(dest);
/* moh/mw < doh/dw ==> moh*dw < doh*mw, where mw,dw>0 */
if (((__s64)moh * atomic_read(&dest->weight) <
@@ -819,7 +809,7 @@ static void __exit ip_vs_lblcr_cleanup(void)
{
unregister_ip_vs_scheduler(&ip_vs_lblcr_scheduler);
unregister_pernet_subsys(&ip_vs_lblcr_ops);
- synchronize_rcu();
+ rcu_barrier();
}
--
1.8.4
^ permalink raw reply related
* [PATCH 3/5] ipvs: do not use dest after ip_vs_dest_put in LBLC
From: Simon Horman @ 2013-09-18 19:45 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Simon Horman
In-Reply-To: <1379533542-9891-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
commit c2a4ffb70eef39 ("ipvs: convert lblc scheduler to rcu")
allows RCU readers to use dest after calling ip_vs_dest_put().
In the corner case it can race with ip_vs_dest_trash_expire()
which can release the dest while it is being returned to the
RCU readers as scheduling result.
To fix the problem do not allow en->dest to be replaced and
defer the ip_vs_dest_put() call by using RCU callback. Now
en->dest does not need to be RCU pointer.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_lblc.c | 68 +++++++++++++++++++----------------------
1 file changed, 31 insertions(+), 37 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index eb814bf..eff13c9 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -93,7 +93,7 @@ struct ip_vs_lblc_entry {
struct hlist_node list;
int af; /* address family */
union nf_inet_addr addr; /* destination IP address */
- struct ip_vs_dest __rcu *dest; /* real server (cache) */
+ struct ip_vs_dest *dest; /* real server (cache) */
unsigned long lastuse; /* last used time */
struct rcu_head rcu_head;
};
@@ -130,20 +130,21 @@ static struct ctl_table vs_vars_table[] = {
};
#endif
-static inline void ip_vs_lblc_free(struct ip_vs_lblc_entry *en)
+static void ip_vs_lblc_rcu_free(struct rcu_head *head)
{
- struct ip_vs_dest *dest;
+ struct ip_vs_lblc_entry *en = container_of(head,
+ struct ip_vs_lblc_entry,
+ rcu_head);
- hlist_del_rcu(&en->list);
- /*
- * We don't kfree dest because it is referred either by its service
- * or the trash dest list.
- */
- dest = rcu_dereference_protected(en->dest, 1);
- ip_vs_dest_put(dest);
- kfree_rcu(en, rcu_head);
+ ip_vs_dest_put(en->dest);
+ kfree(en);
}
+static inline void ip_vs_lblc_del(struct ip_vs_lblc_entry *en)
+{
+ hlist_del_rcu(&en->list);
+ call_rcu(&en->rcu_head, ip_vs_lblc_rcu_free);
+}
/*
* Returns hash value for IPVS LBLC entry
@@ -203,30 +204,23 @@ ip_vs_lblc_new(struct ip_vs_lblc_table *tbl, const union nf_inet_addr *daddr,
struct ip_vs_lblc_entry *en;
en = ip_vs_lblc_get(dest->af, tbl, daddr);
- if (!en) {
- en = kmalloc(sizeof(*en), GFP_ATOMIC);
- if (!en)
- return NULL;
-
- en->af = dest->af;
- ip_vs_addr_copy(dest->af, &en->addr, daddr);
- en->lastuse = jiffies;
+ if (en) {
+ if (en->dest == dest)
+ return en;
+ ip_vs_lblc_del(en);
+ }
+ en = kmalloc(sizeof(*en), GFP_ATOMIC);
+ if (!en)
+ return NULL;
- ip_vs_dest_hold(dest);
- RCU_INIT_POINTER(en->dest, dest);
+ en->af = dest->af;
+ ip_vs_addr_copy(dest->af, &en->addr, daddr);
+ en->lastuse = jiffies;
- ip_vs_lblc_hash(tbl, en);
- } else {
- struct ip_vs_dest *old_dest;
+ ip_vs_dest_hold(dest);
+ en->dest = dest;
- old_dest = rcu_dereference_protected(en->dest, 1);
- if (old_dest != dest) {
- ip_vs_dest_put(old_dest);
- ip_vs_dest_hold(dest);
- /* No ordering constraints for refcnt */
- RCU_INIT_POINTER(en->dest, dest);
- }
- }
+ ip_vs_lblc_hash(tbl, en);
return en;
}
@@ -246,7 +240,7 @@ static void ip_vs_lblc_flush(struct ip_vs_service *svc)
tbl->dead = 1;
for (i=0; i<IP_VS_LBLC_TAB_SIZE; i++) {
hlist_for_each_entry_safe(en, next, &tbl->bucket[i], list) {
- ip_vs_lblc_free(en);
+ ip_vs_lblc_del(en);
atomic_dec(&tbl->entries);
}
}
@@ -281,7 +275,7 @@ static inline void ip_vs_lblc_full_check(struct ip_vs_service *svc)
sysctl_lblc_expiration(svc)))
continue;
- ip_vs_lblc_free(en);
+ ip_vs_lblc_del(en);
atomic_dec(&tbl->entries);
}
spin_unlock(&svc->sched_lock);
@@ -335,7 +329,7 @@ static void ip_vs_lblc_check_expire(unsigned long data)
if (time_before(now, en->lastuse + ENTRY_TIMEOUT))
continue;
- ip_vs_lblc_free(en);
+ ip_vs_lblc_del(en);
atomic_dec(&tbl->entries);
goal--;
}
@@ -511,7 +505,7 @@ ip_vs_lblc_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
* free up entries from the trash at any time.
*/
- dest = rcu_dereference(en->dest);
+ dest = en->dest;
if ((dest->flags & IP_VS_DEST_F_AVAILABLE) &&
atomic_read(&dest->weight) > 0 && !is_overloaded(dest, svc))
goto out;
@@ -631,7 +625,7 @@ static void __exit ip_vs_lblc_cleanup(void)
{
unregister_ip_vs_scheduler(&ip_vs_lblc_scheduler);
unregister_pernet_subsys(&ip_vs_lblc_ops);
- synchronize_rcu();
+ rcu_barrier();
}
--
1.8.4
^ permalink raw reply related
* [PATCH 1/5] ipvs: fix overflow on dest weight multiply
From: Simon Horman @ 2013-09-18 19:45 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Simon Kirby, Simon Horman
In-Reply-To: <1379533542-9891-1-git-send-email-horms@verge.net.au>
From: Simon Kirby <sim@hostway.ca>
Schedulers such as lblc and lblcr require the weight to be as high as the
maximum number of active connections. In commit b552f7e3a9524abcbcdf
("ipvs: unify the formula to estimate the overhead of processing
connections"), the consideration of inactconns and activeconns was cleaned
up to always count activeconns as 256 times more important than inactconns.
In cases where 3000 or more connections are expected, a weight of 3000 *
256 * 3000 connections overflows the 32-bit signed result used to determine
if rescheduling is required.
On amd64, this merely changes the multiply and comparison instructions to
64-bit. On x86, a 64-bit result is already present from imull, so only
a few more comparison instructions are emitted.
Signed-off-by: Simon Kirby <sim@hostway.ca>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
include/net/ip_vs.h | 2 +-
net/netfilter/ipvs/ip_vs_lblc.c | 4 ++--
net/netfilter/ipvs/ip_vs_lblcr.c | 12 ++++++------
net/netfilter/ipvs/ip_vs_nq.c | 8 ++++----
net/netfilter/ipvs/ip_vs_sed.c | 8 ++++----
net/netfilter/ipvs/ip_vs_wlc.c | 6 +++---
6 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index f0d70f0..fe782ed 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1649,7 +1649,7 @@ static inline void ip_vs_conn_drop_conntrack(struct ip_vs_conn *cp)
/* CONFIG_IP_VS_NFCT */
#endif
-static inline unsigned int
+static inline int
ip_vs_dest_conn_overhead(struct ip_vs_dest *dest)
{
/*
diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index 1383b0e..eb814bf 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -443,8 +443,8 @@ __ip_vs_lblc_schedule(struct ip_vs_service *svc)
continue;
doh = ip_vs_dest_conn_overhead(dest);
- if (loh * atomic_read(&dest->weight) >
- doh * atomic_read(&least->weight)) {
+ if ((__s64)loh * atomic_read(&dest->weight) >
+ (__s64)doh * atomic_read(&least->weight)) {
least = dest;
loh = doh;
}
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index 5199448..e65f7c5 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -200,8 +200,8 @@ static inline struct ip_vs_dest *ip_vs_dest_set_min(struct ip_vs_dest_set *set)
continue;
doh = ip_vs_dest_conn_overhead(dest);
- if ((loh * atomic_read(&dest->weight) >
- doh * atomic_read(&least->weight))
+ if (((__s64)loh * atomic_read(&dest->weight) >
+ (__s64)doh * atomic_read(&least->weight))
&& (dest->flags & IP_VS_DEST_F_AVAILABLE)) {
least = dest;
loh = doh;
@@ -246,8 +246,8 @@ static inline struct ip_vs_dest *ip_vs_dest_set_max(struct ip_vs_dest_set *set)
dest = rcu_dereference_protected(e->dest, 1);
doh = ip_vs_dest_conn_overhead(dest);
/* moh/mw < doh/dw ==> moh*dw < doh*mw, where mw,dw>0 */
- if ((moh * atomic_read(&dest->weight) <
- doh * atomic_read(&most->weight))
+ if (((__s64)moh * atomic_read(&dest->weight) <
+ (__s64)doh * atomic_read(&most->weight))
&& (atomic_read(&dest->weight) > 0)) {
most = dest;
moh = doh;
@@ -611,8 +611,8 @@ __ip_vs_lblcr_schedule(struct ip_vs_service *svc)
continue;
doh = ip_vs_dest_conn_overhead(dest);
- if (loh * atomic_read(&dest->weight) >
- doh * atomic_read(&least->weight)) {
+ if ((__s64)loh * atomic_read(&dest->weight) >
+ (__s64)doh * atomic_read(&least->weight)) {
least = dest;
loh = doh;
}
diff --git a/net/netfilter/ipvs/ip_vs_nq.c b/net/netfilter/ipvs/ip_vs_nq.c
index d8d9860..961a6de 100644
--- a/net/netfilter/ipvs/ip_vs_nq.c
+++ b/net/netfilter/ipvs/ip_vs_nq.c
@@ -40,7 +40,7 @@
#include <net/ip_vs.h>
-static inline unsigned int
+static inline int
ip_vs_nq_dest_overhead(struct ip_vs_dest *dest)
{
/*
@@ -59,7 +59,7 @@ ip_vs_nq_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
struct ip_vs_iphdr *iph)
{
struct ip_vs_dest *dest, *least = NULL;
- unsigned int loh = 0, doh;
+ int loh = 0, doh;
IP_VS_DBG(6, "%s(): Scheduling...\n", __func__);
@@ -92,8 +92,8 @@ ip_vs_nq_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
}
if (!least ||
- (loh * atomic_read(&dest->weight) >
- doh * atomic_read(&least->weight))) {
+ ((__s64)loh * atomic_read(&dest->weight) >
+ (__s64)doh * atomic_read(&least->weight))) {
least = dest;
loh = doh;
}
diff --git a/net/netfilter/ipvs/ip_vs_sed.c b/net/netfilter/ipvs/ip_vs_sed.c
index a5284cc..e446b9f 100644
--- a/net/netfilter/ipvs/ip_vs_sed.c
+++ b/net/netfilter/ipvs/ip_vs_sed.c
@@ -44,7 +44,7 @@
#include <net/ip_vs.h>
-static inline unsigned int
+static inline int
ip_vs_sed_dest_overhead(struct ip_vs_dest *dest)
{
/*
@@ -63,7 +63,7 @@ ip_vs_sed_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
struct ip_vs_iphdr *iph)
{
struct ip_vs_dest *dest, *least;
- unsigned int loh, doh;
+ int loh, doh;
IP_VS_DBG(6, "%s(): Scheduling...\n", __func__);
@@ -99,8 +99,8 @@ ip_vs_sed_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
if (dest->flags & IP_VS_DEST_F_OVERLOAD)
continue;
doh = ip_vs_sed_dest_overhead(dest);
- if (loh * atomic_read(&dest->weight) >
- doh * atomic_read(&least->weight)) {
+ if ((__s64)loh * atomic_read(&dest->weight) >
+ (__s64)doh * atomic_read(&least->weight)) {
least = dest;
loh = doh;
}
diff --git a/net/netfilter/ipvs/ip_vs_wlc.c b/net/netfilter/ipvs/ip_vs_wlc.c
index 6dc1fa1..b5b4650 100644
--- a/net/netfilter/ipvs/ip_vs_wlc.c
+++ b/net/netfilter/ipvs/ip_vs_wlc.c
@@ -35,7 +35,7 @@ ip_vs_wlc_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
struct ip_vs_iphdr *iph)
{
struct ip_vs_dest *dest, *least;
- unsigned int loh, doh;
+ int loh, doh;
IP_VS_DBG(6, "ip_vs_wlc_schedule(): Scheduling...\n");
@@ -71,8 +71,8 @@ ip_vs_wlc_schedule(struct ip_vs_service *svc, const struct sk_buff *skb,
if (dest->flags & IP_VS_DEST_F_OVERLOAD)
continue;
doh = ip_vs_dest_conn_overhead(dest);
- if (loh * atomic_read(&dest->weight) >
- doh * atomic_read(&least->weight)) {
+ if ((__s64)loh * atomic_read(&dest->weight) >
+ (__s64)doh * atomic_read(&least->weight)) {
least = dest;
loh = doh;
}
--
1.8.4
^ permalink raw reply related
* [PATCH 0/8 V3] rtlwifi: Patches to fix problems shown by smatch
From: Larry Finger @ 2013-09-18 19:57 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA, Larry Finger,
netdev-u79uwXL29TY76Z2rM5mHXA
Fix smatch warnings and errors in the rtlwifi family of drivers.
V2 addresses comments by David Laight and Sergei Shtylyov.
V3 addresses further comments by David, Sergei, and Kalle Valo. The
dead code is removed, and the variable is left in the struct.
Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
---
Larry Finger (8):
rtlwifi: rtl8192du: Fix smatch errors in /rtl8192de/dm.c
rtlwifi: rtl8192de: Fix smatch warnings in rtl8192de/hw.c
rtlwifi: rtl8192cu: Fix smatch warning in rtl8192cu/trx.c
rtlwifi: rtl8192_common: Fix smatch errors and warnings in
rtl8192c/dm_common.c
rtlwifi: Fix smatch warning in pci.c
rtlwifi: Fix smatch warnings in usb.c
rtlwifi: rtl8188ee: Fix smatch warning in rtl8188ee/hw.c
rtlwifi: Remove all remaining references to variable 'noise'
in rtl_stats struct
drivers/net/wireless/rtlwifi/pci.c | 1 -
drivers/net/wireless/rtlwifi/rtl8188ee/hw.c | 1 +
drivers/net/wireless/rtlwifi/rtl8188ee/trx.c | 1 -
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c | 4 +++-
drivers/net/wireless/rtlwifi/rtl8192ce/trx.c | 1 -
drivers/net/wireless/rtlwifi/rtl8192cu/trx.c | 2 --
drivers/net/wireless/rtlwifi/rtl8192de/dm.c | 8 ++++++--
drivers/net/wireless/rtlwifi/rtl8192de/hw.c | 2 ++
drivers/net/wireless/rtlwifi/rtl8192de/trx.c | 1 -
drivers/net/wireless/rtlwifi/rtl8192se/trx.c | 1 -
drivers/net/wireless/rtlwifi/rtl8723ae/trx.c | 1 -
drivers/net/wireless/rtlwifi/usb.c | 6 ++++--
drivers/net/wireless/rtlwifi/wifi.h | 1 -
13 files changed, 16 insertions(+), 14 deletions(-)
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 2/8 V3] rtlwifi: rtl8192de: Fix smatch warnings in rtl8192de/hw.c
From: Larry Finger @ 2013-09-18 19:57 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA, Larry Finger,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1379534260-14841-1-git-send-email-Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Smatch lists the following:
CHECK drivers/net/wireless/rtlwifi/rtl8192de/hw.c
drivers/net/wireless/rtlwifi/rtl8192de/hw.c:1200 rtl92de_set_qos() info: ignoring unreachable code.
drivers/net/wireless/rtlwifi/rtl8192de/hw.c:1200 rtl92de_set_qos() info: ignoring unreachable code.
The dead code is deleted.
Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
---
drivers/net/wireless/rtlwifi/rtl8192de/hw.c | 19 -------------------
1 file changed, 19 deletions(-)
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192de/hw.c
@@ -1194,25 +1194,6 @@ void rtl92d_linked_set_reg(struct ieee80
* mac80211 will send pkt when scan */
void rtl92de_set_qos(struct ieee80211_hw *hw, int aci)
{
- struct rtl_priv *rtlpriv = rtl_priv(hw);
- rtl92d_dm_init_edca_turbo(hw);
- return;
- switch (aci) {
- case AC1_BK:
- rtl_write_dword(rtlpriv, REG_EDCA_BK_PARAM, 0xa44f);
- break;
- case AC0_BE:
- break;
- case AC2_VI:
- rtl_write_dword(rtlpriv, REG_EDCA_VI_PARAM, 0x5e4322);
- break;
- case AC3_VO:
- rtl_write_dword(rtlpriv, REG_EDCA_VO_PARAM, 0x2f3222);
- break;
- default:
- RT_ASSERT(false, "invalid aci: %d !\n", aci);
- break;
- }
}
void rtl92de_enable_interrupt(struct ieee80211_hw *hw)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/8 V3] rtlwifi: rtl8192du: Fix smatch errors in /rtl8192de/dm.c
From: Larry Finger @ 2013-09-18 19:57 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Larry Finger, netdev
In-Reply-To: <1379534260-14841-1-git-send-email-Larry.Finger@lwfinger.net>
Smatch lists the following:
CHECK drivers/net/wireless/rtlwifi/rtl8192de/dm.c
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1054 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'ofdm_index' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1056 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'ofdm_index' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1126 rtl92d_dm_txpower_tracking_callback_thermalmeter() debug: remove_pools: nr_children over 4000 (4596). (rtlpriv->dbg.global_debuglevel merged)
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1126 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1129 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1132 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1135 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1138 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1141 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1144 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1147 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch1ch13' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1151 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1154 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1157 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1160 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1163 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1166 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1169 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
drivers/net/wireless/rtlwifi/rtl8192de/dm.c:1172 rtl92d_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'cckswing_table_ch14' 33 <= 255
This patch fixes several off-by-one errors. It also removes a comment
referencing variable 'noise' in the rts_stats struct.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
drivers/net/wireless/rtlwifi/rtl8192de/dm.c | 8 ++++++--
drivers/net/wireless/rtlwifi/rtl8192de/trx.c | 1 -
2 files changed, 6 insertions(+), 3 deletions(-)
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192de/dm.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/rtl8192de/dm.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192de/dm.c
@@ -840,9 +840,9 @@ static void rtl92d_dm_txpower_tracking_c
bool internal_pa = false;
long ele_a = 0, ele_d, temp_cck, val_x, value32;
long val_y, ele_c = 0;
- u8 ofdm_index[2];
+ u8 ofdm_index[3];
s8 cck_index = 0;
- u8 ofdm_index_old[2] = {0, 0};
+ u8 ofdm_index_old[3] = {0, 0, 0};
s8 cck_index_old = 0;
u8 index;
int i;
@@ -1118,6 +1118,10 @@ static void rtl92d_dm_txpower_tracking_c
val_x, val_y, ele_a, ele_c, ele_d,
val_x, val_y);
+ if (cck_index >= CCK_TABLE_SIZE)
+ cck_index = CCK_TABLE_SIZE - 1;
+ if (cck_index < 0)
+ cck_index = 0;
if (rtlhal->current_bandtype == BAND_ON_2_4G) {
/* Adjust CCK according to IQK result */
if (!rtlpriv->dm.cck_inch14) {
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192de/trx.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/rtl8192de/trx.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192de/trx.c
@@ -526,7 +526,6 @@ bool rtl92de_rx_query_desc(struct ieee80
}
/*rx_status->qual = stats->signal; */
rx_status->signal = stats->rssi + 10;
- /*rx_status->noise = -stats->noise; */
return true;
}
^ permalink raw reply
* [PATCH 5/8 V3] rtlwifi: Fix smatch warning in pci.c
From: Larry Finger @ 2013-09-18 19:58 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA, Larry Finger,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1379534339-14895-5-git-send-email-Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Smatch reports the following:
CHECK drivers/net/wireless/rtlwifi/pci.c
drivers/net/wireless/rtlwifi/pci.c:739 _rtl_pci_rx_interrupt() warn: assigning (-98) to unsigned variable 'stats.noise'
The variable 'stats.noise' is not used. That initializer is removed.
Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
---
drivers/net/wireless/rtlwifi/pci.c | 1 -
1 file changed, 1 deletion(-)
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/pci.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/pci.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/pci.c
@@ -736,7 +736,6 @@ static void _rtl_pci_rx_interrupt(struct
struct rtl_stats stats = {
.signal = 0,
- .noise = -98,
.rate = 0,
};
int index = rtlpci->rx_ring[rx_queue_idx].idx;
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 4/8 V3] rtlwifi: rtl8192_common: Fix smatch errors and warnings in rtl8192c/dm_common.c
From: Larry Finger @ 2013-09-18 19:58 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Larry Finger, netdev
Smatch lists the following:
CHECK drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:551 rtl92c_dm_pwdb_monitor() info: ignoring unreachable code.
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:551 rtl92c_dm_pwdb_monitor() info: ignoring unreachable code.
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:870 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:870 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:882 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:883 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:891 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:892 rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow 'txpwr_level' 2 <= 2
The unreachable code message is fixed by deleting the code that follows a return.
The errors are fixed by increasing the size of txpwr_level.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c | 25 +------------------------
1 file changed, 1 insertion(+), 24 deletions(-)
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
@@ -541,29 +541,6 @@ EXPORT_SYMBOL(rtl92c_dm_write_dig);
static void rtl92c_dm_pwdb_monitor(struct ieee80211_hw *hw)
{
- struct rtl_priv *rtlpriv = rtl_priv(hw);
- long tmpentry_max_pwdb = 0, tmpentry_min_pwdb = 0xff;
-
- u8 h2c_parameter[3] = { 0 };
-
- return;
-
- if (tmpentry_max_pwdb != 0) {
- rtlpriv->dm.entry_max_undec_sm_pwdb = tmpentry_max_pwdb;
- } else {
- rtlpriv->dm.entry_max_undec_sm_pwdb = 0;
- }
-
- if (tmpentry_min_pwdb != 0xff) {
- rtlpriv->dm.entry_min_undec_sm_pwdb = tmpentry_min_pwdb;
- } else {
- rtlpriv->dm.entry_min_undec_sm_pwdb = 0;
- }
-
- h2c_parameter[2] = (u8) (rtlpriv->dm.undec_sm_pwdb & 0xFF);
- h2c_parameter[0] = 0;
-
- rtl92c_fill_h2c_cmd(hw, H2C_RSSI_REPORT, 3, h2c_parameter);
}
void rtl92c_dm_init_edca_turbo(struct ieee80211_hw *hw)
@@ -673,7 +650,7 @@ static void rtl92c_dm_txpower_tracking_c
s8 cck_index = 0;
int i;
bool is2t = IS_92C_SERIAL(rtlhal->version);
- s8 txpwr_level[2] = {0, 0};
+ s8 txpwr_level[3] = {0, 0, 0};
u8 ofdm_min_index = 6, rf;
rtlpriv->dm.txpower_trackinginit = true;
^ permalink raw reply
* [PATCH 6/8 V3] rtlwifi: Fix smatch warnings in usb.c
From: Larry Finger @ 2013-09-18 19:58 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Larry Finger, netdev
In-Reply-To: <1379534339-14895-5-git-send-email-Larry.Finger@lwfinger.net>
Smatch displays the following:
CHECK drivers/net/wireless/rtlwifi/usb.c
drivers/net/wireless/rtlwifi/usb.c:458 _rtl_usb_rx_process_agg() warn: assigning (-98) to unsigned variable 'stats.noise'
drivers/net/wireless/rtlwifi/usb.c:503 _rtl_usb_rx_process_noagg() warn: assigning (-98) to unsigned variable 'stats.noise'
drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: ignoring unreachable code.
drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: ignoring unreachable code.
The variable 'stats.noise' is not used, thus the initializers are removed.
The unreachable code info is fixed by including the appropriate section inside
#ifdef .. #endif constructions.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
drivers/net/wireless/rtlwifi/usb.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/usb.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/usb.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/usb.c
@@ -455,7 +455,6 @@ static void _rtl_usb_rx_process_agg(stru
struct ieee80211_rx_status rx_status = {0};
struct rtl_stats stats = {
.signal = 0,
- .noise = -98,
.rate = 0,
};
@@ -498,7 +497,6 @@ static void _rtl_usb_rx_process_noagg(st
struct ieee80211_rx_status rx_status = {0};
struct rtl_stats stats = {
.signal = 0,
- .noise = -98,
.rate = 0,
};
@@ -582,12 +580,15 @@ static void _rtl_rx_work(unsigned long p
static unsigned int _rtl_rx_get_padding(struct ieee80211_hdr *hdr,
unsigned int len)
{
+#if NET_IP_ALIGN != 0
unsigned int padding = 0;
+#endif
/* make function no-op when possible */
if (NET_IP_ALIGN == 0 || len < sizeof(*hdr))
return 0;
+#if NET_IP_ALIGN != 0
/* alignment calculation as in lbtf_rx() / carl9170_rx_copy_data() */
/* TODO: deduplicate common code, define helper function instead? */
@@ -608,6 +609,7 @@ static unsigned int _rtl_rx_get_padding(
padding ^= NET_IP_ALIGN;
return padding;
+#endif
}
#define __RADIO_TAP_SIZE_RSV 32
^ permalink raw reply
* [PATCH 7/8 V3] rtlwifi: rtl8188ee: Fix smatch warning in rtl8188ee/hw.c
From: Larry Finger @ 2013-09-18 19:58 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Larry Finger, netdev, Stable
In-Reply-To: <1379534339-14895-5-git-send-email-Larry.Finger@lwfinger.net>
Smatch lists the following:
CHECK drivers/net/wireless/rtlwifi/rtl8188ee/hw.c
drivers/net/wireless/rtlwifi/rtl8188ee/hw.c:149 _rtl88ee_set_fw_clock_on() info: ignoring unreachable code.
drivers/net/wireless/rtlwifi/rtl8188ee/hw.c:149 _rtl88ee_set_fw_clock_on() info: ignoring unreachable code.
This info message is the result of a real error due to a missing break statement
in a "while (1)" loop.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org> [3.10+]
---
drivers/net/wireless/rtlwifi/rtl8188ee/hw.c | 1 +
1 file changed, 1 insertion(+)
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8188ee/hw.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/rtl8188ee/hw.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8188ee/hw.c
@@ -143,6 +143,7 @@ static void _rtl88ee_set_fw_clock_on(str
} else {
rtlhal->fw_clk_change_in_progress = false;
spin_unlock_bh(&rtlpriv->locks.fw_ps_lock);
+ break;
}
}
^ permalink raw reply
* [PATCH 8/8 V3] rtlwifi: Remove all remaining references to variable 'noise' in rtl_stats struct
From: Larry Finger @ 2013-09-18 19:58 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Larry Finger, netdev
In-Reply-To: <1379534339-14895-5-git-send-email-Larry.Finger@lwfinger.net>
This completes removal of all places that reference variable 'noise'
in the rtl_stats struct. The definition of the struct is unchanged.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
drivers/net/wireless/rtlwifi/rtl8188ee/trx.c | 1 -
drivers/net/wireless/rtlwifi/rtl8192ce/trx.c | 1 -
drivers/net/wireless/rtlwifi/rtl8192se/trx.c | 1 -
drivers/net/wireless/rtlwifi/rtl8723ae/trx.c | 1 -
4 files changed, 4 deletions(-)
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8188ee/trx.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/rtl8188ee/trx.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8188ee/trx.c
@@ -477,7 +477,6 @@ bool rtl88ee_rx_query_desc(struct ieee80
/*rx_status->qual = status->signal; */
rx_status->signal = status->recvsignalpower + 10;
- /*rx_status->noise = -status->noise; */
if (status->packet_report_type == TX_REPORT2) {
status->macid_valid_entry[0] =
GET_RX_RPT2_DESC_MACID_VALID_1(pdesc);
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192ce/trx.c
@@ -420,7 +420,6 @@ bool rtl92ce_rx_query_desc(struct ieee80
/*rx_status->qual = stats->signal; */
rx_status->signal = stats->recvsignalpower + 10;
- /*rx_status->noise = -stats->noise; */
return true;
}
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8192se/trx.c
@@ -330,7 +330,6 @@ bool rtl92se_rx_query_desc(struct ieee80
/*rx_status->qual = stats->signal; */
rx_status->signal = stats->rssi + 10;
- /*rx_status->noise = -stats->noise; */
return true;
}
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8723ae/trx.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/rtl8723ae/trx.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/rtl8723ae/trx.c
@@ -359,7 +359,6 @@ bool rtl8723ae_rx_query_desc(struct ieee
/*rx_status->qual = status->signal; */
rx_status->signal = status->recvsignalpower + 10;
- /*rx_status->noise = -status->noise; */
return true;
}
^ permalink raw reply
* [PATCH net RFC 0/2] net: BUG napi_disable sleep while atomic
From: Jacob Keller @ 2013-09-18 20:19 UTC (permalink / raw)
To: netdev
The following series implements a fix for a possible sleep while atomic bug in
the ixgbe driver, caused because napi_disable might sleep. The first patch adds
a might_sleep() call in order to make future sleep while atomic bugs easier to
discover, (as napi_disable does not always sleep!). The second patch is my best
guess at an actual fix for the bug. There is likely a similar bug in every
driver which implemented NET_RX_BUSY_POLL similar to ixgbe. The first patch
will likely cause BUG splats to appear in those drivers unless they are fixed.
---
Jacob Keller (2):
netdevice: add might_sleep() call to napi_disable
ixgbe: fix sleep bug caused by napi_disable inside local_bh_disable()d context
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
include/linux/netdevice.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
--
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply
* [PATCH net RFC 1/2] netdevice: add might_sleep() call to napi_disable
From: Jacob Keller @ 2013-09-18 20:20 UTC (permalink / raw)
To: netdev
Cc: Alex Duyck, Hyong-Youb Kim, Dmitry Kravkov, Amir Vadai,
Eliezer Tamir
In-Reply-To: <20130918201541.8697.3548.stgit@jekeller-desk1.amr.corp.intel.com>
napi_disable potentially calls msleep(1) if the NAPI_STATE_SCHED bit is
previously set by something else. Because it does not always call msleep, it
was difficult to track down a bug related to calling napi_disable within
local_bh_disable()d context. This patch adds a might_sleep() call to the
napi_disable routine in order to aid in the future debugging of similar issues.
This will cause a BUG in drivers which have implemented busy polling in a
similar fashion to ixgbe, and which call napi_disable inside the
local_bh_disable()d section where the vector napi lock is taken.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Cc: Eliezer Tamir <eliezer.tamir@linux.intel.com>
Cc: Alex Duyck <alexander.h.duyck@intel.com>
Cc: Hyong-Youb Kim <hykim@myri.com>
Cc: Amir Vadai <amirv@mellanox.com>
Cc: Dmitry Kravkov <dmitry@broadcom.com>
---
include/linux/netdevice.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3de49ac..81dab00 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -483,6 +483,8 @@ extern void napi_hash_del(struct napi_struct *napi);
*/
static inline void napi_disable(struct napi_struct *n)
{
+ might_sleep();
+
set_bit(NAPI_STATE_DISABLE, &n->state);
while (test_and_set_bit(NAPI_STATE_SCHED, &n->state))
msleep(1);
^ permalink raw reply related
* [PATCH net RFC 2/2] ixgbe: fix sleep bug caused by napi_disable inside local_bh_disable()d context
From: Jacob Keller @ 2013-09-18 20:20 UTC (permalink / raw)
To: netdev
Cc: Alexander Duyck, Hyong-Youb Kim, Dmitry Kravkov, Amir Vadai,
Eliezer Tamir
In-Reply-To: <20130918201541.8697.3548.stgit@jekeller-desk1.amr.corp.intel.com>
This patch fixes a bug caused by calling napi_disable after local_bh_disable.
It is possible for napi_disable to sleep, (though not guarunteed) so it could
cause an atomic sleep bug during the schedule() call in msleep. This patch
resolves the issue by moving the local_bh_disable() calls inside the for loop
in ixgbe_napi_disable_all().
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Cc: Eliezer Tamir <eliezer.tamir@linux.intel.com>
Cc: Alexander Duyck <alexander.duyck@intel.com>
Cc: Hyong-Youb Kim <hykim@myri.com>
Cc: Amir Vadai <amirv@mellanox.com>
Cc: Dmitry Kravkov <dmitry@broadcom.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 0ade0cd..85dbf58 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3893,15 +3893,15 @@ static void ixgbe_napi_disable_all(struct ixgbe_adapter *adapter)
{
int q_idx;
- local_bh_disable(); /* for ixgbe_qv_lock_napi() */
for (q_idx = 0; q_idx < adapter->num_q_vectors; q_idx++) {
napi_disable(&adapter->q_vector[q_idx]->napi);
+ local_bh_disable(); /* for ixgbe_qv_lock_napi() */
while (!ixgbe_qv_lock_napi(adapter->q_vector[q_idx])) {
pr_info("QV %d locked\n", q_idx);
mdelay(1);
}
+ local_bh_enable();
}
- local_bh_enable();
}
#ifdef CONFIG_IXGBE_DCB
^ permalink raw reply related
* Re: [PATCH -net] netpoll: fix NULL pointer dereference in netpoll_cleanup
From: Nikolay Aleksandrov @ 2013-09-18 21:06 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20130918.121539.1213515779326200577.davem@davemloft.net>
On 09/18/2013 06:15 PM, David Miller wrote:
> From: Nikolay Aleksandrov <nikolay@redhat.com>
> Date: Tue, 17 Sep 2013 16:12:35 +0200
>
>> @@ -1284,15 +1284,15 @@ EXPORT_SYMBOL_GPL(__netpoll_free_async);
>>
>> void netpoll_cleanup(struct netpoll *np)
>> {
>> - if (!np->dev)
>> - return;
>> -
>> rtnl_lock();
>> + if (!np->dev) {
>> + rtnl_unlock();
>> + return;
>> + }
>> __netpoll_cleanup(np);
>> - rtnl_unlock();
>> -
>> dev_put(np->dev);
>> np->dev = NULL;
>> + rtnl_unlock();
>> }
>> EXPORT_SYMBOL(netpoll_cleanup);
>
> I know it seems arbitrary, but please do this like:
>
> lock();
> if (condition) {
> ...
> }
> unlock();
>
> rather than having multiple return/unlock code paths.
>
> This style I am suggesting is easier to audit for locking problems.
>
> Thanks.
>
Got it, I'll edit it and re-post.
Thanks,
Nik
^ 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