Netdev List
 help / color / mirror / Atom feed
* BUG?  behaviour mismatch between ipv4 and ipv6 in UDP rx path
From: Chris Friesen @ 2011-02-07 18:29 UTC (permalink / raw)
  To: netdev, Linux Kernel Mailing List, pekkas, davem, yoshfuji


Hi,

One of our guys is seeing occasional dropped ipv4 packets coming in on
an ipv6 udp socket obtained via socket(AF_INET6,  SOCK_DGRAM, IPPROTO_UDP).

Here's what he says:


"The problem happens when release_sock() goes down an interesting code
path.  If (sk->sk_backlog.tail) is non-NULL then release_sock() invokes
__release_sock() which loops over all queue packets and invokes the
socket's backlog receive function for each previously queued packet.

Now for the interesting part.  The UDPv6 backlog receive function (in
net/ipv6/udp.c, udpv6_queue_rcv_skb()) invokes xfrm6_policy_check() to
confirm that the packet is allowed, but the problem is that it calls
this function regardless of whether the packet is IPv4 or IPv6.  The
xfrm6_policy_check() function then assumes that it is an IPv6 packet and
tries to match a policy based on its packet header... but that clearly
won't work because the addresses that it finds when it decodes the skb
are completely bogus."


Looking at the ipv4 code, git commit 9382177 split __udp_queue_rcv_skb()
out of udp_queue_rcv_skb().  It was done for locking purposes, but it
also means that backlog_rcv is bound to __udp_queue_rcv_skb(), which
doesn't call xfrm4_policy_check().


Should a new function __udpv6_queue_rcv_skb() be split out from
udpv6_queue_rcv_skb() and bound to backlog_rcv to resolve the xfrm
issue?  What about the locking that was the reason for the split in the
ipv4 case--is there a similar problem with ipv6?

Thanks,
Chris


-- 
Chris Friesen
Software Developer
GENBAND
chris.friesen@genband.com
www.genband.com

^ permalink raw reply

* RE: [net-next-2.6 2/3] igb: add support for VF Transmit rate limit using iproute2
From: Levy, Lior @ 2011-02-07 18:08 UTC (permalink / raw)
  To: David Miller, Kirsher, Jeffrey T
  Cc: netdev@vger.kernel.org, gospo@redhat.com, bphilips@novell.com
In-Reply-To: <20110128.163817.193721063.davem@davemloft.net>

Thank you Dave for your input.

I agree that there isn't a very good reason to log these messages.
People can use the "ip show" command to query the TX rate limit for each vf.

BTW - this feature is being configured via iproute2 and not ethtool. But still the concept is the same.

We'll re-submit the patch soon.
 
Regards,
Lior.

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of David Miller
Sent: Friday, January 28, 2011 4:38 PM
To: Kirsher, Jeffrey T
Cc: Levy, Lior; netdev@vger.kernel.org; gospo@redhat.com; bphilips@novell.com
Subject: Re: [net-next-2.6 2/3] igb: add support for VF Transmit rate limit using iproute2

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 28 Jan 2011 04:29:38 -0800

> +	if (tx_rate != 0)
> +		dev_info(&adapter->pdev->dev,
> +		         "Setting Transmit rate of %d Mbps for VF %d\n",
> +		         tx_rate, vf);
> +	else
> +		dev_info(&adapter->pdev->dev,
> +		         "Transmit rate limit for VF %d is disabled\n", vf);

If you're going to print this, use netdev_info(netdev, ...).

But I think you shouldn't be logging anything at all.

No other ethtool operation logs what it did except in extremely
exceptional error conditions.  And there is nothing special
about this VF rate limiting ethtool operation to justify these
extraneous logging messages.

If people want to know if the VF is rate limited, and by how much,
then can query the configuration using ethtool.
--
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: About bittiming calculation result
From: Wolfgang Grandegger @ 2011-02-07 15:52 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4D4FDEF9.2030305-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

On 02/07/2011 01:00 PM, Wolfgang Grandegger wrote:
> Hi Tomoya,
> 
> On 02/07/2011 12:38 PM, Tomoya MORINAGA wrote:
>> Hi,
>>
>> I have a question for bittiming-value calculated by Can-core.
>>
>> In case setting like below,
>>  - ip link set can0 type can bitrate 800000
>>  - clock=50MHz
>>  - Use pch_can
>>
>> Can-core calculates like below
>> brp=21
>> seg1=1
>> seg2=1
>> sjw=1
>> prop_seg=0
>>
>> Is "prop_seg=0" true ?
> 
> Well, only prop_seg+phase_seg=tseg1 is relevant and the pch_can driver
> sets the allowed minimum "tseg1_min1" currently to 1:
> 
> static struct can_bittiming_const pch_can_bittiming_const = {
>         .name = KBUILD_MODNAME,
>         .tseg1_min = 1,
>         .tseg1_max = 16,
>         .tseg2_min = 1,
>         .tseg2_max = 8,
>         .sjw_max = 4,
>         .brp_min = 1,
>         .brp_max = 1024, /* 6bit + extended 4bit */
>         .brp_inc = 1,
> };
> 
>> seg1/seg2/sjw/prop_seg must be more than 1 ?
> 
> Then "tseg1_min" should be set to *2*.
> 
>> Also I can see the following kernel error log.
>> bitrate error 0.7%
> 
> A clock frequency of 50 MHz is sub-optimal for CAN and some
> bit-rates cannot be reproduced properly. Here is the output of
> the can-utils program "can-calc-bit-timing" (with an entry for
> the pch-can added):
> 
> $ ./can-calc-bit-timing pch-can
> Bit timing parameters for pch-can with 50.000000 MHz ref clock
> nominal                                 real Bitrt   nom  real SampP
> Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error CNF1 CNF2 CNF3
> 1000000    100   3    3    3   1   5 1000000  0.0% 75.0% 70.0%  6.7% 0x05 0x92 0x02
>  800000    420   0    1    1   1  21  793650  0.8% 80.0% 66.6% 16.8% 0x15 0xff 0x00
>  500000    100   8    8    3   1   5  500000  0.0% 87.5% 85.0%  2.9% 0x05 0xbf 0x02
>  250000    500   3    3    1   1  25  250000  0.0% 87.5% 87.5%  0.0% 0x19 0x92 0x00
>  125000    500   6    7    2   1  25  125000  0.0% 87.5% 87.5%  0.0% 0x19 0xb5 0x01
>  100000    500   8    8    3   1  25  100000  0.0% 87.5% 85.0%  2.9% 0x19 0xbf 0x02
>   50000   2500   3    3    1   1 125   50000  0.0% 87.5% 87.5%  0.0% 0x7d 0x92 0x00
>   20000   2500   8    8    3   1 125   20000  0.0% 87.5% 85.0%  2.9% 0x7d 0xbf 0x02
>   10000  12500   3    3    1   1 625   10000  0.0% 87.5% 87.5%  0.0% 0x71 0x92 0x00
> 
> As you can see, especially 800000 gives rather bad results.

BTW, it's always possible to specify optimized bit-timing parameters
directly, e.g. the following seem better:

   800000     60  12    4    4   4   3  793650  0.8% 80.0% 81.0%  1.2%

You could set these with:

  $ ip link set can0 type can \
    tq 60 prop-seg 12 phase-seg1 4 phase-seg2 4 sjw 4

Wolfgang.

^ permalink raw reply

* Re: 6to4 NAT ?
From: Matthias Urlichs @ 2011-02-07 13:55 UTC (permalink / raw)
  To: Jasper Spaans; +Cc: netdev
In-Reply-To: <20110207124138.GW3731@deee.intern.smurf.noris.de>

Hello,

> Sounds to me like you're looking for nat64. No built in support is
> available afaik, but http://ecdysis.viagenie.ca/ provides a netfilter
> module.

Alternately, there seems to be a stateless program which does the same
thing via a TUN interface:

http://www.litech.org/tayga/

This solutions seems superior to the ecdysis kernel module because
* stateless
* no out-of-tree kernel code with concurrency issues
* while it's userland code, I don't expect to have much traffic
* it can support multiple IPv4 networks on one system
  (we have customers with overlapping RFC1912 address ranges)
* more flexible deployment

-- 
-- Matthias Urlichs

^ permalink raw reply

* Re: 2.6.37 regression: adding main interface to a bridge breaks vlan interface RX
From: Nicolas de Pesloüan @ 2011-02-07 12:09 UTC (permalink / raw)
  To: chriss; +Cc: netdev, Simon Arlott
In-Reply-To: <loom.20110206T203347-475@post.gmane.org>

Le 06/02/2011 20:37, chriss a écrit :
> Jesse Gross<jesse<at>  nicira.com>  writes:
>
>>
>> You should either attached a bridge to a vlan device or stack a vlan
>> device on the bridge port depending on what you are trying to achieve.
>>
>> The bridge handler takes all packets of the device that it is attached
>> to, so having a vlan also attached to the same device will not work.
>> You may get different results on older kernels depending on what NIC
>> you were using but that was a bug.
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo<at>  vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>
>
> Hi and thanks for your reply
>
> I tried that.
>
> I had under 2.6.36.3 following combination:
>
> eth1 in br0 ->  all traffic without vlan id
> eth1.3 ->  traffic with vlan 3
>
> with 2.6.37 i should do the following
> eth1 in br0 ->  traffic w/o vlans
> and br0.3 ->  traffic with vlan id 3
>
> But still there is no rx for vlan 3.
> Any suggestions? Or am i missing something?

I think you should have a look at ebtables command, in particular, the BROUTING chain of broute 
table. If this chain ask the packet to be dropped, then bridge will ignore it and give a chance to 
the upper layer to use it. Upper layer might be IP, or in your particular setup, VLAN.

HTH,

	Nicolas.



In order for eth1.3 to receive the traffic with VLANID=3, while having the bridge get everything 
else, you should add an entry to the

^ permalink raw reply

* Re: About bittiming calculation result
From: Wolfgang Grandegger @ 2011-02-07 12:00 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5009516791F146C49C73FAC57C437313-c0cKtqp5df7I9507bXv2FdBPR1lH4CV8@public.gmane.org>

Hi Tomoya,

On 02/07/2011 12:38 PM, Tomoya MORINAGA wrote:
> Hi,
> 
> I have a question for bittiming-value calculated by Can-core.
> 
> In case setting like below,
>  - ip link set can0 type can bitrate 800000
>  - clock=50MHz
>  - Use pch_can
> 
> Can-core calculates like below
> brp=21
> seg1=1
> seg2=1
> sjw=1
> prop_seg=0
> 
> Is "prop_seg=0" true ?

Well, only prop_seg+phase_seg=tseg1 is relevant and the pch_can driver
sets the allowed minimum "tseg1_min1" currently to 1:

static struct can_bittiming_const pch_can_bittiming_const = {
        .name = KBUILD_MODNAME,
        .tseg1_min = 1,
        .tseg1_max = 16,
        .tseg2_min = 1,
        .tseg2_max = 8,
        .sjw_max = 4,
        .brp_min = 1,
        .brp_max = 1024, /* 6bit + extended 4bit */
        .brp_inc = 1,
};

> seg1/seg2/sjw/prop_seg must be more than 1 ?

Then "tseg1_min" should be set to *2*.

> Also I can see the following kernel error log.
> bitrate error 0.7%

A clock frequency of 50 MHz is sub-optimal for CAN and some
bit-rates cannot be reproduced properly. Here is the output of
the can-utils program "can-calc-bit-timing" (with an entry for
the pch-can added):

$ ./can-calc-bit-timing pch-can
Bit timing parameters for pch-can with 50.000000 MHz ref clock
nominal                                 real Bitrt   nom  real SampP
Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error CNF1 CNF2 CNF3
1000000    100   3    3    3   1   5 1000000  0.0% 75.0% 70.0%  6.7% 0x05 0x92 0x02
 800000    420   0    1    1   1  21  793650  0.8% 80.0% 66.6% 16.8% 0x15 0xff 0x00
 500000    100   8    8    3   1   5  500000  0.0% 87.5% 85.0%  2.9% 0x05 0xbf 0x02
 250000    500   3    3    1   1  25  250000  0.0% 87.5% 87.5%  0.0% 0x19 0x92 0x00
 125000    500   6    7    2   1  25  125000  0.0% 87.5% 87.5%  0.0% 0x19 0xb5 0x01
 100000    500   8    8    3   1  25  100000  0.0% 87.5% 85.0%  2.9% 0x19 0xbf 0x02
  50000   2500   3    3    1   1 125   50000  0.0% 87.5% 87.5%  0.0% 0x7d 0x92 0x00
  20000   2500   8    8    3   1 125   20000  0.0% 87.5% 85.0%  2.9% 0x7d 0xbf 0x02
  10000  12500   3    3    1   1 625   10000  0.0% 87.5% 87.5%  0.0% 0x71 0x92 0x00

As you can see, especially 800000 gives rather bad results.

Wolfgang.

^ permalink raw reply

* About bittiming calculation result
From: Tomoya MORINAGA @ 2011-02-07 11:38 UTC (permalink / raw)
  To: wg, socketcan-core; +Cc: netdev, linux-kernel
In-Reply-To: <20110204.130649.112613896.davem@davemloft.net>

Hi,

I have a question for bittiming-value calculated by Can-core.

In case setting like below,
 - ip link set can0 type can bitrate 800000
 - clock=50MHz
 - Use pch_can

Can-core calculates like below
brp=21
seg1=1
seg2=1
sjw=1
prop_seg=0

Is "prop_seg=0" true ?
seg1/seg2/sjw/prop_seg must be more than 1 ?

Also I can see the following kernel error log.
bitrate error 0.7%

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

^ permalink raw reply

* Re: 6to4 NAT ?
From: Jasper Spaans @ 2011-02-07 10:27 UTC (permalink / raw)
  To: Matthias Urlichs; +Cc: netdev@vger.kernel.org
In-Reply-To: <iiofpe$1j4$1@dough.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

On 07/02/11 10:59, Matthias Urlichs wrote:
> The problem: I have a 10.*/8 ipv4 network which I want to reach via ipv6.
> I.e., what I need is some sort of 6to4 NAT so that traffic from [whereever]
> to e.g. fec0:dead:beef:1234::10.1.2.3 ends up as coming from e.g. 10.0.0.1
> on the IPv4 side.

Sounds to me like you're looking for nat64. No built in support is
available afaik, but http://ecdysis.viagenie.ca/ provides a netfilter
module.

Cheers,
Jasper

-- 
Ir. Jasper Spaans
Fox-IT Experts in IT Security!
T: +31 (0) 15 284 79 99
KvK Haaglanden 27301624



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4121 bytes --]

^ permalink raw reply

* Re: x25: possible skb leak on bad facilities
From: Andrew Hendry @ 2011-02-07 10:08 UTC (permalink / raw)
  To: David Miller; +Cc: apw, john, linux-x25, netdev, linux-kernel, tim.gardner
In-Reply-To: <AANLkTik8hg5G2FYDCoRgh72v-_euGWKix9Fxm89UdXie@mail.gmail.com>


Originally x25_parse_facilities returned
-1 for an error
 0 meaning 0 length facilities
>0 the length of the facilities parsed.

5ef41308f94dc introduced more error checking in x25_parse_facilities
however used 0 to indicate bad parsing
a6331d6f9a429 followed this further for DTE facilities, again using 0 for bad parsing.

The meaning of 0 got confused in the callers.
If the facilities are messed up we can't determine where the data starts.
So patch makes all parsing errors return -1 and ensures callers close and don't use the skb further.

Reported-by: Andy Whitcroft <apw@canonical.com>
Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>


---
 net/x25/x25_facilities.c |   28 +++++++++++++++++++---------
 net/x25/x25_in.c         |   14 +++++++++++---
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/net/x25/x25_facilities.c b/net/x25/x25_facilities.c
index 55187c8..4062075 100644
--- a/net/x25/x25_facilities.c
+++ b/net/x25/x25_facilities.c
@@ -27,9 +27,19 @@
 #include <net/sock.h>
 #include <net/x25.h>
 
-/*
- * Parse a set of facilities into the facilities structures. Unrecognised
- *	facilities are written to the debug log file.
+/**
+ * x25_parse_facilities - Parse facilities from skb into the facilities structs
+ *
+ * @skb: sk_buff to parse
+ * @facilities: Regular facilites, updated as facilities are found
+ * @dte_facs: ITU DTE facilities, updated as DTE facilities are found
+ * @vc_fac_mask: mask is updated with all facilities found
+ *
+ * Return codes:
+ *  -1 - Parsing error, caller should drop call and clean up
+ *   0 - Parse OK, this skb has no facilities
+ *  >0 - Parse OK, returns the length of the facilities header
+ *
  */
 int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 		struct x25_dte_facilities *dte_facs, unsigned long *vc_fac_mask)
@@ -62,7 +72,7 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 		switch (*p & X25_FAC_CLASS_MASK) {
 		case X25_FAC_CLASS_A:
 			if (len < 2)
-				return 0;
+				return -1;
 			switch (*p) {
 			case X25_FAC_REVERSE:
 				if((p[1] & 0x81) == 0x81) {
@@ -107,7 +117,7 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 			break;
 		case X25_FAC_CLASS_B:
 			if (len < 3)
-				return 0;
+				return -1;
 			switch (*p) {
 			case X25_FAC_PACKET_SIZE:
 				facilities->pacsize_in  = p[1];
@@ -130,7 +140,7 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 			break;
 		case X25_FAC_CLASS_C:
 			if (len < 4)
-				return 0;
+				return -1;
 			printk(KERN_DEBUG "X.25: unknown facility %02X, "
 			       "values %02X, %02X, %02X\n",
 			       p[0], p[1], p[2], p[3]);
@@ -139,18 +149,18 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 			break;
 		case X25_FAC_CLASS_D:
 			if (len < p[1] + 2)
-				return 0;
+				return -1;
 			switch (*p) {
 			case X25_FAC_CALLING_AE:
 				if (p[1] > X25_MAX_DTE_FACIL_LEN || p[1] <= 1)
-					return 0;
+					return -1;
 				dte_facs->calling_len = p[2];
 				memcpy(dte_facs->calling_ae, &p[3], p[1] - 1);
 				*vc_fac_mask |= X25_MASK_CALLING_AE;
 				break;
 			case X25_FAC_CALLED_AE:
 				if (p[1] > X25_MAX_DTE_FACIL_LEN || p[1] <= 1)
-					return 0;
+					return -1;
 				dte_facs->called_len = p[2];
 				memcpy(dte_facs->called_ae, &p[3], p[1] - 1);
 				*vc_fac_mask |= X25_MASK_CALLED_AE;
diff --git a/net/x25/x25_in.c b/net/x25/x25_in.c
index f729f02..15de65f 100644
--- a/net/x25/x25_in.c
+++ b/net/x25/x25_in.c
@@ -91,10 +91,10 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp
 {
 	struct x25_address source_addr, dest_addr;
 	int len;
+	struct x25_sock *x25 = x25_sk(sk);
 
 	switch (frametype) {
 		case X25_CALL_ACCEPTED: {
-			struct x25_sock *x25 = x25_sk(sk);
 
 			x25_stop_timer(sk);
 			x25->condition = 0x00;
@@ -113,14 +113,16 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp
 						&dest_addr);
 			if (len > 0)
 				skb_pull(skb, len);
+			else if (len < 0)
+				goto out_clear;
 
 			len = x25_parse_facilities(skb, &x25->facilities,
 						&x25->dte_facilities,
 						&x25->vc_facil_mask);
 			if (len > 0)
 				skb_pull(skb, len);
-			else
-				return -1;
+			else if (len < 0)
+				goto out_clear;
 			/*
 			 *	Copy any Call User Data.
 			 */
@@ -144,6 +146,12 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp
 	}
 
 	return 0;
+
+out_clear:
+	x25_write_internal(sk, X25_CLEAR_REQUEST);
+	x25->state = X25_STATE_2;
+	x25_start_t23timer(sk);
+	return 0;
 }
 
 /*
-- 
1.7.1



On Mon, 2011-02-07 at 17:29 +1100, Andrew Hendry wrote:
> The issue is a bit more complex than Andy's patch, I think I have a full fix.
> Burning it in on test system now, if thats OK ill post patch in a few hours.
> 
> 
> On Mon, Feb 7, 2011 at 3:28 PM, David Miller <davem@davemloft.net> wrote:
> > From: Andrew Hendry <andrew.hendry@gmail.com>
> > Date: Tue, 1 Feb 2011 22:55:13 +1100
> >
> >> There are two callers, when I was crashing it I don't remember it
> >> using the backlog path.
> >> x25_process_rx_frame is called from both x25_backlog_rcv and also
> >> x25_receive_data (via x25_lapb_receive_frame)
> >>
> >> But reviewing that second path now it looks like it will also leak, -1
> >> would make it skip the kfree_skb there as well.
> >> So patch looks good to me, when I have some time I'll run it through
> >> the environment I had setup originally to confirm.
> >
> > Andrew, have you had a chance to do this yet?
> >

^ permalink raw reply related

* 6to4 NAT ?
From: Matthias Urlichs @ 2011-02-07  9:59 UTC (permalink / raw)
  To: netdev

Hello,

The problem: I have a 10.*/8 ipv4 network which I want to reach via ipv6.
I.e., what I need is some sort of 6to4 NAT so that traffic from [whereever]
to e.g. fec0:dead:beef:1234::10.1.2.3 ends up as coming from e.g. 10.0.0.1
on the IPv4 side.

"Classic" NAT, in other words, except that (a) one side is on IPv6, and
(b) the destination's v4 address is the tail end of the v6 address.

Cisco boxes can apparently do that. I want my Linux box to do it too. :-P

Is there something like this already out there, or do I need to copy
the NAT kernel modules and start hacking?

(This is not a tunnel: I don't want the IPv6 packets to get encapsulated
in IPv4. That'd require all the systems on the 10.* net to have their own
local tunnel interface. I can't do that.)
-- 
-- Matthias Urlichs


^ permalink raw reply

* Re: [patch] IPVS: precedence bug in ip_vs_sync_switch_mode()
From: Hans Schillstrom @ 2011-02-07  9:34 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Wensong Zhang, Simon Horman, Julian Anastasov, hans.schillstrom,
	Patrick McHardy, David S. Miller, netdev, lvs-devel,
	netfilter-devel, kernel-janitors

>---- Original Message ----
>From: Dan Carpenter <error27@gmail.com>
>To: "Wensong Zhang" <wensong@linux-vs.org>
>Cc: "Simon Horman" <horms@verge.net.au>, "Julian Anastasov" <ja@ssi.bg>, hans.schillstrom@ericsson.com, "Patrick McHardy" <kaber@trash.net>, "David S. Miller" <davem@davemloft.net>, netdev@vger.kernel.org, lvs-devel@vger.kernel.org, netfilter-devel@vger.kernel.org, kernel-janitors@vger.kernel.org
>Sent: Mon, Feb 7, 2011, 9:39 AM
>Subject: [patch] IPVS: precedence bug in ip_vs_sync_switch_mode()
>
>'!' has higher precedence than '&'.  IP_VS_STATE_MASTER is 0x1 so
>the original code is equivelent to if (!ipvs->sync_state) ...
>
Oops,
Thanks
Hans

>Signed-off-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>

>
>diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
>index 2a2a836..d1b7298 100644
>--- a/net/netfilter/ipvs/ip_vs_sync.c
>+++ b/net/netfilter/ipvs/ip_vs_sync.c
>@@ -392,7 +392,7 @@ void ip_vs_sync_switch_mode(struct net *net, int mode)
> {
> 	struct netns_ipvs *ipvs = net_ipvs(net);
> 
>-	if (!ipvs->sync_state & IP_VS_STATE_MASTER)
>+	if (!(ipvs->sync_state & IP_VS_STATE_MASTER))
> 		return;
> 	if (mode == ipvs->sysctl_sync_ver || !ipvs->sync_buff)
> 		return;
>--



^ permalink raw reply

* Re: x25: possible skb leak on bad facilities
From: John Hughes @ 2011-02-07  9:25 UTC (permalink / raw)
  To: Andy Whitcroft
  Cc: Andrew Hendry, David S. Miller, linux-x25, netdev, linux-kernel,
	Tim Gardner
In-Reply-To: <20110131130826.GC16804@shadowen.org>

On 31/01/11 14:08, Andy Whitcroft wrote:
> Looking at the changes introduced in the commit below, we seem to
> introduce an skb leak when a packet with bad facilities are present:
>
>      commit a6331d6f9a4298173b413cf99a40cc86a9d92c37
>      Author: andrew hendry<andrew.hendry@gmail.com>
>      Date:   Wed Nov 3 12:54:53 2010 +0000
>
>          memory corruption in X.25 facilities parsing
>
> If I am understanding things correctly then we trigger a -1 return to
> the main packet dispatch loop, this being non-zero implies that we have
> requeued the skb and it should not be freed.  As it was not requeued,
> I believe the skb is no longer referenced and then is leaked.
>
> Perhaps someone better aquainted with this code could review my analysis
> in the patch leader below.  If accurate I believe we need the patch below
> to resolve this.  If it is not then I suspect a comment is required on
> the -1 return.
>
> Thoughts?
>    
Sadly, after nearly 30 years (1982-2010) we've just closed our last X.25 
line so I can no longer test this.

Sorry.

^ permalink raw reply

* [PATCH] IPVS: precedence bug in ip_vs_sync_switch_mode()
From: Simon Horman @ 2011-02-07  9:19 UTC (permalink / raw)
  To: netdev, netfilter-devel, netfilter, lvs-devel
  Cc: Julian Anastasov, Hans Schillstrom, Dan Carpenter,
	Patrick McHardy, Simon Horman
In-Reply-To: <1297070381-24377-1-git-send-email-horms@verge.net.au>

From: Dan Carpenter <error27@gmail.com>

'!' has higher precedence than '&'.  IP_VS_STATE_MASTER is 0x1 so
the original code is equivelent to if (!ipvs->sync_state) ...

Signed-off-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_sync.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 2a2a836..d1b7298 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -392,7 +392,7 @@ void ip_vs_sync_switch_mode(struct net *net, int mode)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
-	if (!ipvs->sync_state & IP_VS_STATE_MASTER)
+	if (!(ipvs->sync_state & IP_VS_STATE_MASTER))
 		return;
 	if (mode == ipvs->sysctl_sync_ver || !ipvs->sync_buff)
 		return;
-- 
1.7.2.3


^ permalink raw reply related

* [GIT PULL nf-next-2.6] IPVS
From: Simon Horman @ 2011-02-07  9:19 UTC (permalink / raw)
  To: netdev, netfilter-devel, netfilter, lvs-devel
  Cc: Julian Anastasov, Hans Schillstrom, Dan Carpenter,
	Patrick McHardy

Hi Patrick,

please consider pulling 
git://git.kernel.org/pub/scm/linux/kernel/git/horms/lvs-test-2.6.git master
to get a precedence bug fix from Dan Carpenter.

 net/netfilter/ipvs/ip_vs_sync.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

^ permalink raw reply

* Re: [patch] IPVS: precedence bug in ip_vs_sync_switch_mode()
From: Simon Horman @ 2011-02-07  9:09 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Wensong Zhang, Julian Anastasov, hans.schillstrom,
	Patrick McHardy, David S. Miller, netdev, lvs-devel,
	netfilter-devel, kernel-janitors
In-Reply-To: <20110207083855.GD4384@bicker>

On Mon, Feb 07, 2011 at 11:38:55AM +0300, Dan Carpenter wrote:
> '!' has higher precedence than '&'.  IP_VS_STATE_MASTER is 0x1 so
> the original code is equivelent to if (!ipvs->sync_state) ...

Thanks Dan, I'll push this to Patrick ASAP.

For the record, this seems to have been added as part of the
new synchronisation code and as such is only present in
development trees at this time. i.e. its not in .37 nor scheduled for .38.

> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index 2a2a836..d1b7298 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -392,7 +392,7 @@ void ip_vs_sync_switch_mode(struct net *net, int mode)
>  {
>  	struct netns_ipvs *ipvs = net_ipvs(net);
>  
> -	if (!ipvs->sync_state & IP_VS_STATE_MASTER)
> +	if (!(ipvs->sync_state & IP_VS_STATE_MASTER))
>  		return;
>  	if (mode == ipvs->sysctl_sync_ver || !ipvs->sync_buff)
>  		return;
> --
> To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* [patch] IPVS: precedence bug in ip_vs_sync_switch_mode()
From: Dan Carpenter @ 2011-02-07  8:38 UTC (permalink / raw)
  To: Wensong Zhang
  Cc: Simon Horman, Julian Anastasov, hans.schillstrom, Patrick McHardy,
	David S. Miller, netdev, lvs-devel, netfilter-devel,
	kernel-janitors

'!' has higher precedence than '&'.  IP_VS_STATE_MASTER is 0x1 so
the original code is equivelent to if (!ipvs->sync_state) ...

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 2a2a836..d1b7298 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -392,7 +392,7 @@ void ip_vs_sync_switch_mode(struct net *net, int mode)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
-	if (!ipvs->sync_state & IP_VS_STATE_MASTER)
+	if (!(ipvs->sync_state & IP_VS_STATE_MASTER))
 		return;
 	if (mode == ipvs->sysctl_sync_ver || !ipvs->sync_buff)
 		return;

^ permalink raw reply related

* iputils license issue
From: Noah Meyerhans @ 2011-02-07  7:38 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

Hello.  In ac3fe58b (in the iputils git repo on linux-ipv6.org), code
was added to ping6.c that makes use of some routines from OpenSSL's
libcrypto library.  No explicit licensing is attached to this code, so
due to the concerns about mixing GPL code with OpenSSL code, I'm seeking
clarification about the terms under which it may be distributed.  The
original heading for ping6.c indicates that the file is licensed under
the BSD license.  If the intent is for this new code to also be BSD
licensed, could you state this in the copyright statement in ping6.c?
If the intent was for the code to be licensed under the GPL, would you
be willing to grant an exception similar to the example at
http://lists.debian.org/debian-legal/2002/07/msg00454.html

Thanks.
noah


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: x25: possible skb leak on bad facilities
From: Andrew Hendry @ 2011-02-07  6:29 UTC (permalink / raw)
  To: David Miller; +Cc: apw, john, linux-x25, netdev, linux-kernel, tim.gardner
In-Reply-To: <20110206.202824.260090071.davem@davemloft.net>

The issue is a bit more complex than Andy's patch, I think I have a full fix.
Burning it in on test system now, if thats OK ill post patch in a few hours.


On Mon, Feb 7, 2011 at 3:28 PM, David Miller <davem@davemloft.net> wrote:
> From: Andrew Hendry <andrew.hendry@gmail.com>
> Date: Tue, 1 Feb 2011 22:55:13 +1100
>
>> There are two callers, when I was crashing it I don't remember it
>> using the backlog path.
>> x25_process_rx_frame is called from both x25_backlog_rcv and also
>> x25_receive_data (via x25_lapb_receive_frame)
>>
>> But reviewing that second path now it looks like it will also leak, -1
>> would make it skip the kfree_skb there as well.
>> So patch looks good to me, when I have some time I'll run it through
>> the environment I had setup originally to confirm.
>
> Andrew, have you had a chance to do this yet?
>

^ permalink raw reply

* Re: x25: possible skb leak on bad facilities
From: David Miller @ 2011-02-07  4:28 UTC (permalink / raw)
  To: andrew.hendry; +Cc: apw, john, linux-x25, netdev, linux-kernel, tim.gardner
In-Reply-To: <AANLkTik-ywwtNg2AxAXtsKYJviDp=1zCZ-43JXp9i3g6@mail.gmail.com>

From: Andrew Hendry <andrew.hendry@gmail.com>
Date: Tue, 1 Feb 2011 22:55:13 +1100

> There are two callers, when I was crashing it I don't remember it
> using the backlog path.
> x25_process_rx_frame is called from both x25_backlog_rcv and also
> x25_receive_data (via x25_lapb_receive_frame)
> 
> But reviewing that second path now it looks like it will also leak, -1
> would make it skip the kfree_skb there as well.
> So patch looks good to me, when I have some time I'll run it through
> the environment I had setup originally to confirm.

Andrew, have you had a chance to do this yet?

^ permalink raw reply

* Re: 2.6.37 regression: adding main interface to a bridge breaks vlan interface RX
From: chriss @ 2011-02-06 19:37 UTC (permalink / raw)
  To: netdev
In-Reply-To: <AANLkTi=-+1dnh5ab6nMwjwMx20jpirYyaASZ=dk4OExp@mail.gmail.com>

Jesse Gross <jesse <at> nicira.com> writes:

>
> You should either attached a bridge to a vlan device or stack a vlan
> device on the bridge port depending on what you are trying to achieve.
> 
> The bridge handler takes all packets of the device that it is attached
> to, so having a vlan also attached to the same device will not work.
> You may get different results on older kernels depending on what NIC
> you were using but that was a bug.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo <at> vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 


Hi and thanks for your reply

I tried that.

I had under 2.6.36.3 following combination:

eth1 in br0 -> all traffic without vlan id
eth1.3 -> traffic with vlan 3

with 2.6.37 i should do the following
eth1 in br0 -> traffic w/o vlans
and br0.3 -> traffic with vlan id 3

But still there is no rx for vlan 3.
Any suggestions? Or am i missing something?


regards


^ permalink raw reply

* Re: [PATCH net-next] bnx2x: Proper netdev->ndo_set_rx_mode() implementation.
From: David Miller @ 2011-02-06 19:25 UTC (permalink / raw)
  To: vladz; +Cc: netdev, eilong
In-Reply-To: <201102061850.40458.vladz@broadcom.com>

From: "Vlad Zolotarov" <vladz@broadcom.com>
Date: Sun, 6 Feb 2011 18:50:40 +0200

> Completed the bnx2x_set_rx_mode() to a proper netdev->ndo_set_rx_mode 
> implementation: 
>  - Added a missing configuration of a unicast MAC addresses list.
>  - Changed bp->dma_lock from being a mutex to a spinlock as long as it's taken
> under netdev->addr_list_lock now.
> 
> Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-2.6] bnx2x: Duplication in promisc mode
From: David Miller @ 2011-02-06 19:21 UTC (permalink / raw)
  To: vladz; +Cc: netdev, eilong
In-Reply-To: <1296998053.16759.25.camel@lb-tlvb-vladz>

From: "Vladislav Zolotarov" <vladz@broadcom.com>
Date: Sun, 6 Feb 2011 15:14:13 +0200

> Prevent packets duplication for frames targeting FCoE L2 ring:
> packets were arriving to stack from both L2 RSS and from FCoE
> L2 in a promiscuous mode.
> 
> Configure FCoE L2 ring to DROP_ALL rx mode, when interface is
> configured to PROMISC, and to accept only unicast frames, when
> interface is configured to ALL_MULTI.
> 
> Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] bnx2x: MTU for FCoE L2 ring
From: David Miller @ 2011-02-06 19:21 UTC (permalink / raw)
  To: vladz; +Cc: netdev, eilong
In-Reply-To: <1296996562.16759.9.camel@lb-tlvb-vladz>

From: "Vladislav Zolotarov" <vladz@broadcom.com>
Date: Sun, 6 Feb 2011 14:49:22 +0200

> Always configure an FCoE L2 ring with a mini-jumbo MTU size (2500).
> To do that we had to move the rx_buf_size parameter from per
> function level to a per ring level.
> 
> Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>

Applied.

^ permalink raw reply

* Re: [net-next-2.6 PATCH 0/5] enic: updates to version 2.1.1.6
From: David Miller @ 2011-02-06 19:18 UTC (permalink / raw)
  To: vkolluri; +Cc: netdev
In-Reply-To: <20110205021618.8510.34816.stgit@savbu-pc100.cisco.com>

From: Vasanthy Kolluri <vkolluri@cisco.com>
Date: Fri, 04 Feb 2011 18:17:00 -0800

> The following series implements enic driver updates:
> 
> 1/5 - Clean up: Organize devcmd wrapper routines
> 2/5 - Bug Fix: Fix return values of enic_add/del_station_addr routines
> 3/5 - Bug Fix: Reorder firmware devcmds - CMD_INIT and CMD_IG_VLAN_REWRITE_MODE
> 4/5 - Clean up: Remove support for an older version of hardware
> 5/5 - Update MAINTAINERS
> 
> Signed-off-by: Christian Benvenuti <benve@cisco.com>
> Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
> Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
> Signed-off-by: David Wang <dwang2@cisco.com>

These patches do not apply cleanly to current net-next-2.6, please
respin them.

Thanks.

^ permalink raw reply

* Re: [PATCH 4/4] m68k/atari: ARAnyM - Add support for network access
From: David Miller @ 2011-02-06 19:17 UTC (permalink / raw)
  To: geert
  Cc: linux-m68k, linux-kernel, cz-bobek-lists-aranym, schmitz,
	pstehlik, M.Jurik, netdev
In-Reply-To: <1296989469-7844-5-git-send-email-geert@linux-m68k.org>

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Sun,  6 Feb 2011 11:51:09 +0100

> +	dev->trans_start = jiffies;

Device drivers no longer make this operation, the generic code
does it (see net/core/dev.c:dev_hard_start_xmit() and how it
invokes txq_trans_update() on ->ndo_start_xmit() success).

Therefore, please remove this line.

> +	pr_debug(DRV_NAME ": send %d bytes\n", len);

For consistency with other network drivers, add an appropriate CPP
define for "pr_fmt" and use netdev_info(), netdev_debug(), etc.

In situations where a netdev pointer is not available
(ie. pre-register_netdev()), use "dev_*()" instead.

Thanks.

^ 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