* [PATCH net-next 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12.
From: Daniel Borkmann @ 2013-09-03 7:59 UTC (permalink / raw)
To: davem; +Cc: netdev, David Stevens, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>
i) RFC3810, 9.2. Query Interval [QI] says:
The Query Interval variable denotes the interval between General
Queries sent by the Querier. Default value: 125 seconds. [...]
ii) RFC3810, 9.3. Query Response Interval [QRI] says:
The Maximum Response Delay used to calculate the Maximum Response
Code inserted into the periodic General Queries. Default value:
10000 (10 seconds) [...] The number of seconds represented by the
[Query Response Interval] must be less than the [Query Interval].
iii) RFC3810, 9.12. Older Version Querier Present Timeout [OVQPT] says:
The Older Version Querier Present Timeout is the time-out for
transitioning a host back to MLDv2 Host Compatibility Mode. When an
MLDv1 query is received, MLDv2 hosts set their Older Version Querier
Present Timer to [Older Version Querier Present Timeout].
This value MUST be ([Robustness Variable] times (the [Query Interval]
in the last Query received)) plus ([Query Response Interval]).
Hence, on *default* the timeout results in:
[RV] = 2, [QI] = 125sec, [QRI] = 10sec
[OVQPT] = [RV] * [QI] + [QRI] = 260sec
Having that said, we currently calculate [OVQPT] (here given as 'switchback'
variable) as ...
switchback = (idev->mc_qrv + 1) * max_delay
RFC3810, 9.12. says "the [Query Interval] in the last Query received". In
section "9.14. Configuring timers", it is said:
This section is meant to provide advice to network administrators on
how to tune these settings to their network. Ambitious router
implementations might tune these settings dynamically based upon
changing characteristics of the network. [...]
iv) RFC38010, 9.14.2. Query Interval:
The overall level of periodic MLD traffic is inversely proportional
to the Query Interval. A longer Query Interval results in a lower
overall level of MLD traffic. The value of the Query Interval MUST
be equal to or greater than the Maximum Response Delay used to
calculate the Maximum Response Code inserted in General Query
messages.
I assume that was why switchback is calculated as is (3 * max_delay), although
this setting seems to be meant for routers only to configure their [QI]
interval for non-default intervals. So usage here like this is clearly wrong.
Concluding, the current behaviour in IPv6's multicast code is not conform
to the RFC as switch back is calculated wrongly. That is, it has a too small
value, so MLDv2 hosts switch back again to MLDv2 way too early, i.e. ~30secs
instead of ~260secs on default.
Hence, introduce necessary helper functions and fix this up properly as it
should be.
Introduced in 06da92283 ("[IPV6]: Add MLDv2 support."). Credits to Hannes
Frederic Sowa who also had a hand in this as well. Also thanks to Hangbin Liu
who did initial testing.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: David Stevens <dlstevens@us.ibm.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
06da92283 is linux-history tree.
include/net/if_inet6.h | 9 +++-
include/net/mld.h | 27 +++++++++++-
net/ipv6/mcast.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++---
3 files changed, 143 insertions(+), 9 deletions(-)
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 736b5fb..02ef772 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -171,12 +171,17 @@ struct inet6_dev {
struct ifmcaddr6 *mc_list;
struct ifmcaddr6 *mc_tomb;
spinlock_t mc_lock;
- unsigned char mc_qrv;
+
+ unsigned char mc_qrv; /* Query Robustness Variable */
unsigned char mc_gq_running;
unsigned char mc_ifc_count;
unsigned char mc_dad_count;
- unsigned long mc_v1_seen;
+
+ unsigned long mc_v1_seen; /* Max time we stay in MLDv1 mode */
+ unsigned long mc_qi; /* Query Interval */
+ unsigned long mc_qri; /* Query Response Interval */
unsigned long mc_maxdelay;
+
struct timer_list mc_gq_timer; /* general query timer */
struct timer_list mc_ifc_timer; /* interface change timer */
struct timer_list mc_dad_timer; /* dad complete mc timer */
diff --git a/include/net/mld.h b/include/net/mld.h
index 467143c..2b5421f 100644
--- a/include/net/mld.h
+++ b/include/net/mld.h
@@ -63,7 +63,7 @@ struct mld2_query {
#define mld2q_mrc mld2q_hdr.icmp6_maxdelay
#define mld2q_resv1 mld2q_hdr.icmp6_dataun.un_data16[1]
-/* Max Response Code */
+/* Max Response Code, TODO: transform this to use the below */
#define MLDV2_MASK(value, nb) ((nb)>=32 ? (value) : ((1<<(nb))-1) & (value))
#define MLDV2_EXP(thresh, nbmant, nbexp, value) \
((value) < (thresh) ? (value) : \
@@ -72,4 +72,29 @@ struct mld2_query {
#define MLDV2_MRC(value) MLDV2_EXP(0x8000, 12, 3, value)
+/* RFC3810, 5.1.3. Maximum Response Code:
+ *
+ * If Maximum Response Code >= 32768, Maximum Response Code represents a
+ * floating-point value as follows:
+ *
+ * 0 1 2 3 4 5 6 7 8 9 A B C D E F
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |1| exp | mant |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ */
+#define MLDV2_MRC_EXP(value) (((value) >> 12) & 0x0007)
+#define MLDV2_MRC_MAN(value) ((value) & 0x0fff)
+
+/* RFC3810, 5.1.9. QQIC (Querier's Query Interval Code):
+ *
+ * If QQIC >= 128, QQIC represents a floating-point value as follows:
+ *
+ * 0 1 2 3 4 5 6 7
+ * +-+-+-+-+-+-+-+-+
+ * |1| exp | mant |
+ * +-+-+-+-+-+-+-+-+
+ */
+#define MLDV2_QQIC_EXP(value) (((value) >> 4) & 0x07)
+#define MLDV2_QQIC_MAN(value) ((value) & 0x0f)
+
#endif
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 98ead2b..8992ff2 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -108,6 +108,10 @@ static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
struct inet6_dev *idev);
#define MLD_QRV_DEFAULT 2
+/* RFC3810, 9.2. Query Interval */
+#define MLD_QI_DEFAULT (125 * HZ)
+/* RFC3810, 9.3. Query Response Interval */
+#define MLD_QRI_DEFAULT (10 * HZ)
/* RFC3810, 8.1 Query Version Distinctions */
#define MLD_V1_QUERY_LEN 24
@@ -1112,6 +1116,93 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
return true;
}
+static void mld_set_v1_mode(struct inet6_dev *idev)
+{
+ /* RFC3810, relevant sections:
+ * - 9.1. Robustness Variable
+ * - 9.2. Query Interval
+ * - 9.3. Query Response Interval
+ * - 9.12. Older Version Querier Present Timeout
+ */
+ unsigned long switchback;
+
+ switchback = (idev->mc_qrv * idev->mc_qi) + idev->mc_qri;
+
+ idev->mc_v1_seen = jiffies + switchback;
+}
+
+static void mld_update_qrv(struct inet6_dev *idev,
+ const struct mld2_query *mlh2)
+{
+ /* RFC3810, relevant sections:
+ * - 5.1.8. QRV (Querier's Robustness Variable)
+ * - 9.1. Robustness Variable
+ */
+
+ /* The value of the Robustness Variable MUST NOT be zero,
+ * and SHOULD NOT be one. Catch this here if we ever run
+ * into such a case in future.
+ */
+ WARN_ON(idev->mc_qrv == 0);
+
+ if (mlh2->mld2q_qrv > 0)
+ idev->mc_qrv = mlh2->mld2q_qrv;
+
+ if (unlikely(idev->mc_qrv < 2)) {
+ net_warn_ratelimited("IPv6: MLD: clamping QRV from %u to %u!\n",
+ idev->mc_qrv, MLD_QRV_DEFAULT);
+ idev->mc_qrv = MLD_QRV_DEFAULT;
+ }
+}
+
+static void mld_update_qi(struct inet6_dev *idev,
+ const struct mld2_query *mlh2)
+{
+ /* RFC3810, relevant sections:
+ * - 5.1.9. QQIC (Querier's Query Interval Code)
+ * - 9.2. Query Interval
+ * - 9.12. Older Version Querier Present Timeout
+ * (the [Query Interval] in the last Query received)
+ */
+ unsigned long mc_qqi;
+
+ if (mlh2->mld2q_qqic < 128) {
+ mc_qqi = mlh2->mld2q_qqic;
+ } else {
+ unsigned long mc_man, mc_exp;
+
+ mc_exp = MLDV2_QQIC_EXP(mlh2->mld2q_qqic);
+ mc_man = MLDV2_QQIC_MAN(mlh2->mld2q_qqic);
+
+ mc_qqi = (mc_man | 0x10) << (mc_exp + 3);
+ }
+
+ idev->mc_qi = mc_qqi * HZ;
+}
+
+static void mld_update_qri(struct inet6_dev *idev,
+ const struct mld2_query *mlh2)
+{
+ /* RFC3810, relevant sections:
+ * - 5.1.3. Maximum Response Code
+ * - 9.3. Query Response Interval
+ */
+ unsigned long mc_qri, mc_mrc = ntohs(mlh2->mld2q_mrc);
+
+ if (mc_mrc < 32768) {
+ mc_qri = mc_mrc;
+ } else {
+ unsigned long mc_man, mc_exp;
+
+ mc_exp = MLDV2_MRC_EXP(mc_mrc);
+ mc_man = MLDV2_MRC_MAN(mc_mrc);
+
+ mc_qri = (mc_man | 0x1000) << (mc_exp + 3);
+ }
+
+ idev->mc_qri = msecs_to_jiffies(mc_qri);
+}
+
/* called with rcu_read_lock() */
int igmp6_event_query(struct sk_buff *skb)
{
@@ -1150,12 +1241,15 @@ int igmp6_event_query(struct sk_buff *skb)
return -EINVAL;
if (len == MLD_V1_QUERY_LEN) {
- int switchback;
/* MLDv1 router present */
-
max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
- switchback = (idev->mc_qrv + 1) * max_delay;
- idev->mc_v1_seen = jiffies + switchback;
+
+ mld_set_v1_mode(idev);
+
+ /* cancel MLDv2 report timer */
+ idev->mc_gq_running = 0;
+ if (del_timer(&idev->mc_gq_timer))
+ __in6_dev_put(idev);
/* cancel the interface change timer */
idev->mc_ifc_count = 0;
@@ -1166,6 +1260,10 @@ int igmp6_event_query(struct sk_buff *skb)
} else if (len >= MLD_V2_QUERY_LEN_MIN) {
int srcs_offset = sizeof(struct mld2_query) -
sizeof(struct icmp6hdr);
+
+ /* hosts need to stay in MLDv1 mode, discard MLDv2 queries */
+ if (MLD_V1_SEEN(idev))
+ return 0;
if (!pskb_may_pull(skb, srcs_offset))
return -EINVAL;
@@ -1175,8 +1273,10 @@ int igmp6_event_query(struct sk_buff *skb)
idev->mc_maxdelay = max_delay;
- if (mlh2->mld2q_qrv)
- idev->mc_qrv = mlh2->mld2q_qrv;
+ mld_update_qrv(idev, mlh2);
+ mld_update_qi(idev, mlh2);
+ mld_update_qri(idev, mlh2);
+
if (group_type == IPV6_ADDR_ANY) { /* general query */
if (mlh2->mld2q_nsrcs)
return -EINVAL; /* no sources allowed */
@@ -2337,7 +2437,11 @@ void ipv6_mc_init_dev(struct inet6_dev *idev)
(unsigned long)idev);
setup_timer(&idev->mc_dad_timer, mld_dad_timer_expire,
(unsigned long)idev);
+
idev->mc_qrv = MLD_QRV_DEFAULT;
+ idev->mc_qi = MLD_QI_DEFAULT;
+ idev->mc_qri = MLD_QRI_DEFAULT;
+
idev->mc_maxdelay = unsolicited_report_interval(idev);
idev->mc_v1_seen = 0;
write_unlock_bh(&idev->lock);
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 3/8] net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation
From: Daniel Borkmann @ 2013-09-03 7:59 UTC (permalink / raw)
To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>
Get rid of MLDV2_MRC and use our new macros for mantisse and
exponent to calculate Maximum Response Delay out of the Maximum
Response Code.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
This will give a merge conflict in net/bridge/br_multicast.c with net tree. You can
resolve it by simply taking this version as we already do msecs_to_jiffies() here.
include/net/mld.h | 28 +++++++++++++++++++---------
net/bridge/br_multicast.c | 3 ++-
net/ipv6/mcast.c | 18 ++----------------
3 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/include/net/mld.h b/include/net/mld.h
index 2b5421f..faa1d16 100644
--- a/include/net/mld.h
+++ b/include/net/mld.h
@@ -63,15 +63,6 @@ struct mld2_query {
#define mld2q_mrc mld2q_hdr.icmp6_maxdelay
#define mld2q_resv1 mld2q_hdr.icmp6_dataun.un_data16[1]
-/* Max Response Code, TODO: transform this to use the below */
-#define MLDV2_MASK(value, nb) ((nb)>=32 ? (value) : ((1<<(nb))-1) & (value))
-#define MLDV2_EXP(thresh, nbmant, nbexp, value) \
- ((value) < (thresh) ? (value) : \
- ((MLDV2_MASK(value, nbmant) | (1<<(nbmant))) << \
- (MLDV2_MASK((value) >> (nbmant), nbexp) + (nbexp))))
-
-#define MLDV2_MRC(value) MLDV2_EXP(0x8000, 12, 3, value)
-
/* RFC3810, 5.1.3. Maximum Response Code:
*
* If Maximum Response Code >= 32768, Maximum Response Code represents a
@@ -97,4 +88,23 @@ struct mld2_query {
#define MLDV2_QQIC_EXP(value) (((value) >> 4) & 0x07)
#define MLDV2_QQIC_MAN(value) ((value) & 0x0f)
+static inline unsigned long mldv2_mrc(const struct mld2_query *mlh2)
+{
+ /* RFC3810, 5.1.3. Maximum Response Code */
+ unsigned long ret, mc_mrc = ntohs(mlh2->mld2q_mrc);
+
+ if (mc_mrc < 32768) {
+ ret = mc_mrc;
+ } else {
+ unsigned long mc_man, mc_exp;
+
+ mc_exp = MLDV2_MRC_EXP(mc_mrc);
+ mc_man = MLDV2_MRC_MAN(mc_mrc);
+
+ ret = (mc_man | 0x1000) << (mc_exp + 3);
+ }
+
+ return ret;
+}
+
#endif
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 08e576a..4accd0d 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1203,7 +1203,8 @@ static int br_ip6_multicast_query(struct net_bridge *br,
mld2q = (struct mld2_query *)icmp6_hdr(skb);
if (!mld2q->mld2q_nsrcs)
group = &mld2q->mld2q_mca;
- max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(ntohs(mld2q->mld2q_mrc)) : 1;
+
+ max_delay = max(msecs_to_jiffies(mldv2_mrc(mld2q)), 1UL);
}
br_multicast_query_received(br, port, !ipv6_addr_any(&ip6h->saddr),
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index f86e26b..005b22f 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1195,20 +1195,7 @@ static void mld_update_qri(struct inet6_dev *idev,
* - 5.1.3. Maximum Response Code
* - 9.3. Query Response Interval
*/
- unsigned long mc_qri, mc_mrc = ntohs(mlh2->mld2q_mrc);
-
- if (mc_mrc < 32768) {
- mc_qri = mc_mrc;
- } else {
- unsigned long mc_man, mc_exp;
-
- mc_exp = MLDV2_MRC_EXP(mc_mrc);
- mc_man = MLDV2_MRC_MAN(mc_mrc);
-
- mc_qri = (mc_man | 0x1000) << (mc_exp + 3);
- }
-
- idev->mc_qri = msecs_to_jiffies(mc_qri);
+ idev->mc_qri = msecs_to_jiffies(mldv2_mrc(mlh2));
}
/* called with rcu_read_lock() */
@@ -1277,8 +1264,7 @@ int igmp6_event_query(struct sk_buff *skb)
mlh2 = (struct mld2_query *)skb_transport_header(skb);
- max_delay = max(msecs_to_jiffies(MLDV2_MRC(ntohs(mlh2->mld2q_mrc))), 1UL);
-
+ max_delay = max(msecs_to_jiffies(mldv2_mrc(mlh2)), 1UL);
idev->mc_maxdelay = max_delay;
mld_update_qrv(idev, mlh2);
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
From: Daniel Borkmann @ 2013-09-03 7:59 UTC (permalink / raw)
To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>
RFC3810, 10. Security Considerations says under subsection 10.1.
Query Message:
A forged Version 1 Query message will put MLDv2 listeners on that
link in MLDv1 Host Compatibility Mode. This scenario can be avoided
by providing MLDv2 hosts with a configuration option to ignore
Version 1 messages completely.
Hence, implement a MLDv2-only mode that will ignore MLDv1 traffic:
echo 2 > /proc/sys/net/ipv6/conf/ethX/force_mld_version
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/mcast.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 005b22f..02cd0c5 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1112,9 +1112,21 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
return true;
}
+static bool mld_in_v2_mode_only(const struct inet6_dev *idev)
+{
+ return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 2;
+}
+
+static bool mld_in_v1_mode_only(const struct inet6_dev *idev)
+{
+ return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1;
+}
+
static bool mld_in_v1_mode(const struct inet6_dev *idev)
{
- if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1)
+ if (mld_in_v2_mode_only(idev))
+ return false;
+ if (mld_in_v1_mode_only(idev))
return true;
if (idev->cnf.force_mld_version == 1)
return true;
@@ -1223,7 +1235,6 @@ int igmp6_event_query(struct sk_buff *skb)
return -EINVAL;
idev = __in6_dev_get(skb->dev);
-
if (idev == NULL)
return 0;
@@ -1236,6 +1247,10 @@ int igmp6_event_query(struct sk_buff *skb)
return -EINVAL;
if (len == MLD_V1_QUERY_LEN) {
+ /* Ignore v1 queries */
+ if (mld_in_v2_mode_only(idev))
+ return 0;
+
/* MLDv1 router present */
max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 5/8] net: ipv6: mld: similarly to MLDv2 have min max_delay of 1
From: Daniel Borkmann @ 2013-09-03 7:59 UTC (permalink / raw)
To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>
Similarly as we do in MLDv2 queries, set a forged MLDv1 query with
0 ms mld_maxdelay to minimum timer shot time of 1 jiffies. This is
eventually done in igmp6_group_queried() anyway, so we can simplify
a check there.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/mcast.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 02cd0c5..75a4324 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1040,12 +1040,9 @@ static void igmp6_group_queried(struct ifmcaddr6 *ma, unsigned long resptime)
delay = ma->mca_timer.expires - jiffies;
}
- if (delay >= resptime) {
- if (resptime)
- delay = net_random() % resptime;
- else
- delay = 1;
- }
+ if (delay >= resptime)
+ delay = net_random() % resptime;
+
ma->mca_timer.expires = jiffies + delay;
if (!mod_timer(&ma->mca_timer, jiffies + delay))
atomic_inc(&ma->mca_refcnt);
@@ -1247,12 +1244,15 @@ int igmp6_event_query(struct sk_buff *skb)
return -EINVAL;
if (len == MLD_V1_QUERY_LEN) {
+ unsigned long mldv1_md;
+
/* Ignore v1 queries */
if (mld_in_v2_mode_only(idev))
return 0;
/* MLDv1 router present */
- max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
+ mldv1_md = ntohs(mld->mld_maxdelay);
+ max_delay = max(msecs_to_jiffies(mldv1_md), 1UL);
mld_set_v1_mode(idev);
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 6/8] net: ipv6: mld: refactor query processing into v1/v2 functions
From: Daniel Borkmann @ 2013-09-03 7:59 UTC (permalink / raw)
To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>
Make igmp6_event_query() a bit easier to read by refactoring code
parts into mld_process_v1() and mld_process_v2().
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/mcast.c | 89 +++++++++++++++++++++++++++++++++++---------------------
1 file changed, 56 insertions(+), 33 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 75a4324..b01aa32 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1207,6 +1207,55 @@ static void mld_update_qri(struct inet6_dev *idev,
idev->mc_qri = msecs_to_jiffies(mldv2_mrc(mlh2));
}
+static int mld_process_v1(struct inet6_dev *idev, struct mld_msg *mld,
+ unsigned long *max_delay)
+{
+ unsigned long mldv1_md;
+
+ /* Ignore v1 queries */
+ if (mld_in_v2_mode_only(idev))
+ return -EINVAL;
+
+ /* MLDv1 router present */
+ mldv1_md = ntohs(mld->mld_maxdelay);
+ *max_delay = max(msecs_to_jiffies(mldv1_md), 1UL);
+
+ mld_set_v1_mode(idev);
+
+ /* cancel MLDv2 report timer */
+ idev->mc_gq_running = 0;
+ if (del_timer(&idev->mc_gq_timer))
+ __in6_dev_put(idev);
+
+ /* cancel the interface change timer */
+ idev->mc_ifc_count = 0;
+ if (del_timer(&idev->mc_ifc_timer))
+ __in6_dev_put(idev);
+
+ /* clear deleted report items */
+ mld_clear_delrec(idev);
+
+ return 0;
+}
+
+static int mld_process_v2(struct inet6_dev *idev, struct mld2_query *mld,
+ unsigned long *max_delay)
+{
+ /* hosts need to stay in MLDv1 mode, discard MLDv2 queries */
+ if (mld_in_v1_mode(idev))
+ return -EINVAL;
+
+ *max_delay = max(msecs_to_jiffies(mldv2_mrc(mld)), 1UL);
+
+ mld_update_qrv(idev, mld);
+ mld_update_qi(idev, mld);
+ mld_update_qri(idev, mld);
+
+ idev->mc_maxdelay = *max_delay;
+
+ return 0;
+}
+
/* called with rcu_read_lock() */
int igmp6_event_query(struct sk_buff *skb)
{
@@ -1218,7 +1267,7 @@ int igmp6_event_query(struct sk_buff *skb)
struct mld_msg *mld;
int group_type;
int mark = 0;
- int len;
+ int len, err;
if (!pskb_may_pull(skb, sizeof(struct in6_addr)))
return -EINVAL;
@@ -1244,47 +1293,21 @@ int igmp6_event_query(struct sk_buff *skb)
return -EINVAL;
if (len == MLD_V1_QUERY_LEN) {
- unsigned long mldv1_md;
-
- /* Ignore v1 queries */
- if (mld_in_v2_mode_only(idev))
- return 0;
-
- /* MLDv1 router present */
- mldv1_md = ntohs(mld->mld_maxdelay);
- max_delay = max(msecs_to_jiffies(mldv1_md), 1UL);
-
- mld_set_v1_mode(idev);
-
- /* cancel MLDv2 report timer */
- idev->mc_gq_running = 0;
- if (del_timer(&idev->mc_gq_timer))
- __in6_dev_put(idev);
-
- /* cancel the interface change timer */
- idev->mc_ifc_count = 0;
- if (del_timer(&idev->mc_ifc_timer))
- __in6_dev_put(idev);
- /* clear deleted report items */
- mld_clear_delrec(idev);
+ err = mld_process_v1(idev, mld, &max_delay);
+ if (err < 0)
+ return err;
} else if (len >= MLD_V2_QUERY_LEN_MIN) {
int srcs_offset = sizeof(struct mld2_query) -
sizeof(struct icmp6hdr);
- /* hosts need to stay in MLDv1 mode, discard MLDv2 queries */
- if (mld_in_v1_mode(idev))
- return 0;
if (!pskb_may_pull(skb, srcs_offset))
return -EINVAL;
mlh2 = (struct mld2_query *)skb_transport_header(skb);
- max_delay = max(msecs_to_jiffies(mldv2_mrc(mlh2)), 1UL);
- idev->mc_maxdelay = max_delay;
-
- mld_update_qrv(idev, mlh2);
- mld_update_qi(idev, mlh2);
- mld_update_qri(idev, mlh2);
+ err = mld_process_v2(idev, mlh2, &max_delay);
+ if (err < 0)
+ return err;
if (group_type == IPV6_ADDR_ANY) { /* general query */
if (mlh2->mld2q_nsrcs)
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 7/8] net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions
From: Daniel Borkmann @ 2013-09-03 7:59 UTC (permalink / raw)
To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>
We already have mld_{gq,ifc,dad}_start_timer() functions, so introduce
mld_{gq,ifc,dad}_stop_timer() functions to reduce code size and make it
more readable.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/mcast.c | 41 +++++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index b01aa32..a43028d 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1006,6 +1006,13 @@ static void mld_gq_start_timer(struct inet6_dev *idev)
in6_dev_hold(idev);
}
+static void mld_gq_stop_timer(struct inet6_dev *idev)
+{
+ idev->mc_gq_running = 0;
+ if (del_timer(&idev->mc_gq_timer))
+ __in6_dev_put(idev);
+}
+
static void mld_ifc_start_timer(struct inet6_dev *idev, unsigned long delay)
{
unsigned long tv = net_random() % delay;
@@ -1014,6 +1021,13 @@ static void mld_ifc_start_timer(struct inet6_dev *idev, unsigned long delay)
in6_dev_hold(idev);
}
+static void mld_ifc_stop_timer(struct inet6_dev *idev)
+{
+ idev->mc_ifc_count = 0;
+ if (del_timer(&idev->mc_ifc_timer))
+ __in6_dev_put(idev);
+}
+
static void mld_dad_start_timer(struct inet6_dev *idev, unsigned long delay)
{
unsigned long tv = net_random() % delay;
@@ -1022,6 +1036,12 @@ static void mld_dad_start_timer(struct inet6_dev *idev, unsigned long delay)
in6_dev_hold(idev);
}
+static void mld_dad_stop_timer(struct inet6_dev *idev)
+{
+ if (del_timer(&idev->mc_dad_timer))
+ __in6_dev_put(idev);
+}
+
/*
* IGMP handling (alias multicast ICMPv6 messages)
*/
@@ -1223,15 +1243,9 @@ static int mld_process_v1(struct inet6_dev *idev, struct mld_msg *mld,
mld_set_v1_mode(idev);
/* cancel MLDv2 report timer */
- idev->mc_gq_running = 0;
- if (del_timer(&idev->mc_gq_timer))
- __in6_dev_put(idev);
-
+ mld_gq_stop_timer(idev);
/* cancel the interface change timer */
- idev->mc_ifc_count = 0;
- if (del_timer(&idev->mc_ifc_timer))
- __in6_dev_put(idev);
-
+ mld_ifc_stop_timer(idev);
/* clear deleted report items */
mld_clear_delrec(idev);
@@ -2423,14 +2437,9 @@ void ipv6_mc_down(struct inet6_dev *idev)
/* Withdraw multicast list */
read_lock_bh(&idev->lock);
- idev->mc_ifc_count = 0;
- if (del_timer(&idev->mc_ifc_timer))
- __in6_dev_put(idev);
- idev->mc_gq_running = 0;
- if (del_timer(&idev->mc_gq_timer))
- __in6_dev_put(idev);
- if (del_timer(&idev->mc_dad_timer))
- __in6_dev_put(idev);
+ mld_ifc_stop_timer(idev);
+ mld_gq_stop_timer(idev);
+ mld_dad_stop_timer(idev);
for (i = idev->mc_list; i; i=i->next)
igmp6_group_dropped(i);
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 8/8] net: ipv6: mld: document force_mld_version in ip-sysctl.txt
From: Daniel Borkmann @ 2013-09-03 7:59 UTC (permalink / raw)
To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378195178-21002-1-git-send-email-dborkman@redhat.com>
Document force_mld_version parameter in ip-sysctl.txt.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
Documentation/networking/ip-sysctl.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 1cb3aeb..a46d785 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1358,6 +1358,11 @@ mldv2_unsolicited_report_interval - INTEGER
MLDv2 report retransmit will take place.
Default: 1000 (1 second)
+force_mld_version - INTEGER
+ 0 - (default) No enforcement of a MLD version, MLDv1 fallback allowed
+ 1 - Enforce to use MLD version 1
+ 2 - Enforce to use MLD version 2
+
suppress_frag_ndisc - INTEGER
Control RFC 6980 (Security Implications of IPv6 Fragmentation
with IPv6 Neighbor Discovery) behavior:
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH] ipv6: fix null pointer dereference in __ip6addrlbl_add
From: Michele Baldessari @ 2013-09-03 8:04 UTC (permalink / raw)
To: netdev, Hideaki YOSHIFUJI, David S. Miller, Sasha Levin
In-Reply-To: <20130903001331.GA8820@order.stressinduktion.org>
Hi Hannes,
On Tue, Sep 03, 2013 at 02:13:31AM +0200, Hannes Frederic Sowa wrote:
> On Mon, Sep 02, 2013 at 10:31:28PM +0100, Michele Baldessari wrote:
> > with the latest linux master git tree from Linus
> > (248d296d6d9df384996c2ed95676b367d876d48c - 2 Sep) I can reproduceably oops
> > the kernel with the following commands:
> > ip addrlabel flush
> > ip addrlabel add prefix ::1/128 label 0
> > ip addrlabel add prefix ::/0 label 1
>
> Thanks for the report! This patch should fix this issue:
>
> [PATCH] ipv6: fix null pointer dereference in __ip6addrlbl_add
>
> Commit b67bfe0d42cac56c512dd5da4b1b347a23f4b70a ("hlist: drop
> the node parameter from iterators") changed the behavior of
> hlist_for_each_entry_safe to leave the p argument NULL.
>
> Fix this up by tracking the last argument.
>
> Reported-by: Michele Baldessari <michele@acksyn.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Thanks for the patch, fixes it for me.
Tested-by: Michele Baldessari <michele@acksyn.org>
^ permalink raw reply
* Re: [PATCH RFC net-next 06/21] net: create sysfs symlinks for neighbour devices
From: Veaceslav Falico @ 2013-09-03 8:05 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller,
Eric Dumazet, Alexander Duyck
In-Reply-To: <20130903074853.GB1437@minipsycho.brq.redhat.com>
On Tue, Sep 03, 2013 at 09:48:53AM +0200, Jiri Pirko wrote:
>Mon, Sep 02, 2013 at 11:39:10PM CEST, vfalico@redhat.com wrote:
>>Also, remove the same functionality from bonding - it will be already done
>>for any device that links to its lower/upper neighbour.
>>
>>The links will be created for dev's kobject, and will look like
>>slave_eth0 for lower device eth0 and upper_bridge0 for upper device
>>bridge0.
>
>The pair should be either slave/master or lower/upper. It's undesirable
>to mix these two. Maybe for compatibility reasons we should leave
>current bonding sysfs files and create proper new generic ones?
I completely agree with the slave/master vs lower/upper logic, and you've
correctly understood why I've chosen this path.
I'm ok with any approach, but it looks weird. I've googled a bit and saw
that people use the slave_eth0...
So dunno what's best option here, really.
>
>I think there should be directories under netdev dir like:
>/sys/class/net/em1-
> |-upper-
> |-bond0(link)
> |-vlanx(link)
>/sys/class/net/bond0-
> |-lower-
> |-em1(link)
> |-em2(link)
>
>
>>
>>CC: Jay Vosburgh <fubar@us.ibm.com>
>>CC: Andy Gospodarek <andy@greyhouse.net>
>>CC: "David S. Miller" <davem@davemloft.net>
>>CC: Eric Dumazet <edumazet@google.com>
>>CC: Jiri Pirko <jiri@resnulli.us>
>>CC: Alexander Duyck <alexander.h.duyck@intel.com>
>>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>>---
>> drivers/net/bonding/bond_main.c | 11 +----------
>> drivers/net/bonding/bond_sysfs.c | 25 -------------------------
>> drivers/net/bonding/bonding.h | 2 --
>> net/core/dev.c | 29 +++++++++++++++++++++++++++++
>> 4 files changed, 30 insertions(+), 37 deletions(-)
>>
>>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>index c50679f..b0b753d 100644
>>--- a/drivers/net/bonding/bond_main.c
>>+++ b/drivers/net/bonding/bond_main.c
>>@@ -1635,15 +1635,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>>
>> read_unlock(&bond->lock);
>>
>>- res = bond_create_slave_symlinks(bond_dev, slave_dev);
>>- if (res)
>>- goto err_detach;
>>-
>> res = netdev_rx_handler_register(slave_dev, bond_handle_frame,
>> new_slave);
>> if (res) {
>> pr_debug("Error %d calling netdev_rx_handler_register\n", res);
>>- goto err_dest_symlinks;
>>+ goto err_detach;
>> }
>>
>> pr_info("%s: enslaving %s as a%s interface with a%s link.\n",
>>@@ -1655,9 +1651,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>> return 0;
>>
>> /* Undo stages on error */
>>-err_dest_symlinks:
>>- bond_destroy_slave_symlinks(bond_dev, slave_dev);
>>-
>> err_detach:
>> if (!USES_PRIMARY(bond->params.mode))
>> bond_hw_addr_flush(bond_dev, slave_dev);
>>@@ -1856,8 +1849,6 @@ static int __bond_release_one(struct net_device *bond_dev,
>> bond_dev->name, slave_dev->name, bond_dev->name);
>>
>> /* must do this from outside any spinlocks */
>>- bond_destroy_slave_symlinks(bond_dev, slave_dev);
>>-
>> vlan_vids_del_by_dev(slave_dev, bond_dev);
>>
>> /* If the mode USES_PRIMARY, then this cases was handled above by
>>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>index 5715277..7b962a2 100644
>>--- a/drivers/net/bonding/bond_sysfs.c
>>+++ b/drivers/net/bonding/bond_sysfs.c
>>@@ -168,31 +168,6 @@ static const struct class_attribute class_attr_bonding_masters = {
>> .namespace = bonding_namespace,
>> };
>>
>>-int bond_create_slave_symlinks(struct net_device *master,
>>- struct net_device *slave)
>>-{
>>- char linkname[IFNAMSIZ+7];
>>- int ret = 0;
>>-
>>- /* create a link from the master to the slave */
>>- sprintf(linkname, "slave_%s", slave->name);
>>- ret = sysfs_create_link(&(master->dev.kobj), &(slave->dev.kobj),
>>- linkname);
>>-
>>- return ret;
>>-
>>-}
>>-
>>-void bond_destroy_slave_symlinks(struct net_device *master,
>>- struct net_device *slave)
>>-{
>>- char linkname[IFNAMSIZ+7];
>>-
>>- sprintf(linkname, "slave_%s", slave->name);
>>- sysfs_remove_link(&(master->dev.kobj), linkname);
>>-}
>>-
>>-
>> /*
>> * Show the slaves in the current bond.
>> */
>>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>>index 4abc925..f7a167d 100644
>>--- a/drivers/net/bonding/bonding.h
>>+++ b/drivers/net/bonding/bonding.h
>>@@ -455,8 +455,6 @@ int bond_create(struct net *net, const char *name);
>> int bond_create_sysfs(struct bond_net *net);
>> void bond_destroy_sysfs(struct bond_net *net);
>> void bond_prepare_sysfs_group(struct bonding *bond);
>>-int bond_create_slave_symlinks(struct net_device *master, struct net_device *slave);
>>-void bond_destroy_slave_symlinks(struct net_device *master, struct net_device *slave);
>> int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev);
>> int bond_release(struct net_device *bond_dev, struct net_device *slave_dev);
>> void bond_mii_monitor(struct work_struct *);
>>diff --git a/net/core/dev.c b/net/core/dev.c
>>index 7eecf35..1c3e98d 100644
>>--- a/net/core/dev.c
>>+++ b/net/core/dev.c
>>@@ -4580,6 +4580,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
>> bool upper, void *private)
>> {
>> struct netdev_adjacent *adj, *neigh = NULL;
>>+ char linkname[IFNAMSIZ+7];
>> int ret;
>>
>> adj = __netdev_find_adj(dev, adj_dev, upper, false);
>>@@ -4628,6 +4629,16 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
>> */
>> if (master)
>> neigh->private = private;
>>+
>>+ sprintf(linkname, "slave_%s", adj_dev->name);
>>+ ret = sysfs_create_link(&(dev->dev.kobj),
>>+ &(adj_dev->dev.kobj),
>>+ linkname);
>>+ if (ret) {
>>+ kfree(neigh);
>>+ kfree(adj);
>>+ return ret;
>>+ }
>> list_add_tail_rcu(&neigh->list,
>> &dev->neighbour_dev_list.lower);
>> }
>>@@ -4635,10 +4646,24 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
>> return 0;
>> }
>>
>>+ /* it's upper neighbour, we don't care if it's master or not */
>>+ if (neigh) {
>>+ sprintf(linkname, "upper_%s", adj_dev->name);
>>+ ret = sysfs_create_link(&(dev->dev.kobj), &(adj_dev->dev.kobj),
>>+ linkname);
>>+ if (ret) {
>>+ kfree(neigh);
>>+ kfree(adj);
>>+ return ret;
>>+ }
>>+ }
>>+
>> /* Ensure that master upper link is always the first item in list. */
>> if (master) {
>> ret = sysfs_create_link(&(dev->dev.kobj), &(adj_dev->dev.kobj), "master");
>> if (ret) {
>>+ if (neigh)
>>+ sysfs_remove_link(&(dev->dev.kobj), linkname);
>> kfree(neigh);
>> kfree(adj);
>> return ret;
>>@@ -4678,6 +4703,7 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
>> struct net_device *adj_dev, bool upper)
>> {
>> struct netdev_adjacent *adj, *neighbour;
>>+ char linkname[IFNAMSIZ+7];
>>
>> if (upper) {
>> adj = __netdev_find_upper(dev, adj_dev);
>>@@ -4725,6 +4751,9 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
>> list_del_rcu(&neighbour->list);
>> if (neighbour->master && upper)
>> sysfs_remove_link(&(dev->dev.kobj), "master");
>>+ sprintf(linkname, "%s_%s", upper ? "upper" : "slave",
>>+ adj_dev->name);
>>+ sysfs_remove_link(&(dev->dev.kobj), linkname);
>> dev_put(adj_dev);
>> kfree_rcu(neighbour, rcu);
>> }
>>--
>>1.8.4
>>
^ permalink raw reply
* Re: [PATCH RFC net-next 18/21] net: add a function to get the next/prev private
From: Jiri Pirko @ 2013-09-03 8:10 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: netdev, David S. Miller, Eric Dumazet, Alexander Duyck
In-Reply-To: <1378157965-17537-19-git-send-email-vfalico@redhat.com>
Mon, Sep 02, 2013 at 11:39:22PM CEST, vfalico@redhat.com wrote:
>The boolean flag specifies which direction to go.
>
>CC: "David S. Miller" <davem@davemloft.net>
>CC: Eric Dumazet <edumazet@google.com>
>CC: Jiri Pirko <jiri@resnulli.us>
>CC: Alexander Duyck <alexander.h.duyck@intel.com>
>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>---
> include/linux/netdevice.h | 2 ++
> net/core/dev.c | 24 ++++++++++++++++++++++++
> 2 files changed, 26 insertions(+)
>
>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>index bde2244..a112ccc 100644
>--- a/include/linux/netdevice.h
>+++ b/include/linux/netdevice.h
>@@ -2835,6 +2835,8 @@ extern void *netdev_lower_dev_get_private_rcu(struct net_device *dev,
> struct net_device *lower_dev);
> extern void *netdev_lower_dev_get_private(struct net_device *dev,
> struct net_device *lower_dev);
>+extern void *netdev_lower_dev_get_next_private(struct net_device *dev,
>+ void *private, bool prev);
> extern int skb_checksum_help(struct sk_buff *skb);
> extern struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
> netdev_features_t features, bool tx_path);
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 6df11a0..c694059 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -5163,6 +5163,30 @@ void *netdev_lower_dev_get_private(struct net_device *dev,
> }
> EXPORT_SYMBOL(netdev_lower_dev_get_private);
>
>+extern void *netdev_lower_dev_get_next_private(struct net_device *dev,
>+ void *private, bool prev)
^^^^^^^^^^^
Please rather do this in 2 functions
Next should return next, not previous...
>+{
>+ struct netdev_adjacent *lower;
>+
>+ list_for_each_entry(lower, &dev->neighbour_dev_list.lower, list)
>+ if (lower->private == private)
>+ break;
>+
>+ if (&lower->list == &dev->neighbour_dev_list.lower)
>+ return NULL;
>+
>+ lower = prev ? list_entry(lower->list.prev, struct netdev_adjacent, list) :
>+ list_entry(lower->list.next, struct netdev_adjacent, list);
>+ if (&lower->list != &dev->neighbour_dev_list.lower)
>+ return lower->private;
>+
>+ lower = prev ? list_entry(lower->list.prev, struct netdev_adjacent, list) :
>+ list_entry(lower->list.next, struct netdev_adjacent, list);
>+
>+ return lower->private;
>+}
>+EXPORT_SYMBOL(netdev_lower_dev_get_next_private);
>+
> static void dev_change_rx_flags(struct net_device *dev, int flags)
> {
> const struct net_device_ops *ops = dev->netdev_ops;
>--
>1.8.4
>
^ permalink raw reply
* Re: [PATCH RFC net-next 01/21] net: add neighbour_dev_list to save only neighbours
From: Jiri Pirko @ 2013-09-03 8:29 UTC (permalink / raw)
To: Veaceslav Falico
Cc: netdev, David S. Miller, Eric Dumazet, Alexander Duyck, Cong Wang
In-Reply-To: <1378157965-17537-2-git-send-email-vfalico@redhat.com>
Mon, Sep 02, 2013 at 11:39:05PM CEST, vfalico@redhat.com wrote:
>Currently, we distinguish neighbours (first-level linked devices) from
>non-neighbours by the neighbour bool in the netdev_adjacent. This could be
>quite time-consuming in case we would like to traverse *only* through
>neighbours - cause we'd have to traverse through all devices and check for
>this flag, and in a (quite common) scenario where we have lots of vlans on
>top of bridge, which is on top of a bond - the bonding would have to go
>through all those vlans to get its upper neighbour linked devices.
>
>This situation is really unpleasant, cause there are already a lot of cases
>when a device with slaves needs to go through them in hot path.
>
>To fix this, introduce a new upper/lower device lists structure -
>neighbour_dev_list, which contains only the neighbours. It works always in
>pair with the all_device_list structure (renamed from upper/lower_dev_list),
>i.e. both of them contain the same links, only that all_dev_list contains
>also non-neighbour device links. It's really a small change visible,
>currently, only for __netdev_adjacent_dev_insert/remove(), and doesn't
>change the main linked logic at all.
>
>Also, add some comments a fix a name collision in
>netdev_for_each_upper_dev_rcu().
>
>CC: "David S. Miller" <davem@davemloft.net>
>CC: Eric Dumazet <edumazet@google.com>
>CC: Jiri Pirko <jiri@resnulli.us>
>CC: Alexander Duyck <alexander.h.duyck@intel.com>
>CC: Cong Wang <amwang@redhat.com>
>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>---
> include/linux/netdevice.h | 24 ++++---
> net/core/dev.c | 157 ++++++++++++++++++++++++++++++++++------------
> 2 files changed, 134 insertions(+), 47 deletions(-)
>
>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>index 3ad49b8..df50548 100644
>--- a/include/linux/netdevice.h
>+++ b/include/linux/netdevice.h
>@@ -1124,8 +1124,18 @@ struct net_device {
> struct list_head dev_list;
> struct list_head napi_list;
> struct list_head unreg_list;
>- struct list_head upper_dev_list; /* List of upper devices */
>- struct list_head lower_dev_list;
>+
>+ /* directly linked devices, like slaves for bonding */
>+ struct {
>+ struct list_head upper;
>+ struct list_head lower;
>+ } neighbour_dev_list ;
>+
>+ /* all linked devices, *including* neighbours */
>+ struct {
>+ struct list_head upper;
>+ struct list_head lower;
>+ } all_dev_list ;
I think there is need for some naming consistency for functions and
macros handling these lists.
I propose:
dev_list (drop the "neighbour") and all_dev_list
and exported functions and macros called like:
netdev_lower_get_next_priv()
netdev_lower_for_each_priv()
netdev_all_upper_get_next_rcu()
etc...
>
>
> /* currently active device features */
>@@ -2772,11 +2782,11 @@ extern struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
> struct list_head **iter);
>
> /* iterate through upper list, must be called under RCU read lock */
>-#define netdev_for_each_upper_dev_rcu(dev, upper, iter) \
>- for (iter = &(dev)->upper_dev_list, \
>- upper = netdev_upper_get_next_dev_rcu(dev, &(iter)); \
>- upper; \
>- upper = netdev_upper_get_next_dev_rcu(dev, &(iter)))
>+#define netdev_for_each_upper_dev_rcu(dev, updev, iter) \
>+ for (iter = &(dev)->all_dev_list.upper, \
>+ updev= netdev_upper_get_next_dev_rcu(dev, &(iter)); \
>+ updev; \
>+ updev = netdev_upper_get_next_dev_rcu(dev, &(iter)))
>
> extern struct net_device *netdev_master_upper_dev_get(struct net_device *dev);
> extern struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev);
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 743620e..9e4eb40 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -4373,9 +4373,6 @@ struct netdev_adjacent {
> /* upper master flag, there can only be one master device per list */
> bool master;
>
>- /* indicates that this dev is our first-level lower/upper device */
>- bool neighbour;
>-
> /* counter for the number of times this device was added to us */
> u16 ref_nr;
>
>@@ -4385,12 +4382,17 @@ struct netdev_adjacent {
>
> static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
> struct net_device *adj_dev,
>- bool upper)
>+ bool upper, bool neighbour)
> {
> struct netdev_adjacent *adj;
> struct list_head *dev_list;
>
>- dev_list = upper ? &dev->upper_dev_list : &dev->lower_dev_list;
>+ if (neighbour)
>+ dev_list = upper ? &dev->neighbour_dev_list.upper :
>+ &dev->neighbour_dev_list.lower;
>+ else
>+ dev_list = upper ? &dev->all_dev_list.upper :
>+ &dev->all_dev_list.lower;
>
> list_for_each_entry(adj, dev_list, list) {
> if (adj->dev == adj_dev)
>@@ -4402,13 +4404,25 @@ static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
> static inline struct netdev_adjacent *__netdev_find_upper(struct net_device *dev,
> struct net_device *udev)
> {
>- return __netdev_find_adj(dev, udev, true);
>+ return __netdev_find_adj(dev, udev, true, false);
> }
>
> static inline struct netdev_adjacent *__netdev_find_lower(struct net_device *dev,
> struct net_device *ldev)
> {
>- return __netdev_find_adj(dev, ldev, false);
>+ return __netdev_find_adj(dev, ldev, false, false);
>+}
>+
>+static inline struct netdev_adjacent *__netdev_find_upper_neighbour(struct net_device *dev,
>+ struct net_device *udev)
>+{
>+ return __netdev_find_adj(dev, udev, true, true);
>+}
>+
>+static inline struct netdev_adjacent *__netdev_find_lower_neighbour(struct net_device *dev,
>+ struct net_device *ldev)
>+{
>+ return __netdev_find_adj(dev, ldev, false, true);
> }
>
> /**
>@@ -4440,7 +4454,7 @@ bool netdev_has_any_upper_dev(struct net_device *dev)
> {
> ASSERT_RTNL();
>
>- return !list_empty(&dev->upper_dev_list);
>+ return !list_empty(&dev->all_dev_list.upper);
> }
> EXPORT_SYMBOL(netdev_has_any_upper_dev);
>
>@@ -4457,10 +4471,10 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
>
> ASSERT_RTNL();
>
>- if (list_empty(&dev->upper_dev_list))
>+ if (list_empty(&dev->neighbour_dev_list.upper))
> return NULL;
>
>- upper = list_first_entry(&dev->upper_dev_list,
>+ upper = list_first_entry(&dev->neighbour_dev_list.upper,
> struct netdev_adjacent, list);
> if (likely(upper->master))
> return upper->dev;
>@@ -4484,7 +4498,7 @@ struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
>
> upper = list_entry_rcu((*iter)->next, struct netdev_adjacent, list);
>
>- if (&upper->list == &dev->upper_dev_list)
>+ if (&upper->list == &dev->all_dev_list.upper)
> return NULL;
>
> *iter = &upper->list;
>@@ -4504,7 +4518,7 @@ struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev)
> {
> struct netdev_adjacent *upper;
>
>- upper = list_first_or_null_rcu(&dev->upper_dev_list,
>+ upper = list_first_or_null_rcu(&dev->neighbour_dev_list.upper,
> struct netdev_adjacent, list);
> if (upper && likely(upper->master))
> return upper->dev;
>@@ -4517,11 +4531,12 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
> bool neighbour, bool master,
> bool upper)
> {
>- struct netdev_adjacent *adj;
>+ struct netdev_adjacent *adj, *neigh = NULL;
>
>- adj = __netdev_find_adj(dev, adj_dev, upper);
>+ adj = __netdev_find_adj(dev, adj_dev, upper, false);
>
> if (adj) {
>+ /* we cannot insert a neighbour device twice */
> BUG_ON(neighbour);
> adj->ref_nr++;
> return 0;
>@@ -4533,24 +4548,50 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
>
> adj->dev = adj_dev;
> adj->master = master;
>- adj->neighbour = neighbour;
> adj->ref_nr = 1;
>-
> dev_hold(adj_dev);
>+
>+ if (neighbour) {
>+ neigh = kmalloc(sizeof(*neigh), GFP_KERNEL);
>+ if (!neigh) {
>+ kfree(adj);
>+ return -ENOMEM;
>+ }
>+ neigh->dev = adj_dev;
>+ neigh->master = master;
>+ neigh->ref_nr = 1;
>+ dev_hold(adj_dev);
>+ }
>+
> pr_debug("dev_hold for %s, because of %s link added from %s to %s\n",
> adj_dev->name, upper ? "upper" : "lower", dev->name,
> adj_dev->name);
>+ if (neigh)
>+ pr_debug("dev_hold for %s, because of %s link added from %s"
>+ " to %s (neighbour)\n",
>+ adj_dev->name, upper ? "upper" : "lower", dev->name,
>+ adj_dev->name);
>
> if (!upper) {
>- list_add_tail_rcu(&adj->list, &dev->lower_dev_list);
>+ if (neigh)
>+ list_add_tail_rcu(&neigh->list,
>+ &dev->neighbour_dev_list.lower);
>+ list_add_tail_rcu(&adj->list, &dev->all_dev_list.lower);
> return 0;
> }
>
> /* Ensure that master upper link is always the first item in list. */
>- if (master)
>- list_add_rcu(&adj->list, &dev->upper_dev_list);
>- else
>- list_add_tail_rcu(&adj->list, &dev->upper_dev_list);
>+ if (master) {
>+ if (neigh)
>+ list_add_rcu(&neigh->list,
>+ &dev->neighbour_dev_list.upper);
>+ list_add_rcu(&adj->list, &dev->all_dev_list.upper);
>+ } else {
>+ if (neigh)
>+ list_add_tail_rcu(&neigh->list,
>+ &dev->neighbour_dev_list.upper);
>+ list_add_tail_rcu(&adj->list, &dev->all_dev_list.upper);
>+ }
>
> return 0;
> }
>@@ -4574,17 +4615,36 @@ static inline int __netdev_lower_dev_insert(struct net_device *dev,
> void __netdev_adjacent_dev_remove(struct net_device *dev,
> struct net_device *adj_dev, bool upper)
> {
>- struct netdev_adjacent *adj;
>+ struct netdev_adjacent *adj, *neighbour;
>
>- if (upper)
>+ if (upper) {
> adj = __netdev_find_upper(dev, adj_dev);
>- else
>+ neighbour = __netdev_find_upper_neighbour(dev, adj_dev);
>+ } else {
> adj = __netdev_find_lower(dev, adj_dev);
>+ neighbour = __netdev_find_lower_neighbour(dev, adj_dev);
>+ }
>
>- if (!adj)
>+ if (!adj) {
>+ pr_err("tried to remove %s device %s from %s\n",
>+ upper ? "upper" : "lower", dev->name, adj_dev->name);
> BUG();
>+ }
>
> if (adj->ref_nr > 1) {
>+ pr_debug("rec_cnt-- for link to %s, because of %s link removed"
>+ " from %s to %s, remains %d\n",
>+ adj_dev->name, upper ? "upper" : "lower", dev->name,
>+ adj_dev->name, adj->ref_nr-1);
>+ if (neighbour) {
>+ pr_debug("rec_cnt-- for link to %s, because of %s link"
>+ " removed from %s to %s, remain %d (neigh)\n",
>+ adj_dev->name, upper ? "upper" : "lower",
>+ dev->name, adj_dev->name,
>+ neighbour->ref_nr-1);
>+ BUG_ON(adj->ref_nr != neighbour->ref_nr);
>+ neighbour->ref_nr--;
>+ }
> adj->ref_nr--;
> return;
> }
>@@ -4595,6 +4655,15 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
> adj_dev->name);
> dev_put(adj_dev);
> kfree_rcu(adj, rcu);
>+ if (neighbour) {
>+ pr_debug("dev_put for %s, because of %s link removed from %s"
>+ " to %s (neighbour)\n",
>+ adj_dev->name, upper ? "upper" : "lower", dev->name,
>+ adj_dev->name);
>+ list_del_rcu(&neighbour->list);
>+ dev_put(adj_dev);
>+ kfree_rcu(neighbour, rcu);
>+ }
> }
>
> static inline void __netdev_upper_dev_remove(struct net_device *dev,
>@@ -4675,12 +4744,14 @@ static int __netdev_upper_dev_link(struct net_device *dev,
> return ret;
>
> /* Now that we linked these devs, make all the upper_dev's
>- * upper_dev_list visible to every dev's lower_dev_list and vice
>+ * all_dev_list.upper visible to every dev's all_dev_list.lower an
> * versa, and don't forget the devices itself. All of these
> * links are non-neighbours.
> */
>- list_for_each_entry(i, &dev->lower_dev_list, list) {
>- list_for_each_entry(j, &upper_dev->upper_dev_list, list) {
>+ list_for_each_entry(i, &dev->all_dev_list.lower, list) {
>+ list_for_each_entry(j, &upper_dev->all_dev_list.upper, list) {
>+ pr_debug("Interlinking %s with %s, non-neighbour\n",
>+ i->dev->name, j->dev->name);
> ret = __netdev_adjacent_dev_link(i->dev, j->dev);
> if (ret)
> goto rollback_mesh;
>@@ -4688,14 +4759,18 @@ static int __netdev_upper_dev_link(struct net_device *dev,
> }
>
> /* add dev to every upper_dev's upper device */
>- list_for_each_entry(i, &upper_dev->upper_dev_list, list) {
>+ list_for_each_entry(i, &upper_dev->all_dev_list.upper, list) {
>+ pr_debug("linking %s's upper device %s with %s\n", upper_dev->name,
>+ i->dev->name, dev->name);
> ret = __netdev_adjacent_dev_link(dev, i->dev);
> if (ret)
> goto rollback_upper_mesh;
> }
>
> /* add upper_dev to every dev's lower device */
>- list_for_each_entry(i, &dev->lower_dev_list, list) {
>+ list_for_each_entry(i, &dev->all_dev_list.lower, list) {
>+ pr_debug("linking %s's lower device %s with %s\n", dev->name,
>+ i->dev->name, upper_dev->name);
> ret = __netdev_adjacent_dev_link(i->dev, upper_dev);
> if (ret)
> goto rollback_lower_mesh;
>@@ -4706,7 +4781,7 @@ static int __netdev_upper_dev_link(struct net_device *dev,
>
> rollback_lower_mesh:
> to_i = i;
>- list_for_each_entry(i, &dev->lower_dev_list, list) {
>+ list_for_each_entry(i, &dev->all_dev_list.lower, list) {
> if (i == to_i)
> break;
> __netdev_adjacent_dev_unlink(i->dev, upper_dev);
>@@ -4716,7 +4791,7 @@ rollback_lower_mesh:
>
> rollback_upper_mesh:
> to_i = i;
>- list_for_each_entry(i, &upper_dev->upper_dev_list, list) {
>+ list_for_each_entry(i, &upper_dev->all_dev_list.upper, list) {
> if (i == to_i)
> break;
> __netdev_adjacent_dev_unlink(dev, i->dev);
>@@ -4727,8 +4802,8 @@ rollback_upper_mesh:
> rollback_mesh:
> to_i = i;
> to_j = j;
>- list_for_each_entry(i, &dev->lower_dev_list, list) {
>- list_for_each_entry(j, &upper_dev->upper_dev_list, list) {
>+ list_for_each_entry(i, &dev->all_dev_list.lower, list) {
>+ list_for_each_entry(j, &upper_dev->all_dev_list.upper, list) {
> if (i == to_i && j == to_j)
> break;
> __netdev_adjacent_dev_unlink(i->dev, j->dev);
>@@ -4797,17 +4872,17 @@ void netdev_upper_dev_unlink(struct net_device *dev,
> * devices from all upper_dev's upper devices and vice
> * versa, to maintain the graph relationship.
> */
>- list_for_each_entry(i, &dev->lower_dev_list, list)
>- list_for_each_entry(j, &upper_dev->upper_dev_list, list)
>+ list_for_each_entry(i, &dev->all_dev_list.lower, list)
>+ list_for_each_entry(j, &upper_dev->all_dev_list.upper, list)
> __netdev_adjacent_dev_unlink(i->dev, j->dev);
>
> /* remove also the devices itself from lower/upper device
> * list
> */
>- list_for_each_entry(i, &dev->lower_dev_list, list)
>+ list_for_each_entry(i, &dev->all_dev_list.lower, list)
> __netdev_adjacent_dev_unlink(i->dev, upper_dev);
>
>- list_for_each_entry(i, &upper_dev->upper_dev_list, list)
>+ list_for_each_entry(i, &upper_dev->all_dev_list.upper, list)
> __netdev_adjacent_dev_unlink(dev, i->dev);
>
> call_netdevice_notifiers(NETDEV_CHANGEUPPER, dev);
>@@ -6069,8 +6144,10 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
> INIT_LIST_HEAD(&dev->napi_list);
> INIT_LIST_HEAD(&dev->unreg_list);
> INIT_LIST_HEAD(&dev->link_watch_list);
>- INIT_LIST_HEAD(&dev->upper_dev_list);
>- INIT_LIST_HEAD(&dev->lower_dev_list);
>+ INIT_LIST_HEAD(&dev->neighbour_dev_list.upper);
>+ INIT_LIST_HEAD(&dev->neighbour_dev_list.lower);
>+ INIT_LIST_HEAD(&dev->all_dev_list.upper);
>+ INIT_LIST_HEAD(&dev->all_dev_list.lower);
> dev->priv_flags = IFF_XMIT_DST_RELEASE;
> setup(dev);
>
>--
>1.8.4
>
^ permalink raw reply
* Re: [PATCH RFC net-next 01/21] net: add neighbour_dev_list to save only neighbours
From: Veaceslav Falico @ 2013-09-03 8:34 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David S. Miller, Eric Dumazet, Alexander Duyck, Cong Wang
In-Reply-To: <20130903082943.GD1437@minipsycho.brq.redhat.com>
On Tue, Sep 03, 2013 at 10:29:43AM +0200, Jiri Pirko wrote:
>Mon, Sep 02, 2013 at 11:39:05PM CEST, vfalico@redhat.com wrote:
...snip...
>>- struct list_head upper_dev_list; /* List of upper devices */
>>- struct list_head lower_dev_list;
>>+
>>+ /* directly linked devices, like slaves for bonding */
>>+ struct {
>>+ struct list_head upper;
>>+ struct list_head lower;
>>+ } neighbour_dev_list ;
>>+
>>+ /* all linked devices, *including* neighbours */
>>+ struct {
>>+ struct list_head upper;
>>+ struct list_head lower;
>>+ } all_dev_list ;
>
>
>I think there is need for some naming consistency for functions and
>macros handling these lists.
>
>I propose:
>dev_list (drop the "neighbour") and all_dev_list
Agreed.
>
>and exported functions and macros called like:
>netdev_lower_get_next_priv()
>netdev_lower_for_each_priv()
>netdev_all_upper_get_next_rcu()
>etc...
Yep, will try to convert and see how it goes.
Thanks a lot!
^ permalink raw reply
* Re: Is fallback vhost_net to qemu for live migrate available？
From: Wei Liu @ 2013-09-03 8:40 UTC (permalink / raw)
To: Qin Chuanyu
Cc: Wei Liu, Anthony Liguori, Michael S. Tsirkin, jasowang, KVM list,
netdev, qianhuibin, xen-devel@lists.xen.org, wangfuhai, likunyun,
liuyongan, liuyingdong
In-Reply-To: <52253B2B.6050909@huawei.com>
On Tue, Sep 03, 2013 at 09:28:11AM +0800, Qin Chuanyu wrote:
> On 2013/9/2 15:57, Wei Liu wrote:
> >On Sat, Aug 31, 2013 at 12:45:11PM +0800, Qin Chuanyu wrote:
> >>On 2013/8/30 0:08, Anthony Liguori wrote:
> >>>Hi Qin,
> >>
> >>>>By change the memory copy and notify mechanism ，currently virtio-net with
> >>>>vhost_net could run on Xen with good performance。
> >>>
> >>>I think the key in doing this would be to implement a property
> >>>ioeventfd and irqfd interface in the driver domain kernel. Just
> >>>hacking vhost_net with Xen specific knowledge would be pretty nasty
> >>>IMHO.
> >>>
> >>Yes, I add a kernel module which persist virtio-net pio_addr and
> >>msix address as what kvm module did. Guest wake up vhost thread by
> >>adding a hook func in evtchn_interrupt.
> >>
> >>>Did you modify the front end driver to do grant table mapping or is
> >>>this all being done by mapping the domain's memory?
> >>>
> >>There is nothing changed in front end driver. Currently I use
> >>alloc_vm_area to get address space， and map the domain's memory as
> >>what what qemu did.
> >>
> >
> >You mean you're using xc_map_foreign_range and friends in the backend to
> >map guest memory? That's not very desirable as it violates Xen's
> >security model. It would not be too hard to pass grant references
> >instead of guest physical memory address IMHO.
> >
> In fact, I did what virtio-net have done in Qemu. I think security
> is a pseudo question because Dom0 is under control.
>
Consider that you might have driver domains. Not every domain is under
control or trusted. Also consider that security model like XSM can be
used to audit operations to enhance security so your foreign mapping
approach might not always work.
In short term foreign mapping can save you some time implementing the
prototype. In long term using grant table is the proper way to go. And
IMHO the benifit outweights the cost.
Wei.
> Host could access memory of guest in KVM much easier than Xen,
> but I hadn't heard someone said KVM is un-secret.
>
> Regards
> Qin chuanyu
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Is fallback vhost_net to qemu for live migrate available？
From: Michael S. Tsirkin @ 2013-09-03 8:55 UTC (permalink / raw)
To: Wei Liu
Cc: Qin Chuanyu, Anthony Liguori, jasowang, KVM list, netdev,
qianhuibin, xen-devel@lists.xen.org, wangfuhai, likunyun,
liuyongan, liuyingdong
In-Reply-To: <20130903084047.GF14104@zion.uk.xensource.com>
On Tue, Sep 03, 2013 at 09:40:48AM +0100, Wei Liu wrote:
> On Tue, Sep 03, 2013 at 09:28:11AM +0800, Qin Chuanyu wrote:
> > On 2013/9/2 15:57, Wei Liu wrote:
> > >On Sat, Aug 31, 2013 at 12:45:11PM +0800, Qin Chuanyu wrote:
> > >>On 2013/8/30 0:08, Anthony Liguori wrote:
> > >>>Hi Qin,
> > >>
> > >>>>By change the memory copy and notify mechanism ，currently virtio-net with
> > >>>>vhost_net could run on Xen with good performance。
> > >>>
> > >>>I think the key in doing this would be to implement a property
> > >>>ioeventfd and irqfd interface in the driver domain kernel. Just
> > >>>hacking vhost_net with Xen specific knowledge would be pretty nasty
> > >>>IMHO.
> > >>>
> > >>Yes, I add a kernel module which persist virtio-net pio_addr and
> > >>msix address as what kvm module did. Guest wake up vhost thread by
> > >>adding a hook func in evtchn_interrupt.
> > >>
> > >>>Did you modify the front end driver to do grant table mapping or is
> > >>>this all being done by mapping the domain's memory?
> > >>>
> > >>There is nothing changed in front end driver. Currently I use
> > >>alloc_vm_area to get address space， and map the domain's memory as
> > >>what what qemu did.
> > >>
> > >
> > >You mean you're using xc_map_foreign_range and friends in the backend to
> > >map guest memory? That's not very desirable as it violates Xen's
> > >security model. It would not be too hard to pass grant references
> > >instead of guest physical memory address IMHO.
> > >
> > In fact, I did what virtio-net have done in Qemu. I think security
> > is a pseudo question because Dom0 is under control.
> >
>
> Consider that you might have driver domains. Not every domain is under
> control or trusted.
I don't see anything that will prevent using driver domains here.
> Also consider that security model like XSM can be
> used to audit operations to enhance security so your foreign mapping
> approach might not always work.
It could be nice to have as an option, sure.
XSM is disabled by default though so I don't think lack of support for
that makes it a prototype.
> In short term foreign mapping can save you some time implementing the
> prototype.
> In long term using grant table is the proper way to go. And
> IMHO the benifit outweights the cost.
>
> Wei.
I'm guessing direct access could be quite a bit faster.
But someone would have to implement your idea in order to
do a cost/benefit analysis.
> > Host could access memory of guest in KVM much easier than Xen,
> > but I hadn't heard someone said KVM is un-secret.
> >
> > Regards
> > Qin chuanyu
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [patch] caif: add a sanity check to the tty name
From: Dan Carpenter @ 2013-09-03 9:02 UTC (permalink / raw)
To: Dmitry Tarnyagin; +Cc: netdev, kernel-janitors
"tty->name" and "name" are a 64 character buffers. My static checker
complains because we add the "cf" on the front so it look like we are
copying a 66 character string into a 64 character buffer.
Also if the name is larger than IFNAMSIZ (16) it triggers a BUG_ON()
inside the call to alloc_netdev().
This is all under CAP_SYS_ADMIN so it's not a security fix, it just adds
a little robustness.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index 34dea95..88a6a58 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -347,7 +347,9 @@ static int ldisc_open(struct tty_struct *tty)
/* release devices to avoid name collision */
ser_release(NULL);
- sprintf(name, "cf%s", tty->name);
+ result = snprintf(name, sizeof(name), "cf%s", tty->name);
+ if (result >= IFNAMSIZ)
+ return -EINVAL;
dev = alloc_netdev(sizeof(*ser), name, caifdev_setup);
if (!dev)
return -ENOMEM;
^ permalink raw reply related
* [patch] x25: add a sanity check parsing X.25 facilities
From: Dan Carpenter @ 2013-09-03 9:03 UTC (permalink / raw)
To: Andrew Hendry; +Cc: David S. Miller, linux-x25, netdev, kernel-janitors
This was found with a manual audit and I don't have a reproducer. We
limit ->calling_len and ->called_len when we get them from
copy_from_user() in x25_ioctl() so when they come from skb->data then
we should cap them there as well.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Possibly should be applied to -stable. My guess is that this leads to
memory corruption, but I don't know.
diff --git a/net/x25/x25_facilities.c b/net/x25/x25_facilities.c
index 66c63873..b825325 100644
--- a/net/x25/x25_facilities.c
+++ b/net/x25/x25_facilities.c
@@ -156,6 +156,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
case X25_FAC_CALLING_AE:
if (p[1] > X25_MAX_DTE_FACIL_LEN || p[1] <= 1)
return -1;
+ if (p[2] > X25_MAX_AE_LEN)
+ return -1;
dte_facs->calling_len = p[2];
memcpy(dte_facs->calling_ae, &p[3], p[1] - 1);
*vc_fac_mask |= X25_MASK_CALLING_AE;
@@ -163,6 +165,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
case X25_FAC_CALLED_AE:
if (p[1] > X25_MAX_DTE_FACIL_LEN || p[1] <= 1)
return -1;
+ if (p[2] > X25_MAX_AE_LEN)
+ return -1;
dte_facs->called_len = p[2];
memcpy(dte_facs->called_ae, &p[3], p[1] - 1);
*vc_fac_mask |= X25_MASK_CALLED_AE;
^ permalink raw reply related
* [patch] qlcnic: remove a stray semicolon
From: Dan Carpenter @ 2013-09-03 9:13 UTC (permalink / raw)
To: Himanshu Madhani
Cc: Rajesh Borundia, Shahed Shaikh, Jitendra Kalsaria, Sony Chacko,
Sucheta Chakraborty, linux-driver, netdev, kernel-janitors
Just remove a small semicolon.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index 332aa71..4d7ad00 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -1709,7 +1709,7 @@ qlcnic_set_dump(struct net_device *netdev, struct ethtool_dump *val)
qlcnic_dev_request_reset(adapter, val->flag);
adapter->flags &= ~QLCNIC_FW_RESET_OWNER;
break;
-;
+
case QLCNIC_SET_QUIESCENT:
case QLCNIC_RESET_QUIESCENT:
state = QLC_SHARED_REG_RD32(adapter, QLCNIC_CRB_DEV_STATE);
^ permalink raw reply related
* VLAN HW accel, performance advantage?
From: Joakim Tjernlund @ 2013-09-03 9:05 UTC (permalink / raw)
To: netdev
I am considering impl. VLAN HW acceleration in Freescales ucc_geth driver
to improve
performance, primarily for bridged interfaces.
I am hoping the community has some insights as to what performance gains
one
could expect?
Jocke
^ permalink raw reply
* Re: Is fallback vhost_net to qemu for live migrate available？
From: Wei Liu @ 2013-09-03 9:15 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Wei Liu, Qin Chuanyu, Anthony Liguori, jasowang, KVM list, netdev,
qianhuibin, xen-devel@lists.xen.org, wangfuhai, likunyun,
liuyongan, liuyingdong
In-Reply-To: <20130903085556.GD18901@redhat.com>
On Tue, Sep 03, 2013 at 11:55:56AM +0300, Michael S. Tsirkin wrote:
> On Tue, Sep 03, 2013 at 09:40:48AM +0100, Wei Liu wrote:
> > On Tue, Sep 03, 2013 at 09:28:11AM +0800, Qin Chuanyu wrote:
> > > On 2013/9/2 15:57, Wei Liu wrote:
> > > >On Sat, Aug 31, 2013 at 12:45:11PM +0800, Qin Chuanyu wrote:
> > > >>On 2013/8/30 0:08, Anthony Liguori wrote:
> > > >>>Hi Qin,
> > > >>
> > > >>>>By change the memory copy and notify mechanism ，currently virtio-net with
> > > >>>>vhost_net could run on Xen with good performance。
> > > >>>
> > > >>>I think the key in doing this would be to implement a property
> > > >>>ioeventfd and irqfd interface in the driver domain kernel. Just
> > > >>>hacking vhost_net with Xen specific knowledge would be pretty nasty
> > > >>>IMHO.
> > > >>>
> > > >>Yes, I add a kernel module which persist virtio-net pio_addr and
> > > >>msix address as what kvm module did. Guest wake up vhost thread by
> > > >>adding a hook func in evtchn_interrupt.
> > > >>
> > > >>>Did you modify the front end driver to do grant table mapping or is
> > > >>>this all being done by mapping the domain's memory?
> > > >>>
> > > >>There is nothing changed in front end driver. Currently I use
> > > >>alloc_vm_area to get address space， and map the domain's memory as
> > > >>what what qemu did.
> > > >>
> > > >
> > > >You mean you're using xc_map_foreign_range and friends in the backend to
> > > >map guest memory? That's not very desirable as it violates Xen's
> > > >security model. It would not be too hard to pass grant references
> > > >instead of guest physical memory address IMHO.
> > > >
> > > In fact, I did what virtio-net have done in Qemu. I think security
> > > is a pseudo question because Dom0 is under control.
> > >
> >
> > Consider that you might have driver domains. Not every domain is under
> > control or trusted.
>
> I don't see anything that will prevent using driver domains here.
>
There is nothing technically stopping driver domains to work. It's about
the boundary of trust.
> > Also consider that security model like XSM can be
> > used to audit operations to enhance security so your foreign mapping
> > approach might not always work.
>
> It could be nice to have as an option, sure.
> XSM is disabled by default though so I don't think lack of support for
> that makes it a prototype.
>
XSM is there already, someone in a while might just want to give it a
shot despite it's not well supported. Then all of a sudden they find out
something works before (the foreign mapping backend) would not work
anymore. That would just lead to confusion. It's like a cycle of bad
things. Users try something lack of support -> it doesn't work -> users
won't use it anymore -> less support because nobody seems to be
interested.
> > In short term foreign mapping can save you some time implementing the
> > prototype.
> > In long term using grant table is the proper way to go. And
> > IMHO the benifit outweights the cost.
> >
> > Wei.
>
> I'm guessing direct access could be quite a bit faster.
> But someone would have to implement your idea in order to
> do a cost/benefit analysis.
>
I'm not sure direct access will be faster. Either way hypervisor is
involved to update guest's page table.
Anyway, my point is, switching to grant table is not as hard as you
think - pass along grant refs instead of guest pseudo address in the
ring and use grant table API instead of foreign mapping API in the
backend.
Wei.
> > > Host could access memory of guest in KVM much easier than Xen,
> > > but I hadn't heard someone said KVM is un-secret.
> > >
> > > Regards
> > > Qin chuanyu
> > >
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net-next 1/1] qlcnic: Fix sparse warning.
From: Sucheta Chakraborty @ 2013-09-03 9:07 UTC (permalink / raw)
To: davem; +Cc: netdev, Dept-HSGLinuxNICDev
This patch fixes warning "warning: symbol 'qlcnic_set_dcb_ops' was
not declared. Should it be static?"
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_dcb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_dcb.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_dcb.c
index 2e10e79..d62d5ce 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_dcb.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_dcb.c
@@ -248,7 +248,7 @@ static inline void __qlcnic_init_dcbnl_ops(struct qlcnic_adapter *adapter)
adapter->netdev->dcbnl_ops = &qlcnic_dcbnl_ops;
}
-void qlcnic_set_dcb_ops(struct qlcnic_adapter *adapter)
+static void qlcnic_set_dcb_ops(struct qlcnic_adapter *adapter)
{
if (qlcnic_82xx_check(adapter))
adapter->dcb->ops = &qlcnic_82xx_dcb_ops;
--
1.8.1.4
^ permalink raw reply related
* Re: [PATCH net-next 5/6] bonding: restructure and simplify bond_for_each_slave_next()
From: Nikolay Aleksandrov @ 2013-09-03 10:22 UTC (permalink / raw)
To: Ding Tianhong
Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller, Veaceslav Falico,
Netdev
In-Reply-To: <52206E4E.6010507@huawei.com>
On 08/30/2013 12:05 PM, Ding Tianhong wrote:
> remove the wordy int and add bond_for_each_slave_next_rcu() for future use.
>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> Cc: Nikolay Aleksandrov <nikolay@redhat.com>
> ---
> drivers/net/bonding/bond_alb.c | 3 +--
> drivers/net/bonding/bond_main.c | 6 ++----
> drivers/net/bonding/bonding.h | 19 +++++++++++++++----
> 3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 3a5db7b..d266c56 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -383,7 +383,6 @@ static struct slave *rlb_next_rx_slave(struct bonding *bond)
> {
> struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
> struct slave *rx_slave, *slave, *start_at;
> - int i = 0;
>
> if (bond_info->next_rx_slave)
> start_at = bond_info->next_rx_slave;
> @@ -392,7 +391,7 @@ static struct slave *rlb_next_rx_slave(struct bonding *bond)
>
> rx_slave = NULL;
>
> - bond_for_each_slave_from(bond, slave, i, start_at) {
> + bond_for_each_slave_from(bond, slave, start_at) {
> if (SLAVE_IS_OK(slave)) {
> if (!rx_slave) {
> rx_slave = slave;
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 4264a76..8c9902a 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -904,7 +904,6 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
> struct slave *new_active, *old_active;
> struct slave *bestslave = NULL;
> int mintime = bond->params.updelay;
> - int i;
>
> new_active = bond->curr_active_slave;
>
> @@ -923,7 +922,7 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
> /* remember where to stop iterating over the slaves */
> old_active = new_active;
>
> - bond_for_each_slave_from(bond, new_active, i, old_active) {
> + bond_for_each_slave_from(bond, new_active, old_active) {
> if (new_active->link == BOND_LINK_UP) {
> return new_active;
> } else if (new_active->link == BOND_LINK_BACK &&
> @@ -2891,7 +2890,6 @@ do_failover:
> static void bond_ab_arp_probe(struct bonding *bond)
> {
> struct slave *slave, *next_slave;
> - int i;
>
> read_lock(&bond->curr_slave_lock);
>
> @@ -2923,7 +2921,7 @@ static void bond_ab_arp_probe(struct bonding *bond)
>
> /* search for next candidate */
> next_slave = bond_next_slave(bond, bond->current_arp_slave);
> - bond_for_each_slave_from(bond, slave, i, next_slave) {
> + bond_for_each_slave_from(bond, slave, next_slave) {
> if (IS_UP(slave->dev)) {
> slave->link = BOND_LINK_BACK;
> bond_set_slave_active_flags(slave);
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 9898493..a3ab47f 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -119,14 +119,25 @@
> * bond_for_each_slave_from - iterate the slaves list from a starting point
> * @bond: the bond holding this list.
> * @pos: current slave.
> - * @cnt: counter for max number of moves
> * @start: starting point.
> *
> * Caller must hold bond->lock
> */
> -#define bond_for_each_slave_from(bond, pos, cnt, start) \
> - for (cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
> - cnt++, pos = bond_next_slave(bond, pos))
> +#define bond_for_each_slave_from(bond, pos, start) \
> + for (int cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
> + cnt++, pos = bond_next_slave(bond, pos))
> +
Please read below the argument against using the nested cnt definition.
> +/**
> + * bond_for_each_slave_from_rcu - iterate the slaves list from a starting point
> + * @bond: the bond holding this list.
> + * @pos: current slave.
> + * @start: starting point.
> + *
> + * Caller must hold rcu_read_lock
> + */
> +#define bond_for_each_slave_from_rcu(bond, pos, start) \
> + for (int cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
> + cnt++, pos = bond_next_slave_rcu(bond, pos))
>
I don't think you can rely on slave_cnt in RCU, you may go overboard and pass
twice over the same slave if a slave gets removed, or the opposite. Also this
definition of cnt is troublesome because the name is quite common, I don't know
if it's accepted, but it if it is at least change the name to something like
__cnt or anything that is less likely to be defined.
The cnt argument goes for bond_for_each_slave_from as well.
> /**
> * bond_for_each_slave - iterate over all slaves
>
^ permalink raw reply
* Re: [PATCH net-next 6/6] bonding: use RCU protection for alb xmit path
From: Nikolay Aleksandrov @ 2013-09-03 10:22 UTC (permalink / raw)
To: Ding Tianhong
Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller, Veaceslav Falico,
Netdev, Hideaki YOSHIFUJI
In-Reply-To: <52206E54.4040001@huawei.com>
On 08/30/2013 12:05 PM, Ding Tianhong wrote:
> The commit 278b20837511776dc9d5f6ee1c7fabd5479838bb
> (bonding: initial RCU conversion) has convert the roundrobin, active-backup,
> broadcast and xor xmit path to rcu protection, the performance will be better
> for these mode, so this time, convert xmit path for alb mode.
>
> Suggested-by: Nikolay Aleksandrov <nikolay@redhat.com>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> Cc: Nikolay Aleksandrov <nikolay@redhat.com>
> Cc: Veaceslav Falico <vfalico@redhat.com>
> ---
> drivers/net/bonding/bond_alb.c | 20 +++++++++-----------
> drivers/net/bonding/bonding.h | 2 +-
> 2 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index d266c56..e94a5d0 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -229,7 +229,7 @@ static struct slave *tlb_get_least_loaded_slave(struct bonding *bond)
> max_gap = LLONG_MIN;
>
> /* Find the slave with the largest gap */
> - bond_for_each_slave(bond, slave) {
> + bond_for_each_slave_rcu(bond, slave) {
> if (SLAVE_IS_OK(slave)) {
> long long gap = compute_gap(slave);
>
> @@ -625,10 +625,12 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
> {
> struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
> struct arp_pkt *arp = arp_pkt(skb);
> - struct slave *assigned_slave;
> + struct slave *assigned_slave, *curr_active_slave;
> struct rlb_client_info *client_info;
> u32 hash_index = 0;
>
> + curr_active_slave = rcu_dereference(bond->curr_active_slave);
> +
I think this code is prone to a race condition with the slave removal, because
without the lock __bond_release_one() may think that it has done the LB release
(bond_alb_deinit_slave) and cleared the slave while you may set it after locking.
I guess you should move the dereference after the lock so to make sure that the
LB release hasn't passed (or it has and it'll be different).
> _lock_rx_hashtbl(bond);
>
> hash_index = _simple_hash((u8 *)&arp->ip_dst, sizeof(arp->ip_dst));
> @@ -654,9 +656,9 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
> * move the old client to primary (curr_active_slave) so
> * that the new client can be assigned to this entry.
> */
> - if (bond->curr_active_slave &&
> - client_info->slave != bond->curr_active_slave) {
> - client_info->slave = bond->curr_active_slave;
> + if (curr_active_slave &&
> + client_info->slave != curr_active_slave) {
> + client_info->slave = curr_active_slave;
> rlb_update_client(client_info);
> }
> }
> @@ -1336,8 +1338,6 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>
> /* make sure that the curr_active_slave do not change during tx
> */
> - read_lock(&bond->lock);
> - read_lock(&bond->curr_slave_lock);
>
> switch (ntohs(skb->protocol)) {
> case ETH_P_IP: {
> @@ -1420,12 +1420,12 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>
> if (!tx_slave) {
> /* unbalanced or unassigned, send through primary */
> - tx_slave = bond->curr_active_slave;
> + tx_slave = rcu_dereference(bond->curr_active_slave);
> bond_info->unbalanced_load += skb->len;
> }
>
> if (tx_slave && SLAVE_IS_OK(tx_slave)) {
> - if (tx_slave != bond->curr_active_slave) {
> + if (tx_slave != rcu_dereference(bond->curr_active_slave)) {
> memcpy(eth_data->h_source,
> tx_slave->dev->dev_addr,
> ETH_ALEN);
> @@ -1440,8 +1440,6 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
> }
> }
>
> - read_unlock(&bond->curr_slave_lock);
> - read_unlock(&bond->lock);
> if (res) {
> /* no suitable interface, frame not sent */
> kfree_skb(skb);
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index a3ab47f..9bc3af0 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -536,7 +536,7 @@ static inline struct slave *bond_slave_has_mac(struct bonding *bond,
> {
> struct slave *tmp;
>
> - bond_for_each_slave(bond, tmp)
> + bond_for_each_slave_rcu(bond, tmp)
> if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr))
> return tmp;
>
>
^ permalink raw reply
* [PATCH net-next 0/2] net_random_N, reciprocal_divide helper updates
From: Daniel Borkmann @ 2013-09-03 10:26 UTC (permalink / raw)
To: davem; +Cc: netdev
Introduce a net_random_N() helper instead of reimplementing it over and
over, plus use reciprocal_divide() helper instead of reimplementing it
as well.
Daniel Borkmann (2):
net: introduce generic net_random_N helper
net: use reciprocal_divide instead of reimplementing it
drivers/net/team/team_mode_random.c | 7 +------
include/linux/net.h | 7 +++++++
include/net/red.h | 2 +-
net/802/garp.c | 2 +-
net/802/mrp.c | 2 +-
net/core/dev.c | 2 +-
net/core/flow_dissector.c | 5 ++---
net/ipv4/inet_hashtables.c | 2 +-
net/ipv4/netfilter/ipt_CLUSTERIP.c | 2 +-
net/ipv4/udp.c | 6 +++---
net/ipv6/inet6_hashtables.c | 2 +-
net/ipv6/udp.c | 4 ++--
net/netfilter/nf_conntrack_core.c | 2 +-
net/netfilter/nf_conntrack_expect.c | 2 +-
net/netfilter/nf_nat_core.c | 4 ++--
net/netfilter/xt_HMARK.c | 2 +-
net/netfilter/xt_NFQUEUE.c | 4 ++--
net/netfilter/xt_cluster.c | 2 +-
net/netfilter/xt_hashlimit.c | 2 +-
net/packet/af_packet.c | 2 +-
net/sched/sch_choke.c | 8 +-------
net/sched/sch_fq_codel.c | 2 +-
22 files changed, 34 insertions(+), 39 deletions(-)
--
1.7.11.7
^ permalink raw reply
* [PATCH net-next 1/2] net: introduce generic net_random_N helper
From: Daniel Borkmann @ 2013-09-03 10:26 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1378204010-27050-1-git-send-email-dborkman@redhat.com>
We have implemented the same function over and over, so introduce a
generic helper net_random_N() that unifies these implementations.
It internally used net_random() which eventually resolves to
prandom_u32(). Explicit include of reciprocal_div.h is not necessary.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
drivers/net/team/team_mode_random.c | 7 +------
include/linux/net.h | 7 +++++++
include/net/red.h | 2 +-
net/802/garp.c | 2 +-
net/802/mrp.c | 2 +-
net/packet/af_packet.c | 2 +-
net/sched/sch_choke.c | 8 +-------
7 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/net/team/team_mode_random.c b/drivers/net/team/team_mode_random.c
index 7f032e2..b2294f8 100644
--- a/drivers/net/team/team_mode_random.c
+++ b/drivers/net/team/team_mode_random.c
@@ -16,17 +16,12 @@
#include <linux/reciprocal_div.h>
#include <linux/if_team.h>
-static u32 random_N(unsigned int N)
-{
- return reciprocal_divide(prandom_u32(), N);
-}
-
static bool rnd_transmit(struct team *team, struct sk_buff *skb)
{
struct team_port *port;
int port_index;
- port_index = random_N(team->en_port_count);
+ port_index = net_random_N(team->en_port_count);
port = team_get_port_by_index_rcu(team, port_index);
if (unlikely(!port))
goto drop;
diff --git a/include/linux/net.h b/include/linux/net.h
index 4f27575..86ffce7 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -24,6 +24,7 @@
#include <linux/fcntl.h> /* For O_CLOEXEC and O_NONBLOCK */
#include <linux/kmemcheck.h>
#include <linux/rcupdate.h>
+#include <linux/reciprocal_div.h>
#include <uapi/linux/net.h>
struct poll_table_struct;
@@ -243,6 +244,12 @@ do { \
#define net_random() prandom_u32()
#define net_srandom(seed) prandom_seed((__force u32)(seed))
+/* deliver a random number between 0 and N - 1 */
+static inline u32 net_random_N(u32 N)
+{
+ return reciprocal_divide(net_random(), N);
+}
+
extern int kernel_sendmsg(struct socket *sock, struct msghdr *msg,
struct kvec *vec, size_t num, size_t len);
extern int kernel_recvmsg(struct socket *sock, struct msghdr *msg,
diff --git a/include/net/red.h b/include/net/red.h
index ef46058..b17ed60 100644
--- a/include/net/red.h
+++ b/include/net/red.h
@@ -303,7 +303,7 @@ static inline unsigned long red_calc_qavg(const struct red_parms *p,
static inline u32 red_random(const struct red_parms *p)
{
- return reciprocal_divide(net_random(), p->max_P_reciprocal);
+ return net_random_N(p->max_P_reciprocal);
}
static inline int red_mark_probability(const struct red_parms *p,
diff --git a/net/802/garp.c b/net/802/garp.c
index 5d9630a..c6fe7ca 100644
--- a/net/802/garp.c
+++ b/net/802/garp.c
@@ -397,7 +397,7 @@ static void garp_join_timer_arm(struct garp_applicant *app)
{
unsigned long delay;
- delay = (u64)msecs_to_jiffies(garp_join_time) * net_random() >> 32;
+ delay = net_random_N(msecs_to_jiffies(garp_join_time));
mod_timer(&app->join_timer, jiffies + delay);
}
diff --git a/net/802/mrp.c b/net/802/mrp.c
index 1eb05d8..7338a7c 100644
--- a/net/802/mrp.c
+++ b/net/802/mrp.c
@@ -578,7 +578,7 @@ static void mrp_join_timer_arm(struct mrp_applicant *app)
{
unsigned long delay;
- delay = (u64)msecs_to_jiffies(mrp_join_time) * net_random() >> 32;
+ delay = net_random_N(msecs_to_jiffies(mrp_join_time));
mod_timer(&app->join_timer, jiffies + delay);
}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2e8286b..7ac669e 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1162,7 +1162,7 @@ static unsigned int fanout_demux_rnd(struct packet_fanout *f,
struct sk_buff *skb,
unsigned int num)
{
- return reciprocal_divide(prandom_u32(), num);
+ return net_random_N(num);
}
static unsigned int fanout_demux_rollover(struct packet_fanout *f,
diff --git a/net/sched/sch_choke.c b/net/sched/sch_choke.c
index ef53ab8..e19b20be 100644
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -77,12 +77,6 @@ struct choke_sched_data {
struct sk_buff **tab;
};
-/* deliver a random number between 0 and N - 1 */
-static u32 random_N(unsigned int N)
-{
- return reciprocal_divide(prandom_u32(), N);
-}
-
/* number of elements in queue including holes */
static unsigned int choke_len(const struct choke_sched_data *q)
{
@@ -233,7 +227,7 @@ static struct sk_buff *choke_peek_random(const struct choke_sched_data *q,
int retrys = 3;
do {
- *pidx = (q->head + random_N(choke_len(q))) & q->tab_mask;
+ *pidx = (q->head + net_random_N(choke_len(q))) & q->tab_mask;
skb = q->tab[*pidx];
if (skb)
return skb;
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 2/2] net: use reciprocal_divide instead of reimplementing it
From: Daniel Borkmann @ 2013-09-03 10:26 UTC (permalink / raw)
To: davem; +Cc: netdev, netfilter-devel
In-Reply-To: <1378204010-27050-1-git-send-email-dborkman@redhat.com>
Replace these places with reciprocal_divide() inline function instead of
reimplementing it each time, thus it will become easier to read. We do not
need to explicitly include its header as it's pulled in from linux/net.h
anyway.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: netfilter-devel@vger.kernel.org
---
net/core/dev.c | 2 +-
net/core/flow_dissector.c | 5 ++---
net/ipv4/inet_hashtables.c | 2 +-
net/ipv4/netfilter/ipt_CLUSTERIP.c | 2 +-
net/ipv4/udp.c | 6 +++---
net/ipv6/inet6_hashtables.c | 2 +-
net/ipv6/udp.c | 4 ++--
net/netfilter/nf_conntrack_core.c | 2 +-
net/netfilter/nf_conntrack_expect.c | 2 +-
net/netfilter/nf_nat_core.c | 4 ++--
net/netfilter/xt_HMARK.c | 2 +-
net/netfilter/xt_NFQUEUE.c | 4 ++--
net/netfilter/xt_cluster.c | 2 +-
net/netfilter/xt_hashlimit.c | 2 +-
net/sched/sch_fq_codel.c | 2 +-
15 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 6fbb0c9..bc7c839 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3040,7 +3040,7 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
}
if (map) {
- tcpu = map->cpus[((u64) skb->rxhash * map->len) >> 32];
+ tcpu = map->cpus[reciprocal_divide(skb->rxhash, map->len)];
if (cpu_online(tcpu)) {
cpu = tcpu;
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 159737c..59521e0 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -233,7 +233,7 @@ u16 __skb_tx_hash(const struct net_device *dev, const struct sk_buff *skb,
hash = (__force u16) skb->protocol;
hash = jhash_1word(hash, hashrnd);
- return (u16) (((u64) hash * qcount) >> 32) + qoffset;
+ return (u16) reciprocal_divide(hash, qcount) + qoffset;
}
EXPORT_SYMBOL(__skb_tx_hash);
@@ -324,8 +324,7 @@ static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
hash = (__force u16) skb->protocol ^
skb->rxhash;
hash = jhash_1word(hash, hashrnd);
- queue_index = map->queues[
- ((u64)hash * map->len) >> 32];
+ queue_index = map->queues[reciprocal_divide(hash, map->len)];
}
if (unlikely(queue_index >= dev->real_num_tx_queues))
queue_index = -1;
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 7bd8983..731dcbf 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -204,7 +204,7 @@ begin:
}
} else if (score == hiscore && reuseport) {
matches++;
- if (((u64)phash * matches) >> 32 == 0)
+ if (reciprocal_divide(phash, matches) == 0)
result = sk;
phash = next_pseudo_random32(phash);
}
diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 0b732ef..0fede8b 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -273,7 +273,7 @@ clusterip_hashfn(const struct sk_buff *skb,
}
/* node numbers are 1..n, not 0..n */
- return (((u64)hashval * config->num_total_nodes) >> 32) + 1;
+ return reciprocal_divide(hashval, config->num_total_nodes) + 1;
}
static inline int
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 74d2c95..5db0725 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -223,7 +223,7 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
remaining = (high - low) + 1;
rand = net_random();
- first = (((u64)rand * remaining) >> 32) + low;
+ first = reciprocal_divide(rand, remaining) + low;
/*
* force rand to be an odd multiple of UDP_HTABLE_SIZE
*/
@@ -435,7 +435,7 @@ begin:
}
} else if (score == badness && reuseport) {
matches++;
- if (((u64)hash * matches) >> 32 == 0)
+ if (reciprocal_divide(hash, matches) == 0)
result = sk;
hash = next_pseudo_random32(hash);
}
@@ -516,7 +516,7 @@ begin:
}
} else if (score == badness && reuseport) {
matches++;
- if (((u64)hash * matches) >> 32 == 0)
+ if (reciprocal_divide(hash, matches) == 0)
result = sk;
hash = next_pseudo_random32(hash);
}
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index 32b4a16..d9d8908 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -187,7 +187,7 @@ begin:
}
} else if (score == hiscore && reuseport) {
matches++;
- if (((u64)phash * matches) >> 32 == 0)
+ if (reciprocal_divide(phash, matches) == 0)
result = sk;
phash = next_pseudo_random32(phash);
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index f405815..610013a 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -226,7 +226,7 @@ begin:
goto exact_match;
} else if (score == badness && reuseport) {
matches++;
- if (((u64)hash * matches) >> 32 == 0)
+ if (reciprocal_divide(hash, matches) == 0)
result = sk;
hash = next_pseudo_random32(hash);
}
@@ -306,7 +306,7 @@ begin:
}
} else if (score == badness && reuseport) {
matches++;
- if (((u64)hash * matches) >> 32 == 0)
+ if (reciprocal_divide(hash, matches) == 0)
result = sk;
hash = next_pseudo_random32(hash);
}
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 5d892fe..6378441 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -97,7 +97,7 @@ static u32 hash_conntrack_raw(const struct nf_conntrack_tuple *tuple, u16 zone)
static u32 __hash_bucket(u32 hash, unsigned int size)
{
- return ((u64)hash * size) >> 32;
+ return reciprocal_divide(hash, size);
}
static u32 hash_bucket(u32 hash, const struct net *net)
diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index 4fd1ca9..c0865e6 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -83,7 +83,7 @@ static unsigned int nf_ct_expect_dst_hash(const struct nf_conntrack_tuple *tuple
hash = jhash2(tuple->dst.u3.all, ARRAY_SIZE(tuple->dst.u3.all),
(((tuple->dst.protonum ^ tuple->src.l3num) << 16) |
(__force __u16)tuple->dst.u.all) ^ nf_conntrack_hash_rnd);
- return ((u64)hash * nf_ct_expect_hsize) >> 32;
+ return reciprocal_divide(hash, nf_ct_expect_hsize);
}
struct nf_conntrack_expect *
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 6f0f4f7..ce9083e 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -126,7 +126,7 @@ hash_by_src(const struct net *net, u16 zone,
/* Original src, to ensure we map it consistently if poss. */
hash = jhash2((u32 *)&tuple->src, sizeof(tuple->src) / sizeof(u32),
tuple->dst.protonum ^ zone ^ nf_conntrack_hash_rnd);
- return ((u64)hash * net->ct.nat_htable_size) >> 32;
+ return reciprocal_divide(hash, net->ct.nat_htable_size);
}
/* Is this tuple already taken? (not by us) */
@@ -274,7 +274,7 @@ find_best_ips_proto(u16 zone, struct nf_conntrack_tuple *tuple,
}
var_ipp->all[i] = (__force __u32)
- htonl(minip + (((u64)j * dist) >> 32));
+ htonl(minip + reciprocal_divide(j, dist));
if (var_ipp->all[i] != range->max_addr.all[i])
full_range = true;
diff --git a/net/netfilter/xt_HMARK.c b/net/netfilter/xt_HMARK.c
index 73b73f6..3dd05c4 100644
--- a/net/netfilter/xt_HMARK.c
+++ b/net/netfilter/xt_HMARK.c
@@ -126,7 +126,7 @@ hmark_hash(struct hmark_tuple *t, const struct xt_hmark_info *info)
hash = jhash_3words(src, dst, t->uports.v32, info->hashrnd);
hash = hash ^ (t->proto & info->proto_mask);
- return (((u64)hash * info->hmodulus) >> 32) + info->hoffset;
+ return reciprocal_divide(hash, info->hmodulus) + info->hoffset;
}
static void
diff --git a/net/netfilter/xt_NFQUEUE.c b/net/netfilter/xt_NFQUEUE.c
index 1e2fae3..88443c1 100644
--- a/net/netfilter/xt_NFQUEUE.c
+++ b/net/netfilter/xt_NFQUEUE.c
@@ -83,10 +83,10 @@ nfqueue_hash(const struct sk_buff *skb, const struct xt_action_param *par)
u32 queue = info->queuenum;
if (par->family == NFPROTO_IPV4)
- queue += ((u64) hash_v4(skb) * info->queues_total) >> 32;
+ queue += reciprocal_divide(hash_v4(skb), info->queues_total);
#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
else if (par->family == NFPROTO_IPV6)
- queue += ((u64) hash_v6(skb) * info->queues_total) >> 32;
+ queue += reciprocal_divide(hash_v6(skb), info->queues_total);
#endif
return queue;
diff --git a/net/netfilter/xt_cluster.c b/net/netfilter/xt_cluster.c
index f4af1bf..845fc16 100644
--- a/net/netfilter/xt_cluster.c
+++ b/net/netfilter/xt_cluster.c
@@ -55,7 +55,7 @@ xt_cluster_hash(const struct nf_conn *ct,
WARN_ON(1);
break;
}
- return (((u64)hash * info->total_nodes) >> 32);
+ return reciprocal_divide(hash, info->total_nodes);
}
static inline bool
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 9ff035c..9af9021 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -135,7 +135,7 @@ hash_dst(const struct xt_hashlimit_htable *ht, const struct dsthash_dst *dst)
* give results between [0 and cfg.size-1] and same hash distribution,
* but using a multiply, less expensive than a divide
*/
- return ((u64)hash * ht->cfg.size) >> 32;
+ return reciprocal_divide(hash, ht->cfg.size);
}
static struct dsthash_ent *
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index 5578628..4869671 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -77,7 +77,7 @@ static unsigned int fq_codel_hash(const struct fq_codel_sched_data *q,
hash = jhash_3words((__force u32)keys.dst,
(__force u32)keys.src ^ keys.ip_proto,
(__force u32)keys.ports, q->perturbation);
- return ((u64)hash * q->flows_cnt) >> 32;
+ return reciprocal_divide(hash, q->flows_cnt);
}
static unsigned int fq_codel_classify(struct sk_buff *skb, struct Qdisc *sch,
--
1.7.11.7
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox