netdev - Re: ICMP packets - ll_temac with Microblaze

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 21 Dec 2011 16:59:04 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	monstr@...str.eu
Cc:	David Miller <davem@...emloft.net>,
	John Williams <john.williams@...alogix.com>,
	netdev@...r.kernel.org
Subject: Re: ICMP packets - ll_temac with Microblaze

Le mercredi 21 décembre 2011 à 15:24 +0100, Michal Simek a écrit :
> Eric Dumazet wrote:
> > Le mercredi 21 décembre 2011 à 14:28 +0100, Michal Simek a écrit :
> > 
> >> ok. Can you provide me any background why size should be setup by
> >> size = SKB_WITH_OVERHEAD(ksize(data));
> >> and not to use size which is passed to kmalloc in __alloc_skb.
> > 
> > Its all about memory accounting (based on skb->truesize)
> > 
> > Prior to the patch, we could fool memory accounting because skbs claimed
> > to use less memory than what they really used.
> > 
> > And crash machines eventually.
> > 
> > Now memory accouting is fixed, we probably need to change some points in
> > the kernel, where we previously accepted a small skb, but not a very
> > large one.
> > 
> > Since "ping" probably uses SOCK_RAW sockets, I'll try this one :
> > 
> > (We dont care of _this_ skb truesize, only on the count of previously
> > queued packets)
> > 
> > 
> > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> > index 0da505c..a809a48 100644
> > --- a/net/packet/af_packet.c
> > +++ b/net/packet/af_packet.c
> > @@ -1631,8 +1631,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
> >  	if (snaplen > res)
> >  		snaplen = res;
> >  
> > -	if (atomic_read(&sk->sk_rmem_alloc) + skb->truesize >=
> > -	    (unsigned)sk->sk_rcvbuf)
> > +	if (atomic_read(&sk->sk_rmem_alloc) >= (unsigned)sk->sk_rcvbuf)
> >  		goto drop_n_acct;
> >  
> >  	if (skb_shared(skb)) {
> > @@ -1763,7 +1762,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
> >  	if (po->tp_version <= TPACKET_V2) {
> >  		if (macoff + snaplen > po->rx_ring.frame_size) {
> >  			if (po->copy_thresh &&
> > -				atomic_read(&sk->sk_rmem_alloc) + skb->truesize
> > +				atomic_read(&sk->sk_rmem_alloc)
> >  				< (unsigned)sk->sk_rcvbuf) {
> >  				if (skb_shared(skb)) {
> >  					copy_skb = skb_clone(skb, GFP_ATOMIC);
> > 
> > 
> > 
> > 
> 
> It doesn't work too.
> It is possible to see this behavior on qemu. What about if I prepare you package with cross toolchain, rootfs
> and you can add debug message where you want?
> 
> I have also tried ll_temac driver with ppc440 and behavior is the same.
> 
> Max FRAME_SIZE pass to netdev_alloc_skb_ip_align is 7966. For this value ping works.
> (For ll_temac driver it is #define XTE_JUMBO_MTU 7948 from ll_temac.h)
> 

I wonder if you applied/tested my patch correctly, since it really
should had help in your case  (allowing first received packet to be
queued, even if very big)

Given the way NIC drivers work (pre-allocating big buffers before
hardware fill frames in them), SO_RCVBUF limits are difficult to
respect.

We should allow at least one frame, even with a very small SO_RCVBUF
setting, or silently cap a minimum sane value.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html