* [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 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 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 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 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 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 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 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 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 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
* [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 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
* 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
* 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: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: [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 ] 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: 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 ] ip6tnl: do route updating for redirect in ip6_tnl_err()
From: Duan Jiong @ 2013-09-18 17:17 UTC (permalink / raw)
To: Duan Jiong, David Miller; +Cc: netdev, hannes
In-Reply-To: <5239970F.8000105@cn.fujitsu.com>
于 2013/9/18 20:05, Duan Jiong 写道:
> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>
> After the ip6_tnl_err() is called, the rel_msg is assigned
> to 0, and the ip4ip6_err()will return directly, the instruction
> below will not be executed.
>
> In rfc2473, we can know that the tunnel ICMP redirect
> message should not be reported to the source of the
> original packet, so we can handle it in ip4ip6_err().
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> ---
> net/ipv6/ip6_tunnel.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index 2d8f482..35c4b70 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -529,6 +529,9 @@ ip6_tnl_err(struct sk_buff *skb, __u8 ipproto, struct inet6_skb_parm *opt,
> rel_msg = 1;
> }
> break;
> + case NDISC_REDIRECT:
> + ip6_redirect(skb, net, 0, 0);
sorry, it should be
ip6_redirect(skb, dev_net(skb->dev), skb->dev->ifindex, 0);
> + break;
> }
>
> *type = rel_type;
> @@ -576,9 +579,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
> rel_type = ICMP_DEST_UNREACH;
> rel_code = ICMP_FRAG_NEEDED;
> break;
> - case NDISC_REDIRECT:
> - rel_type = ICMP_REDIRECT;
> - rel_code = ICMP_REDIR_HOST;
> default:
> return 0;
> }
> @@ -637,8 +637,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>
> skb_dst(skb2)->ops->update_pmtu(skb_dst(skb2), NULL, skb2, rel_info);
> }
> - if (rel_type == ICMP_REDIRECT)
> - skb_dst(skb2)->ops->redirect(skb_dst(skb2), NULL, skb2);
>
> icmp_send(skb2, rel_type, rel_code, htonl(rel_info));
>
^ permalink raw reply
* [PATCH] mrp: add periodictimer to retry lost packets
From: Noel Burton-Krahn @ 2013-09-18 17:09 UTC (permalink / raw)
To: netdev
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)
Cheers,
--
Noel
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..edcd458 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,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);
+ mod_timer(&app->periodic_timer, jiffies + delay);
+}
+
+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 +870,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 +898,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);
^ permalink raw reply related
* Re: [PATCH] USBNET: fix handling padding packet
From: Oliver Neukum @ 2013-09-18 17:06 UTC (permalink / raw)
To: Bjørn Mork
Cc: Ming Lei, David S. Miller, Greg Kroah-Hartman,
Network Development, linux-usb
In-Reply-To: <87zjra5dpx.fsf@nemi.mork.no>
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.
> 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.
Regards
Oliver
^ permalink raw reply
* Re: [PATCH] USBNET: fix handling padding packet
From: Ming Lei @ 2013-09-18 17:03 UTC (permalink / raw)
To: Bjørn Mork
Cc: David S. Miller, Greg Kroah-Hartman, Oliver Neukum,
Network Development, linux-usb, Oliver Neukum
In-Reply-To: <87zjra5dpx.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
On Wed, Sep 18, 2013 at 11:52 PM, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> wrote:
> Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> writes:
>
>>> Are you really sure that the one driver/device using this really need
>>> the padding byte? If you could just make FLAG_SEND_ZLP part of the
>>> condition for enabling can_dma_sg, then all this extra complexity would
>>> be unnecessary. As the comment in front of the padding code says:
>>
>> At least for the effected driver of ax88179, the padding packet is needed
>> since the driver does set the padding flag in the header, see
>> ax88179_tx_fixup().
>
> Yes, I noticed that the driver doesn't set the flag. I just didn't put
> too much into that. I don't think that necessarily means that the
> padding is needed. It probably just means that the driver worked with
> the default padding enabled, so the author left it there.
>
> This flag should really have been inverted. ZLP should be the default
> for all new usbnet drivers.
In theory, it is, but who knows the reality.
>
> 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.
Could you or other guys share how to build such packet so that I can
do the test?
Once we can confirm the padding isn't needed, we can remove the
padding flag. But if the padding flag is still there, this patch should be
dropped.
>
>>> "strictly conforming cdc-ether devices should expect the ZLP here"
>>>
>>> There shouldn't be any problems requiring this conformance as a
>>> precondition for allowing SG. The requirements are already strict.
>>
>> There is no reason to forbid DMA SG for one driver which requires
>> padding, right?
>
> Yes there is: Added complexity for everybody, based on a combination of
> features which just does not make any sense.
>
> 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
> 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.
See above, if we can prove that padding isn't needed, we can remove
the padding, then the patch can be dropped. If we can't, the patch is needed.
>
> 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?
If so, we can remove the assignment of zero to skb->data[skb->len],
but probably it might cause regression, that is why I wouldn't like to
do that. Also looks introducing one extra dev->padding_frame doesn't
cause much complexity.
Thanks
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net] ip: generate unique IP identificator if local fragmentation is allowed
From: David Miller @ 2013-09-18 16:55 UTC (permalink / raw)
To: aatteka; +Cc: netdev
In-Reply-To: <CAA=3Oq=Kxfe8KKnWV7QjLnT1Art3UEMJBTE=tXBLZPa6diug6g@mail.gmail.com>
From: Ansis Atteka <aatteka@nicira.com>
Date: Wed, 18 Sep 2013 09:50:31 -0700
> On Wed, Sep 18, 2013 at 9:20 AM, David Miller <davem@davemloft.net> wrote:
>> From: Ansis Atteka <aatteka@nicira.com>
>> Date: Tue, 17 Sep 2013 15:25:00 -0700
>>
>>> @@ -1317,6 +1317,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
>>> ttl = ip_select_ttl(inet, &rt->dst);
>>>
>>> iph = (struct iphdr *)skb->data;
>>> +
>>> iph->version = 4;
>>> iph->ihl = 5;
>>> iph->tos = inet->tos;
>>> @@ -1324,7 +1325,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
>>> iph->ttl = ttl;
>>> iph->protocol = sk->sk_protocol;
>>> ip_copy_addrs(iph, fl4);
>>> - ip_select_ident(iph, &rt->dst, sk);
>>> + ip_select_ident(skb, &rt->dst, sk);
>>>
>>> if (opt) {
>>> iph->ihl += opt->optlen>>2;
>>
>> This transformation is not equivalent.
>>
>> ip_select_ident() is going to use ip_hdr() which obtains the header
>> pointer via skb_network_header().
>>
>> But here, the code is using skb->data for this, which is different.
>
> I dared to do that because three lines below ip_options_build() seems
> to be already doing the same thing, if IP options are present:
>
> struct sk_buff *__ip_make_skb() {
> ...
> ip_select_ident(skb, &rt->dst, sk);
> if (opt) {
> iph->ihl += opt->optlen>>2;
> ip_options_build(skb, opt, cork->addr, rt, 0);
> }
>
> and
>
> void ip_options_build(struct sk_buff *skb, struct ip_options *opt,
> __be32 daddr, struct rtable *rt, int is_frag) {
> unsigned char *iph = skb_network_header(skb);
> ....
Then change the first hunk to use ip_hdr() if you are so confident.
Having inconsistent calculations of the ip header pointer is just
asking for trouble.
^ permalink raw reply
* Re: [PATCH net] ip: generate unique IP identificator if local fragmentation is allowed
From: Ansis Atteka @ 2013-09-18 16:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20130918.122018.1088822749792394509.davem@davemloft.net>
On Wed, Sep 18, 2013 at 9:20 AM, David Miller <davem@davemloft.net> wrote:
> From: Ansis Atteka <aatteka@nicira.com>
> Date: Tue, 17 Sep 2013 15:25:00 -0700
>
>> @@ -1317,6 +1317,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
>> ttl = ip_select_ttl(inet, &rt->dst);
>>
>> iph = (struct iphdr *)skb->data;
>> +
>> iph->version = 4;
>> iph->ihl = 5;
>> iph->tos = inet->tos;
>> @@ -1324,7 +1325,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
>> iph->ttl = ttl;
>> iph->protocol = sk->sk_protocol;
>> ip_copy_addrs(iph, fl4);
>> - ip_select_ident(iph, &rt->dst, sk);
>> + ip_select_ident(skb, &rt->dst, sk);
>>
>> if (opt) {
>> iph->ihl += opt->optlen>>2;
>
> This transformation is not equivalent.
>
> ip_select_ident() is going to use ip_hdr() which obtains the header
> pointer via skb_network_header().
>
> But here, the code is using skb->data for this, which is different.
I dared to do that because three lines below ip_options_build() seems
to be already doing the same thing, if IP options are present:
struct sk_buff *__ip_make_skb() {
...
ip_select_ident(skb, &rt->dst, sk);
if (opt) {
iph->ihl += opt->optlen>>2;
ip_options_build(skb, opt, cork->addr, rt, 0);
}
and
void ip_options_build(struct sk_buff *skb, struct ip_options *opt,
__be32 daddr, struct rtable *rt, int is_frag) {
unsigned char *iph = skb_network_header(skb);
....
Another solution would be to pass local fragmentation flag as fourth
argument to ip_select_ident(). If you think that would be cleaner,
then I can reimplement it that way?
^ permalink raw reply
* Re: mvneta: oops in __rcu_read_lock on mirabox
From: Willy Tarreau @ 2013-09-18 16:49 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Ethan Tuttle, Russell King - ARM Linux, Andrew Lunn, Jason Cooper,
netdev, Ezequiel Garcia, Gregory Clément, linux-arm-kernel
In-Reply-To: <20130918183549.3e7b8f4c@skate>
On Wed, Sep 18, 2013 at 06:35:49PM +0200, Thomas Petazzoni wrote:
> Dear Ethan Tuttle,
>
> On Tue, 17 Sep 2013 23:30:56 -0700, Ethan Tuttle wrote:
> > On Mon, Sep 16, 2013 at 11:01 PM, Willy Tarreau <w@1wt.eu> wrote:
> > > Next step should be that you test both kernels to be sure.
> >
> > Thanks for the kernel images, Willy. I'm still experimenting but
> > initial results are strange: I haven't seen a crash from the -ethan
> > image you provided, nor by a kernel with that config that I built
> > myself. The config is only different from my crashing config by a few
> > options. So perhaps some combination of options prevents the crash.
> > I'll see if I can narrow it down.
>
> A toolchain generating some crappy code maybe? Ethan, Willy, comparing
> your toolchain (compiler version, origin of the toolchain) could be
> interesting.
I thought about this but it looks suspicious, I don't see why the toolchain
would produce random bitflips.
My toolchain is a linaro 4.7 gcc into which I have added support for a
"pj4b" CPU target which is essentially the same as cortex-a9 plus support
for the IDIV instruction in thumb mode.
But I can send it to Ethan if that helps.
Willy
^ 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