Netdev List
 help / color / mirror / Atom feed
* [PATCH net 1/3] openvswitch: Fix bad merge resolution.
From: Jesse Gross @ 2013-08-14 23:19 UTC (permalink / raw)
  To: David Miller; +Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1376522347-39531-1-git-send-email-jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>

git silently included an extra hunk in vport_cmd_set() during
automatic merging. This code is unreachable so it does not actually
introduce a problem but it is clearly incorrect.

Signed-off-by: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
---
 net/openvswitch/datapath.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index f7e3a0d..f2ed760 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -2076,9 +2076,6 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
 	ovs_notify(reply, info, &ovs_dp_vport_multicast_group);
 	return 0;
 
-	rtnl_unlock();
-	return 0;
-
 exit_free:
 	kfree_skb(reply);
 exit_unlock:
-- 
1.8.1.2

^ permalink raw reply related

* [GIT net] Open vSwitch
From: Jesse Gross @ 2013-08-14 23:19 UTC (permalink / raw)
  To: David Miller; +Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA

Three bug fixes that are fairly small either way but resolve obviously
incorrect code. For net/3.11.

The following changes since commit ad81f0545ef01ea651886dddac4bef6cec930092:

  Linux 3.11-rc1 (2013-07-14 15:18:27 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jesse/openvswitch.git fixes

for you to fetch changes up to 36bf5cc66d60868bcc10aff209defed5a7b71c1d:

  openvswitch: Reset tunnel key between input and output. (2013-08-14 15:50:36 -0700)

----------------------------------------------------------------
Jesse Gross (2):
      openvswitch: Fix bad merge resolution.
      openvswitch: Reset tunnel key between input and output.

Pravin B Shelar (1):
      openvswitch: Use correct type while allocating flex array.

 net/openvswitch/actions.c  | 1 +
 net/openvswitch/datapath.c | 3 ---
 net/openvswitch/flow.c     | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

^ permalink raw reply

* Re: Fwd: RFC 6980 on Security Implications of IPv6 Fragmentation with IPv6 Neighbor Discovery
From: Hannes Frederic Sowa @ 2013-08-14 23:06 UTC (permalink / raw)
  To: Fernando Gont; +Cc: netdev
In-Reply-To: <520B3D81.9070506@gont.com.ar>

On Wed, Aug 14, 2013 at 05:19:13AM -0300, Fernando Gont wrote:
> Folks,
> 
> FYI. -- this is an important piece when it comes to First Hop (i.e.,
> "local link") Security.

Thanks for the heads-up, Fernando!

I sketched up a patch to protect the receiving side. I still don't know if I
should make this behaviour default or configurable via a sysctl knob. I really
don't want to break existing installations.

As an extra plus, we now discard packets with nested fragment headers at once.
Those packets should never have been accepted.

Thanks,

  Hannes


diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 77a4784..9ac5047 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -103,6 +103,7 @@ struct inet6_skb_parm {
 #define IP6SKB_FORWARDED	2
 #define IP6SKB_REROUTED		4
 #define IP6SKB_ROUTERALERT	8
+#define IP6SKB_FRAGMENTED      16
 };
 
 #define IP6CB(skb)	((struct inet6_skb_parm*)((skb)->cb))
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 79aa965..5c91616 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1524,6 +1524,9 @@ int ndisc_rcv(struct sk_buff *skb)
 	if (skb_linearize(skb))
 		return 0;
 
+	if (IP6CB(skb)->flags & IP6SKB_FRAGMENTED)
+		return 0;
+
 	msg = (struct nd_msg *)skb_transport_header(skb);
 
 	__skb_push(skb, skb->data - skb_transport_header(skb));
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 790d9f4..1aeb473 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -490,6 +490,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
 	ipv6_hdr(head)->payload_len = htons(payload_len);
 	ipv6_change_dsfield(ipv6_hdr(head), 0xff, ecn);
 	IP6CB(head)->nhoff = nhoff;
+	IP6CB(head)->flags |= IP6SKB_FRAGMENTED;
 
 	/* Yes, and fold redundant checksum back. 8) */
 	if (head->ip_summed == CHECKSUM_COMPLETE)
@@ -524,6 +525,9 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
 	struct net *net = dev_net(skb_dst(skb)->dev);
 	int evicted;
 
+	if (IP6CB(skb)->flags & IP6SKB_FRAGMENTED)
+		goto fail_hdr;
+
 	IP6_INC_STATS_BH(net, ip6_dst_idev(skb_dst(skb)), IPSTATS_MIB_REASMREQDS);
 
 	/* Jumbo payload inhibits frag. header */
@@ -544,6 +548,7 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
 				 ip6_dst_idev(skb_dst(skb)), IPSTATS_MIB_REASMOKS);
 
 		IP6CB(skb)->nhoff = (u8 *)fhdr - skb_network_header(skb);
+		IP6CB(skb)->flags |= IP6SKB_FRAGMENTED;
 		return 1;
 	}
 

^ permalink raw reply related

* Re: [PATCH] r8169: fix invalid register dump
From: Francois Romieu @ 2013-08-14 22:47 UTC (permalink / raw)
  To: Peter Wu; +Cc: netdev, nic_swsd
In-Reply-To: <2045708.ru9COLib4d@al>

Peter Wu <lekensteyn@gmail.com> :
[...]
> Checklist:
> 1. super large regs->len: won't be greater than R8169_REGS_SIZE (256)
> 2. regs->len == 0: 0 < 0 is false, nothing is copied
> 3. regs->len is 1, 2 or 3: i = 0, at most 3 bytes will be copied
> 4. regs->len is 4, i < 4 - 4, skip loop, 0 < regs->len, copy 4
> 5. regs->len is 5, i < 5 - 4, copy; 4 < regs->len, copy 1

Spartan implementation:

	u32 __iomem *ioaddr = tp->mmio_addr;
	u32 *dw = p;
	int i;

	...

	for (i = regs->len; i > 0; i -= 4)
		memcpy_fromio(dw++, ioaddr++, min(i, 4));

or (min() verges on gluttony):

	for (i = regs->len; i >= 4; i -= 4)
		memcpy_fromio(dw++, ioaddr++, 4);

	if (i > 0)
		memcpy_fromio(dw, ioaddr, i);

-- 
Ueimor

^ permalink raw reply

* [PATCH] net: rfkill: Do not ignore errors from regulator_enable()
From: Luis Henriques @ 2013-08-14 22:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: netdev, linux-wireless, David S. Miller, Johannes Berg,
	John W. Linville

Function regulator_enable() may return an error that has to be checked.
This patch changes function rfkill_regulator_set_block() so that it checks
for the return code.  Also, rfkill_data->reg_enabled is set to 'true' only
if there is no error.

This fixes the following compilation warning:

net/rfkill/rfkill-regulator.c:43:20: warning: ignoring return value of 'regulator_enable', declared with attribute warn_unused_result [-Wunused-result]

Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
 net/rfkill/rfkill-regulator.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/rfkill/rfkill-regulator.c b/net/rfkill/rfkill-regulator.c
index d11ac79..cf5b145 100644
--- a/net/rfkill/rfkill-regulator.c
+++ b/net/rfkill/rfkill-regulator.c
@@ -30,6 +30,7 @@ struct rfkill_regulator_data {
 static int rfkill_regulator_set_block(void *data, bool blocked)
 {
 	struct rfkill_regulator_data *rfkill_data = data;
+	int ret = 0;
 
 	pr_debug("%s: blocked: %d\n", __func__, blocked);
 
@@ -40,15 +41,16 @@ static int rfkill_regulator_set_block(void *data, bool blocked)
 		}
 	} else {
 		if (!rfkill_data->reg_enabled) {
-			regulator_enable(rfkill_data->vcc);
-			rfkill_data->reg_enabled = true;
+			ret = regulator_enable(rfkill_data->vcc);
+			if (!ret)
+				rfkill_data->reg_enabled = true;
 		}
 	}
 
 	pr_debug("%s: regulator_is_enabled after set_block: %d\n", __func__,
 		regulator_is_enabled(rfkill_data->vcc));
 
-	return 0;
+	return ret;
 }
 
 static struct rfkill_ops rfkill_regulator_ops = {
-- 
1.8.3.2

^ permalink raw reply related

* Re: [PATCH] drivers/net/ethernet/via/via-velocity.c: update napi implementation
From: Sergei Shtylyov @ 2013-08-14 21:42 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Francois Romieu, roszenrami, dshwatrz, kernel-janitors, netdev,
	linux-kernel, grant.likely, rob.herring
In-Reply-To: <1376490413-12067-1-git-send-email-Julia.Lawall@lip6.fr>

Hello.

On 08/14/2013 06:26 PM, Julia Lawall wrote:

> From: Julia Lawall <Julia.Lawall@lip6.fr>

> Drivers supporting NAPI should use a NAPI-specific function for receiving
> packets.  Hence netif_rx is changed to netif_receive_skb.

> Furthermore netif_napi_del should be used in the probe and remove function
> to clean up the NAPI resource information.

> Thanks to Francois Romieu, David Shwatrz and Rami Rosen for their help on
> this patch.

> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

> ---
> I still don't understand the reason for choosing netif_receive_skb,
> specifically.  This patch is not tested.

    This driver is not the only one with the netif_rx() used with NAPI issue. 
I also overlooked this when converting drivers/net/ethernet/renesas/sh_eth.c 
to NAPI. I'll try to change the call and see if netperf would behave better...

WBR, Sergei

^ permalink raw reply

* Re: TCP Connection teardown seems to violate TCP specification
From: Rick Jones @ 2013-08-14 21:38 UTC (permalink / raw)
  To: Damian Lukowski; +Cc: Yuchung Cheng, netdev
In-Reply-To: <1376515097.9293.6.camel@nexus>

On 08/14/2013 02:18 PM, Damian Lukowski wrote:
> At least in curl, the close() seems to be implicit as it occurs
> at the very end of the trace:
>
> close(3)                                = 0
> exit_group(0)                           = ?
> +++ exited with 0 +++
>
> Nevertheless, shouldn't the stack keep on reading the input
> even if the local application is not interested in it?
> The other side might rely on it, and I've seen webserver logs
> which indicate SSL read errors likely because of this.

Ignoring for a moment the SSL matter, if you get a tutorial on (BSD) 
sockets or a copy of the works of the likes of the late W. Richard 
Stevens you will find that the semantics of close() are such that there 
is no way for the received data to be consumed by an application - it no 
longer has a reference to the socket, so to where can the data go?

If then data arrives, what is TCP to do?  Clearly, from the standpoint 
of TCP, something is amiss.  The local application has indicated it is 
not expecting any more data (by calling close()) and more data has 
arrived.  TCP could discard the data and not ACK it, which would simply 
leave the remote TCP retransmitting until a limit was reached at the 
remote.  TCP could bit-bucket the data and send-back ACKs, but that is 
giving a false sense of "success" to the remote application.  So, TCP 
does the only thing it can do - it sends a Reset (RST) segment back to 
the sender.  This then is an indication that something went wrong - in 
this case there was an application-layer error.  Sadly, TCP has no way 
to say that rather than some other TCP-level error - it has just the one 
RST bit.

If the other side relies on the data it is sending being consumed, and 
the local side close()es, that is an indication of an application-layer 
failure - they did not "handshake" their goodbye correctly.

Also, close() is not the half-close of which you read in the TCP RFC. 
In the BSD Sockets interface (not to be confused with the "sockets" 
mentioned in the RFC) the call one makes to effect a half-close of a 
connection is shutdown(SHUT_WR).  In that case, the application retains 
a reference to the socket, and so can consume data until the remote end 
itself then calls either SHUTDOWN(SHUT_WR) or close() (close() being OK 
here at the remote because the connection is already half-closed so the 
remote isn't going to be receiving any data in the first place).

So, at the client side or more accurately the side initiating TCP 
connection shutdown the sequence should be something like:

1) shutdown(SHUT_WR) - this will cause TCP to send a FINished segment to 
the remote TCP, and the remote TCP will indicate this to the remote 
application via a read return of zero.
2) wait for a read return of zero, perhaps bit bucketing data - this 
read return of zero will indicate that the remote application has gotten 
the notification and has itself closed
3) close()

Now, step two can be ever so slightly problematic - how long to wait?

As for SSL...  I cannot begin to pretend to be an SSL protocol expert, 
but in looking at some traces recently, and at one of the more recent 
RFCs, I think they have a slight hole in their specification.  The 
(current?) spec says that an SSL Close Notify alert message is to be 
sent (exchanged?) when terminating the SSL session.  The specs also say 
that a client can just send the Close Notify and go away. (eg a close - 
implicit or explicit)

Trouble is, the remote side will want to send a Close Notify of its own. 
  If the client has Close Notified and scooted, or even just close()ed 
without a Close Notify sent, the remote/server's Close Notify will hit 
the client's TCP stack and elicit a RST segment.  Even better is when 
there is a statefull firewall between client and server, which then 
closes-off the four-tuple.  If that RST is lost on the way back to the 
server, the server TCP will continue retransmitting the Close Notify 
message, which will hit the firewall and be dropped.  The firewall may 
then log "Hey, I dropped this packet trying to get in" entries for each 
of those retransmissions, which then may cause people looking at said 
logs to become "concerned..."

rick jones

^ permalink raw reply

* [PATCH] net_sched: restore "linklayer atm" handling
From: Jesper Dangaard Brouer @ 2013-08-14 21:47 UTC (permalink / raw)
  To: David S. Miller, Dave Taht, netdev
  Cc: Jesper Dangaard Brouer, Stephen Hemminger

commit 56b765b79 ("htb: improved accuracy at high rates")
broke the "linklayer atm" handling.

 tc class add ... htb rate X ceil Y linklayer atm

The linklayer setting is implemented by modifying the rate table
which is send to the kernel.  No direct parameter were
transferred to the kernel indicating the linklayer setting.

The commit 56b765b79 ("htb: improved accuracy at high rates")
removed the use of the rate table system.

To keep compatible with older iproute2 utils, this patch detects
the linklayer by parsing the rate table.  It also supports future
versions of iproute2 to send this linklayer parameter to the
kernel directly. This is done by using the __reserved field in
struct tc_ratespec, to convey the choosen linklayer option, but
only using the lower 4 bits of this field.

Linklayer detection is limited to speeds below 100Mbit/s, because
at high rates the rtab is gets too inaccurate, so bad that
several fields contain the same values, this resembling the ATM
detect.  Fields even start to contain "0" time to send, e.g. at
1000Mbit/s sending a 96 bytes packet cost "0", thus the rtab have
been more broken than we first realized.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---

 include/net/sch_generic.h      |    9 ++++++++-
 include/uapi/linux/pkt_sched.h |   10 +++++++++-
 net/sched/sch_api.c            |   41 ++++++++++++++++++++++++++++++++++++++++
 net/sched/sch_generic.c        |    1 +
 net/sched/sch_htb.c            |   13 +++++++++++++
 5 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 6eab633..e5ae0c5 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -683,13 +683,19 @@ struct psched_ratecfg {
 	u64	rate_bytes_ps; /* bytes per second */
 	u32	mult;
 	u16	overhead;
+	u8	linklayer;
 	u8	shift;
 };
 
 static inline u64 psched_l2t_ns(const struct psched_ratecfg *r,
 				unsigned int len)
 {
-	return ((u64)(len + r->overhead) * r->mult) >> r->shift;
+	len += r->overhead;
+
+	if (unlikely(r->linklayer == TC_LINKLAYER_ATM))
+		return ((u64)(DIV_ROUND_UP(len,48)*53) * r->mult) >> r->shift;
+
+	return ((u64)len * r->mult) >> r->shift;
 }
 
 extern void psched_ratecfg_precompute(struct psched_ratecfg *r, const struct tc_ratespec *conf);
@@ -700,6 +706,7 @@ static inline void psched_ratecfg_getrate(struct tc_ratespec *res,
 	memset(res, 0, sizeof(*res));
 	res->rate = r->rate_bytes_ps;
 	res->overhead = r->overhead;
+	res->linklayer = (r->linklayer & TC_LINKLAYER_MASK);
 }
 
 #endif
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index dbd71b0..09d62b9 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -73,9 +73,17 @@ struct tc_estimator {
 #define TC_H_ROOT	(0xFFFFFFFFU)
 #define TC_H_INGRESS    (0xFFFFFFF1U)
 
+/* Need to corrospond to iproute2 tc/tc_core.h "enum link_layer" */
+enum tc_link_layer {
+	TC_LINKLAYER_UNAWARE, /* Indicate unaware old iproute2 util */
+	TC_LINKLAYER_ETHERNET,
+	TC_LINKLAYER_ATM,
+};
+#define TC_LINKLAYER_MASK 0x0F /* limit use to lower 4 bits */
+
 struct tc_ratespec {
 	unsigned char	cell_log;
-	unsigned char	__reserved;
+	__u8		linklayer; /* lower 4 bits */
 	unsigned short	overhead;
 	short		cell_align;
 	unsigned short	mpu;
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 281c1bd..51b968d 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -285,6 +285,45 @@ static struct Qdisc_ops *qdisc_lookup_ops(struct nlattr *kind)
 	return q;
 }
 
+/* The linklayer setting were not transferred from iproute2, in older
+ * versions, and the rate tables lookup systems have been dropped in
+ * the kernel. To keep backward compatible with older iproute2 tc
+ * utils, we detect the linklayer setting by detecting if the rate
+ * table were modified.
+ *
+ * For linklayer ATM table entries, the rate table will be aligned to
+ * 48 bytes, thus some table entries will contain the same value.  The
+ * mpu (min packet unit) is also encoded into the old rate table, thus
+ * starting from the mpu, we find low and high table entries for
+ * mapping this cell.  If these entries contain the same value, when
+ * the rate tables have been modified for linklayer ATM.
+ *
+ * This is done by rounding mpu to the nearest 48 bytes cell/entry,
+ * and then roundup to the next cell, calc the table entry one below,
+ * and compare.
+ */
+static __u8 __detect_linklayer(struct tc_ratespec *r, __u32 *rtab)
+{
+	int low       = roundup(r->mpu, 48);
+	int high      = roundup(low+1, 48);
+	int cell_low  = low >> r->cell_log;
+	int cell_high = (high >> r->cell_log) - 1;
+
+	/* rtab is too inaccurate at rates > 100Mbit/s */
+	if ((r->rate > (100000000/8)) || (rtab[0] == 0)) {
+		pr_debug("TC linklayer: Giving up ATM detection\n");
+		return TC_LINKLAYER_ETHERNET;
+	}
+
+	if ((cell_high > cell_low) && (cell_high < 256)
+	    && (rtab[cell_low] == rtab[cell_high])) {
+		pr_debug("TC linklayer: Detected ATM, low(%d)=high(%d)=%u\n",
+			 cell_low, cell_high, rtab[cell_high]);
+		return TC_LINKLAYER_ATM;
+	}
+	return TC_LINKLAYER_ETHERNET;
+}
+
 static struct qdisc_rate_table *qdisc_rtab_list;
 
 struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r, struct nlattr *tab)
@@ -308,6 +347,8 @@ struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r, struct nlattr *ta
 		rtab->rate = *r;
 		rtab->refcnt = 1;
 		memcpy(rtab->data, nla_data(tab), 1024);
+		if (r->linklayer == TC_LINKLAYER_UNAWARE)
+			r->linklayer = __detect_linklayer(r, rtab->data);
 		rtab->next = qdisc_rtab_list;
 		qdisc_rtab_list = rtab;
 	}
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index eeb8276..48be3d5 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -909,6 +909,7 @@ void psched_ratecfg_precompute(struct psched_ratecfg *r,
 	memset(r, 0, sizeof(*r));
 	r->overhead = conf->overhead;
 	r->rate_bytes_ps = conf->rate;
+	r->linklayer = (conf->linklayer & TC_LINKLAYER_MASK);
 	r->mult = 1;
 	/*
 	 * The deal here is to replace a divide by a reciprocal one
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 45e7515..c2178b1 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1329,6 +1329,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
 	struct htb_sched *q = qdisc_priv(sch);
 	struct htb_class *cl = (struct htb_class *)*arg, *parent;
 	struct nlattr *opt = tca[TCA_OPTIONS];
+	struct qdisc_rate_table *rtab = NULL, *ctab = NULL;
 	struct nlattr *tb[TCA_HTB_MAX + 1];
 	struct tc_htb_opt *hopt;
 
@@ -1350,6 +1351,18 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
 	if (!hopt->rate.rate || !hopt->ceil.rate)
 		goto failure;
 
+	/* Keeping backward compatible with rate_table based iproute2 tc */
+	if (hopt->rate.linklayer == TC_LINKLAYER_UNAWARE) {
+		rtab = qdisc_get_rtab(&hopt->rate, tb[TCA_HTB_RTAB]);
+		if (rtab)
+			qdisc_put_rtab(rtab);
+	}
+	if (hopt->ceil.linklayer == TC_LINKLAYER_UNAWARE) {
+		ctab = qdisc_get_rtab(&hopt->ceil, tb[TCA_HTB_CTAB]);
+		if (ctab)
+			qdisc_put_rtab(ctab);
+	}
+
 	if (!cl) {		/* new class */
 		struct Qdisc *new_q;
 		int prio;

^ permalink raw reply related

* Re: [PATCH] r8169: fix invalid register dump
From: Peter Wu @ 2013-08-14 21:31 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev, nic_swsd
In-Reply-To: <20130814195829.GA1613@electric-eye.fr.zoreil.com>

On Wednesday 14 August 2013 21:58:29 Francois Romieu wrote:
> > -     memcpy_fromio(p, tp->mmio_addr, regs->len);
> > +     if (regs->len >= 4) {
> > +             for (i = 0; i < regs->len - 4; i += 4)
> > +                     memcpy_fromio(bytes + i, tp->mmio_addr + i, 4);
> > +     }
> > +     if (i < regs->len)
> 
> Comparison with random stack stuff when regs->len < 4. :o/

Right, let's rm $OLD_PATCH and consider this one.

Checklist:
1. super large regs->len: won't be greater than R8169_REGS_SIZE (256)
2. regs->len == 0: 0 < 0 is false, nothing is copied
3. regs->len is 1, 2 or 3: i = 0, at most 3 bytes will be copied
4. regs->len is 4, i < 4 - 4, skip loop, 0 < regs->len, copy 4
5. regs->len is 5, i < 5 - 4, copy; 4 < regs->len, copy 1

With this I can now say with confidence that I haven't overlooked something
related to integer overflow. You have a very sharp eye, thanks for
catching my mistakes.

Regards,
Peter
---
From: Peter Wu <lekensteyn@gmail.com>

For some reason, my PCIe RTL8111E onboard NIC on a GA-Z68X-UD3H-B3
motherboard reads as FFs when reading from MMIO with a block size
larger than 7. Therefore change to reading blocks of four bytes.

Signed-off-by: Peter Wu <lekensteyn@gmail.com>
---
 drivers/net/ethernet/realtek/r8169.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index b5eb419..19524c0 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1897,12 +1897,19 @@ static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 			     void *p)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
+	char *bytes = p;
+	int i = 0;
 
 	if (regs->len > R8169_REGS_SIZE)
 		regs->len = R8169_REGS_SIZE;
 
 	rtl_lock_work(tp);
-	memcpy_fromio(p, tp->mmio_addr, regs->len);
+	if (regs->len >= 4) {
+		for (; i < regs->len - 4; i += 4)
+			memcpy_fromio(bytes + i, tp->mmio_addr + i, 4);
+	}
+	if (i < regs->len)
+		memcpy_fromio(bytes + i, tp->mmio_addr + i, regs->len - i);
 	rtl_unlock_work(tp);
 }
 
-- 
1.8.3.4

^ permalink raw reply related

* Re: TCP Connection teardown seems to violate TCP specification
From: Damian Lukowski @ 2013-08-14 21:18 UTC (permalink / raw)
  To: Rick Jones; +Cc: Yuchung Cheng, netdev
In-Reply-To: <520BE51F.40208@hp.com>

At least in curl, the close() seems to be implicit as it occurs 
at the very end of the trace:

close(3)                                = 0
exit_group(0)                           = ?
+++ exited with 0 +++

Nevertheless, shouldn't the stack keep on reading the input
even if the local application is not interested in it?
The other side might rely on it, and I've seen webserver logs
which indicate SSL read errors likely because of this.

Kind regards
 Damian

Am Mittwoch, den 14.08.2013, 13:14 -0700 schrieb Rick Jones:
> On 08/14/2013 12:50 PM, Yuchung Cheng wrote:
> > On Wed, Aug 14, 2013 at 12:21 PM, Damian Lukowski
> > <damian@tvk.rwth-aachen.de> wrote:
> >> Hi,
> >>
> >> the TCP specification states that an endpoint has to accept
> >> packets from the other side even after it has half-closed the
> >> connection locally. This does not seem to be the case
> >> under Linux?
> >>
> >> For example, when I wget -O /dev/null https://www.verisign.com/
> >> --max-redir 0, the local TCP stack sends an FIN,ACK close to the end.
> >> However, the webserver has more data to send (Encrypted Alert).
> >> Instead of ACKing the packet, the local side sends a RST.
> >> This seems wrong to me.
> >
> > Which wget are you using? it's not doing half-close (i.e.,
> > shutdown(SHUT_WR)) on my wget version 1.13.4
> 
> I suspect that when Damian system call traces his wget he will find it 
> is doing either:
> 
> close()
> 
> or
> shutdown(SHUT_WR)
> close()
> without waiting for the read return of zero from the remote.
> 
> it may even be just exiting after getting the last bytes of the URL, 
> which would be an implicit close()
> 
> Over the years I've seen examples of all three behaviours.
> 
> rick jones
> 
> >
> >
> >>
> >> Regards
> >>   Damian
> >>
> >> --
> >> 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
> > --
> > 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: [PATCHv2 net-next] 6lowpan: handle only real link-local addresses
From: Alexander Aring @ 2013-08-14 21:17 UTC (permalink / raw)
  To: David Miller
  Cc: alex.bluesman.smirnov, dbaryshkov, linux-zigbee-devel, netdev
In-Reply-To: <20130814.125346.585011183381258117.davem@davemloft.net>

Hi David,

On Wed, Aug 14, 2013 at 12:53:46PM -0700, David Miller wrote:
> From: Alexander Aring <alex.aring@gmail.com>
> Date: Wed, 14 Aug 2013 13:01:07 +0200
> 
> > A link-local address isn't fe80::/10 it's fe80::/64
> > see http://tools.ietf.org/html/rfc4291#section-2.5.6
> > for more details.
> > 
> > Also fix a comment issue "local link" -> "link-local"
> > 
> > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > Reviewed-by: Werner Almesberger <werner@almesberger.net>
> 
> You still haven't addressed my biggest concern.
> 
Sorry, I thought that was the missing "net-next" tag in the subject.

> What order do I apply this patch relative to the other 6?
> 
This patch is a fix for the compression of an address. To make the right
compression on a link-local address.

So this patch has nothing to do for the address uncompression fix
(the patch serie should fix the uncompression of addresses). This is a
bigger patch serie because we make a major rewrite of this function.

The order doesn't matter for this patch. Necessary is that the patch
series will apply in the order 1 to 6. So you can apply this patch
before or afterwards the patch-series.

> Don't make me guess, instead place this patch into the necessary
> place within the series of 6 patches.
> 
I can understand that. This patches only fixes some address behaviour
which is in the current implementation wrong. But there are several
other problems with the current 6LoWPAN implementation.
I don't know which is the right place for these patches, because it is
a big change and the 6LoWPAN implementation is broken anyway.

> I'm not applying any of this until you can submit it properly.
> 
Ok, please let me know what should I do to submit it properly.

Thanks

Regards
Alex

^ permalink raw reply

* [PATCH] ip monitor: Enable prefix labels when monitoring more than 1 object
From: Martin Schwenke @ 2013-08-14 21:05 UTC (permalink / raw)
  To: netdev

Prefix labelling is currently only activated when monitoring "all"
objects.  However, the output can still be confusing when monitoring
more than 1 object, so enable prefix labels in this case.

Signed-off-by: Martin Schwenke <martin@meltin.net>
---
 ip/ipmonitor.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index 86c473e..62401ea 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -146,6 +146,7 @@ int do_ipmonitor(int argc, char **argv)
 	int lprefix=0;
 	int lneigh=0;
 	int lnetconf=0;
+	int num_objects = 0;
 
 	rtnl_close(&rth);
 	ipaddr_reset_filter(1);
@@ -159,24 +160,31 @@ int do_ipmonitor(int argc, char **argv)
 			file = *argv;
 		} else if (matches(*argv, "link") == 0) {
 			llink=1;
+			num_objects++;
 			groups = 0;
 		} else if (matches(*argv, "address") == 0) {
 			laddr=1;
+			num_objects++;
 			groups = 0;
 		} else if (matches(*argv, "route") == 0) {
 			lroute=1;
+			num_objects++;
 			groups = 0;
 		} else if (matches(*argv, "mroute") == 0) {
 			lmroute=1;
+			num_objects++;
 			groups = 0;
 		} else if (matches(*argv, "prefix") == 0) {
 			lprefix=1;
+			num_objects++;
 			groups = 0;
 		} else if (matches(*argv, "neigh") == 0) {
 			lneigh = 1;
+			num_objects++;
 			groups = 0;
 		} else if (matches(*argv, "netconf") == 0) {
 			lnetconf = 1;
+			num_objects++;
 			groups = 0;
 		} else if (strcmp(*argv, "all") == 0) {
 			groups = ~RTMGRP_TC;
@@ -223,6 +231,9 @@ int do_ipmonitor(int argc, char **argv)
 		if (!preferred_family || preferred_family == AF_INET6)
 			groups |= nl_mgrp(RTNLGRP_IPV6_NETCONF);
 	}
+	if (num_objects > 1) {
+		prefix_banner = 1;
+	}
 	if (file) {
 		FILE *fp;
 		fp = fopen(file, "r");
-- 
1.7.10.4


Can be pulled from git://git.ozlabs.org/~martins/iproute2.git/ctdb.git

I'm not on the list so please CC: me on responses...

peace & happiness,
martin

^ permalink raw reply related

* Re: Quick Blind TCP Connection Spoofing with SYN Cookies
From: some one @ 2013-08-14 21:02 UTC (permalink / raw)
  To: Jakob Lell; +Cc: netdev, full-disclosure, oss-security
In-Reply-To: <520A3B4A.1050704@jakoblell.com>


[-- Attachment #1.1: Type: text/plain, Size: 60 bytes --]

Good write up that Jakob and an interesting read.
Thanks ,)

[-- Attachment #1.2: Type: text/html, Size: 81 bytes --]

[-- Attachment #2: Type: text/plain, Size: 200 bytes --]

_______________________________________________
Full-Disclosure - We believe in it.
Charter: http://lists.grok.org.uk/full-disclosure-charter.html
Hosted and sponsored by Secunia - http://secunia.com/

^ permalink raw reply

* Re: [PATCH v2] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
From: Michael S. Tsirkin @ 2013-08-14 20:52 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <520BE700.7020302@redhat.com>

On Wed, Aug 14, 2013 at 04:22:24PM -0400, Vlad Yasevich wrote:
> On 08/14/2013 03:24 PM, Michael S. Tsirkin wrote:
> >On Wed, Aug 14, 2013 at 01:03:40PM -0400, Vlad Yasevich wrote:
> >>When the user turns off IFF_VNET_HDR flag, attempts to change
> >>offload features via TUNSETOFFLOAD do not work.  This could cause
> >>GSO packets to be delivered to the user when the user is
> >>not prepared to handle them.
> >
> >Just to clarify - is there some userspace that actually
> >triggers this?
> 
> Yes.  The configuration that triggers is running windows guest
> which uses non-virtio driver over macvtap.
> 
> The issue is that in non-virtio configuration, libvirt turns off
> IFF_VNET_HDR support in macvtap.  Later, windows guest end up trying
> to change offload capabilities.  Win7 (and others that don't support
> GSO) will try to turn off offloads and that operation will fail.  Thus
> GSO packets will get queued to the socket without vnet hdr support.
> 
> -vlad

OK so I'm guessing the root of the problem is that offloads
default to on for macvtap.

> >
> >
> >>To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
> >>disabled.  Treat any attempt to enable offloads as an error in
> >>this case.
> >>We also need to update the TUN_FEATURES mask to include all checksum
> >>options as the underlying device may have something other then
> >>HW_CSUM set.
> >>
> >>Change since v1:
> >>   - Removed the call to update offloads when IFF_VNET_HDR is turned off.
> >>   - Changed the macvtap version of TUN_OFFLOADS to include all checksum
> >>     offloads since the physical nic may have them set.
> >>   - Treat enabling of offloads without vnet_hdr support as error.
> >>
> >>Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> >>---
> >>  drivers/net/macvtap.c | 12 +++++++-----
> >>  1 file changed, 7 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> >>index a98fb0e..3acfc37 100644
> >>--- a/drivers/net/macvtap.c
> >>+++ b/drivers/net/macvtap.c
> >>@@ -65,7 +65,7 @@ static struct cdev macvtap_cdev;
> >>
> >>  static const struct proto_ops macvtap_socket_ops;
> >>
> >>-#define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
> >>+#define TUN_OFFLOADS (NETIF_F_ALL_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
> >>  		      NETIF_F_TSO6 | NETIF_F_UFO)
> >>  #define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
> >>  /*
> >>@@ -1024,6 +1024,12 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
> >>  	if (!vlan)
> >>  		return -ENOLINK;
> >>
> >>+	/* If the user is trying to set offloads while IFF_VNET_HDR is
> >>+	 * off, report it as an error.
> >>+	 */
> >>+	if (!(q->flags & IFF_VNET_HDR) && arg)
> >>+		return -EINVAL;
> >>+

So now you can disable hdr versus offloads in any order,
but you still must first enable hdr and then the offloads.
Which looks asymmetrical.
Maybe just drop this completely? You can get an invalid
state temporarily but we live with it e.g. for packet sockets.

> >>  	features = vlan->dev->features;
> >>
> >>  	if (arg & TUN_F_CSUM) {
> >>@@ -1155,10 +1161,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
> >>  			    TUN_F_TSO_ECN | TUN_F_UFO))
> >>  			return -EINVAL;
> >>
> >>-		/* TODO: only accept frames with the features that
> >>-			 got enabled for forwarded frames */
> >>-		if (!(q->flags & IFF_VNET_HDR))
> >>-			return  -EINVAL;
> >>  		rtnl_lock();
> >>  		ret = set_offload(q, arg);
> >>  		rtnl_unlock();
> >>--
> >>1.8.1.4

^ permalink raw reply

* Re: TCP Connection teardown seems to violate TCP specification
From: Damian Lukowski @ 2013-08-14 20:46 UTC (permalink / raw)
  To: Yuchung Cheng; +Cc: netdev
In-Reply-To: <CAK6E8=fWOe3eEvw6sC=Da415qGp+wXsm2bpe9Vbbukit9yU8sQ@mail.gmail.com>

Hi,

I've seen this with wget 1.14 and curl 7.30.0 on 3.11-rc5 (Gentoo),
with wget 1.15 and curl 7.26.0 on 3.5.7 (other Gentoo machine),
with wget 1.12 and curl 7.19.7 on 2.6.32 (Centos 6.4),

Regards
 Damian

Am Mittwoch, den 14.08.2013, 12:50 -0700 schrieb Yuchung Cheng:
> On Wed, Aug 14, 2013 at 12:21 PM, Damian Lukowski
> <damian@tvk.rwth-aachen.de> wrote:
> > Hi,
> >
> > the TCP specification states that an endpoint has to accept
> > packets from the other side even after it has half-closed the
> > connection locally. This does not seem to be the case
> > under Linux?
> >
> > For example, when I wget -O /dev/null https://www.verisign.com/
> > --max-redir 0, the local TCP stack sends an FIN,ACK close to the end.
> > However, the webserver has more data to send (Encrypted Alert).
> > Instead of ACKing the packet, the local side sends a RST.
> > This seems wrong to me.
> 
> Which wget are you using? it's not doing half-close (i.e.,
> shutdown(SHUT_WR)) on my wget version 1.13.4
> 
> 
> >
> > Regards
> >  Damian
> >
> > --
> > 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

* ethernet: ti: cpsw/mdio: suspend/resume
From: Daniel Mack @ 2013-08-14 20:46 UTC (permalink / raw)
  To: Vaibhav Bedia, mugunthanvnm; +Cc: netdev, Sebastian Siewior

Hi Mugunthan,

Vaibhav told me that you are working on a patch to add suspend/resume
functions to the cpsw and davinci_mdio driver, but searching my archives
didn't bring up anything.

May I ask whether you have a link to your work, or can I help here in
any way?


Thanks,
Daniel

^ permalink raw reply

* Re: [Patch net-next] net: sync some IP headers with glibc
From: David Miller @ 2013-08-14 20:42 UTC (permalink / raw)
  To: amwang; +Cc: netdev, tmb, libc-alpha, yoshfuji, carlos
In-Reply-To: <1376383054-9184-1-git-send-email-amwang@redhat.com>

From: Cong Wang <amwang@redhat.com>
Date: Tue, 13 Aug 2013 16:37:33 +0800

> @@ -23,31 +23,54 @@
>  
>  /* Standard well-defined IP protocols.  */
>  enum {
> -  IPPROTO_IP = 0,		/* Dummy protocol for TCP		*/
> -  IPPROTO_ICMP = 1,		/* Internet Control Message Protocol	*/

> +  IPPROTO_IP = 0,		/* Dummy protocol for TCP. */

Please don't do unrelated things like reformat comments, it makes the
path much harder to audit.

> -
> +#if __UAPI_DEF_IN6_ADDR
>  struct in6_addr {
>  	union {
>  		__u8		u6_addr8[16];
> +#if __UAPI_DEF_IN6_ADDR
>  		__be16		u6_addr16[8];

The same CPP test twice inside of itself, the second test seems
superfluous.  I bet this second one was supposed to be
__UAPI_DEF_IN6_ADDR_ALT instead of __UAPI_DEF_IN6_ADDR.

> -#define IPPROTO_HOPOPTS		0	/* IPv6 hop-by-hop options	*/
> -#define IPPROTO_ROUTING		43	/* IPv6 routing header		*/
> -#define IPPROTO_FRAGMENT	44	/* IPv6 fragmentation header	*/
> -#define IPPROTO_ICMPV6		58	/* ICMPv6			*/
> -#define IPPROTO_NONE		59	/* IPv6 no next header		*/
> -#define IPPROTO_DSTOPTS		60	/* IPv6 destination options	*/
> -#define IPPROTO_MH		135	/* IPv6 mobility header		*/
> +#if __UAPI_DEF_IPPROTO_V6
> +enum {
> +  IPPROTO_HOPOPTS = 0,		/* IPv6 hop-by-hop options      */

Again, do not reformat things, it's an unrelated change and makes
this patch harder to review.

^ permalink raw reply

* Re: [PATCH net-next] ipip: Add room for user-specified custom header
From: David Miller @ 2013-08-14 20:39 UTC (permalink / raw)
  To: kristian.evensen; +Cc: netdev
In-Reply-To: <1376296778-1983-1-git-send-email-kristian.evensen@gmail.com>

From: Kristian Evensen <kristian.evensen@gmail.com>
Date: Mon, 12 Aug 2013 10:39:38 +0200

> This patch adds support for specifying the size of a custom header to be
> inserted between the two IP headers. The actual content will be inserted later,
> for example by a module that is attached to a hook.
> 
> A use-case for the feature provided by this patch is to ease the implementation
> of custom tunneling protocols. Instead of implementing them directly in the
> kernel and having to look out for changes in the kernel behavior/APIs, a
> tunneling protocol can be implemented as an independent module.
> 
> To avoid breaking user space (by updating ip_tunnel_params), it is only possible
> to specify the custom header length when an IPIP-tunnel is created using
> netlink-messages.
> 
> Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>

Sorry I'm not even going to consider applying this until you post
use-cases for this feature, because right now it just looks like a
random way to facilitate some third party proprietary kernel module
or similar.

^ permalink raw reply

* Re: [PATCH v2] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
From: Vlad Yasevich @ 2013-08-14 20:22 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev
In-Reply-To: <20130814192437.GA20226@redhat.com>

On 08/14/2013 03:24 PM, Michael S. Tsirkin wrote:
> On Wed, Aug 14, 2013 at 01:03:40PM -0400, Vlad Yasevich wrote:
>> When the user turns off IFF_VNET_HDR flag, attempts to change
>> offload features via TUNSETOFFLOAD do not work.  This could cause
>> GSO packets to be delivered to the user when the user is
>> not prepared to handle them.
>
> Just to clarify - is there some userspace that actually
> triggers this?

Yes.  The configuration that triggers is running windows guest
which uses non-virtio driver over macvtap.

The issue is that in non-virtio configuration, libvirt turns off
IFF_VNET_HDR support in macvtap.  Later, windows guest end up trying
to change offload capabilities.  Win7 (and others that don't support
GSO) will try to turn off offloads and that operation will fail.  Thus
GSO packets will get queued to the socket without vnet hdr support.

-vlad
>
>
>> To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
>> disabled.  Treat any attempt to enable offloads as an error in
>> this case.
>> We also need to update the TUN_FEATURES mask to include all checksum
>> options as the underlying device may have something other then
>> HW_CSUM set.
>>
>> Change since v1:
>>    - Removed the call to update offloads when IFF_VNET_HDR is turned off.
>>    - Changed the macvtap version of TUN_OFFLOADS to include all checksum
>>      offloads since the physical nic may have them set.
>>    - Treat enabling of offloads without vnet_hdr support as error.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>>   drivers/net/macvtap.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
>> index a98fb0e..3acfc37 100644
>> --- a/drivers/net/macvtap.c
>> +++ b/drivers/net/macvtap.c
>> @@ -65,7 +65,7 @@ static struct cdev macvtap_cdev;
>>
>>   static const struct proto_ops macvtap_socket_ops;
>>
>> -#define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
>> +#define TUN_OFFLOADS (NETIF_F_ALL_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
>>   		      NETIF_F_TSO6 | NETIF_F_UFO)
>>   #define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
>>   /*
>> @@ -1024,6 +1024,12 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
>>   	if (!vlan)
>>   		return -ENOLINK;
>>
>> +	/* If the user is trying to set offloads while IFF_VNET_HDR is
>> +	 * off, report it as an error.
>> +	 */
>> +	if (!(q->flags & IFF_VNET_HDR) && arg)
>> +		return -EINVAL;
>> +
>>   	features = vlan->dev->features;
>>
>>   	if (arg & TUN_F_CSUM) {
>> @@ -1155,10 +1161,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
>>   			    TUN_F_TSO_ECN | TUN_F_UFO))
>>   			return -EINVAL;
>>
>> -		/* TODO: only accept frames with the features that
>> -			 got enabled for forwarded frames */
>> -		if (!(q->flags & IFF_VNET_HDR))
>> -			return  -EINVAL;
>>   		rtnl_lock();
>>   		ret = set_offload(q, arg);
>>   		rtnl_unlock();
>> --
>> 1.8.1.4

^ permalink raw reply

* RE: Query: Regarding 8211E Phy module support in drivers/net/phy
From: Sharma Bhupesh-B45370 @ 2013-08-14 20:18 UTC (permalink / raw)
  To: 'Giuseppe CAVALLARO'; +Cc: 'netdev@vger.kernel.org'
In-Reply-To: <520B8F2C.5090308@st.com>


> -----Original Message-----
> From: Giuseppe CAVALLARO [mailto:peppe.cavallaro@st.com]
> Sent: Wednesday, August 14, 2013 7:38 PM
> To: Sharma Bhupesh-B45370
> Cc: 'netdev@vger.kernel.org'
> Subject: Re: Query: Regarding 8211E Phy module support in drivers/net/phy
> 
> On 8/2/2013 8:50 PM, Sharma Bhupesh-B45370 wrote:
> > Hi Peppe,
> >
> > This regarding the patch prepared by you for adding RTL8211E support
> > in drivers/net/phy/realtek.c
> >
> > commit ef3d90491a15f0d5cf1ec39a38a45dac6968fb2a
> > Author: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
> > Date:   Wed Jan 23 00:30:03 2013 +0000
> >
> >      net: phy: realtek: add rtl8211e driver
> >
> >      This patch adds the minimal driver to manage the
> >      Realtek RTL8211E 10/100/1000 Transceivers.
> >
> > I see a macro specific to 8211E:
> > +#define        RTL8211E_INER_LINK_STAT 0x10
> >
> > I was having a look at the RTL8211E(G) datasheet and could see the
> > following bit positions for INER register (Address 0x12):
> >
> > [7:1] as Reserved
> > [0] as Jabber Interrupt
> >
> > If I am not wrong a value 0x10 written to the INER register tries to
> > set bit [4] which seems Reserved.
> 
> Hello Sharma
> you are right RTL8211E_INER_LINK_STAT should be 0x400 I will test it
> again (just to find the hw) and prepare/send a new patch.
>

Hi Peppe,

Glad to hear from you. Please test with the h/w and send the relevant patch
as I plan to send a patch supporting a new RTL phy module rebased on top of your patch.

Thanks for your help.
Regards,
Bhupesh

 
> Many thanks
> 
> peppe
> 
> >
> > Or am I missing something here.
> > Please let me know your views on the same.
> >
> > Thanks,
> > Bhupesh
> >
> >
> >
> 

^ permalink raw reply

* Re: TCP Connection teardown seems to violate TCP specification
From: Rick Jones @ 2013-08-14 20:14 UTC (permalink / raw)
  To: Yuchung Cheng; +Cc: Damian Lukowski, netdev
In-Reply-To: <CAK6E8=fWOe3eEvw6sC=Da415qGp+wXsm2bpe9Vbbukit9yU8sQ@mail.gmail.com>

On 08/14/2013 12:50 PM, Yuchung Cheng wrote:
> On Wed, Aug 14, 2013 at 12:21 PM, Damian Lukowski
> <damian@tvk.rwth-aachen.de> wrote:
>> Hi,
>>
>> the TCP specification states that an endpoint has to accept
>> packets from the other side even after it has half-closed the
>> connection locally. This does not seem to be the case
>> under Linux?
>>
>> For example, when I wget -O /dev/null https://www.verisign.com/
>> --max-redir 0, the local TCP stack sends an FIN,ACK close to the end.
>> However, the webserver has more data to send (Encrypted Alert).
>> Instead of ACKing the packet, the local side sends a RST.
>> This seems wrong to me.
>
> Which wget are you using? it's not doing half-close (i.e.,
> shutdown(SHUT_WR)) on my wget version 1.13.4

I suspect that when Damian system call traces his wget he will find it 
is doing either:

close()

or
shutdown(SHUT_WR)
close()
without waiting for the read return of zero from the remote.

it may even be just exiting after getting the last bytes of the URL, 
which would be an implicit close()

Over the years I've seen examples of all three behaviours.

rick jones

>
>
>>
>> Regards
>>   Damian
>>
>> --
>> 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
> --
> 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: [PATCH] r8169: fix invalid register dump
From: Francois Romieu @ 2013-08-14 19:58 UTC (permalink / raw)
  To: Peter Wu; +Cc: netdev, nic_swsd
In-Reply-To: <3465288.NFVErF5E7l@al>

Peter Wu <lekensteyn@gmail.com> :
[...]
> Heh, I forgot about that case. Fixed it in the below patch. By the way,
> have you observed similar behavior on other hardware?

Notwithstanding really old PCI adapters - sometimes - none that I remember of.

[...]
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index b5eb419..d367763 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -1897,12 +1897,19 @@ static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
>  			     void *p)
>  {
>  	struct rtl8169_private *tp = netdev_priv(dev);
> +	char *bytes = p;
> +	int i;
>  
>  	if (regs->len > R8169_REGS_SIZE)
>  		regs->len = R8169_REGS_SIZE;
>  
>  	rtl_lock_work(tp);
> -	memcpy_fromio(p, tp->mmio_addr, regs->len);
> +	if (regs->len >= 4) {
> +		for (i = 0; i < regs->len - 4; i += 4)
> +			memcpy_fromio(bytes + i, tp->mmio_addr + i, 4);
> +	}
> +	if (i < regs->len)

Comparison with random stack stuff when regs->len < 4. :o/

-- 
Ueimor

^ permalink raw reply

* Re: [PATCHv2 net-next] 6lowpan: handle only real link-local addresses
From: David Miller @ 2013-08-14 19:53 UTC (permalink / raw)
  To: alex.aring; +Cc: alex.bluesman.smirnov, dbaryshkov, linux-zigbee-devel, netdev
In-Reply-To: <1376478067-3371-1-git-send-email-alex.aring@gmail.com>

From: Alexander Aring <alex.aring@gmail.com>
Date: Wed, 14 Aug 2013 13:01:07 +0200

> A link-local address isn't fe80::/10 it's fe80::/64
> see http://tools.ietf.org/html/rfc4291#section-2.5.6
> for more details.
> 
> Also fix a comment issue "local link" -> "link-local"
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> Reviewed-by: Werner Almesberger <werner@almesberger.net>

You still haven't addressed my biggest concern.

What order do I apply this patch relative to the other 6?

Don't make me guess, instead place this patch into the necessary
place within the series of 6 patches.

I'm not applying any of this until you can submit it properly.

Thanks.

^ permalink raw reply

* Re: TCP Connection teardown seems to violate TCP specification
From: Yuchung Cheng @ 2013-08-14 19:50 UTC (permalink / raw)
  To: Damian Lukowski; +Cc: netdev
In-Reply-To: <1376508095.4596.12.camel@nexus>

On Wed, Aug 14, 2013 at 12:21 PM, Damian Lukowski
<damian@tvk.rwth-aachen.de> wrote:
> Hi,
>
> the TCP specification states that an endpoint has to accept
> packets from the other side even after it has half-closed the
> connection locally. This does not seem to be the case
> under Linux?
>
> For example, when I wget -O /dev/null https://www.verisign.com/
> --max-redir 0, the local TCP stack sends an FIN,ACK close to the end.
> However, the webserver has more data to send (Encrypted Alert).
> Instead of ACKing the packet, the local side sends a RST.
> This seems wrong to me.

Which wget are you using? it's not doing half-close (i.e.,
shutdown(SHUT_WR)) on my wget version 1.13.4


>
> Regards
>  Damian
>
> --
> 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: [patch] rtnetlink: remove an unneeded test
From: Eric W. Biederman @ 2013-08-14 19:35 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David S. Miller, John Fastabend, Jiri Pirko, Vlad Yasevich,
	netdev, kernel-janitors
In-Reply-To: <20130814093542.GC521@elgon.mountain>

Dan Carpenter <dan.carpenter@oracle.com> writes:

> We know that "dev" is a valid pointer at this point, so we can remove
> the test and clean up a little.

Quite right.

Reviewed-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 618dbe4..e7e058e 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1867,10 +1867,10 @@ replay:
>  		else
>  			err = register_netdevice(dev);
>  
> -		if (err < 0 && !IS_ERR(dev))
> +		if (err < 0) {
>  			free_netdev(dev);
> -		if (err < 0)
>  			goto out;
> +		}
>  
>  		err = rtnl_configure_link(dev, ifm);
>  		if (err < 0)

^ 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