Netdev List
 help / color / mirror / Atom feed
* [Patch net-next] tunnel: use iptunnel_xmit() again
From: Cong Wang @ 2013-03-10  2:38 UTC (permalink / raw)
  To: netdev; +Cc: Pravin B Shelar, Stephen Hemminger, David S. Miller, Cong Wang

From: Cong Wang <xiyou.wangcong@gmail.com>

With recent patches from Pravin, most tunnels can't use iptunnel_xmit()
any more, due to ip_select_ident() and skb->ip_summed. But we can just
move these operations out of iptunnel_xmit(), so that tunnels can
use it again.

This by the way fixes a bug in vxlan (missing nf_reset()) for net-next.

Cc: Pravin B Shelar <pshelar@nicira.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

---
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index f057ec0..f3a135c 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -855,7 +855,6 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 	__u16 src_port;
 	__be16 df = 0;
 	__u8 tos, ttl;
-	int err;
 	bool did_rsc = false;
 	const struct vxlan_fdb *f;
 
@@ -980,18 +979,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (handle_offloads(skb))
 		goto drop;
 
-	err = ip_local_out(skb);
-	if (likely(net_xmit_eval(err) == 0)) {
-		struct vxlan_stats *stats = this_cpu_ptr(vxlan->stats);
-
-		u64_stats_update_begin(&stats->syncp);
-		stats->tx_packets++;
-		stats->tx_bytes += pkt_len;
-		u64_stats_update_end(&stats->syncp);
-	} else {
-		dev->stats.tx_errors++;
-		dev->stats.tx_aborted_errors++;
-	}
+	iptunnel_xmit(skb, dev);
 	return NETDEV_TX_OK;
 
 drop:
diff --git a/include/net/ipip.h b/include/net/ipip.h
index fd19625..0c046e3 100644
--- a/include/net/ipip.h
+++ b/include/net/ipip.h
@@ -51,13 +51,10 @@ struct ip_tunnel_prl_entry {
 static inline void iptunnel_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	int err;
-	struct iphdr *iph = ip_hdr(skb);
 	int pkt_len = skb->len - skb_transport_offset(skb);
 	struct pcpu_tstats *tstats = this_cpu_ptr(dev->tstats);
 
 	nf_reset(skb);
-	skb->ip_summed = CHECKSUM_NONE;
-	ip_select_ident(iph, skb_dst(skb), NULL);
 
 	err = ip_local_out(skb);
 	if (likely(net_xmit_eval(err) == 0)) {
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index d0ef0e6..a13a097 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -762,7 +762,6 @@ error:
 
 static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct pcpu_tstats *tstats = this_cpu_ptr(dev->tstats);
 	struct ip_tunnel *tunnel = netdev_priv(dev);
 	const struct iphdr  *old_iph;
 	const struct iphdr  *tiph;
@@ -778,7 +777,6 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
 	int    mtu;
 	u8     ttl;
 	int    err;
-	int    pkt_len;
 
 	skb = handle_offloads(tunnel, skb);
 	if (IS_ERR(skb)) {
@@ -1022,19 +1020,7 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
 		}
 	}
 
-	nf_reset(skb);
-
-	pkt_len = skb->len - skb_transport_offset(skb);
-	err = ip_local_out(skb);
-	if (likely(net_xmit_eval(err) == 0)) {
-		u64_stats_update_begin(&tstats->syncp);
-		tstats->tx_bytes += pkt_len;
-		tstats->tx_packets++;
-		u64_stats_update_end(&tstats->syncp);
-	} else {
-		dev->stats.tx_errors++;
-		dev->stats.tx_aborted_errors++;
-	}
+	iptunnel_xmit(skb, dev);
 	return NETDEV_TX_OK;
 
 #if IS_ENABLED(CONFIG_IPV6)
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index b50435b..34e006f 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -478,8 +478,6 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
 	__be32 dst = tiph->daddr;
 	struct flowi4 fl4;
 	int    mtu;
-	int err;
-	int pkt_len;
 
 	if (skb->protocol != htons(ETH_P_IP))
 		goto tx_error;
@@ -600,21 +598,7 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
 	if ((iph->ttl = tiph->ttl) == 0)
 		iph->ttl	=	old_iph->ttl;
 
-	nf_reset(skb);
-
-	pkt_len = skb->len - skb_transport_offset(skb);
-	err = ip_local_out(skb);
-	if (likely(net_xmit_eval(err) == 0)) {
-		struct pcpu_tstats *tstats = this_cpu_ptr(dev->tstats);
-
-		u64_stats_update_begin(&tstats->syncp);
-		tstats->tx_bytes += pkt_len;
-		tstats->tx_packets++;
-		u64_stats_update_end(&tstats->syncp);
-	} else {
-		dev->stats.tx_errors++;
-		dev->stats.tx_aborted_errors++;
-	}
+	iptunnel_xmit(skb, dev);
 
 	return NETDEV_TX_OK;
 
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 02f96dc..898e671 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -899,6 +899,8 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
 	if ((iph->ttl = tiph->ttl) == 0)
 		iph->ttl	=	iph6->hop_limit;
 
+	skb->ip_summed = CHECKSUM_NONE;
+	ip_select_ident(iph, skb_dst(skb), NULL);
 	iptunnel_xmit(skb, dev);
 	return NETDEV_TX_OK;
 

^ permalink raw reply related

* [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Vimalkumar @ 2013-03-10  3:20 UTC (permalink / raw)
  To: netdev, eric.dumazet, shemminger; +Cc: Vimalkumar

Since rate values are passed around between kernel and tc
in bytes/sec, 2**32 bytes/sec is around 34Gb/s.  Beyond that
rate, htb, tbf, hfsc, etc. can never be configured correctly.

Signed-off-by: Vimalkumar <j.vimal@gmail.com>
---
 include/linux/gen_stats.h |    2 +-
 include/linux/pkt_sched.h |   10 +++++-----
 misc/ifstat.c             |    4 ++--
 tc/m_police.c             |    2 +-
 tc/q_cbq.c                |    2 +-
 tc/q_choke.c              |    2 +-
 tc/q_gred.c               |    2 +-
 tc/q_hfsc.c               |    6 ++++--
 tc/q_red.c                |    2 +-
 tc/tc_cbq.c               |    4 ++--
 tc/tc_cbq.h               |    4 ++--
 tc/tc_core.c              |    4 ++--
 tc/tc_core.h              |    4 ++--
 tc/tc_util.c              |    6 +++---
 tc/tc_util.h              |    6 +++---
 15 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/include/linux/gen_stats.h b/include/linux/gen_stats.h
index 552c8a0..5ca6015 100644
--- a/include/linux/gen_stats.h
+++ b/include/linux/gen_stats.h
@@ -33,7 +33,7 @@ struct gnet_stats_basic_packed {
  * @pps: current packet rate
  */
 struct gnet_stats_rate_est {
-	__u32	bps;
+	__u64	bps;
 	__u32	pps;
 };
 
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 32aef0a..d6bc658 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -35,7 +35,7 @@ struct tc_stats {
 	__u32	drops;			/* Packets dropped because of lack of resources */
 	__u32	overlimits;		/* Number of throttle events when this
 					 * flow goes out of allocated bandwidth */
-	__u32	bps;			/* Current flow byte rate */
+	__u64	bps;			/* Current flow byte rate */
 	__u32	pps;			/* Current flow packet rate */
 	__u32	qlen;
 	__u32	backlog;
@@ -79,7 +79,7 @@ struct tc_ratespec {
 	unsigned short	overhead;
 	short		cell_align;
 	unsigned short	mpu;
-	__u32		rate;
+	__u64		rate;
 };
 
 #define TC_RTAB_SIZE	1024
@@ -368,9 +368,9 @@ struct tc_hfsc_qopt {
 };
 
 struct tc_service_curve {
-	__u32	m1;		/* slope of the first segment in bps */
+	__u64	m1;		/* slope of the first segment in bps */
 	__u32	d;		/* x-projection of the first segment in us */
-	__u32	m2;		/* slope of the second segment in bps */
+	__u64	m2;		/* slope of the second segment in bps */
 };
 
 struct tc_hfsc_stats {
@@ -541,7 +541,7 @@ struct tc_netem_corrupt {
 };
 
 struct tc_netem_rate {
-	__u32	rate;	/* byte/s */
+	__u64	rate;	/* byte/s */
 	__s32	packet_overhead;
 	__u32	cell_size;
 	__s32	cell_overhead;
diff --git a/misc/ifstat.c b/misc/ifstat.c
index 6d0ad8c..67a97eb 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -181,7 +181,7 @@ static void load_raw_table(FILE *fp)
 		p = next;
 
 		for (i=0; i<MAXS; i++) {
-			unsigned rate;
+			__u64 rate;
 			if (!(next = strchr(p, ' ')))
 				abort();
 			*next++ = 0;
@@ -192,7 +192,7 @@ static void load_raw_table(FILE *fp)
 			if (!(next = strchr(p, ' ')))
 				abort();
 			*next++ = 0;
-			if (sscanf(p, "%u", &rate) != 1)
+			if (sscanf(p, "%llu", &rate) != 1)
 				abort();
 			n->rate[i] = rate;
 			p = next;
diff --git a/tc/m_police.c b/tc/m_police.c
index 53cbefc..b187440 100644
--- a/tc/m_police.c
+++ b/tc/m_police.c
@@ -131,7 +131,7 @@ int act_parse_police(struct action_util *a,int *argc_p, char ***argv_p, int tca_
 	struct tc_police p;
 	__u32 rtab[256];
 	__u32 ptab[256];
-	__u32 avrate = 0;
+	__u64 avrate = 0;
 	int presult = 0;
 	unsigned buffer=0, mtu=0, mpu=0;
 	unsigned short overhead=0;
diff --git a/tc/q_cbq.c b/tc/q_cbq.c
index 3c5e72c..a742248 100644
--- a/tc/q_cbq.c
+++ b/tc/q_cbq.c
@@ -190,7 +190,7 @@ static int cbq_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
 	unsigned mpu=0;
 	int cell_log=-1;
 	int ewma_log=-1;
-	unsigned bndw = 0;
+	__u64 bndw = 0;
 	unsigned minburst=0, maxburst=0;
 	unsigned short overhead=0;
 	unsigned int linklayer = LINKLAYER_ETHERNET; /* Assume ethernet */
diff --git a/tc/q_choke.c b/tc/q_choke.c
index 6fbcadf..6cc691c 100644
--- a/tc/q_choke.c
+++ b/tc/q_choke.c
@@ -38,7 +38,7 @@ static int choke_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	unsigned burst = 0;
 	unsigned avpkt = 1000;
 	double probability = 0.02;
-	unsigned rate = 0;
+	__u64 rate = 0;
 	int ecn_ok = 0;
 	int wlog;
 	__u8 sbuf[256];
diff --git a/tc/q_gred.c b/tc/q_gred.c
index a4df3a6..3b9e079 100644
--- a/tc/q_gred.c
+++ b/tc/q_gred.c
@@ -122,7 +122,7 @@ static int gred_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct n
 	unsigned burst = 0;
 	unsigned avpkt = 0;
 	double probability = 0.02;
-	unsigned rate = 0;
+	__u64 rate = 0;
 	int wlog;
 	__u8 sbuf[256];
 	struct rtattr *tail;
diff --git a/tc/q_hfsc.c b/tc/q_hfsc.c
index 03539ec..ff3b8a0 100644
--- a/tc/q_hfsc.c
+++ b/tc/q_hfsc.c
@@ -294,7 +294,8 @@ hfsc_get_sc1(int *argcp, char ***argvp, struct tc_service_curve *sc)
 {
 	char **argv = *argvp;
 	int argc = *argcp;
-	unsigned int m1 = 0, d = 0, m2 = 0;
+	unsigned int d = 0;
+	__u64 m1 = 0, m2 = 0;
 
 	if (matches(*argv, "m1") == 0) {
 		NEXT_ARG();
@@ -337,7 +338,8 @@ hfsc_get_sc2(int *argcp, char ***argvp, struct tc_service_curve *sc)
 {
 	char **argv = *argvp;
 	int argc = *argcp;
-	unsigned int umax = 0, dmax = 0, rate = 0;
+	unsigned int umax = 0, dmax = 0;
+	__u64 rate;
 
 	if (matches(*argv, "umax") == 0) {
 		NEXT_ARG();
diff --git a/tc/q_red.c b/tc/q_red.c
index 89e7320..2fd9f61 100644
--- a/tc/q_red.c
+++ b/tc/q_red.c
@@ -39,7 +39,7 @@ static int red_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
 	unsigned burst = 0;
 	unsigned avpkt = 0;
 	double probability = 0.02;
-	unsigned rate = 0;
+	__u64 rate = 0;
 	int wlog;
 	__u8 sbuf[256];
 	__u32 max_P;
diff --git a/tc/tc_cbq.c b/tc/tc_cbq.c
index 0bb262e..46b9957 100644
--- a/tc/tc_cbq.c
+++ b/tc/tc_cbq.c
@@ -24,7 +24,7 @@
 #include "tc_core.h"
 #include "tc_cbq.h"
 
-unsigned tc_cbq_calc_maxidle(unsigned bndw, unsigned rate, unsigned avpkt,
+unsigned tc_cbq_calc_maxidle(__u64 bndw, __u64 rate, unsigned avpkt,
 			     int ewma_log, unsigned maxburst)
 {
 	double maxidle;
@@ -41,7 +41,7 @@ unsigned tc_cbq_calc_maxidle(unsigned bndw, unsigned rate, unsigned avpkt,
 	return tc_core_time2tick(maxidle*(1<<ewma_log)*TIME_UNITS_PER_SEC);
 }
 
-unsigned tc_cbq_calc_offtime(unsigned bndw, unsigned rate, unsigned avpkt,
+unsigned tc_cbq_calc_offtime(__u64 bndw, __u64 rate, unsigned avpkt,
 			     int ewma_log, unsigned minburst)
 {
 	double g = 1.0 - 1.0/(1<<ewma_log);
diff --git a/tc/tc_cbq.h b/tc/tc_cbq.h
index 8f95649..b485919 100644
--- a/tc/tc_cbq.h
+++ b/tc/tc_cbq.h
@@ -1,9 +1,9 @@
 #ifndef _TC_CBQ_H_
 #define _TC_CBQ_H_ 1
 
-unsigned tc_cbq_calc_maxidle(unsigned bndw, unsigned rate, unsigned avpkt,
+unsigned tc_cbq_calc_maxidle(__u64 bndw, __u64 rate, unsigned avpkt,
 			     int ewma_log, unsigned maxburst);
-unsigned tc_cbq_calc_offtime(unsigned bndw, unsigned rate, unsigned avpkt,
+unsigned tc_cbq_calc_offtime(__u64 bndw, __u64 rate, unsigned avpkt,
 			     int ewma_log, unsigned minburst);
 
 #endif
diff --git a/tc/tc_core.c b/tc/tc_core.c
index 85b072e..a3cacc1 100644
--- a/tc/tc_core.c
+++ b/tc/tc_core.c
@@ -56,12 +56,12 @@ unsigned tc_core_ktime2time(unsigned ktime)
 	return ktime / clock_factor;
 }
 
-unsigned tc_calc_xmittime(unsigned rate, unsigned size)
+unsigned tc_calc_xmittime(__u64 rate, unsigned size)
 {
 	return tc_core_time2tick(TIME_UNITS_PER_SEC*((double)size/rate));
 }
 
-unsigned tc_calc_xmitsize(unsigned rate, unsigned ticks)
+unsigned tc_calc_xmitsize(__u64 rate, unsigned ticks)
 {
 	return ((double)rate*tc_core_tick2time(ticks))/TIME_UNITS_PER_SEC;
 }
diff --git a/tc/tc_core.h b/tc/tc_core.h
index 5a693ba..8a63b79 100644
--- a/tc/tc_core.h
+++ b/tc/tc_core.h
@@ -18,8 +18,8 @@ unsigned tc_core_time2tick(unsigned time);
 unsigned tc_core_tick2time(unsigned tick);
 unsigned tc_core_time2ktime(unsigned time);
 unsigned tc_core_ktime2time(unsigned ktime);
-unsigned tc_calc_xmittime(unsigned rate, unsigned size);
-unsigned tc_calc_xmitsize(unsigned rate, unsigned ticks);
+unsigned tc_calc_xmittime(__u64 rate, unsigned size);
+unsigned tc_calc_xmitsize(__u64 rate, unsigned ticks);
 int tc_calc_rtable(struct tc_ratespec *r, __u32 *rtab,
 		   int cell_log, unsigned mtu, enum link_layer link_layer);
 int tc_calc_size_table(struct tc_sizespec *s, __u16 **stab);
diff --git a/tc/tc_util.c b/tc/tc_util.c
index 8e62a01..0939536 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -143,7 +143,7 @@ static const struct rate_suffix {
 };
 
 
-int get_rate(unsigned *rate, const char *str)
+int get_rate(__u64 *rate, const char *str)
 {
 	char *p;
 	double bps = strtod(str, &p);
@@ -167,7 +167,7 @@ int get_rate(unsigned *rate, const char *str)
 	return -1;
 }
 
-void print_rate(char *buf, int len, __u32 rate)
+void print_rate(char *buf, int len, __u64 rate)
 {
 	double tmp = (double)rate*8;
 	extern int use_iec;
@@ -189,7 +189,7 @@ void print_rate(char *buf, int len, __u32 rate)
 	}
 }
 
-char * sprint_rate(__u32 rate, char *buf)
+char * sprint_rate(__u64 rate, char *buf)
 {
 	print_rate(buf, SPRINT_BSIZE-1, rate);
 	return buf;
diff --git a/tc/tc_util.h b/tc/tc_util.h
index 4f54436..45d5d1a 100644
--- a/tc/tc_util.h
+++ b/tc/tc_util.h
@@ -57,18 +57,18 @@ extern struct qdisc_util *get_qdisc_kind(const char *str);
 extern struct filter_util *get_filter_kind(const char *str);
 
 extern int get_qdisc_handle(__u32 *h, const char *str);
-extern int get_rate(unsigned *rate, const char *str);
+extern int get_rate(__u64 *rate, const char *str);
 extern int get_size(unsigned *size, const char *str);
 extern int get_size_and_cell(unsigned *size, int *cell_log, char *str);
 extern int get_time(unsigned *time, const char *str);
 extern int get_linklayer(unsigned *val, const char *arg);
 
-extern void print_rate(char *buf, int len, __u32 rate);
+extern void print_rate(char *buf, int len, __u64 rate);
 extern void print_size(char *buf, int len, __u32 size);
 extern void print_qdisc_handle(char *buf, int len, __u32 h);
 extern void print_time(char *buf, int len, __u32 time);
 extern void print_linklayer(char *buf, int len, unsigned linklayer);
-extern char * sprint_rate(__u32 rate, char *buf);
+extern char * sprint_rate(__u64 rate, char *buf);
 extern char * sprint_size(__u32 size, char *buf);
 extern char * sprint_qdisc_handle(__u32 h, char *buf);
 extern char * sprint_tc_classid(__u32 h, char *buf);
-- 
1.7.5.3

^ permalink raw reply related

* [PATCH] sch: Rate (in bits/sec) should be u64 so it doesn't overflow at 40Gbit.
From: Vimalkumar @ 2013-03-10  3:24 UTC (permalink / raw)
  To: netdev, eric.dumazet, shemminger, davem, jiri, jhs; +Cc: Vimalkumar


Signed-off-by: Vimalkumar <j.vimal@gmail.com>
---
 include/net/sch_generic.h      |    4 ++--
 include/uapi/linux/gen_stats.h |    2 +-
 include/uapi/linux/pkt_sched.h |   10 +++++-----
 net/sched/sch_generic.c        |    2 +-
 net/sched/sch_hfsc.c           |    8 ++++----
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index f10818f..c9098c8 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -690,9 +690,9 @@ static inline u64 psched_l2t_ns(const struct psched_ratecfg *r,
 	return ((u64)len * r->mult) >> r->shift;
 }
 
-extern void psched_ratecfg_precompute(struct psched_ratecfg *r, u32 rate);
+extern void psched_ratecfg_precompute(struct psched_ratecfg *r, u64 rate);
 
-static inline u32 psched_ratecfg_getrate(const struct psched_ratecfg *r)
+static inline u64 psched_ratecfg_getrate(const struct psched_ratecfg *r)
 {
 	return r->rate_bps >> 3;
 }
diff --git a/include/uapi/linux/gen_stats.h b/include/uapi/linux/gen_stats.h
index 552c8a0..5ca6015 100644
--- a/include/uapi/linux/gen_stats.h
+++ b/include/uapi/linux/gen_stats.h
@@ -33,7 +33,7 @@ struct gnet_stats_basic_packed {
  * @pps: current packet rate
  */
 struct gnet_stats_rate_est {
-	__u32	bps;
+	__u64	bps;
 	__u32	pps;
 };
 
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 32aef0a..d6bc658 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -35,7 +35,7 @@ struct tc_stats {
 	__u32	drops;			/* Packets dropped because of lack of resources */
 	__u32	overlimits;		/* Number of throttle events when this
 					 * flow goes out of allocated bandwidth */
-	__u32	bps;			/* Current flow byte rate */
+	__u64	bps;			/* Current flow byte rate */
 	__u32	pps;			/* Current flow packet rate */
 	__u32	qlen;
 	__u32	backlog;
@@ -79,7 +79,7 @@ struct tc_ratespec {
 	unsigned short	overhead;
 	short		cell_align;
 	unsigned short	mpu;
-	__u32		rate;
+	__u64		rate;
 };
 
 #define TC_RTAB_SIZE	1024
@@ -368,9 +368,9 @@ struct tc_hfsc_qopt {
 };
 
 struct tc_service_curve {
-	__u32	m1;		/* slope of the first segment in bps */
+	__u64	m1;		/* slope of the first segment in bps */
 	__u32	d;		/* x-projection of the first segment in us */
-	__u32	m2;		/* slope of the second segment in bps */
+	__u64	m2;		/* slope of the second segment in bps */
 };
 
 struct tc_hfsc_stats {
@@ -541,7 +541,7 @@ struct tc_netem_corrupt {
 };
 
 struct tc_netem_rate {
-	__u32	rate;	/* byte/s */
+	__u64	rate;	/* byte/s */
 	__s32	packet_overhead;
 	__u32	cell_size;
 	__s32	cell_overhead;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index ffad481..37a178c 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -898,7 +898,7 @@ void dev_shutdown(struct net_device *dev)
 	WARN_ON(timer_pending(&dev->watchdog_timer));
 }
 
-void psched_ratecfg_precompute(struct psched_ratecfg *r, u32 rate)
+void psched_ratecfg_precompute(struct psched_ratecfg *r, u64 rate)
 {
 	u64 factor;
 	u64 mult;
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 9facea0..f06b447 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -419,7 +419,7 @@ seg_y2x(u64 y, u64 ism)
 
 /* Convert m (bps) into sm (bytes/psched us) */
 static u64
-m2sm(u32 m)
+m2sm(u64 m)
 {
 	u64 sm;
 
@@ -431,7 +431,7 @@ m2sm(u32 m)
 
 /* convert m (bps) into ism (psched us/byte) */
 static u64
-m2ism(u32 m)
+m2ism(u64 m)
 {
 	u64 ism;
 
@@ -458,13 +458,13 @@ d2dx(u32 d)
 }
 
 /* convert sm (bytes/psched us) into m (bps) */
-static u32
+static u64
 sm2m(u64 sm)
 {
 	u64 m;
 
 	m = (sm * PSCHED_TICKS_PER_SEC) >> SM_SHIFT;
-	return (u32)m;
+	return m;
 }
 
 /* convert dx (psched us) into d (us) */
-- 
1.7.5.3

^ permalink raw reply related

* [PATCH] print_rate: use knowledge in rate_suffix table for human readable rates.
From: Vimalkumar @ 2013-03-10  3:26 UTC (permalink / raw)
  To: netdev, shemminger, eric.dumazet; +Cc: Vimalkumar


Signed-off-by: Vimalkumar <j.vimal@gmail.com>
---
 tc/tc_util.c |   26 ++++++++++++--------------
 1 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index 0939536..6e68d87 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -139,7 +139,8 @@ static const struct rate_suffix {
 	{ "GBps",	8000000000. },
 	{ "TiBps",	8.*1024.*1024.*1024.*1024. },
 	{ "TBps",	8000000000000. },
-	{ NULL }
+	{ NULL },
+	{ NULL },
 };
 
 
@@ -171,21 +172,18 @@ void print_rate(char *buf, int len, __u64 rate)
 {
 	double tmp = (double)rate*8;
 	extern int use_iec;
+	int start = 0;
+	const struct rate_suffix *s;
 
 	if (use_iec) {
-		if (tmp >= 1000.0*1024.0*1024.0)
-			snprintf(buf, len, "%.0fMibit", tmp/(1024.0*1024.0));
-		else if (tmp >= 1000.0*1024)
-			snprintf(buf, len, "%.0fKibit", tmp/1024);
-		else
-			snprintf(buf, len, "%.0fbit", tmp);
-	} else {
-		if (tmp >= 1000.0*1000000.0)
-			snprintf(buf, len, "%.0fMbit", tmp/1000000.0);
-		else if (tmp >= 1000.0 * 1000.0)
-			snprintf(buf, len, "%.0fKbit", tmp/1000.0);
-		else
-			snprintf(buf, len, "%.0fbit",  tmp);
+		start = 1;
+	}
+
+	snprintf(buf, len, "%.0fbit", tmp);
+	for (s = &suffixes[start]; s->name; s += 2) {
+		if (tmp >= s->scale) {
+			snprintf(buf, len, "%.3f%s", tmp / s->scale, s->name);
+		}
 	}
 }
 
-- 
1.7.5.3

^ permalink raw reply related

* Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Eric Dumazet @ 2013-03-10  4:03 UTC (permalink / raw)
  To: Vimalkumar; +Cc: netdev, shemminger
In-Reply-To: <1362885604-14006-1-git-send-email-j.vimal@gmail.com>

On Sat, 2013-03-09 at 19:20 -0800, Vimalkumar wrote:
> Since rate values are passed around between kernel and tc
> in bytes/sec, 2**32 bytes/sec is around 34Gb/s.  Beyond that
> rate, htb, tbf, hfsc, etc. can never be configured correctly.
> 
> Signed-off-by: Vimalkumar <j.vimal@gmail.com>
> ---
>  include/linux/gen_stats.h |    2 +-
>  include/linux/pkt_sched.h |   10 +++++-----
>  misc/ifstat.c             |    4 ++--
>  tc/m_police.c             |    2 +-
>  tc/q_cbq.c                |    2 +-
>  tc/q_choke.c              |    2 +-
>  tc/q_gred.c               |    2 +-
>  tc/q_hfsc.c               |    6 ++++--
>  tc/q_red.c                |    2 +-
>  tc/tc_cbq.c               |    4 ++--
>  tc/tc_cbq.h               |    4 ++--
>  tc/tc_core.c              |    4 ++--
>  tc/tc_core.h              |    4 ++--
>  tc/tc_util.c              |    6 +++---
>  tc/tc_util.h              |    6 +++---
>  15 files changed, 31 insertions(+), 29 deletions(-)
> 
> diff --git a/include/linux/gen_stats.h b/include/linux/gen_stats.h
> index 552c8a0..5ca6015 100644
> --- a/include/linux/gen_stats.h
> +++ b/include/linux/gen_stats.h
> @@ -33,7 +33,7 @@ struct gnet_stats_basic_packed {
>   * @pps: current packet rate
>   */
>  struct gnet_stats_rate_est {
> -	__u32	bps;
> +	__u64	bps;
>  	__u32	pps;
>  };
>  
> diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
> index 32aef0a..d6bc658 100644
> --- a/include/linux/pkt_sched.h
> +++ b/include/linux/pkt_sched.h
> @@ -35,7 +35,7 @@ struct tc_stats {
>  	__u32	drops;			/* Packets dropped because of lack of resources */
>  	__u32	overlimits;		/* Number of throttle events when this
>  					 * flow goes out of allocated bandwidth */
> -	__u32	bps;			/* Current flow byte rate */
> +	__u64	bps;			/* Current flow byte rate */
>  	__u32	pps;			/* Current flow packet rate */
>  	__u32	qlen;
>  	__u32	backlog;
> @@ -79,7 +79,7 @@ struct tc_ratespec {
>  	unsigned short	overhead;
>  	short		cell_align;
>  	unsigned short	mpu;
> -	__u32		rate;
> +	__u64		rate;
>  };
>  

You cannot do that without breaking user land tools.

Not only tc, but all user applications as well.

We need to support compatibility, before considering adding such
changes.

^ permalink raw reply

* Re: [PATCH] sch: Rate (in bits/sec) should be u64 so it doesn't overflow at 40Gbit.
From: Eric Dumazet @ 2013-03-10  4:06 UTC (permalink / raw)
  To: Vimalkumar; +Cc: netdev, shemminger, davem, jiri, jhs
In-Reply-To: <1362885896-14123-1-git-send-email-j.vimal@gmail.com>

On Sat, 2013-03-09 at 19:24 -0800, Vimalkumar wrote:
> Signed-off-by: Vimalkumar <j.vimal@gmail.com>
> ---
>  include/net/sch_generic.h      |    4 ++--
>  include/uapi/linux/gen_stats.h |    2 +-
>  include/uapi/linux/pkt_sched.h |   10 +++++-----
>  net/sched/sch_generic.c        |    2 +-
>  net/sched/sch_hfsc.c           |    8 ++++----
>  5 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index f10818f..c9098c8 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -690,9 +690,9 @@ static inline u64 psched_l2t_ns(const struct psched_ratecfg *r,
>  	return ((u64)len * r->mult) >> r->shift;
>  }
>  
> -extern void psched_ratecfg_precompute(struct psched_ratecfg *r, u32 rate);
> +extern void psched_ratecfg_precompute(struct psched_ratecfg *r, u64 rate);
>  
> -static inline u32 psched_ratecfg_getrate(const struct psched_ratecfg *r)
> +static inline u64 psched_ratecfg_getrate(const struct psched_ratecfg *r)
>  {
>  	return r->rate_bps >> 3;
>  }
> diff --git a/include/uapi/linux/gen_stats.h b/include/uapi/linux/gen_stats.h
> index 552c8a0..5ca6015 100644
> --- a/include/uapi/linux/gen_stats.h
> +++ b/include/uapi/linux/gen_stats.h
> @@ -33,7 +33,7 @@ struct gnet_stats_basic_packed {
>   * @pps: current packet rate
>   */
>  struct gnet_stats_rate_est {
> -	__u32	bps;
> +	__u64	bps;
>  	__u32	pps;
>  };
>  
> diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
> index 32aef0a..d6bc658 100644
> --- a/include/uapi/linux/pkt_sched.h
> +++ b/include/uapi/linux/pkt_sched.h
> @@ -35,7 +35,7 @@ struct tc_stats {
>  	__u32	drops;			/* Packets dropped because of lack of resources */
>  	__u32	overlimits;		/* Number of throttle events when this
>  					 * flow goes out of allocated bandwidth */
> -	__u32	bps;			/* Current flow byte rate */
> +	__u64	bps;			/* Current flow byte rate */
>  	__u32	pps;			/* Current flow packet rate */
>  	__u32	qlen;
>  	__u32	backlog;
> @@ -79,7 +79,7 @@ struct tc_ratespec {
>  	unsigned short	overhead;
>  	short		cell_align;
>  	unsigned short	mpu;
> -	__u32		rate;
> +	__u64		rate;
>  };
>  

If this was so easy, we would have done that 10 years ago.

This change breaks userland, and thats absolutely not doable.

^ permalink raw reply

* Re: [PATCH] print_rate: use knowledge in rate_suffix table for human readable rates.
From: Eric Dumazet @ 2013-03-10  4:08 UTC (permalink / raw)
  To: Vimalkumar; +Cc: netdev, shemminger
In-Reply-To: <1362885998-14292-1-git-send-email-j.vimal@gmail.com>

On Sat, 2013-03-09 at 19:26 -0800, Vimalkumar wrote:
> Signed-off-by: Vimalkumar <j.vimal@gmail.com>
> ---
>  tc/tc_util.c |   26 ++++++++++++--------------
>  1 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/tc/tc_util.c b/tc/tc_util.c
> index 0939536..6e68d87 100644
> --- a/tc/tc_util.c
> +++ b/tc/tc_util.c
> @@ -139,7 +139,8 @@ static const struct rate_suffix {
>  	{ "GBps",	8000000000. },
>  	{ "TiBps",	8.*1024.*1024.*1024.*1024. },
>  	{ "TBps",	8000000000000. },
> -	{ NULL }
> +	{ NULL },
> +	{ NULL },
>  };
>  

You'll have to explicitly add in the title a way to say its a patch for
iproute2.

[PATCH iproute2] tc:  use knowledge in rate_suffix table for human readable rates.

To avoid the confusion, as netdev is more likely to review kernel patches.

^ permalink raw reply

* Re: [Patch net-next] tunnel: use iptunnel_xmit() again
From: Eric Dumazet @ 2013-03-10  4:29 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, Pravin B Shelar, Stephen Hemminger, David S. Miller
In-Reply-To: <1362883119-4099-1-git-send-email-xiyou.wangcong@gmail.com>

On Sun, 2013-03-10 at 10:38 +0800, Cong Wang wrote:
> From: Cong Wang <xiyou.wangcong@gmail.com>
> 
> With recent patches from Pravin, most tunnels can't use iptunnel_xmit()
> any more, due to ip_select_ident() and skb->ip_summed. But we can just
> move these operations out of iptunnel_xmit(), so that tunnels can
> use it again.
> 
> This by the way fixes a bug in vxlan (missing nf_reset()) for net-next.
> 
> Cc: Pravin B Shelar <pshelar@nicira.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> 
> ---

missing diffstat

otherwise, patch looks good to me

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH RFC] unix: account skb memory to receiving socket's sk_rmem_alloc on sending
From: Eric Dumazet @ 2013-03-10  4:31 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: netdev, yannick, xiyou.wangcong, davem
In-Reply-To: <20130309214333.GI28531@order.stressinduktion.org>

On Sat, 2013-03-09 at 22:43 +0100, Hannes Frederic Sowa wrote:

> I had this patch left from the last net-next submission timeframe. In
> the meantime I did some updates I would love to have a review on. It
> e.g. includes poll handling now.
> 
> This patch lacks documentation updates for max_dgram_qlen. I'll update
> the documentation on the next submission.
> 
> Btw, iproute from current git has the ability to report socket memory for
> unix domain sockets, too. So these changes should be better observable.

I am busy this week attending Netfilter Workshop in Copenhagen.

Do you have a user test program ?

^ permalink raw reply

* Re: [PATCH RFC] unix: account skb memory to receiving socket's sk_rmem_alloc on sending
From: Hannes Frederic Sowa @ 2013-03-10  4:40 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, yannick, xiyou.wangcong, davem
In-Reply-To: <1362889861.4051.15.camel@edumazet-glaptop>

On Sun, Mar 10, 2013 at 05:31:01AM +0100, Eric Dumazet wrote:
> On Sat, 2013-03-09 at 22:43 +0100, Hannes Frederic Sowa wrote:
> 
> > I had this patch left from the last net-next submission timeframe. In
> > the meantime I did some updates I would love to have a review on. It
> > e.g. includes poll handling now.
> > 
> > This patch lacks documentation updates for max_dgram_qlen. I'll update
> > the documentation on the next submission.
> > 
> > Btw, iproute from current git has the ability to report socket memory for
> > unix domain sockets, too. So these changes should be better observable.
> 
> I am busy this week attending Netfilter Workshop in Copenhagen.
> 
> Do you have a user test program ?

I used a couple of perl scripts. I'll bring them in shape and will post them
here, hopefully tomorrow.

Have great fun in Copenhagen!

^ permalink raw reply

* Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Vimal @ 2013-03-10  4:53 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, shemminger
In-Reply-To: <1362888229.4051.2.camel@edumazet-glaptop>

Ok, do you have suggestions on how to do this?  Maybe a better way to
do this would be to introduce an additional "multipler" option for
rates, which is set to 1 as default, so actual rate can be computed as
multipler * rate supplied.

On 9 March 2013 20:03, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2013-03-09 at 19:20 -0800, Vimalkumar wrote:
>> Since rate values are passed around between kernel and tc
>> in bytes/sec, 2**32 bytes/sec is around 34Gb/s.  Beyond that
>> rate, htb, tbf, hfsc, etc. can never be configured correctly.
>>
>> Signed-off-by: Vimalkumar <j.vimal@gmail.com>
>> ---
>>  include/linux/gen_stats.h |    2 +-
>>  include/linux/pkt_sched.h |   10 +++++-----
>>  misc/ifstat.c             |    4 ++--
>>  tc/m_police.c             |    2 +-
>>  tc/q_cbq.c                |    2 +-
>>  tc/q_choke.c              |    2 +-
>>  tc/q_gred.c               |    2 +-
>>  tc/q_hfsc.c               |    6 ++++--
>>  tc/q_red.c                |    2 +-
>>  tc/tc_cbq.c               |    4 ++--
>>  tc/tc_cbq.h               |    4 ++--
>>  tc/tc_core.c              |    4 ++--
>>  tc/tc_core.h              |    4 ++--
>>  tc/tc_util.c              |    6 +++---
>>  tc/tc_util.h              |    6 +++---
>>  15 files changed, 31 insertions(+), 29 deletions(-)
>>
>> diff --git a/include/linux/gen_stats.h b/include/linux/gen_stats.h
>> index 552c8a0..5ca6015 100644
>> --- a/include/linux/gen_stats.h
>> +++ b/include/linux/gen_stats.h
>> @@ -33,7 +33,7 @@ struct gnet_stats_basic_packed {
>>   * @pps: current packet rate
>>   */
>>  struct gnet_stats_rate_est {
>> -     __u32   bps;
>> +     __u64   bps;
>>       __u32   pps;
>>  };
>>
>> diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
>> index 32aef0a..d6bc658 100644
>> --- a/include/linux/pkt_sched.h
>> +++ b/include/linux/pkt_sched.h
>> @@ -35,7 +35,7 @@ struct tc_stats {
>>       __u32   drops;                  /* Packets dropped because of lack of resources */
>>       __u32   overlimits;             /* Number of throttle events when this
>>                                        * flow goes out of allocated bandwidth */
>> -     __u32   bps;                    /* Current flow byte rate */
>> +     __u64   bps;                    /* Current flow byte rate */
>>       __u32   pps;                    /* Current flow packet rate */
>>       __u32   qlen;
>>       __u32   backlog;
>> @@ -79,7 +79,7 @@ struct tc_ratespec {
>>       unsigned short  overhead;
>>       short           cell_align;
>>       unsigned short  mpu;
>> -     __u32           rate;
>> +     __u64           rate;
>>  };
>>
>
> You cannot do that without breaking user land tools.
>
> Not only tc, but all user applications as well.
>
> We need to support compatibility, before considering adding such
> changes.
>
>
>



--
Vimal

^ permalink raw reply

* Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Eric Dumazet @ 2013-03-10  5:05 UTC (permalink / raw)
  To: Vimal; +Cc: netdev, shemminger
In-Reply-To: <CAK3Ji11Mij3Y_sGC6nLS01rxfccZo0SPJmQy-LAmskz1OEdmkQ@mail.gmail.com>

On Sat, 2013-03-09 at 20:53 -0800, Vimal wrote:
> Ok, do you have suggestions on how to do this?  Maybe a better way to
> do this would be to introduce an additional "multipler" option for
> rates, which is set to 1 as default, so actual rate can be computed as
> multipler * rate supplied.

How an old program, in binary form, will automatically knows it has to
change its behavior to use an inexistent field ?

I can use an old distro, and update kernel to upstream kernel, it must
continue to work.

^ permalink raw reply

* Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Eric Dumazet @ 2013-03-10  5:54 UTC (permalink / raw)
  To: Bill Fink; +Cc: Vimal, netdev, shemminger
In-Reply-To: <20130310004904.de508bfa.billfink@mindspring.com>

On Sun, 2013-03-10 at 00:49 -0500, Bill Fink wrote:

> I don't see the problem.  An old program would not know about
> the new multiplier, would thus get the default multiplier of 1,
> and get the same behavior as always, with the same limitation
> of ~34 Gbps.  But someone with a newer tc/kernel could for example
> specify a multiplier of 10, which would then support rates up to
> about 340 Gbps.  It sounds like a reasonable approach to me.

Hopefully, some of us see the problem here and are able to reject
patches before breaking user land.

^ permalink raw reply

* Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
From: Bill Fink @ 2013-03-10  5:49 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Vimal, netdev, shemminger
In-Reply-To: <1362891937.4051.25.camel@edumazet-glaptop>

On Sun, 10 Mar 2013, Eric Dumazet wrote:

> On Sat, 2013-03-09 at 20:53 -0800, Vimal wrote:
> > Ok, do you have suggestions on how to do this?  Maybe a better way to
> > do this would be to introduce an additional "multipler" option for
> > rates, which is set to 1 as default, so actual rate can be computed as
> > multipler * rate supplied.
> 
> How an old program, in binary form, will automatically knows it has to
> change its behavior to use an inexistent field ?
> 
> I can use an old distro, and update kernel to upstream kernel, it must
> continue to work.

I don't see the problem.  An old program would not know about
the new multiplier, would thus get the default multiplier of 1,
and get the same behavior as always, with the same limitation
of ~34 Gbps.  But someone with a newer tc/kernel could for example
specify a multiplier of 10, which would then support rates up to
about 340 Gbps.  It sounds like a reasonable approach to me.

						-Bill

^ permalink raw reply

* Re: [Patch net-next] tunnel: use iptunnel_xmit() again
From: David Miller @ 2013-03-10  7:05 UTC (permalink / raw)
  To: eric.dumazet; +Cc: xiyou.wangcong, netdev, pshelar, stephen
In-Reply-To: <1362889787.4051.14.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 10 Mar 2013 05:29:47 +0100

> On Sun, 2013-03-10 at 10:38 +0800, Cong Wang wrote:
>> From: Cong Wang <xiyou.wangcong@gmail.com>
>> 
>> With recent patches from Pravin, most tunnels can't use iptunnel_xmit()
>> any more, due to ip_select_ident() and skb->ip_summed. But we can just
>> move these operations out of iptunnel_xmit(), so that tunnels can
>> use it again.
>> 
>> This by the way fixes a bug in vxlan (missing nf_reset()) for net-next.
>> 
>> Cc: Pravin B Shelar <pshelar@nicira.com>
>> Cc: Stephen Hemminger <stephen@networkplumber.org>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>> 
>> ---
> 
> missing diffstat
> 
> otherwise, patch looks good to me
> 
> Acked-by: Eric Dumazet <edumazet@google.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next 2/2] bnx2x: use the default NAPI weight
From: Herbert Xu @ 2013-03-10  8:38 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, eilong, jhs, Tom Herbert
In-Reply-To: <1362606777.15793.198.camel@edumazet-glaptop>

On Wed, Mar 06, 2013 at 01:52:57PM -0800, Eric Dumazet wrote:
>
> - BQL (incurring more TX completion rounds and possibility to
> block/unblock a qdisc)
> - ticket spinlocks, and even with the guard of qdisc busylock
> 
> -> we can have a starvation problem.

This only happens in cases where we aren't using multiqueue or
we're using it incorrectly, resulting in TX work from being split
over CPUs.

In that case it's not clear that it is starvation if we keep the
TX processing on one CPU.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH net-next] don't compare skb->network_header with skb->tail
From: Hong Zhiguo @ 2013-03-10  8:42 UTC (permalink / raw)
  To: netdev, davem, stephen; +Cc: zhiguo.hong, Hong Zhiguo

in the case of NET_SKBUFF_DATA_USES_OFFSET(on 64-bit arch),
skb->network_header is just offset over skb->head.

Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
---
 net/core/dev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 90cee5b..378e0b9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1705,7 +1705,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
 			skb_reset_mac_header(skb2);
 
 			if (skb_network_header(skb2) < skb2->data ||
-			    skb2->network_header > skb2->tail) {
+			    skb_network_header(skb2) > skb2->tail) {
 				net_crit_ratelimited("protocol %04x is buggy, dev %s\n",
 						     ntohs(skb2->protocol),
 						     dev->name);
-- 
1.7.10.4

^ permalink raw reply related

* [Patch net-next] ipv6: introduce ip6tunnel_xmit() helper
From: Cong Wang @ 2013-03-10  9:00 UTC (permalink / raw)
  To: netdev; +Cc: Eric Dumazet, Pravin B Shelar, David S. Miller, Cong Wang

From: Cong Wang <xiyou.wangcong@gmail.com>

Similar to iptunnel_xmit(), group these operations into a
helper function.

This by the way fixes the missing u64_stats_update_begin()
and u64_stats_update_end() for 32 bit arch.

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Pravin B Shelar <pshelar@nicira.com>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

---
 include/net/ip6_tunnel.h |   20 ++++++++++++++++++++
 net/ipv6/ip6_gre.c       |   17 +----------------
 net/ipv6/ip6_tunnel.c    |   15 +--------------
 3 files changed, 22 insertions(+), 30 deletions(-)

diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index e03047f..ebdef7f 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -68,4 +68,24 @@ __u16 ip6_tnl_parse_tlv_enc_lim(struct sk_buff *skb, __u8 *raw);
 __u32 ip6_tnl_get_cap(struct ip6_tnl *t, const struct in6_addr *laddr,
 			     const struct in6_addr *raddr);
 
+static inline void ip6tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct net_device_stats *stats = &dev->stats;
+	int pkt_len, err;
+
+	nf_reset(skb);
+	pkt_len = skb->len;
+	err = ip6_local_out(skb);
+
+	if (net_xmit_eval(err) == 0) {
+		struct pcpu_tstats *tstats = this_cpu_ptr(dev->tstats);
+		u64_stats_update_begin(&tstats->syncp);
+		tstats->tx_bytes += pkt_len;
+		tstats->tx_packets++;
+		u64_stats_update_end(&tstats->syncp);
+	} else {
+		stats->tx_errors++;
+		stats->tx_aborted_errors++;
+	}
+}
 #endif
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index e4efffe..6a6ba73 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -667,7 +667,6 @@ static netdev_tx_t ip6gre_xmit2(struct sk_buff *skb,
 	struct net_device_stats *stats = &tunnel->dev->stats;
 	int err = -1;
 	u8 proto;
-	int pkt_len;
 	struct sk_buff *new_skb;
 
 	if (dev->type == ARPHRD_ETHER)
@@ -801,23 +800,9 @@ static netdev_tx_t ip6gre_xmit2(struct sk_buff *skb,
 		}
 	}
 
-	nf_reset(skb);
-	pkt_len = skb->len;
-	err = ip6_local_out(skb);
-
-	if (net_xmit_eval(err) == 0) {
-		struct pcpu_tstats *tstats = this_cpu_ptr(tunnel->dev->tstats);
-
-		tstats->tx_bytes += pkt_len;
-		tstats->tx_packets++;
-	} else {
-		stats->tx_errors++;
-		stats->tx_aborted_errors++;
-	}
-
+	ip6tunnel_xmit(skb, dev);
 	if (ndst)
 		ip6_tnl_dst_store(tunnel, ndst);
-
 	return 0;
 tx_err_link_failure:
 	stats->tx_carrier_errors++;
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index fff83cb..bef3fed 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -955,7 +955,6 @@ static int ip6_tnl_xmit2(struct sk_buff *skb,
 	unsigned int max_headroom = sizeof(struct ipv6hdr);
 	u8 proto;
 	int err = -1;
-	int pkt_len;
 
 	if (!fl6->flowi6_mark)
 		dst = ip6_tnl_dst_check(t);
@@ -1035,19 +1034,7 @@ static int ip6_tnl_xmit2(struct sk_buff *skb,
 	ipv6h->nexthdr = proto;
 	ipv6h->saddr = fl6->saddr;
 	ipv6h->daddr = fl6->daddr;
-	nf_reset(skb);
-	pkt_len = skb->len;
-	err = ip6_local_out(skb);
-
-	if (net_xmit_eval(err) == 0) {
-		struct pcpu_tstats *tstats = this_cpu_ptr(t->dev->tstats);
-
-		tstats->tx_bytes += pkt_len;
-		tstats->tx_packets++;
-	} else {
-		stats->tx_errors++;
-		stats->tx_aborted_errors++;
-	}
+	ip6tunnel_xmit(skb, dev);
 	if (ndst)
 		ip6_tnl_dst_store(t, ndst);
 	return 0;

^ permalink raw reply related

* [PATCH net-next] don't compare skb->network_header with skb->tail
From: Hong Zhiguo @ 2013-03-10  9:15 UTC (permalink / raw)
  To: netdev, davem, stephen; +Cc: zhiguo.hong, Hong Zhiguo

in the case of NET_SKBUFF_DATA_USES_OFFSET(on 64-bit arch),
skb->network_header is just offset over skb->head.

Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
---
 net/core/dev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 90cee5b..378e0b9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1705,7 +1705,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
 			skb_reset_mac_header(skb2);
 
 			if (skb_network_header(skb2) < skb2->data ||
-			    skb2->network_header > skb2->tail) {
+			    skb_network_header(skb2) > skb2->tail) {
 				net_crit_ratelimited("protocol %04x is buggy, dev %s\n",
 						     ntohs(skb2->protocol),
 						     dev->name);
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCHv2 net-next 01/15] net: add skb_dst_set_unref
From: David Miller @ 2013-03-10  9:17 UTC (permalink / raw)
  To: ja; +Cc: horms, lvs-devel, netdev
In-Reply-To: <1362863815-5898-2-git-send-email-ja@ssi.bg>

From: Julian Anastasov <ja@ssi.bg>
Date: Sat,  9 Mar 2013 23:16:41 +0200

> 	skb_dst_set_unref will use noref version even for
> DST_NOCACHE entries because DST_NOCACHE means dst is not
> cached in routing structures, still dst could be cached
> by routing users and used to produce noref instances.
> 
> Signed-off-by: Julian Anastasov <ja@ssi.bg>

I'm fine with this approach, but I think the name of this
interface could be better.

In fact you could do something like:

1) Rename skb_dst_set_noref() to __skb_dst_set_noref() and add
   a new "bool force" parameter.  DST_NOCACHE check is overriden
   when 'force' is true.

2) skb_dst_set_noref() is an inline that passes 'force' as false.

3) New interface skb_dst_set_noref_force() passes 'force' as true
   and will be used by your IPVS changes.

Then all of the RCU checks etc. happen in one shared function.

^ permalink raw reply

* Re: [PATCH 0/3] net: netlink info leak fixes
From: David Miller @ 2013-03-10  9:20 UTC (permalink / raw)
  To: minipli; +Cc: netdev
In-Reply-To: <1362844341-12591-1-git-send-email-minipli@googlemail.com>

From: Mathias Krause <minipli@googlemail.com>
Date: Sat,  9 Mar 2013 16:52:18 +0100

> a few more info leak fixes -- this time in rtnl.
> 
> Please apply!

All applied and queued up for -stable, thanks!

^ permalink raw reply

* Re: [PATCH net-next] don't compare skb->network_header with skb->tail
From: Eric Dumazet @ 2013-03-10  9:48 UTC (permalink / raw)
  To: Hong Zhiguo; +Cc: netdev, davem, stephen, zhiguo.hong
In-Reply-To: <1362906925-45308-1-git-send-email-honkiko@gmail.com>

On Sun, 2013-03-10 at 17:15 +0800, Hong Zhiguo wrote:
> in the case of NET_SKBUFF_DATA_USES_OFFSET(on 64-bit arch),
> skb->network_header is just offset over skb->head.
> 
> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
> ---
>  net/core/dev.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 90cee5b..378e0b9 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1705,7 +1705,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
>  			skb_reset_mac_header(skb2);
>  
>  			if (skb_network_header(skb2) < skb2->data ||
> -			    skb2->network_header > skb2->tail) {
> +			    skb_network_header(skb2) > skb2->tail) {
>  				net_crit_ratelimited("protocol %04x is buggy, dev %s\n",
>  						     ntohs(skb2->protocol),
>  						     dev->name);

Sorry I dont understand this patch.

Could you elaborate ?

^ permalink raw reply

* [PATCH net-next] fix buf of assigning skb->tail to network_header
From: Hong Zhiguo @ 2013-03-10  9:48 UTC (permalink / raw)
  To: netdev, davem, stephen; +Cc: zhiguo.hong, Hong Zhiguo

in the case of NET_SKBUFF_DATA_USES_OFFSET,  direct pointer
assignment to skb->network_header is a dangerous bug.

Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
---
 net/ipv4/ipmr.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 5f95b3a..553409b 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -980,7 +980,7 @@ static int ipmr_cache_report(struct mr_table *mrt,
 
 	/* Copy the IP header */
 
-	skb->network_header = skb->tail;
+	skb_set_network_header(skb, skb->tail - skb->data);
 	skb_put(skb, ihl);
 	skb_copy_to_linear_data(skb, pkt->data, ihl);
 	ip_hdr(skb)->protocol = 0;	/* Flag to the kernel this is a route add */
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH net-next] fix buf of assigning skb->tail to network_header
From: Eric Dumazet @ 2013-03-10  9:58 UTC (permalink / raw)
  To: Hong Zhiguo; +Cc: netdev, davem, stephen, zhiguo.hong
In-Reply-To: <1362908909-45697-1-git-send-email-honkiko@gmail.com>

On Sun, 2013-03-10 at 17:48 +0800, Hong Zhiguo wrote:
> in the case of NET_SKBUFF_DATA_USES_OFFSET,  direct pointer
> assignment to skb->network_header is a dangerous bug.

But skb->tail is not a pointer.

Really you need to explain more than this confusing changelog

^ permalink raw reply

* Re: [net-next (TAKE 2) 0/4] IPv6 over Firewire
From: Stephan Gatzka @ 2013-03-10 10:03 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev, linux1394-devel, davem, stefanr
In-Reply-To: <51310507.7030904@gmail.com>

O.k., I rechecked all checksums send by IPv6 over FireWire from Linux
and from MacOS.

The checksums from Yoshis implementation are correct, MacOS is wrong.

What I don't understand is that using my proof of concept patch. I could
ping6 MacOSX from my Linux box. Even with CHECKSUM_UNNECESSARY MacOSX
should also throw away the packets send from Linux. Hm, I have to
recheck this.

Regards,

Stephan

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox