Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] vlan: Do not support clearing VLAN_FLAG_REORDER_HDR
From: Ben Greear @ 2011-05-23 22:23 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: David Miller, shemminger, nicolas.2p.debian, jpirko, xiaosuo,
	netdev, kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <m1y61x2htp.fsf@fess.ebiederm.org>

On 05/23/2011 03:05 PM, Eric W. Biederman wrote:
> David Miller<davem@davemloft.net>  writes:
>
>> From: Stephen Hemminger<shemminger@linux-foundation.org>
>> Date: Mon, 23 May 2011 14:00:48 -0700
>>
>>> IMHO the REORDER_HDR flag was a design mistake. It means supporting
>>> two different API's for the application. For a packet capture application
>>> it means the format of the packet will be different based on how
>>> the VLAN was created. And the vlan was created outside the application.
>>>
>>> If there was a requirement to support both formats, then it should
>>> be a property of the AF_PACKET socket.  The application can then do
>>> an setsockopt() to control the format of the data. The issue is
>>> for things like mmap/AF_PACKET the re-inserted form will be slower
>>> since data has to be copied.
>>
>> Indeed, it was a foolish thing to support in the first place.
>>
>> I guess we couldn't see the hw offloads in the future at that
>> point.
>>
>> I'm all for finding a way to deprecate this over time.
>
>
> There are two very similar issues here, that affect two
> slightly different cases.
>
> Let's assume the configuration is:
> eth0 - Talks to the outside world.
> vlan2000 - Is the vlan interface of interest.
>
>
> With REORDER_HDR set when I tcpdump on vlan2000 I don't see the
> vlan header, however I continue to see the vlan header when I tcpdump on eth0.

That seems good.  This is what originally let dhcp function.  Not sure
if it's still required for dhcp, but there could easily be other programs that
have similar issues.

With REORDER_HDR off, we should see vlan tagged packets on both eth0
and vlan2000.

If REORDER_HDR dissappears entirely, I think you have to default to
stripping the header on vlan2000.

>
> With vlan hardware acceleration.  When I tcpdump on eth0 I don't
> see the vlan header.  Nor do I see the vlan header when I tcpdump
> on vlan2000.

I think you should see the header on eth0 regardless of hw acceleration
or not.  Users should not have to know if their NIC/driver supports
vlan tag stripping in one mode or another.


> Right now both cases are problematic in Linus's tree.
>
> For inbound packets We are testing the wrong interface for the to see if
> we should play with REORDER_HDR.
>
> Hardware acceleration vlan tagging we are hiding the vlan header from
> portable applications that simply dump eth0.  Which is non-intuitive
> and apparent to everyone now that this happens on both software and
> hardware interfaces.
>
>
> So we have a couple of questions.
> 1) Do we revert the software emulation of vlan header tagging to fix
>     it's implementation issues in 2.6.40?
>
>     The big issues.
>     - vlan_untag is currently reading undefined data.
>     - Clearing REORDER_HDR no longer works.
>
>     I expect the issues are small enough that we can address them now.
>
> 2) Do we instead move the software implementation of vlan header
>     acceleration back into the net-next.
>
> 3) What do we do with pf_packet and vlan hardware acceleration when
>     dumping not the vlan interface but the interface below the vlan
>     interface?
>
>     Do we provide an option to keep the vlan header?  Should that option
>     be on by default?

At the least we need to have the header kept when using pf_packet on eth0.

I think it's best to have it available on vlan2000, but perhaps have it
stripped by default for backwards compatibility.

Thanks,
Ben


>
> Eric


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply

* Re: 2.6.38.x, 2.6.39 sfq? kernel panic in sfq_enqueue
From: Denys Fedoryshchenko @ 2011-05-23 22:35 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, hadi
In-Reply-To: <1306163146.20687.19.camel@edumazet-laptop>

 On Mon, 23 May 2011 17:05:46 +0200, Eric Dumazet wrote:
> Le lundi 23 mai 2011 à 17:27 +0300, Denys Fedoryshchenko a écrit :
>
>> > Thanks !
>>  By objdump or he must recompile kernel with DEBUG_INFO and use gdb?
>>
>>
>>
>
>
> Just objdump of sch_sfq.o (or sch_sfq.ko) file, (to keep existing
> offsets), thanks
 Sorry for delay, i had to contact person who run that kernel :)

 www.nuclearcat.com/files/disasm.txt
 www.nuclearcat.com/files/sch_sfq.ko



^ permalink raw reply

* Re: [PATCH 1/3] vlan: Do not support clearing VLAN_FLAG_REORDER_HDR
From: Eric W. Biederman @ 2011-05-23 22:48 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David Miller, greearb, nicolas.2p.debian, jpirko, xiaosuo, netdev,
	kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <20110523151631.2c34740f@nehalam>

Stephen Hemminger <shemminger@linux-foundation.org> writes:

> On Mon, 23 May 2011 15:05:54 -0700
> ebiederm@xmission.com (Eric W. Biederman) wrote:
>
>> David Miller <davem@davemloft.net> writes:
>> 
>> > From: Stephen Hemminger <shemminger@linux-foundation.org>
>> > Date: Mon, 23 May 2011 14:00:48 -0700
>> >
>> >> IMHO the REORDER_HDR flag was a design mistake. It means supporting
>> >> two different API's for the application. For a packet capture application
>> >> it means the format of the packet will be different based on how
>> >> the VLAN was created. And the vlan was created outside the application.
>> >> 
>> >> If there was a requirement to support both formats, then it should
>> >> be a property of the AF_PACKET socket.  The application can then do
>> >> an setsockopt() to control the format of the data. The issue is
>> >> for things like mmap/AF_PACKET the re-inserted form will be slower
>> >> since data has to be copied.
>> >
>> > Indeed, it was a foolish thing to support in the first place.
>> >
>> > I guess we couldn't see the hw offloads in the future at that
>> > point.
>> >
>> > I'm all for finding a way to deprecate this over time.
>> 
>> 
>> There are two very similar issues here, that affect two
>> slightly different cases.
>> 
>> Let's assume the configuration is:
>> eth0 - Talks to the outside world.
>> vlan2000 - Is the vlan interface of interest.
>> 
>> 
>> With REORDER_HDR set when I tcpdump on vlan2000 I don't see the
>> vlan header, however I continue to see the vlan header when I tcpdump on eth0.
>> 
>> With vlan hardware acceleration.  When I tcpdump on eth0 I don't
>> see the vlan header.  Nor do I see the vlan header when I tcpdump
>> on vlan2000.
>> 
>> 
>> Right now both cases are problematic in Linus's tree.
>> 
>> For inbound packets We are testing the wrong interface for the to see if
>> we should play with REORDER_HDR.
>> 
>> Hardware acceleration vlan tagging we are hiding the vlan header from
>> portable applications that simply dump eth0.  Which is non-intuitive
>> and apparent to everyone now that this happens on both software and
>> hardware interfaces.
>> 
>> 
>> So we have a couple of questions.
>> 1) Do we revert the software emulation of vlan header tagging to fix
>>    it's implementation issues in 2.6.40?
>> 
>>    The big issues.
>>    - vlan_untag is currently reading undefined data.
>>    - Clearing REORDER_HDR no longer works.
>> 
>>    I expect the issues are small enough that we can address them now.
>> 
>> 2) Do we instead move the software implementation of vlan header
>>    acceleration back into the net-next.
>> 
>> 3) What do we do with pf_packet and vlan hardware acceleration when
>>    dumping not the vlan interface but the interface below the vlan
>>    interface?
>> 
>>    Do we provide an option to keep the vlan header?  Should that option
>>    be on by default?
>
> Why not just make libpcap smarter? it already has Linux specific
> stuff for checksum offload. The portability to BSD for AF_PACKET is
> a non-issue.

That is not an option I had considered, and it sounds intriguing.

Even if libpcap can cope we are still not backwards compatible with
older linux applications.  Some of which work up to 2.6.39 because
the interface the vlan tagged packets come in on does not implement
vlan hardware acceleration.

There is also the issue that socket filters can't currently see
skb->tci so we are really messed up in the case we want vlan hardware
acceleration.

Eric




^ permalink raw reply

* Re: [PATCH 1/3] vlan: Do not support clearing VLAN_FLAG_REORDER_HDR
From: Eric W. Biederman @ 2011-05-23 23:02 UTC (permalink / raw)
  To: Ben Greear
  Cc: David Miller, shemminger, nicolas.2p.debian, jpirko, xiaosuo,
	netdev, kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <4DDADE52.8000008@candelatech.com>

Ben Greear <greearb@candelatech.com> writes:

> On 05/23/2011 03:05 PM, Eric W. Biederman wrote:
>
> If REORDER_HDR dissappears entirely, I think you have to default to
> stripping the header on vlan2000.

Which is what patches that started this thread are doing.

>> With vlan hardware acceleration.  When I tcpdump on eth0 I don't
>> see the vlan header.  Nor do I see the vlan header when I tcpdump
>> on vlan2000.
>
> I think you should see the header on eth0 regardless of hw acceleration
> or not.  Users should not have to know if their NIC/driver supports
> vlan tag stripping in one mode or another.

But is it acceptable to fix this in libpcap?

>> 3) What do we do with pf_packet and vlan hardware acceleration when
>>     dumping not the vlan interface but the interface below the vlan
>>     interface?
>>
>>     Do we provide an option to keep the vlan header?  Should that option
>>     be on by default?
>
> At the least we need to have the header kept when using pf_packet on
> eth0.

Start with the assumption that vlan hardware acceleration is in place
and the hardware has stripped the vlan tag and put it in skb->tci.
Sure there are dumb divers out there that don't do this today but
either we need to throw out vlan hardware acceleration completely
or emulate it in software because otherwise the test matrix is just
too big.

> I think it's best to have it available on vlan2000, but perhaps have it
> stripped by default for backwards compatibility.

Anything that deals with raw packets pretty much breaks if you don't
strip the vlan header from visibility on vlan2000.  Plus you loose
any advantage there is from vlan hardware acceleration, which is
available on must modern NICs.  So I don't think we can seriously
consider having the vlan header for present on the vlan2000 device.

All that is interesting is what to do with eth0, and pf_packet sockets.
And the only question that seems really interesting there is do we put
the vlan header back on with libpcap magic or with pf_packet logic.

We have to start with a the assumption that we come in with a pf_packet
with the vlan tag only in skb->tci.

Eric

^ permalink raw reply

* [PATCH] vlan: Fix the b0rked ingress VLAN_FLAG_REORDER_HDR check.
From: Eric W. Biederman @ 2011-05-24  0:11 UTC (permalink / raw)
  To: David Miller
  Cc: shemminger, greearb, nicolas.2p.debian, jpirko, xiaosuo, netdev,
	kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <20110523.172047.1438754754048434316.davem@davemloft.net>


Testing of VLAN_FLAG_REORDER_HDR does not belong in vlan_untag but
rather in vlan_do_receive.  In attempting to test VLAN_FLAG_REORDER_HDR
we are attempting to deference vlan private network device members a non
vlan device, which is just wrong.  Move the test into vlan_do_receive so
that the vlan header will not be properly put on the packet in the case
of vlan header acceleration.

As we remove the check from vlan_check_reorder_header rename it
vlan_reorder_header to keep the naming clean.

Hopefully at somepoint we will just declare the case where
VLAN_FLAG_REORDER_HDR is cleared as unsupported and remove the code.
Until then this keeps it working correctly.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 include/linux/if_vlan.h |   25 ++++++++++++++++++++++---
 net/8021q/vlan_core.c   |   25 ++++++++++++++-----------
 2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 290bd8a..26f3c92 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -220,7 +220,7 @@ static inline int vlan_hwaccel_receive_skb(struct sk_buff *skb,
 }
 
 /**
- * __vlan_put_tag - regular VLAN tag inserting
+ * vlan_insert_tag - regular VLAN tag inserting
  * @skb: skbuff to tag
  * @vlan_tci: VLAN TCI to insert
  *
@@ -229,8 +229,10 @@ static inline int vlan_hwaccel_receive_skb(struct sk_buff *skb,
  *
  * Following the skb_unshare() example, in case of error, the calling function
  * doesn't have to worry about freeing the original skb.
+ *
+ * Does not change skb->protocol so this function can be used for receive.
  */
-static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
+static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb, u16 vlan_tci)
 {
 	struct vlan_ethhdr *veth;
 
@@ -250,8 +252,25 @@ static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
 	/* now, the TCI */
 	veth->h_vlan_TCI = htons(vlan_tci);
 
-	skb->protocol = htons(ETH_P_8021Q);
+	return skb;
+}
 
+/**
+ * __vlan_put_tag - regular VLAN tag inserting
+ * @skb: skbuff to tag
+ * @vlan_tci: VLAN TCI to insert
+ *
+ * Inserts the VLAN tag into @skb as part of the payload
+ * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ */
+static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
+{
+	skb = vlan_insert_tag(skb, vlan_tci);
+	if (skb)
+		skb->protocol = htons(ETH_P_8021Q);
 	return skb;
 }
 
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 41495dc..c08dae7 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -23,6 +23,11 @@ bool vlan_do_receive(struct sk_buff **skbp)
 		return false;
 
 	skb->dev = vlan_dev;
+	if (unlikely(!(vlan_dev_info(vlan_dev)->flags & VLAN_FLAG_REORDER_HDR))) {
+		skb = *skbp = vlan_insert_tag(skb, skb->vlan_tci);
+		if (!skb)
+			return false;
+	}
 	skb->priority = vlan_get_ingress_priority(vlan_dev, skb->vlan_tci);
 	skb->vlan_tci = 0;
 
@@ -89,17 +94,15 @@ gro_result_t vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
 }
 EXPORT_SYMBOL(vlan_gro_frags);
 
-static struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
+static struct sk_buff *vlan_reorder_header(struct sk_buff *skb)
 {
-	if (vlan_dev_info(skb->dev)->flags & VLAN_FLAG_REORDER_HDR) {
-		if (skb_cow(skb, skb_headroom(skb)) < 0)
-			skb = NULL;
-		if (skb) {
-			/* Lifted from Gleb's VLAN code... */
-			memmove(skb->data - ETH_HLEN,
-				skb->data - VLAN_ETH_HLEN, 12);
-			skb->mac_header += VLAN_HLEN;
-		}
+	if (skb_cow(skb, skb_headroom(skb)) < 0)
+		skb = NULL;
+	if (skb) {
+		/* Lifted from Gleb's VLAN code... */
+		memmove(skb->data - ETH_HLEN,
+			skb->data - VLAN_ETH_HLEN, 12);
+		skb->mac_header += VLAN_HLEN;
 	}
 	return skb;
 }
@@ -161,7 +164,7 @@ struct sk_buff *vlan_untag(struct sk_buff *skb)
 	skb_pull_rcsum(skb, VLAN_HLEN);
 	vlan_set_encap_proto(skb, vhdr);
 
-	skb = vlan_check_reorder_header(skb);
+	skb = vlan_reorder_header(skb);
 	if (unlikely(!skb))
 		goto err_free;
 
-- 
1.7.5.1.217.g4e3aa


^ permalink raw reply related

* Re: linux-next: build failure after merge of the final tree
From: Mike Frysinger @ 2011-05-24  2:06 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Linus, linux-next, linux-kernel, David S. Miller, netdev,
	Andrew Morton, Mel Gorman, linux-mm, Alexander Viro,
	linux-fsdevel, Paul E. McKenney, Dipankar Sarma
In-Reply-To: <20110520161816.dda6f1fd.sfr@canb.auug.org.au>

On Fri, May 20, 2011 at 02:18, Stephen Rothwell wrote:
> Caused by commit e66eed651fd1 ("list: remove prefetching from regular list
> iterators").
>
> I added the following patch for today:

probably should get added to whatever tree that commit is coming from
so we dont have bisect hell ?

more failures:
drivers/usb/host/isp1362-hcd.c: In function 'isp1362_write_ptd':
drivers/usb/host/isp1362-hcd.c:355: error: implicit declaration of
function 'prefetch'
drivers/usb/host/isp1362-hcd.c: In function 'isp1362_read_ptd':
drivers/usb/host/isp1362-hcd.c:377: error: implicit declaration of
function 'prefetchw'
make[3]: *** [drivers/usb/host/isp1362-hcd.o] Error 1

drivers/usb/musb/musb_core.c: In function 'musb_write_fifo':
drivers/usb/musb/musb_core.c:219: error: implicit declaration of
function 'prefetch'
make[3]: *** [drivers/usb/musb/musb_core.o] Error 1

although it seems like it should be fairly trivial to look at the
funcs in linux/prefetch.h, grep the tree, and find a pretty good list
of the files that are missing the include
-mike

^ permalink raw reply

* Re: linux-next: build failure after merge of the final tree
From: Greg KH @ 2011-05-24  2:51 UTC (permalink / raw)
  To: Mike Frysinger
  Cc: Stephen Rothwell, Linus, linux-next, linux-kernel,
	David S. Miller, netdev, Andrew Morton, Mel Gorman, linux-mm,
	Alexander Viro, linux-fsdevel, Paul E. McKenney, Dipankar Sarma
In-Reply-To: <BANLkTimjzzqTS1fELmpb0UivqseLsYOfPw@mail.gmail.com>

On Mon, May 23, 2011 at 10:06:40PM -0400, Mike Frysinger wrote:
> On Fri, May 20, 2011 at 02:18, Stephen Rothwell wrote:
> > Caused by commit e66eed651fd1 ("list: remove prefetching from regular list
> > iterators").
> >
> > I added the following patch for today:
> 
> probably should get added to whatever tree that commit is coming from
> so we dont have bisect hell ?
> 
> more failures:
> drivers/usb/host/isp1362-hcd.c: In function 'isp1362_write_ptd':
> drivers/usb/host/isp1362-hcd.c:355: error: implicit declaration of
> function 'prefetch'
> drivers/usb/host/isp1362-hcd.c: In function 'isp1362_read_ptd':
> drivers/usb/host/isp1362-hcd.c:377: error: implicit declaration of
> function 'prefetchw'
> make[3]: *** [drivers/usb/host/isp1362-hcd.o] Error 1
> 
> drivers/usb/musb/musb_core.c: In function 'musb_write_fifo':
> drivers/usb/musb/musb_core.c:219: error: implicit declaration of
> function 'prefetch'
> make[3]: *** [drivers/usb/musb/musb_core.o] Error 1
> 
> although it seems like it should be fairly trivial to look at the
> funcs in linux/prefetch.h, grep the tree, and find a pretty good list
> of the files that are missing the include

How did this not show up in linux-next?  Where did the patch that caused
this show up from?

totally confused,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH 03/15] ehea: Remove force_irq logic in napi poll routine
From: Benjamin Herrenschmidt @ 2011-05-24  3:32 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: leitao, michael, jesse, bhutchings, netdev
In-Reply-To: <20110512005622.766206152@samba.org>

On Thu, 2011-05-12 at 10:52 +1000, Anton Blanchard wrote:
> plain text document attachment (ehea_3.patch)
> commit 18604c548545 (ehea: NAPI multi queue TX/RX path for SMP) added
> driver specific logic for exiting napi mode. I'm not sure what it was
> trying to solve and it should be up to the network stack to decide when
> we are done polling so remove it.

I -think- the force_irq trick was meant to allow migration of the
workload if the interrupt moved. IE. Without it, continuous traffic
would cause NAPI to always happen on the same CPU, regardless of the
affinity of the corresponding queue IRQ.

I suspect all of that is obsoleted by the newer generic infrastructure.

Cheers,
Ben.

> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> 
> Index: linux-net/drivers/net/ehea/ehea_main.c
> ===================================================================
> --- linux-net.orig/drivers/net/ehea/ehea_main.c	2011-05-12 07:47:49.640116670 +1000
> +++ linux-net/drivers/net/ehea/ehea_main.c	2011-05-12 07:47:51.960153456 +1000
> @@ -927,7 +927,6 @@ static struct ehea_cqe *ehea_proc_cqes(s
>  	return cqe;
>  }
>  
> -#define EHEA_NAPI_POLL_NUM_BEFORE_IRQ 16
>  #define EHEA_POLL_MAX_CQES 65535
>  
>  static int ehea_poll(struct napi_struct *napi, int budget)
> @@ -937,18 +936,13 @@ static int ehea_poll(struct napi_struct
>  	struct net_device *dev = pr->port->netdev;
>  	struct ehea_cqe *cqe;
>  	struct ehea_cqe *cqe_skb = NULL;
> -	int force_irq, wqe_index;
> +	int wqe_index;
>  	int rx = 0;
>  
> -	force_irq = (pr->poll_counter > EHEA_NAPI_POLL_NUM_BEFORE_IRQ);
>  	cqe_skb = ehea_proc_cqes(pr, EHEA_POLL_MAX_CQES);
> +	rx += ehea_proc_rwqes(dev, pr, budget - rx);
>  
> -	if (!force_irq)
> -		rx += ehea_proc_rwqes(dev, pr, budget - rx);
> -
> -	while ((rx != budget) || force_irq) {
> -		pr->poll_counter = 0;
> -		force_irq = 0;
> +	while ((rx != budget)) {
>  		napi_complete(napi);
>  		ehea_reset_cq_ep(pr->recv_cq);
>  		ehea_reset_cq_ep(pr->send_cq);
> @@ -968,7 +962,6 @@ static int ehea_poll(struct napi_struct
>  		rx += ehea_proc_rwqes(dev, pr, budget - rx);
>  	}
>  
> -	pr->poll_counter++;
>  	return rx;
>  }
>  
> Index: linux-net/drivers/net/ehea/ehea.h
> ===================================================================
> --- linux-net.orig/drivers/net/ehea/ehea.h	2011-05-12 07:47:49.640116670 +1000
> +++ linux-net/drivers/net/ehea/ehea.h	2011-05-12 07:47:51.960153456 +1000
> @@ -383,7 +383,6 @@ struct ehea_port_res {
>  	u64 tx_bytes;
>  	u64 rx_packets;
>  	u64 rx_bytes;
> -	u32 poll_counter;
>  	struct net_lro_mgr lro_mgr;
>  	struct net_lro_desc lro_desc[MAX_LRO_DESCRIPTORS];
>  	int sq_restart_flag;
> 
> 
> --
> 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: linux-next: build failure after merge of the final tree
From: Stephen Rothwell @ 2011-05-24  3:59 UTC (permalink / raw)
  To: Greg KH
  Cc: Mike Frysinger, Linus, linux-next, linux-kernel, David S. Miller,
	netdev, Andrew Morton, Mel Gorman, linux-mm, Alexander Viro,
	linux-fsdevel, Paul E. McKenney, Dipankar Sarma
In-Reply-To: <20110524025151.GA26939@kroah.com>

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

Hi Greg,

On Mon, 23 May 2011 19:51:51 -0700 Greg KH <greg@kroah.com> wrote:
>
> On Mon, May 23, 2011 at 10:06:40PM -0400, Mike Frysinger wrote:
> > On Fri, May 20, 2011 at 02:18, Stephen Rothwell wrote:
> > > Caused by commit e66eed651fd1 ("list: remove prefetching from regular list
> > > iterators").
> > >
> > > I added the following patch for today:
> > 
> > probably should get added to whatever tree that commit is coming from
> > so we dont have bisect hell ?
> > 
> > more failures:
> > drivers/usb/host/isp1362-hcd.c: In function 'isp1362_write_ptd':
> > drivers/usb/host/isp1362-hcd.c:355: error: implicit declaration of
> > function 'prefetch'
> > drivers/usb/host/isp1362-hcd.c: In function 'isp1362_read_ptd':
> > drivers/usb/host/isp1362-hcd.c:377: error: implicit declaration of
> > function 'prefetchw'
> > make[3]: *** [drivers/usb/host/isp1362-hcd.o] Error 1
> > 
> > drivers/usb/musb/musb_core.c: In function 'musb_write_fifo':
> > drivers/usb/musb/musb_core.c:219: error: implicit declaration of
> > function 'prefetch'
> > make[3]: *** [drivers/usb/musb/musb_core.o] Error 1
> > 
> > although it seems like it should be fairly trivial to look at the
> > funcs in linux/prefetch.h, grep the tree, and find a pretty good list
> > of the files that are missing the include
> 
> How did this not show up in linux-next?  Where did the patch that caused
> this show up from?
> 
> totally confused,

:-)

sfr said above:
> Caused by commit e66eed651fd1 ("list: remove prefetching from regular
> list iterators").

The cause was a patch from Linus ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* Re: linux-next: build failure after merge of the final tree
From: Linus Torvalds @ 2011-05-24  4:01 UTC (permalink / raw)
  To: Mike Frysinger
  Cc: Stephen Rothwell, linux-next, linux-kernel, David S. Miller,
	netdev, Andrew Morton, Mel Gorman, linux-mm, Alexander Viro,
	linux-fsdevel, Paul E. McKenney, Dipankar Sarma
In-Reply-To: <BANLkTimjzzqTS1fELmpb0UivqseLsYOfPw@mail.gmail.com>

On Mon, May 23, 2011 at 7:06 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
>
> more failures:

Is this blackfin or something?

I did an allyesconfig with a special x86 patch that should have caught
everything that didn't have the proper prefetch.h include, but non-x86
drivers would have passed that.

And I guess I didn't do my "force staging drivers on" hack for that test either.

                      Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: linux-next: build failure after merge of the final tree
From: Mike Frysinger @ 2011-05-24  4:10 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Stephen Rothwell, linux-next, linux-kernel, David S. Miller,
	netdev, Andrew Morton, Mel Gorman, linux-mm, Alexander Viro,
	linux-fsdevel, Paul E. McKenney, Dipankar Sarma
In-Reply-To: <BANLkTine2kobQA8TkmtiuXdKL=07NCo2vA@mail.gmail.com>

On Tue, May 24, 2011 at 00:01, Linus Torvalds wrote:
> On Mon, May 23, 2011 at 7:06 PM, Mike Frysinger wrote:
>>
>> more failures:
>
> Is this blackfin or something?

let's go with "something" ...

> I did an allyesconfig with a special x86 patch that should have caught
> everything that didn't have the proper prefetch.h include, but non-x86
> drivers would have passed that.

the isp1362-hcd failure probably is before your
268bb0ce3e87872cb9290c322b0d35bce230d88f.  i think i was reading a log
that is a few days old (ive been traveling and am playing catch up
atm).  i'll refresh and see what's what still.

the common musb code only allows it to be built if the arch glue is
available, and there is no x86 glue.  so an allyesconfig on x86
wouldnt have picked up the failure.  it'll bomb though for any target
which does have the glue.
-mike

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH 1/3] vlan: Do not support clearing VLAN_FLAG_REORDER_HDR
From: Ben Greear @ 2011-05-24  4:20 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: David Miller, shemminger, nicolas.2p.debian, jpirko, xiaosuo,
	netdev, kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <m1boyt10n4.fsf@fess.ebiederm.org>

On 05/23/2011 04:02 PM, Eric W. Biederman wrote:
> Ben Greear<greearb@candelatech.com>  writes:
>
>> On 05/23/2011 03:05 PM, Eric W. Biederman wrote:
>>
>> If REORDER_HDR dissappears entirely, I think you have to default to
>> stripping the header on vlan2000.
>
> Which is what patches that started this thread are doing.
>
>>> With vlan hardware acceleration.  When I tcpdump on eth0 I don't
>>> see the vlan header.  Nor do I see the vlan header when I tcpdump
>>> on vlan2000.
>>
>> I think you should see the header on eth0 regardless of hw acceleration
>> or not.  Users should not have to know if their NIC/driver supports
>> vlan tag stripping in one mode or another.
>
> But is it acceptable to fix this in libpcap?

No, because libpcap is not the only thing that opens packet
sockets.  Our user-space network emulator bridges two ethernet
interfaces using packet-sockets.  We have to have the VLAN header
available to properly bridge a VLAN packet out the other side.
It would be possible to use some aux data, but I think that is
a very nasty hack.

Surely we and libpcap are not the only users...

Also, anything using a packet filter might need the header in place.

>>> 3) What do we do with pf_packet and vlan hardware acceleration when
>>>      dumping not the vlan interface but the interface below the vlan
>>>      interface?
>>>
>>>      Do we provide an option to keep the vlan header?  Should that option
>>>      be on by default?
>>
>> At the least we need to have the header kept when using pf_packet on
>> eth0.
>
> Start with the assumption that vlan hardware acceleration is in place
> and the hardware has stripped the vlan tag and put it in skb->tci.
> Sure there are dumb divers out there that don't do this today but
> either we need to throw out vlan hardware acceleration completely
> or emulate it in software because otherwise the test matrix is just
> too big.

It's a binary case..either tag exists in skb or it doesn't.  It might double
the test cases, but that is still a reasonable number.  We should be able
to write a test app to mostly automate testing for that matter.

I still haven't had time to do detailed testing, but I *believe* that
even smart drivers like e1000e and igb don't strip tags if you do not
have any VLANs created on the NIC.  So, sometimes you get tags on
eth0 even when using fancy NICs.

>
>> I think it's best to have it available on vlan2000, but perhaps have it
>> stripped by default for backwards compatibility.
>
> Anything that deals with raw packets pretty much breaks if you don't
> strip the vlan header from visibility on vlan2000.  Plus you loose
> any advantage there is from vlan hardware acceleration, which is
> available on must modern NICs.  So I don't think we can seriously
> consider having the vlan header for present on the vlan2000 device.

I'm fine with stripping them from the vlan2000 frame, though
hopefully one day we can optimize to only do this if there
are actual consumers of the raw packet in user-space.

> All that is interesting is what to do with eth0, and pf_packet sockets.
> And the only question that seems really interesting there is do we put
> the vlan header back on with libpcap magic or with pf_packet logic.
>
> We have to start with a the assumption that we come in with a pf_packet
> with the vlan tag only in skb->tci.

Then I think we should put it back with pf_packet logic.  Possibly with
a per-socket option to disable this and send it as only aux data if that
is more efficient.

If it turns out the NIC is not stripping VLAN tags for whatever reason,
we might be able to optimize things so that it never does the HW emulation
so that it never has to un-do it later.

If we can agree on the desired behaviour, I'll put some effort into
a test system that can test a 4-port system:

eth0 { loopback cable }  eth1 { pf-socket based software bridge } eth2 { looped-back-cable } eth3

Could iterate through a matrix of vlans in various places and test each time to make sure
eth0 and eth3 receive expected data.  I could split the sender/consumer logic into one process
and the bridge into another so that it could be done on two systems with 2 ports each.

Thanks,
Ben

>
> Eric


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [PATCH]net:8021q:vlan.c Fix pr_info to read on line in the syslog.
From: Justin P. Mattock @ 2011-05-24  4:33 UTC (permalink / raw)
  To: Joe Perches; +Cc: netdev, davem, linux-kernel
In-Reply-To: <1306167781.8687.20.camel@Joe-Laptop>

On 05/23/2011 09:23 AM, Joe Perches wrote:
> On Mon, 2011-05-23 at 09:04 -0700, Justin P. Mattock wrote:
>> The patch below changes the pr_info so that it reads on one line in the syslog
>> rather than two.
>>
>> before:
>>
>> [   30.438203] 802.1Q VLAN Support v1.8 Ben Greear<greearb@candelatech.com>
>> [   30.441542] All bugs added by David S. Miller<davem@redhat.com>
>>
>>
>> after:
>> [   29.356282] 802.1Q VLAN Support v1.8 Ben Greear<greearb@candelatech.com>  All bugs added by: David S. Miller<davem@redhat.com>
>>
>> Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
>>
>> ---
>>   net/8021q/vlan.c |    4 ++--
>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
>> index b2274d1..02f4d8b 100644
>> --- a/net/8021q/vlan.c
>> +++ b/net/8021q/vlan.c
>> @@ -673,8 +673,8 @@ static int __init vlan_proto_init(void)
>>   {
>>   	int err;
>>
>> -	pr_info("%s v%s %s\n", vlan_fullname, vlan_version, vlan_copyright);
>> -	pr_info("All bugs added by %s\n", vlan_buggyright);
>> +	pr_info("%s v%s %s All bugs added by: %s\n", vlan_fullname, vlan_version,
>> +		vlan_copyright, vlan_buggyright);
>
> Might as well avoid the format string then too.
>
> 	pr_info(vlan_fullname " v" vlan_version " " vlan_copyright
> 		" All bugs added by: " vlan_buggyright "\n");
>
> or just kill the otherwise unused vlan_<foo>  variables
>
> 	pr_info("802.1Q VLAN Support v" vlan_version
> 		" Ben Greear<greearb@candelatech.com>"
> 		" All bugs added by: David S. Miller<davem@redhat.com>\n");
>
> though I think that emitting names on startup isn't necessary and
> this is enough:
>
> 	pr_info("802.1Q VLAN Support v" vlan_version "\n");
>
>
>

alright, I will resend this with what you have then.

Justin P. Mattock

^ permalink raw reply

* Re: [PATCH] vlan: Fix the b0rked ingress VLAN_FLAG_REORDER_HDR check.
From: David Miller @ 2011-05-24  4:54 UTC (permalink / raw)
  To: ebiederm
  Cc: shemminger, greearb, nicolas.2p.debian, jpirko, xiaosuo, netdev,
	kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <m1oc2tyn20.fsf_-_@fess.ebiederm.org>

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Mon, 23 May 2011 17:11:51 -0700

> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index 41495dc..c08dae7 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -23,6 +23,11 @@ bool vlan_do_receive(struct sk_buff **skbp)
>  		return false;
>  
>  	skb->dev = vlan_dev;
> +	if (unlikely(!(vlan_dev_info(vlan_dev)->flags & VLAN_FLAG_REORDER_HDR))) {
> +		skb = *skbp = vlan_insert_tag(skb, skb->vlan_tci);
> +		if (!skb)
> +			return false;
> +	}
>  	skb->priority = vlan_get_ingress_priority(vlan_dev, skb->vlan_tci);
>  	skb->vlan_tci = 0;
>  

Below we do a eth_hdr(skb) based check on the ethernet address in the
PACKET_OTHERHOST case.

Won't this VLAN tag insertion change skb->mac_header and thus screw up
that test?

Touching this code requires surgical precision and long audits, because
there are so many subtble dependencies all over the place like this.

^ permalink raw reply

* Re: [PATCH] ipv6: xfrm6: fix dubious code
From: Steffen Klassert @ 2011-05-24  5:02 UTC (permalink / raw)
  To: David Miller; +Cc: bhutchings, eric.dumazet, netdev
In-Reply-To: <20110523.163302.1123967560115611293.davem@davemloft.net>

On Mon, May 23, 2011 at 04:33:02PM -0400, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Mon, 23 May 2011 08:36:00 -0700
> 
> > On Mon, 2011-05-23 at 10:42 +0200, Eric Dumazet wrote:
> >> net/ipv6/xfrm6_tunnel.c: In function ‘xfrm6_tunnel_rcv’:
> >> net/ipv6/xfrm6_tunnel.c:244:53: warning: the omitted middle operand
> >> in ?: will always be ‘true’, suggest explicit middle operand
> >> 
> >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> >> ---
> >>  net/ipv6/xfrm6_tunnel.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c
> >> index a6770a0..fb9b0c3 100644
> >> --- a/net/ipv6/xfrm6_tunnel.c
> >> +++ b/net/ipv6/xfrm6_tunnel.c
> >> @@ -241,7 +241,7 @@ static int xfrm6_tunnel_rcv(struct sk_buff *skb)
> >>  	__be32 spi;
> >>  
> >>  	spi = xfrm6_tunnel_spi_lookup(net, (const xfrm_address_t *)&iph->saddr);
> >> -	return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi) > 0 ? : 0;
> >> +	return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi) > 0 ? 1 : 0;
> >>  }
> >>  
> >>  static int xfrm6_tunnel_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
> > 
> > I suspect that this was intended to return the result of xfrm6_rcv_spi()
> > if > 0.
> 
> I also suspect this was the intent, but I'm not sure why it matters
> at all.
> 
> The equivalent code implementing the same operations on the ipv4
> side return xfrm4_rcv_spi()'s return value directly.
> 
> So we need to either decide that we can do the same thing here on the
> ipv6 side, or document exactly why we can't.

I think we can return the value directly like ipv4 does it. xfrm6_rcv_spi()
returns the return value of xfrm_input() which returns 0 in any case.

^ permalink raw reply

* Re: [PATCH] ipv6: xfrm6: fix dubious code
From: David Miller @ 2011-05-24  5:12 UTC (permalink / raw)
  To: steffen.klassert; +Cc: bhutchings, eric.dumazet, netdev
In-Reply-To: <20110524050242.GD8013@secunet.com>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 24 May 2011 07:02:42 +0200

> I think we can return the value directly like ipv4 does it. xfrm6_rcv_spi()
> returns the return value of xfrm_input() which returns 0 in any case.

Indeed, works for me:

--------------------
ipv6: Fix return of xfrm6_tunnel_rcv()

Like ipv4, just return xfrm6_rcv_spi()'s return value directly.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipv6/xfrm6_tunnel.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c
index a6770a0..4fe1db1 100644
--- a/net/ipv6/xfrm6_tunnel.c
+++ b/net/ipv6/xfrm6_tunnel.c
@@ -241,7 +241,7 @@ static int xfrm6_tunnel_rcv(struct sk_buff *skb)
 	__be32 spi;
 
 	spi = xfrm6_tunnel_spi_lookup(net, (const xfrm_address_t *)&iph->saddr);
-	return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi) > 0 ? : 0;
+	return xfrm6_rcv_spi(skb, IPPROTO_IPV6, spi);
 }
 
 static int xfrm6_tunnel_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
-- 
1.7.4.4


^ permalink raw reply related

* Re: [patch 1/1] net/irda: convert bfin_sir to common Blackfin UART header
From: David Miller @ 2011-05-24  5:13 UTC (permalink / raw)
  To: akpm; +Cc: samuel, netdev, vapier
In-Reply-To: <201105232217.p4NMH9uH015477@imap1.linux-foundation.org>

From: akpm@linux-foundation.org
Date: Mon, 23 May 2011 15:17:09 -0700

> From: Mike Frysinger <vapier@gentoo.org>
> 
> No need to duplicate these defines now that the common Blackfin code has
> unified these for all UART devices.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Cc: Samuel Ortiz <samuel@sortiz.org>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Applied.

^ permalink raw reply

* Re: [patch 1/1] net: convert %p usage to %pK
From: David Miller @ 2011-05-24  5:13 UTC (permalink / raw)
  To: akpm
  Cc: netdev, drosenberg, a.p.zijlstra, eparis, eric.dumazet, eugeneteo,
	jmorris, kees.cook, mingo, tgraf
In-Reply-To: <201105232217.p4NMHZiC015498@imap1.linux-foundation.org>

From: akpm@linux-foundation.org
Date: Mon, 23 May 2011 15:17:35 -0700

> From: Dan Rosenberg <drosenberg@vsecurity.com>
> 
> The %pK format specifier is designed to hide exposed kernel pointers,
> specifically via /proc interfaces.  Exposing these pointers provides an
> easy target for kernel write vulnerabilities, since they reveal the
> locations of writable structures containing easily triggerable function
> pointers.  The behavior of %pK depends on the kptr_restrict sysctl.
> 
> If kptr_restrict is set to 0, no deviation from the standard %p behavior
> occurs.  If kptr_restrict is set to 1, the default, if the current user
> (intended to be a reader via seq_printf(), etc.) does not have CAP_SYSLOG
> (currently in the LSM tree), kernel pointers using %pK are printed as 0's.
>  If kptr_restrict is set to 2, kernel pointers using %pK are printed as
> 0's regardless of privileges.  Replacing with 0's was chosen over the
> default "(null)", which cannot be parsed by userland %p, which expects
> "(nil)".
> 
> The supporting code for kptr_restrict and %pK are currently in the -mm
> tree.  This patch converts users of %p in net/ to %pK.  Cases of printing
> pointers to the syslog are not covered, since this would eliminate useful
> information for postmortem debugging and the reading of the syslog is
> already optionally protected by the dmesg_restrict sysctl.
> 
> Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Applied.

^ permalink raw reply

* Re: [PATCH 1/3] vlan: Do not support clearing VLAN_FLAG_REORDER_HDR
From: David Miller @ 2011-05-24  5:19 UTC (permalink / raw)
  To: ebiederm
  Cc: shemminger, greearb, nicolas.2p.debian, jpirko, xiaosuo, netdev,
	kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <m1y61x2htp.fsf@fess.ebiederm.org>

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Mon, 23 May 2011 15:05:54 -0700

> 3) What do we do with pf_packet and vlan hardware acceleration when
>    dumping not the vlan interface but the interface below the vlan
>    interface?
> 
>    Do we provide an option to keep the vlan header?  Should that option
>    be on by default?
> 

The vlan_tci in the V2 pf_packet auxdata was intended for this
purpose.

So no matter what variant of behavior is occurring, apps can properly
reconstitute the VLAN header if they inspect the vlan_tci in the
auxdata.

The only thing that seems to be missing is an indication that a VLAN
tag was present at all, ie. vlan_tx_tag_present(), in this manner an
application could then differentiate between no VLAN header and a VLAN
tag of zero.

^ permalink raw reply

* [PATCH v2]net:8021q:vlan.c Fix pr_info to just give the vlan fullname and version.
From: Justin P. Mattock @ 2011-05-24  5:40 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Justin P. Mattock, Joe Perches, David S. Miller,
	Ben Greear

The below patch removes vlan_buggyright and vlan_copyright from vlan_proto_init, 
so that it prints out just the fullname of vlan and the version number.

before:

[   30.438203] 802.1Q VLAN Support v1.8 Ben Greear <greearb@candelatech.com>
[   30.441542] All bugs added by David S. Miller <davem@redhat.com>

after:

[   31.513910] 802.1Q VLAN Support v1.8

Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
CC: Joe Perches <joe@perches.com>
CC: David S. Miller <davem@davemloft.net>
CC: Ben Greear <greearb@candelatech.com>
---
 net/8021q/vlan.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index b2274d1..9df3fcb 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -673,8 +673,7 @@ static int __init vlan_proto_init(void)
 {
 	int err;
 
-	pr_info("%s v%s %s\n", vlan_fullname, vlan_version, vlan_copyright);
-	pr_info("All bugs added by %s\n", vlan_buggyright);
+	pr_info("%s v%s\n", vlan_fullname, vlan_version);
 
 	err = register_pernet_subsys(&vlan_net_ops);
 	if (err < 0)
-- 
1.7.5.1

^ permalink raw reply related

* Re: [PATCH v2]net:8021q:vlan.c Fix pr_info to just give the vlan fullname and version.
From: Eric Dumazet @ 2011-05-24  5:45 UTC (permalink / raw)
  To: Justin P. Mattock
  Cc: netdev, linux-kernel, Joe Perches, David S. Miller, Ben Greear
In-Reply-To: <1306215647-2857-1-git-send-email-justinmattock@gmail.com>

Le lundi 23 mai 2011 à 22:40 -0700, Justin P. Mattock a écrit :
> The below patch removes vlan_buggyright and vlan_copyright from vlan_proto_init, 
> so that it prints out just the fullname of vlan and the version number.
> 
> before:
> 
> [   30.438203] 802.1Q VLAN Support v1.8 Ben Greear <greearb@candelatech.com>
> [   30.441542] All bugs added by David S. Miller <davem@redhat.com>
> 
> after:
> 
> [   31.513910] 802.1Q VLAN Support v1.8
> 
> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
> CC: Joe Perches <joe@perches.com>
> CC: David S. Miller <davem@davemloft.net>
> CC: Ben Greear <greearb@candelatech.com>
> ---
>  net/8021q/vlan.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index b2274d1..9df3fcb 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -673,8 +673,7 @@ static int __init vlan_proto_init(void)
>  {
>  	int err;
>  
> -	pr_info("%s v%s %s\n", vlan_fullname, vlan_version, vlan_copyright);
> -	pr_info("All bugs added by %s\n", vlan_buggyright);
> +	pr_info("%s v%s\n", vlan_fullname, vlan_version);
>  
>  	err = register_pernet_subsys(&vlan_net_ops);
>  	if (err < 0)

This reminds me a discussion with Harald Welte in netfilter workshop
2010.

Sometime these strings help lot guys working to fight GPL violations.

^ permalink raw reply

* RE: [PATCH] be2net: hash key for rss-config cmd not set
From: Sathya.Perla @ 2011-05-24  5:46 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <20110523.155206.345121915872826257.davem@davemloft.net>

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, May 24, 2011 1:22 AM
> To: Perla, Sathya
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH] be2net: hash key for rss-config cmd not set
> 
> From: Sathya Perla <sathya.perla@emulex.com>
> Date: Mon, 23 May 2011 17:55:27 +0530
> 
> > Need a random hash key to effectively hash incoming connections into
> > multiple RX rings.
> >
> > Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
> 
> You're saying you need a random hash key, yet you're assigning
> a constant.
> 
> Why not use a real random number, via get_random() or similar?

I meant to say a non-zero, non-descript value is needed as the hash key. That was the reason I had left the hash variable un-initialized; but on some systems I get a zero value and hashing is not effective. The constant value I tried (not of any significance) seems to work fine.

Pls let me know if you want me to re-send the patch with a more-descriptive changelog...

thanks.
-Sathya

^ permalink raw reply

* Re: [PATCH v2]net:8021q:vlan.c Fix pr_info to just give the vlan fullname and version.
From: Joe Perches @ 2011-05-24  5:54 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Justin P. Mattock, netdev, linux-kernel, David S. Miller,
	Ben Greear
In-Reply-To: <1306215958.2638.29.camel@edumazet-laptop>

On Tue, 2011-05-24 at 07:45 +0200, Eric Dumazet wrote:
> Le lundi 23 mai 2011 à 22:40 -0700, Justin P. Mattock a écrit :
> > The below patch removes vlan_buggyright and vlan_copyright from vlan_proto_init, 
> > so that it prints out just the fullname of vlan and the version number.
[]
> > diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> > -	pr_info("%s v%s %s\n", vlan_fullname, vlan_version, vlan_copyright);
> > -	pr_info("All bugs added by %s\n", vlan_buggyright);
> > +	pr_info("%s v%s\n", vlan_fullname, vlan_version);
> >  
> >  	err = register_pernet_subsys(&vlan_net_ops);
> >  	if (err < 0)
> This reminds me a discussion with Harald Welte in netfilter workshop
> 2010.
> Sometime these strings help lot guys working to fight GPL violations.

How so?

^ permalink raw reply

* Re: [PATCH 1/3] vlan: Do not support clearing VLAN_FLAG_REORDER_HDR
From: Jiri Pirko @ 2011-05-24  5:58 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Eric W. Biederman, Changli Gao, Ben Greear, David Miller, netdev,
	shemminger, kaber, fubar, eric.dumazet, andy, Jesse Gross
In-Reply-To: <4DDAB9FC.8050207@gmail.com>

Mon, May 23, 2011 at 09:48:12PM CEST, nicolas.2p.debian@gmail.com wrote:
>Le 23/05/2011 12:43, Jiri Pirko a écrit :
>>Mon, May 23, 2011 at 11:41:22AM CEST, ebiederm@xmission.com wrote:
>>>Changli Gao<xiaosuo@gmail.com>  writes:
>>>
>>>>On Mon, May 23, 2011 at 9:45 AM, Eric W. Biederman
>>>><ebiederm@xmission.com>  wrote:
>>>>>>In another side, is there a specification which defines the
>>>>>>hw-accel-vlan-rx?
>>>>>
>>>>>I don't know.
>>>>>
>>>>>I have just been trying to clean up the mess since some of the
>>>>>hw-accel-vlan code broke my use case, by delivering packets with
>>>>>priority but no vlan (aka vlan 0 packets) twice to my pf_packet sockets.
>>>>>
>>>>
>>>>OK. But if we have decided to simulate the hw-accel-vlan-rx, I think
>>>>we'd better adjust the place where we put the emulation code. The very
>>>>beginnings of netif_rx() and neif_receive_skb() are better. Then rps
>>>>can support vlan packets without any change.
>>>
>>>That sounds nice. Patches are welcome.
>>>
>>>In principle it should be doable with some code motion.  I don't think
>>>moving vlan_untag earlier constitutes a bug fix.
>>
>>I do not think that is doable. Consider multi tagged packets. The place
>>just after "another_round" takes care about that.
>>
>>Btw what's the rationale to move untag to earlier position?
>
>Maybe simply because we try to mimic hw-accel, and hw-accel untagging
>definitely happens before we enter __netif_receive_skb and only
>happens once.
>
>So having software untagging inside the __netif_receive_skb loop looks different.

I understand. But what code prior to current vlan_untag position needs
to see the skb untagged?

>
>	Nicolas.

^ permalink raw reply

* Re: [patch 1/1] net: convert %p usage to %pK
From: Eric Dumazet @ 2011-05-24  6:17 UTC (permalink / raw)
  To: David Miller
  Cc: akpm, netdev, drosenberg, a.p.zijlstra, eparis, eugeneteo,
	jmorris, kees.cook, mingo, tgraf
In-Reply-To: <20110524.011330.1077020828173889583.davem@davemloft.net>

Le mardi 24 mai 2011 à 01:13 -0400, David Miller a écrit :
> From: akpm@linux-foundation.org
> Date: Mon, 23 May 2011 15:17:35 -0700
> 
> > From: Dan Rosenberg <drosenberg@vsecurity.com>
> > 
> > The %pK format specifier is designed to hide exposed kernel pointers,
> > specifically via /proc interfaces.  Exposing these pointers provides an
> > easy target for kernel write vulnerabilities, since they reveal the
> > locations of writable structures containing easily triggerable function
> > pointers.  The behavior of %pK depends on the kptr_restrict sysctl.
> > 
> > If kptr_restrict is set to 0, no deviation from the standard %p behavior
> > occurs.  If kptr_restrict is set to 1, the default, if the current user
> > (intended to be a reader via seq_printf(), etc.) does not have CAP_SYSLOG
> > (currently in the LSM tree), kernel pointers using %pK are printed as 0's.
> >  If kptr_restrict is set to 2, kernel pointers using %pK are printed as
> > 0's regardless of privileges.  Replacing with 0's was chosen over the
> > default "(null)", which cannot be parsed by userland %p, which expects
> > "(nil)".
> > 
> > The supporting code for kptr_restrict and %pK are currently in the -mm
> > tree.  This patch converts users of %p in net/ to %pK.  Cases of printing
> > pointers to the syslog are not covered, since this would eliminate useful
> > information for postmortem debugging and the reading of the syslog is
> > already optionally protected by the dmesg_restrict sysctl.
> > 
> > Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> 
> Applied.

We probably need to extend this to inet_diag as well.

"ss -e" currently expose kernel pointers, like /proc files used to do
before this patch.

Thanks

[PATCH] inet_diag: hide socket pointers

Provide a mayber_hide_ptr() helper and use it in inet_diag to not
disclose kernel pointers to user, with kptr_restrict logic :

kptr_restrict = 0 : kernel pointers are not mangled
kptr_restrict = 1 : if the current user does not have CAP_SYSLOG,
kernel pointers are replaced by 0
kptr_restrict = 2 : kernel pointers are replaced by 0

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/printk.h |    1 +
 lib/vsprintf.c         |   15 +++++++++++----
 net/ipv4/inet_diag.c   |    6 ++++--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index ee048e7..47c0cef 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -111,6 +111,7 @@ extern bool printk_timed_ratelimit(unsigned long *caller_jiffies,
 extern int printk_delay_msec;
 extern int dmesg_restrict;
 extern int kptr_restrict;
+void *maybe_hide_ptr(void *ptr);
 
 void log_buf_kexec_setup(void);
 #else
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 1d659d7..20d3576 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -799,6 +799,16 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
 
 int kptr_restrict __read_mostly;
 
+void *maybe_hide_ptr(void *ptr)
+{
+	if (!((kptr_restrict == 0) ||
+	      (kptr_restrict == 1 &&
+	       has_capability_noaudit(current, CAP_SYSLOG))))
+		ptr = NULL;
+	return ptr;
+}
+EXPORT_SYMBOL(maybe_hide_ptr);
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format
@@ -911,10 +921,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 				spec.field_width = 2 * sizeof(void *);
 			return string(buf, end, "pK-error", spec);
 		}
-		if (!((kptr_restrict == 0) ||
-		      (kptr_restrict == 1 &&
-		       has_capability_noaudit(current, CAP_SYSLOG))))
-			ptr = NULL;
+		ptr = maybe_hide_ptr(ptr);
 		break;
 	}
 	spec.flags |= SMALL;
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 6ffe94c..b5646a3 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -84,6 +84,7 @@ static int inet_csk_diag_fill(struct sock *sk,
 	struct inet_diag_meminfo  *minfo = NULL;
 	unsigned char	 *b = skb_tail_pointer(skb);
 	const struct inet_diag_handler *handler;
+	u64 ptr;
 
 	handler = inet_diag_table[unlh->nlmsg_type];
 	BUG_ON(handler == NULL);
@@ -114,8 +115,9 @@ static int inet_csk_diag_fill(struct sock *sk,
 	r->idiag_retrans = 0;
 
 	r->id.idiag_if = sk->sk_bound_dev_if;
-	r->id.idiag_cookie[0] = (u32)(unsigned long)sk;
-	r->id.idiag_cookie[1] = (u32)(((unsigned long)sk >> 31) >> 1);
+	ptr = (u64)maybe_hide_ptr(sk);
+	r->id.idiag_cookie[0] = (u32)ptr;
+	r->id.idiag_cookie[1] = (u32)(ptr >> 32);
 
 	r->id.idiag_sport = inet->inet_sport;
 	r->id.idiag_dport = inet->inet_dport;



^ permalink raw reply related


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