Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 2/3] tcp: Initial repair mode
From: Pavel Emelyanov @ 2012-03-29  9:53 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Linux Netdev List, David Miller
In-Reply-To: <1332967158.2624.42.camel@bwh-desktop.uk.solarflarecom.com>

On 03/29/2012 12:39 AM, Ben Hutchings wrote:
> On Wed, 2012-03-28 at 19:37 +0400, Pavel Emelyanov wrote:
> [...]
>> * Ability to forcibly bind a socket to a port
>>
>> The sk->sk_reuse is set to 2 denoting, that the socket is question
>> should be bound as if all the others in the system are configured
>> with the SO_REUSEADDR option.
> 
> Shouldn't this constant be named?

Agree, I will fix this up.

> [...]
>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
> [...]
>> +	case TCP_REPAIR_QUEUE:
>> +		if (!tp->repair)
>> +			err = -EPERM;
>> +		else if (val <= TCP_QUEUES_NR)
> 
> Off-by-one.

Oops :( Thanks for noticing!

>> +			tp->repair_queue = val;
>> +		else
>> +			err = -EINVAL;
>> +		break;
> [...]
> 

^ permalink raw reply

* Re: [PATCH 3/3] tcp: Repair socket queues
From: Li Yu @ 2012-03-29 10:30 UTC (permalink / raw)
  To: Pavel Emelyanov; +Cc: Linux Netdev List, David Miller
In-Reply-To: <4F733062.9020800@parallels.com>

于 2012年03月28日 23:38, Pavel Emelyanov 写道:
> Reading queues under repair mode is done with recvmsg call.
> The queue-under-repair set by TCP_REPAIR_QUEUE option is used
> to determine which queue should be read. Thus both send and
> receive queue can be read with this.
>
> Caller must pass the MSG_PEEK flag.
>
> Writing to queues is done with sendmsg call and yet again --
> the repair-queue option can be used to push data into the
> receive queue.
>
> When putting an skb into receive queue a zero tcp header is
> appented to its head to address the tcp_hdr(skb)->syn and
> the ->fin checks by the (after repair) tcp_recvmsg. These
> flags flags are both set to zero and that's why.
>
> The fin cannot be met in the queue while reading the source
> socket, since the repair only works for closed/established
> sockets and queueing fin packet always changes its state.
>
> The syn in the queue denotes that the respective skb's seq
> is "off-by-one" as compared to the actual payload lenght. Thus,
> at the rcv queue refill we can just drop this flag and set the
> skb's sequences to precice values. IOW -- emulate the situation
> when the packet with data and syn is splitted into two -- a
> packet with syn and a packet with data and the former one is
> already "eaten".
>
> When the repair mode is turned off, the write queue seqs are
> updated so that the whole queue is considered to be 'already sent,
> waiting for ACKs' (write_seq = snd_nxt<= snd_una). From the
> protocol POV the send queue looks like it was sent, but the data
> between the write_seq and snd_nxt is lost in the network.
>
> This helps to avoid another sockoption for setting the snd_nxt
> sequence. Leaving the whole queue in a 'not yet sent' state (as
> it will be after sendmsg-s) will not allow to receive any acks
> from the peer since the ack_seq will be after the snd_nxt. Thus
> even the ack for the window probe will be dropped and the
> connection will be 'locked' with the zero peer window.
>

Do we need to restore various TCP options switch bits. e.g. window
scale factor, sack_ok and so on.

En, I think the recorded mss_cache may be need to restored too.

Thanks.

Yu

> Signed-off-by: Pavel Emelyanov<xemul@parallels.com>
> ---
>   net/ipv4/tcp.c        |   89 +++++++++++++++++++++++++++++++++++++++++++++++--
>   net/ipv4/tcp_output.c |    1 +
>   2 files changed, 87 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 65ae921..2ab3a31 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -911,6 +911,39 @@ static inline int select_size(const struct sock *sk, bool sg)
>   	return tmp;
>   }
>
> +static int tcp_send_rcvq(struct sock *sk, struct msghdr *msg, size_t size)
> +{
> +	struct sk_buff *skb;
> +	struct tcp_skb_cb *cb;
> +	struct tcphdr *th;
> +
> +	skb = alloc_skb(size + sizeof(*th), sk->sk_allocation);
> +	if (!skb)
> +		goto err;
> +
> +	th = (struct tcphdr *)skb_put(skb, sizeof(*th));
> +	skb_reset_transport_header(skb);
> +	memset(th, 0, sizeof(*th));
> +
> +	if (memcpy_fromiovec(skb_put(skb, size), msg->msg_iov, size))
> +		goto err_free;
> +
> +	cb = TCP_SKB_CB(skb);
> +
> +	TCP_SKB_CB(skb)->seq = tcp_sk(sk)->rcv_nxt;
> +	TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(skb)->seq + size;
> +	TCP_SKB_CB(skb)->ack_seq = tcp_sk(sk)->snd_una - 1;
> +
> +	tcp_queue_rcv(sk, skb, sizeof(*th));
> +
> +	return size;
> +
> +err_free:
> +	kfree_skb(skb);
> +err:
> +	return -ENOMEM;
> +}
> +
>   int tcp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
>   		size_t size)
>   {
> @@ -932,6 +965,19 @@ int tcp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
>   		if ((err = sk_stream_wait_connect(sk,&timeo)) != 0)
>   			goto out_err;
>
> +	if (unlikely(tp->repair)) {
> +		if (tp->repair_queue == TCP_RECV_QUEUE) {
> +			copied = tcp_send_rcvq(sk, msg, size);
> +			goto out;
> +		}
> +
> +		err = -EINVAL;
> +		if (tp->repair_queue == TCP_NO_QUEUE)
> +			goto out_err;
> +
> +		/* 'common' sending to sendq */
> +	}
> +
>   	/* This should be in poll */
>   	clear_bit(SOCK_ASYNC_NOSPACE,&sk->sk_socket->flags);
>
> @@ -1089,7 +1135,7 @@ new_segment:
>   			if ((seglen -= copy) == 0&&  iovlen == 0)
>   				goto out;
>
> -			if (skb->len<  max || (flags&  MSG_OOB))
> +			if (skb->len<  max || (flags&  MSG_OOB) || tp->repair)
>   				continue;
>
>   			if (forced_push(tp)) {
> @@ -1102,7 +1148,7 @@ new_segment:
>   wait_for_sndbuf:
>   			set_bit(SOCK_NOSPACE,&sk->sk_socket->flags);
>   wait_for_memory:
> -			if (copied)
> +			if (copied&&  !tp->repair)
>   				tcp_push(sk, flags&  ~MSG_MORE, mss_now, TCP_NAGLE_PUSH);
>
>   			if ((err = sk_stream_wait_memory(sk,&timeo)) != 0)
> @@ -1113,7 +1159,7 @@ wait_for_memory:
>   	}
>
>   out:
> -	if (copied)
> +	if (copied&&  !tp->repair)
>   		tcp_push(sk, flags, mss_now, tp->nonagle);
>   	release_sock(sk);
>   	return copied;
> @@ -1187,6 +1233,24 @@ static int tcp_recv_urg(struct sock *sk, struct msghdr *msg, int len, int flags)
>   	return -EAGAIN;
>   }
>
> +static int tcp_peek_sndq(struct sock *sk, struct msghdr *msg, int len)
> +{
> +	struct sk_buff *skb;
> +	int copied = 0, err = 0;
> +
> +	/* XXX -- need to support SO_PEEK_OFF */
> +
> +	skb_queue_walk(&sk->sk_write_queue, skb) {
> +		err = skb_copy_datagram_iovec(skb, 0, msg->msg_iov, skb->len);
> +		if (err)
> +			break;
> +
> +		copied += skb->len;
> +	}
> +
> +	return err ?: copied;
> +}
> +
>   /* Clean up the receive buffer for full frames taken by the user,
>    * then send an ACK if necessary.  COPIED is the number of bytes
>    * tcp_recvmsg has given to the user so far, it speeds up the
> @@ -1432,6 +1496,21 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
>   	if (flags&  MSG_OOB)
>   		goto recv_urg;
>
> +	if (unlikely(tp->repair)) {
> +		err = -EPERM;
> +		if (!(flags&  MSG_PEEK))
> +			goto out;
> +
> +		if (tp->repair_queue == TCP_SEND_QUEUE)
> +			goto recv_sndq;
> +
> +		err = -EINVAL;
> +		if (tp->repair_queue == TCP_NO_QUEUE)
> +			goto out;
> +
> +		/* 'common' recv queue MSG_PEEK-ing */
> +	}
> +
>   	seq =&tp->copied_seq;
>   	if (flags&  MSG_PEEK) {
>   		peek_seq = tp->copied_seq;
> @@ -1783,6 +1862,10 @@ out:
>   recv_urg:
>   	err = tcp_recv_urg(sk, msg, len, flags);
>   	goto out;
> +
> +recv_sndq:
> +	err = tcp_peek_sndq(sk, msg, len);
> +	goto out;
>   }
>   EXPORT_SYMBOL(tcp_recvmsg);
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 4e2ce39..b29d612 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2796,6 +2796,7 @@ void tcp_send_window_probe(struct sock *sk)
>   {
>   	if (sk->sk_state == TCP_ESTABLISHED) {
>   		tcp_sk(sk)->snd_wl1 = tcp_sk(sk)->rcv_nxt - 1;
> +		tcp_sk(sk)->snd_nxt = tcp_sk(sk)->write_seq;
>   		tcp_xmit_probe_skb(sk, 0);
>   	}
>   }

^ permalink raw reply

* Re: [PATCH 3/3] tcp: Repair socket queues
From: Pavel Emelyanov @ 2012-03-29 10:36 UTC (permalink / raw)
  To: Li Yu; +Cc: Linux Netdev List, David Miller
In-Reply-To: <4F7439AE.6050006@gmail.com>

On 03/29/2012 02:30 PM, Li Yu wrote:
> 于 2012年03月28日 23:38, Pavel Emelyanov 写道:
>> Reading queues under repair mode is done with recvmsg call.
>> The queue-under-repair set by TCP_REPAIR_QUEUE option is used
>> to determine which queue should be read. Thus both send and
>> receive queue can be read with this.
>>
>> Caller must pass the MSG_PEEK flag.
>>
>> Writing to queues is done with sendmsg call and yet again --
>> the repair-queue option can be used to push data into the
>> receive queue.
>>
>> When putting an skb into receive queue a zero tcp header is
>> appented to its head to address the tcp_hdr(skb)->syn and
>> the ->fin checks by the (after repair) tcp_recvmsg. These
>> flags flags are both set to zero and that's why.
>>
>> The fin cannot be met in the queue while reading the source
>> socket, since the repair only works for closed/established
>> sockets and queueing fin packet always changes its state.
>>
>> The syn in the queue denotes that the respective skb's seq
>> is "off-by-one" as compared to the actual payload lenght. Thus,
>> at the rcv queue refill we can just drop this flag and set the
>> skb's sequences to precice values. IOW -- emulate the situation
>> when the packet with data and syn is splitted into two -- a
>> packet with syn and a packet with data and the former one is
>> already "eaten".
>>
>> When the repair mode is turned off, the write queue seqs are
>> updated so that the whole queue is considered to be 'already sent,
>> waiting for ACKs' (write_seq = snd_nxt<= snd_una). From the
>> protocol POV the send queue looks like it was sent, but the data
>> between the write_seq and snd_nxt is lost in the network.
>>
>> This helps to avoid another sockoption for setting the snd_nxt
>> sequence. Leaving the whole queue in a 'not yet sent' state (as
>> it will be after sendmsg-s) will not allow to receive any acks
>> from the peer since the ack_seq will be after the snd_nxt. Thus
>> even the ack for the window probe will be dropped and the
>> connection will be 'locked' with the zero peer window.
>>
> 
> Do we need to restore various TCP options switch bits. e.g. window
> scale factor, sack_ok and so on.

SACK-s -- yes, this is in TODO list. Various window stuff -- not necessary.
TCP will eventually negotiate proper values again.

> En, I think the recorded mss_cache may be need to restored too.

Same with mss. As far as I understand this one will be re-detected after
a connection restore.

> Thanks.
> 
> Yu

^ permalink raw reply

* Re: [PATCH 3/3] tcp: Repair socket queues
From: Li Yu @ 2012-03-29 10:41 UTC (permalink / raw)
  To: Pavel Emelyanov; +Cc: Linux Netdev List, David Miller
In-Reply-To: <4F743B32.4050107@parallels.com>

于 2012年03月29日 18:36, Pavel Emelyanov 写道:
> On 03/29/2012 02:30 PM, Li Yu wrote:
>> 于 2012年03月28日 23:38, Pavel Emelyanov 写道:
>>> Reading queues under repair mode is done with recvmsg call.
>>> The queue-under-repair set by TCP_REPAIR_QUEUE option is used
>>> to determine which queue should be read. Thus both send and
>>> receive queue can be read with this.
>>>
>>> Caller must pass the MSG_PEEK flag.
>>>
>>> Writing to queues is done with sendmsg call and yet again --
>>> the repair-queue option can be used to push data into the
>>> receive queue.
>>>
>>> When putting an skb into receive queue a zero tcp header is
>>> appented to its head to address the tcp_hdr(skb)->syn and
>>> the ->fin checks by the (after repair) tcp_recvmsg. These
>>> flags flags are both set to zero and that's why.
>>>
>>> The fin cannot be met in the queue while reading the source
>>> socket, since the repair only works for closed/established
>>> sockets and queueing fin packet always changes its state.
>>>
>>> The syn in the queue denotes that the respective skb's seq
>>> is "off-by-one" as compared to the actual payload lenght. Thus,
>>> at the rcv queue refill we can just drop this flag and set the
>>> skb's sequences to precice values. IOW -- emulate the situation
>>> when the packet with data and syn is splitted into two -- a
>>> packet with syn and a packet with data and the former one is
>>> already "eaten".
>>>
>>> When the repair mode is turned off, the write queue seqs are
>>> updated so that the whole queue is considered to be 'already sent,
>>> waiting for ACKs' (write_seq = snd_nxt<= snd_una). From the
>>> protocol POV the send queue looks like it was sent, but the data
>>> between the write_seq and snd_nxt is lost in the network.
>>>
>>> This helps to avoid another sockoption for setting the snd_nxt
>>> sequence. Leaving the whole queue in a 'not yet sent' state (as
>>> it will be after sendmsg-s) will not allow to receive any acks
>>> from the peer since the ack_seq will be after the snd_nxt. Thus
>>> even the ack for the window probe will be dropped and the
>>> connection will be 'locked' with the zero peer window.
>>>
>>
>> Do we need to restore various TCP options switch bits. e.g. window
>> scale factor, sack_ok and so on.
>
> SACK-s -- yes, this is in TODO list. Various window stuff -- not necessary.
> TCP will eventually negotiate proper values again.
>
>> En, I think the recorded mss_cache may be need to restored too.
>
> Same with mss. As far as I understand this one will be re-detected after
> a connection restore.
>

After the connection are repaired, it directly enter ESTABLISHED state,
so this TCP connection has no chance to negotiate such optional
features, such negotiation only can occurs at 3WHS.

Thanks.

Yu

>> Thanks.
>>
>> Yu
>

^ permalink raw reply

* Re: [PATCH 3/3] tcp: Repair socket queues
From: Li Yu @ 2012-03-29 10:41 UTC (permalink / raw)
  To: Pavel Emelyanov; +Cc: Linux Netdev List, David Miller
In-Reply-To: <4F743B32.4050107@parallels.com>

于 2012年03月29日 18:36, Pavel Emelyanov 写道:
> On 03/29/2012 02:30 PM, Li Yu wrote:
>> 于 2012年03月28日 23:38, Pavel Emelyanov 写道:
>>> Reading queues under repair mode is done with recvmsg call.
>>> The queue-under-repair set by TCP_REPAIR_QUEUE option is used
>>> to determine which queue should be read. Thus both send and
>>> receive queue can be read with this.
>>>
>>> Caller must pass the MSG_PEEK flag.
>>>
>>> Writing to queues is done with sendmsg call and yet again --
>>> the repair-queue option can be used to push data into the
>>> receive queue.
>>>
>>> When putting an skb into receive queue a zero tcp header is
>>> appented to its head to address the tcp_hdr(skb)->syn and
>>> the ->fin checks by the (after repair) tcp_recvmsg. These
>>> flags flags are both set to zero and that's why.
>>>
>>> The fin cannot be met in the queue while reading the source
>>> socket, since the repair only works for closed/established
>>> sockets and queueing fin packet always changes its state.
>>>
>>> The syn in the queue denotes that the respective skb's seq
>>> is "off-by-one" as compared to the actual payload lenght. Thus,
>>> at the rcv queue refill we can just drop this flag and set the
>>> skb's sequences to precice values. IOW -- emulate the situation
>>> when the packet with data and syn is splitted into two -- a
>>> packet with syn and a packet with data and the former one is
>>> already "eaten".
>>>
>>> When the repair mode is turned off, the write queue seqs are
>>> updated so that the whole queue is considered to be 'already sent,
>>> waiting for ACKs' (write_seq = snd_nxt<= snd_una). From the
>>> protocol POV the send queue looks like it was sent, but the data
>>> between the write_seq and snd_nxt is lost in the network.
>>>
>>> This helps to avoid another sockoption for setting the snd_nxt
>>> sequence. Leaving the whole queue in a 'not yet sent' state (as
>>> it will be after sendmsg-s) will not allow to receive any acks
>>> from the peer since the ack_seq will be after the snd_nxt. Thus
>>> even the ack for the window probe will be dropped and the
>>> connection will be 'locked' with the zero peer window.
>>>
>>
>> Do we need to restore various TCP options switch bits. e.g. window
>> scale factor, sack_ok and so on.
>
> SACK-s -- yes, this is in TODO list. Various window stuff -- not necessary.
> TCP will eventually negotiate proper values again.
>
>> En, I think the recorded mss_cache may be need to restored too.
>
> Same with mss. As far as I understand this one will be re-detected after
> a connection restore.
>

After the connection are repaired, it directly enter ESTABLISHED state,
so this TCP connection has no chance to negotiate such optional
features, such negotiation only can occur at 3WHS.

Thanks.

Yu

>> Thanks.
>>
>> Yu
>

^ permalink raw reply

* Re: [BUGFIX][PATCH 2/3] memcg/tcp: remove static_branch_slow_dec() at changing limit
From: Glauber Costa @ 2012-03-29 10:58 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki; +Cc: netdev, David Miller, Andrew Morton
In-Reply-To: <4F740A41.6040002@jp.fujitsu.com>

On 03/29/2012 09:07 AM, KAMEZAWA Hiroyuki wrote:
> tcp memcontrol uses static_branch to optimize limit=RESOURCE_MAX case.
> If all cgroup's limit=RESOUCE_MAX, resource usage is not accounted.
> But it's buggy now.
> 
> For example, do following
>   # while sleep 1;do
>     echo 9223372036854775807>  /cgroup/memory/A/memory.kmem.tcp.limit_in_bytes;
>     echo 300M>  /cgroup/memory/A/memory.kmem.tcp.limit_in_bytes;
>     done
> 
> and run network application under A. tcp's usage is sometimes accounted
> and sometimes not accounted because of frequent changes of static_branch.
> Then, finally, you can see broken tcp.usage_in_bytes.
> WARN_ON() is printed because res_counter->usage goes below 0.
> ==
> kernel: ------------[ cut here ]----------
> kernel: WARNING: at kernel/res_counter.c:96 res_counter_uncharge_locked+0x37/0x40()
>   <snip>
> kernel: Pid: 17753, comm: bash Tainted: G  W    3.3.0+ #99
> kernel: Call Trace:
> kernel:<IRQ>   [<ffffffff8104cc9f>] warn_slowpath_common+0x7f/0xc0
> kernel: [<ffffffff810d7e88>] ? rb_reserve__next_event+0x68/0x470
> kernel: [<ffffffff8104ccfa>] warn_slowpath_null+0x1a/0x20
> kernel: [<ffffffff810b4e37>] res_counter_uncharge_locked+0x37/0x40
> ...
> ==
> 
> This patch removes static_branch_slow_dec() at changing res_counter's
> limit to RESOUCE_MAX. By this, once accounting started, the accountting
> will continue until the tcp cgroup is destroyed.
> 
> I think this will not be problem in real use.
> 

So...

Are the warnings still there if you have your other patch in this series?
Maybe what we should do is, flush the resource counters so they go back
to 0 besides decrementing the static branch. This way we get a more
consistent behavior.

Another thing to keep in mind, is that the static branch will only be
inactive if we turn off *all* controllers. You see this happening
because you are only testing with one.
So even if we go to the route you're proposing, we could probably try
doing something on the
global level, instead of a per-memcg boolean flat.

^ permalink raw reply

* Re: [PATCH linux-next] dmaengine: add context parameter fixups
From: Vinod Koul @ 2012-03-29 10:59 UTC (permalink / raw)
  To: Mark Brown
  Cc: fabio.estevam, alsa-devel, samuel, paul.gortmaker, Takashi Iwai,
	netdev, linux-kernel, Alexandre Bounine, linux-next,
	dan.j.williams, Fabio Estevam, lrg
In-Reply-To: <20120328155106.GY3232@opensource.wolfsonmicro.com>

On Wed, 2012-03-28 at 16:51 +0100, Mark Brown wrote:
> On Wed, Mar 28, 2012 at 05:44:22PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > No, not yet - it was only sent after the merge window.  Quite why nobody
> > > managed to notice it before then I don't know.  It'll go to him soon, or
> > > at least to Takashi, but don't know if it'll make -rc1 or not.
> 
> > How is the situation now?
> 
> > FYI, I'm going to send a pull request to Linus tomorrow or on Friday.
> > If anything needed to be merged in rc1, it must be there in time.
> 
> It's in my tree, I didn't see an enormous rush to get it in TBH.
I should be sending the pull request for slave-dma to Linus this
evening.
Can you please include the fix I did in your tree in your pull request.


-- 
~Vinod

^ permalink raw reply

* Re: 答复: 答复: [PATCH] set fake_rtable's dst to NULL to avoid kernel Oops.
From: Eric Dumazet @ 2012-03-29 11:31 UTC (permalink / raw)
  To: Peter Huang (Peng); +Cc: linux-kernel, harry.majun, zhoukang7, 'netdev'
In-Reply-To: <004501cd0d8f$c4e133b0$4ea39b10$%huangpeng@huawei.com>

On Thu, 2012-03-29 at 17:38 +0800, Peter Huang (Peng) wrote:
> Thks for your mail.
> 
> >Check net/bridge/br_netfilter.c and commits e688a6048076 (net: introduce
> >DST_NOPEER dst flag )  4adf0af6818f3ea5 (bridge: send correct MTU value
> >in PMTU (revised))
> 
> This patch already included in kernel-3.3, but for our case, virtual tap device's delayed
> Deletion will also cause kernel oops even in kernel3.3.

I was suggesting you take a look at the commit content ;)

Then you can see the code in br_nf_local_in(), a bit cleaner than yours.

^ permalink raw reply

* 答复: 答复: 答复: [PATCH] set fake_rtable's dst to NULL to avoid kernel Oops.
From: Peter Huang (Peng) @ 2012-03-29 11:41 UTC (permalink / raw)
  To: 'Eric Dumazet'
  Cc: linux-kernel, harry.majun, zhoukang7, 'netdev'
In-Reply-To: <1333020682.2325.517.camel@edumazet-glaptop>

> >Check net/bridge/br_netfilter.c and commits e688a6048076 (net: introduce
> >DST_NOPEER dst flag )  4adf0af6818f3ea5 (bridge: send correct MTU value
> >in PMTU (revised))
> 
> This patch already included in kernel-3.3, but for our case, virtual tap device's delayed
> Deletion will also cause kernel oops even in kernel3.3.

>I was suggesting you take a look at the commit content ;)

>Then you can see the code in br_nf_local_in(), a bit cleaner than yours.

Yes, I will look into it, and trying to find out why this patch didn't work for our case.
Thanks for your advice:)

^ permalink raw reply

* Re: [PATCH] fix a bug in emitting the 16-bit immediate operand of AND
From: Eric Dumazet @ 2012-03-29 11:45 UTC (permalink / raw)
  To: zhuangfeiran@ict.ac.cn; +Cc: davem, netdev, linux-kernel
In-Reply-To: <4F742AE4.6050006@ict.ac.cn>

Le jeudi 29 mars 2012 à 17:27 +0800, zhuangfeiran@ict.ac.cn a écrit :
> When K >= 0xFFFF0000, AND needs the two least significant bytes of K as
> its operand, but EMIT2() gives it the least significant byte of K and
> 0x2. EMIT() should be used here to replace EMIT2().
> 
> Signed-off-by: Feiran Zhuang  <zhuangfeiran@ict.ac.cn>
> ---
>  arch/x86/net/bpf_jit_comp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 5671752..5a5b6e4 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -289,7 +289,7 @@ void bpf_jit_compile(struct sk_filter *fp)
>  					EMIT2(0x24, K & 0xFF); /* and imm8,%al */
>  				} else if (K >= 0xFFFF0000) {
>  					EMIT2(0x66, 0x25);	/* and imm16,%ax */
> -					EMIT2(K, 2);
> +					EMIT(K, 2);
>  				} else {
>  					EMIT1_off32(0x25, K);	/* and imm32,%eax */
>  				}

Good catch, thanks !

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

^ permalink raw reply

* Re: [REGRESSION][PATCH] bpf_jit drops the ball on indirect negative mem references
From: Jan Seiffert @ 2012-03-29 11:54 UTC (permalink / raw)
  To: netdev; +Cc: Eric Dumazet, linux-kernel, David S. Miller, Matt Evans
In-Reply-To: <1332967172.2325.22.camel@edumazet-glaptop>

Eric Dumazet schrieb:
> On Wed, 2012-03-28 at 22:26 +0200, Jan Seiffert wrote:
> [snip]
>> Say you have a UDP socket, and you want to filter for bogus source
>> addresses (drop already in kernel to save the context switch).
>> To have only one bpf program for ipv4 and ipv6 (you have to checked
>> the same bogus v4 addresses in mapped space), there is a point where
>> it elegant to have a negative offset saved in the X register.
> Cool, thats a valid use, thanks.
>
>
> Problem is you slow down the jit in its normal use, for a very specific
> use. 
>
> Please rework your patch so that absolute loads of positive offsets
> (known at compile time) dont have to test negative offsets at run time.
>
> You add two instructions per load, and thats not good.
>
> Something like :
>
> sk_load_word:
>         .globl  sk_load_word
>  
>        test    %esi,%esi
>        js      bpf_slow_path_word_neg
>
> sk_load_word_positive_offset:
> 	.globl sk_load_word_positive_offset
>
>         mov     %r9d,%eax               # hlen
>         sub     %esi,%eax               # hlen - offset
>         cmp     $3,%eax
>
> ...
>
>
>
Ok, to keep the ball rolling here is a V2 with the changes you suggested:

Consider the following test program:

#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <pcap-bpf.h>

#define die(x) do {perror(x); return 1;} while (0)
struct bpf_insn udp_filter[] = {
	/*   0 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(0)), /* leax	net[0] */
	/*   1 */ BPF_STMT(BPF_LD|BPF_B|BPF_IND, 0),             /* ldb	[x+0] */
	/*   2 */ BPF_STMT(BPF_RET|BPF_A, 0),                    /* ret	a */
};

int main(int argc, char *argv[])
{
	char buf[512];
	struct sockaddr_in addr;
	struct bpf_program prg;
	socklen_t addr_s;
	ssize_t res;
	int fd;

	addr.sin_family = AF_INET;
	addr.sin_port = htons(5000);
	addr.sin_addr.s_addr = 0;
	addr_s = sizeof(addr);
	prg.bf_len = sizeof(udp_filter)/sizeof(udp_filter[0]);
	prg.bf_insns = udp_filter;
	if(-1 == (fd = socket(AF_INET, SOCK_DGRAM, 0)))
		die("socket");
	if(-1 == bind(fd, (struct sockaddr *)&addr, sizeof(addr)))
		die("bind");
	if(-1 == setsockopt(fd, SOL_SOCKET, SO_ATTACH_FILTER, &prg, sizeof(prg)))
		die("setsockopt");
	res = recvfrom(fd, buf, sizeof(buf), 0, (struct sockaddr *)&addr, &addr_s);
	if(res != -1)
		printf("packet received: %zi bytes\n", res);
	else
		die("recvfrom");
	return 0;
}

when used with the bpf jit disabled works:
console 1 $ ./bpf
console 2 $ echo "hello" | nc -u localhost 5000
console 1: packet received: 6 bytes

When the bpf jit gets enabled (echo 100 >
/proc/sys/net/core/bpf_jit_enable) the same program stops working:
console 1 $ ./bpf
console 2 $ echo "hello" | nc -u localhost 5000
console 1:

The reason is that both jits (x86 and powerpc) do not handle negative
memory references like SKF_NET_OFF or SKF_LL_OFF, only the simple
ancillary data references are supported (by mapping to special
instructions).
In the case of an absolute reference, the jit aborts the translation
if a negative reference is seen, also a negative k on the indirect
load aborts the translation, but if X is negative to begin with, only
the error handler is reached at runtime which drops the whole packet.

I propose the following patch to fix this situation.
Lightly tested on x86, but the powerpc asm part is prop. wrong.


Signed-of-by: Jan Seiffert <kaffeemonster@googlemail.com>

diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index af1ab5e..e9b57b3 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -49,6 +49,10 @@
  * Assembly helpers from arch/powerpc/net/bpf_jit.S:
  */
 extern u8 sk_load_word[], sk_load_half[], sk_load_byte[], sk_load_byte_msh[];
+extern u8 sk_load_word_positive_offset[], sk_load_half_positive_offset[];
+extern u8 sk_load_byte_positive_offset[], sk_load_byte_msh_positive_offset[];
+extern u8 sk_load_word_negative_offset[], sk_load_half_negative_offset[];
+extern u8 sk_load_byte_negative_offset[], sk_load_byte_msh_negative_offset[];
 
 #define FUNCTION_DESCR_SIZE	24
 
diff --git a/arch/powerpc/net/bpf_jit_64.S b/arch/powerpc/net/bpf_jit_64.S
index ff4506e..e590aa5 100644
--- a/arch/powerpc/net/bpf_jit_64.S
+++ b/arch/powerpc/net/bpf_jit_64.S
@@ -31,14 +31,13 @@
  * then branch directly to slow_path_XXX if required.  (In fact, could
  * load a spare GPR with the address of slow_path_generic and pass size
  * as an argument, making the call site a mtlr, li and bllr.)
- *
- * Technically, the "is addr < 0" check is unnecessary & slowing down
- * the ABS path, as it's statically checked on generation.
  */
 	.globl	sk_load_word
 sk_load_word:
 	cmpdi	r_addr, 0
-	blt	bpf_error
+	blt	bpf_slow_path_word_neg
+	.globl	sk_load_word_positive_offset
+sk_load_word_positive_offset:
 	/* Are we accessing past headlen? */
 	subi	r_scratch1, r_HL, 4
 	cmpd	r_scratch1, r_addr
@@ -51,7 +50,9 @@ sk_load_word:
 	.globl	sk_load_half
 sk_load_half:
 	cmpdi	r_addr, 0
-	blt	bpf_error
+	blt	bpf_slow_path_half_neg
+	.globl	sk_load_half_positive_offset
+sk_load_half_positive_offset:
 	subi	r_scratch1, r_HL, 2
 	cmpd	r_scratch1, r_addr
 	blt	bpf_slow_path_half
@@ -61,7 +62,9 @@ sk_load_half:
 	.globl	sk_load_byte
 sk_load_byte:
 	cmpdi	r_addr, 0
-	blt	bpf_error
+	blt	bpf_slow_path_byte_neg
+	.globl	sk_load_byte_positive_offset
+sk_load_byte_positive_offset:
 	cmpd	r_HL, r_addr
 	ble	bpf_slow_path_byte
 	lbzx	r_A, r_D, r_addr
@@ -69,22 +72,20 @@ sk_load_byte:
 
 /*
  * BPF_S_LDX_B_MSH: ldxb  4*([offset]&0xf)
- * r_addr is the offset value, already known positive
+ * r_addr is the offset value
  */
 	.globl sk_load_byte_msh
 sk_load_byte_msh:
+	cmpdi	r_addr, 0
+	blt	bpf_slow_path_byte_msh_neg
+	.globl sk_load_byte_msh_positive_offset
+sk_load_byte_msh_positive_offset:
 	cmpd	r_HL, r_addr
 	ble	bpf_slow_path_byte_msh
 	lbzx	r_X, r_D, r_addr
 	rlwinm	r_X, r_X, 2, 32-4-2, 31-2
 	blr
 
-bpf_error:
-	/* Entered with cr0 = lt */
-	li	r3, 0
-	/* Generated code will 'blt epilogue', returning 0. */
-	blr
-
 /* Call out to skb_copy_bits:
  * We'll need to back up our volatile regs first; we have
  * local variable space at r1+(BPF_PPC_STACK_BASIC).
@@ -136,3 +137,85 @@ bpf_slow_path_byte_msh:
 	lbz	r_X, BPF_PPC_STACK_BASIC+(2*8)(r1)
 	rlwinm	r_X, r_X, 2, 32-4-2, 31-2
 	blr
+
+/* Call out to bpf_internal_load_pointer_neg_helper:
+ * We'll need to back up our volatile regs first; we have
+ * local variable space at r1+(BPF_PPC_STACK_BASIC).
+ * Allocate a new stack frame here to remain ABI-compliant in
+ * stashing LR.
+ */
+#define sk_negative_common(SIZE)				\
+	mflr	r0;						\
+	std	r0, 16(r1);					\
+	/* R3 goes in parameter space of caller's frame */	\
+	std	r_skb, (BPF_PPC_STACKFRAME+48)(r1);		\
+	std	r_A, (BPF_PPC_STACK_BASIC+(0*8))(r1);		\
+	std	r_X, (BPF_PPC_STACK_BASIC+(1*8))(r1);		\
+	stdu	r1, -BPF_PPC_SLOWPATH_FRAME(r1);		\
+	/* R3 = r_skb, as passed */				\
+	mr	r4, r_addr;					\
+	li	r5, SIZE;					\
+	bl	bpf_internal_load_pointer_neg_helper;		\
+	/* R3 != 0 on success */				\
+	addi	r1, r1, BPF_PPC_SLOWPATH_FRAME;			\
+	ld	r0, 16(r1);					\
+	ld	r_A, (BPF_PPC_STACK_BASIC+(0*8))(r1);		\
+	ld	r_X, (BPF_PPC_STACK_BASIC+(1*8))(r1);		\
+	mtlr	r0;						\
+	cmpldi	r3, 0;						\
+	beq	bpf_error_slow;	/* cr0 = EQ */			\
+	mr	r_addr, r3;					\
+	ld	r_skb, (BPF_PPC_STACKFRAME+48)(r1);		\
+	/* Great success! */
+
+bpf_slow_path_word_neg:
+	lis     r_scratch1,-32	/* SKF_LL_OFF */
+	cmpd	r_addr, r_scratch1	/* addr < SKF_* */
+	blt	bpf_error	/* cr0 = LT */
+	.globl	sk_load_word_negative_offset
+sk_load_word_negative_offset:
+	sk_negative_common(4)
+	lwz	r_A, 0(r_addr)
+	blr
+
+bpf_slow_path_half_neg:
+	lis     r_scratch1,-32	/* SKF_LL_OFF */
+	cmpd	r_addr, r_scratch1	/* addr < SKF_* */
+	blt	bpf_error	/* cr0 = LT */
+	.globl	sk_load_half_negative_offset
+sk_load_half_negative_offset:
+	sk_negative_common(2)
+	lhz	r_A, 0(r_addr)
+	blr
+
+bpf_slow_path_byte_neg:
+	lis     r_scratch1,-32	/* SKF_LL_OFF */
+	cmpd	r_addr, r_scratch1	/* addr < SKF_* */
+	blt	bpf_error	/* cr0 = LT */
+	.globl	sk_load_byte_negative_offset
+sk_load_byte_negative_offset:
+	sk_negative_common(1)
+	lbz	r_A, 0(r_addr)
+	blr
+
+bpf_slow_path_byte_msh_neg:
+	lis     r_scratch1,-32	/* SKF_LL_OFF */
+	cmpd	r_addr, r_scratch1	/* addr < SKF_* */
+	blt	bpf_error	/* cr0 = LT */
+	.globl	sk_load_byte_msh_negative_offset
+sk_load_byte_msh_negative_offset:
+	sk_negative_common(1)
+	lbz	r_X, 0(r_addr)
+	rlwinm	r_X, r_X, 2, 32-4-2, 31-2
+	blr
+
+bpf_error_slow:
+	/* fabricate a cr0 = lt */
+	li	r_scratch1, -1
+	cmpdi	r_scratch1, 0
+bpf_error:
+	/* Entered with cr0 = lt */
+	li	r3, 0
+	/* Generated code will 'blt epilogue', returning 0. */
+	blr
+
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 73619d3..2dc8b14 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -127,6 +127,9 @@ static void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx)
 	PPC_BLR();
 }
 
+#define CHOOSE_LOAD_FUNC(K, func) \
+	((int)K < 0 ? ((int)K >= SKF_LL_OFF ? func##_negative_offset : func) : func##_positive_offset)
+
 /* Assemble the body code between the prologue & epilogue. */
 static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
 			      struct codegen_context *ctx,
@@ -391,21 +394,16 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
 
 			/*** Absolute loads from packet header/data ***/
 		case BPF_S_LD_W_ABS:
-			func = sk_load_word;
+			func = CHOOSE_LOAD_FUNC(K, sk_load_word);
 			goto common_load;
 		case BPF_S_LD_H_ABS:
-			func = sk_load_half;
+			func = CHOOSE_LOAD_FUNC(K, sk_load_half);
 			goto common_load;
 		case BPF_S_LD_B_ABS:
-			func = sk_load_byte;
+			func = CHOOSE_LOAD_FUNC(K, sk_load_byte);
 		common_load:
-			/*
-			 * Load from [K].  Reference with the (negative)
-			 * SKF_NET_OFF/SKF_LL_OFF offsets is unsupported.
-			 */
+			/* Load from [K]. */
 			ctx->seen |= SEEN_DATAREF;
-			if ((int)K < 0)
-				return -ENOTSUPP;
 			PPC_LI64(r_scratch1, func);
 			PPC_MTLR(r_scratch1);
 			PPC_LI32(r_addr, K);
@@ -429,7 +427,7 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
 		common_load_ind:
 			/*
 			 * Load from [X + K].  Negative offsets are tested for
-			 * in the helper functions, and result in a 'ret 0'.
+			 * in the helper functions.
 			 */
 			ctx->seen |= SEEN_DATAREF | SEEN_XREG;
 			PPC_LI64(r_scratch1, func);
@@ -443,13 +441,7 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
 			break;
 
 		case BPF_S_LDX_B_MSH:
-			/*
-			 * x86 version drops packet (RET 0) when K<0, whereas
-			 * interpreter does allow K<0 (__load_pointer, special
-			 * ancillary data).  common_load returns ENOTSUPP if K<0,
-			 * so we fall back to interpreter & filter works.
-			 */
-			func = sk_load_byte_msh;
+			func = CHOOSE_LOAD_FUNC(K, sk_load_byte_msh);
 			goto common_load;
 			break;
 
diff --git a/arch/x86/net/bpf_jit.S b/arch/x86/net/bpf_jit.S
index 6687022..63ae130 100644
--- a/arch/x86/net/bpf_jit.S
+++ b/arch/x86/net/bpf_jit.S
@@ -18,17 +18,18 @@
  * r9d : hlen = skb->len - skb->data_len
  */
 #define SKBDATA	%r8
+#define SKF_MAX_NEG_OFF    $(-0x200000) /* SKF_LL_OFF from filter.h */
 
-sk_load_word_ind:
-	.globl	sk_load_word_ind
-
-	add	%ebx,%esi	/* offset += X */
-#	test    %esi,%esi	/* if (offset < 0) goto bpf_error; */
-	js	bpf_error
-
+	.p2align 1
 sk_load_word:
 	.globl	sk_load_word
 
+	test	%esi,%esi
+	js	bpf_slow_path_word_neg
+
+sk_load_word_positive_offset:
+	.globl	sk_load_word_positive_offset
+
 	mov	%r9d,%eax		# hlen
 	sub	%esi,%eax		# hlen - offset
 	cmp	$3,%eax
@@ -37,16 +38,16 @@ sk_load_word:
 	bswap   %eax  			/* ntohl() */
 	ret
 
-
-sk_load_half_ind:
-	.globl sk_load_half_ind
-
-	add	%ebx,%esi	/* offset += X */
-	js	bpf_error
-
+	.p2align 1
 sk_load_half:
 	.globl	sk_load_half
 
+	test	%esi,%esi
+	js	bpf_slow_path_half_neg
+
+sk_load_half_positive_offset:
+	.globl	sk_load_half_positive_offset
+
 	mov	%r9d,%eax
 	sub	%esi,%eax		#	hlen - offset
 	cmp	$1,%eax
@@ -55,14 +56,16 @@ sk_load_half:
 	rol	$8,%ax			# ntohs()
 	ret
 
-sk_load_byte_ind:
-	.globl sk_load_byte_ind
-	add	%ebx,%esi	/* offset += X */
-	js	bpf_error
-
+	.p2align 1
 sk_load_byte:
 	.globl	sk_load_byte
 
+	test	%esi,%esi
+	js	bpf_slow_path_byte_neg
+
+sk_load_byte_positive_offset:
+	.globl	sk_load_byte_positive_offset
+
 	cmp	%esi,%r9d   /* if (offset >= hlen) goto bpf_slow_path_byte */
 	jle	bpf_slow_path_byte
 	movzbl	(SKBDATA,%rsi),%eax
@@ -73,25 +76,22 @@ sk_load_byte:
  *
  * Implements BPF_S_LDX_B_MSH : ldxb  4*([offset]&0xf)
  * Must preserve A accumulator (%eax)
- * Inputs : %esi is the offset value, already known positive
+ * Inputs : %esi is the offset value
  */
-ENTRY(sk_load_byte_msh)
-	CFI_STARTPROC
+	.p2align 1
+sk_load_byte_msh:
+	.globl	sk_load_byte_msh
+	test	%esi,%esi
+	js	bpf_slow_path_byte_msh_neg
+
+sk_load_byte_msh_positive_offset:
+	.globl	sk_load_byte_msh_positive_offset
 	cmp	%esi,%r9d      /* if (offset >= hlen) goto bpf_slow_path_byte_msh */
 	jle	bpf_slow_path_byte_msh
 	movzbl	(SKBDATA,%rsi),%ebx
 	and	$15,%bl
 	shl	$2,%bl
 	ret
-	CFI_ENDPROC
-ENDPROC(sk_load_byte_msh)
-
-bpf_error:
-# force a return 0 from jit handler
-	xor		%eax,%eax
-	mov		-8(%rbp),%rbx
-	leaveq
-	ret
 
 /* rsi contains offset and can be scratched */
 #define bpf_slow_path_common(LEN)		\
@@ -108,6 +108,7 @@ bpf_error:
 	pop	%rdi
 
 
+	.p2align 1
 bpf_slow_path_word:
 	bpf_slow_path_common(4)
 	js	bpf_error
@@ -115,6 +116,7 @@ bpf_slow_path_word:
 	bswap	%eax
 	ret
 
+	.p2align 1
 bpf_slow_path_half:
 	bpf_slow_path_common(2)
 	js	bpf_error
@@ -123,12 +125,14 @@ bpf_slow_path_half:
 	movzwl	%ax,%eax
 	ret
 
+	.p2align 1
 bpf_slow_path_byte:
 	bpf_slow_path_common(1)
 	js	bpf_error
 	movzbl	-12(%rbp),%eax
 	ret
 
+	.p2align 1
 bpf_slow_path_byte_msh:
 	xchg	%eax,%ebx /* dont lose A , X is about to be scratched */
 	bpf_slow_path_common(1)
@@ -138,3 +142,73 @@ bpf_slow_path_byte_msh:
 	shl	$2,%al
 	xchg	%eax,%ebx
 	ret
+
+#define sk_negative_common(SIZE)				\
+	push	%rdi;	/* save skb */				\
+	push	%r9;						\
+	push	SKBDATA;					\
+/* rsi already has offset */					\
+	mov	$SIZE,%ecx;	/* size */			\
+	call	bpf_internal_load_pointer_neg_helper;		\
+	test	%rax,%rax;					\
+	pop	SKBDATA;					\
+	pop	%r9;						\
+	pop	%rdi;						\
+	jz	bpf_error
+
+
+	.p2align 1
+bpf_slow_path_word_neg:
+	cmp	SKF_MAX_NEG_OFF, %esi	/* test range */
+	jl	bpf_error	/* offset lower -> error  */
+sk_load_word_negative_offset:
+	.globl	sk_load_word_negative_offset
+	sk_negative_common(4)
+	mov	(%rax), %eax
+	bswap	%eax
+	ret
+
+	.p2align 1
+bpf_slow_path_half_neg:
+	cmp	SKF_MAX_NEG_OFF, %esi
+	jl	bpf_error
+sk_load_half_negative_offset:
+	.globl	sk_load_half_negative_offset
+	sk_negative_common(2)
+	mov	(%rax),%ax
+	rol	$8,%ax
+	movzwl	%ax,%eax
+	ret
+
+	.p2align 1
+bpf_slow_path_byte_neg:
+	cmp	SKF_MAX_NEG_OFF, %esi
+	jl	bpf_error
+sk_load_byte_negative_offset:
+	.globl	sk_load_byte_negative_offset
+	sk_negative_common(1)
+	movzbl	(%rax), %eax
+	ret
+
+	.p2align 1
+bpf_slow_path_byte_msh_neg:
+	cmp	SKF_MAX_NEG_OFF, %esi
+	jl	bpf_error
+sk_load_byte_msh_negative_offset:
+	.globl	sk_load_byte_msh_negative_offset
+	xchg	%eax,%ebx /* dont lose A , X is about to be scratched */
+	sk_negative_common(1)
+	movzbl	(%rax),%eax
+	and	$15,%al
+	shl	$2,%al
+	xchg	%eax,%ebx
+	ret
+
+	.p2align 1
+bpf_error:
+# force a return 0 from jit handler
+	xor		%eax,%eax
+	mov		-8(%rbp),%rbx
+	leaveq
+	ret
+
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 5671752..c20374f 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -30,7 +30,10 @@ int bpf_jit_enable __read_mostly;
  * assembly code in arch/x86/net/bpf_jit.S
  */
 extern u8 sk_load_word[], sk_load_half[], sk_load_byte[], sk_load_byte_msh[];
-extern u8 sk_load_word_ind[], sk_load_half_ind[], sk_load_byte_ind[];
+extern u8 sk_load_word_positive_offset[], sk_load_half_positive_offset[];
+extern u8 sk_load_byte_positive_offset[], sk_load_byte_msh_positive_offset[];
+extern u8 sk_load_word_negative_offset[], sk_load_half_negative_offset[];
+extern u8 sk_load_byte_negative_offset[], sk_load_byte_msh_negative_offset[];
 
 static inline u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
 {
@@ -117,6 +120,8 @@ static inline void bpf_flush_icache(void *start, void *end)
 	set_fs(old_fs);
 }
 
+#define CHOOSE_LOAD_FUNC(K, func) \
+	((int)K < 0 ? ((int)K >= SKF_LL_OFF ? func##_negative_offset : func) : func##_positive_offset)
 
 void bpf_jit_compile(struct sk_filter *fp)
 {
@@ -473,44 +478,42 @@ void bpf_jit_compile(struct sk_filter *fp)
 #endif
 				break;
 			case BPF_S_LD_W_ABS:
-				func = sk_load_word;
+				func = CHOOSE_LOAD_FUNC(K, sk_load_word);
 common_load:			seen |= SEEN_DATAREF;
-				if ((int)K < 0) {
-					/* Abort the JIT because __load_pointer() is needed. */
-					goto out;
-				}
 				t_offset = func - (image + addrs[i]);
 				EMIT1_off32(0xbe, K); /* mov imm32,%esi */
 				EMIT1_off32(0xe8, t_offset); /* call */
 				break;
 			case BPF_S_LD_H_ABS:
-				func = sk_load_half;
+				func = CHOOSE_LOAD_FUNC(K, sk_load_half);
 				goto common_load;
 			case BPF_S_LD_B_ABS:
-				func = sk_load_byte;
+				func = CHOOSE_LOAD_FUNC(K, sk_load_byte);
 				goto common_load;
 			case BPF_S_LDX_B_MSH:
-				if ((int)K < 0) {
-					/* Abort the JIT because __load_pointer() is needed. */
-					goto out;
-				}
+				func = CHOOSE_LOAD_FUNC(K, sk_load_byte_msh);
 				seen |= SEEN_DATAREF | SEEN_XREG;
-				t_offset = sk_load_byte_msh - (image + addrs[i]);
+				t_offset = func - (image + addrs[i]);
 				EMIT1_off32(0xbe, K);	/* mov imm32,%esi */
 				EMIT1_off32(0xe8, t_offset); /* call sk_load_byte_msh */
 				break;
 			case BPF_S_LD_W_IND:
-				func = sk_load_word_ind;
+				func = sk_load_word;
 common_load_ind:		seen |= SEEN_DATAREF | SEEN_XREG;
 				t_offset = func - (image + addrs[i]);
-				EMIT1_off32(0xbe, K);	/* mov imm32,%esi   */
+				if (K) {
+					EMIT2(0x8d, 0xb3); /* lea imm32(%rbx),%esi */
+					EMIT(K, 4);
+				} else {
+					EMIT2(0x89,0xde); /* mov %ebx,%esi */
+				}
 				EMIT1_off32(0xe8, t_offset);	/* call sk_load_xxx_ind */
 				break;
 			case BPF_S_LD_H_IND:
-				func = sk_load_half_ind;
+				func = sk_load_half;
 				goto common_load_ind;
 			case BPF_S_LD_B_IND:
-				func = sk_load_byte_ind;
+				func = sk_load_byte;
 				goto common_load_ind;
 			case BPF_S_JMP_JA:
 				t_offset = addrs[i + K] - addrs[i];
diff --git a/net/core/filter.c b/net/core/filter.c
index 5dea452..04ca613 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -41,7 +41,7 @@
 #include <linux/ratelimit.h>
 
 /* No hurry in this branch */
-static void *__load_pointer(const struct sk_buff *skb, int k, unsigned int size)
+void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, unsigned int size)
 {
 	u8 *ptr = NULL;
 
@@ -54,13 +54,14 @@ static void *__load_pointer(const struct sk_buff *skb, int k, unsigned int size)
 		return ptr;
 	return NULL;
 }
+EXPORT_SYMBOL(bpf_internal_load_pointer_neg_helper);
 
 static inline void *load_pointer(const struct sk_buff *skb, int k,
 				 unsigned int size, void *buffer)
 {
 	if (k >= 0)
 		return skb_header_pointer(skb, k, size, buffer);
-	return __load_pointer(skb, k, size);
+	return bpf_internal_load_pointer_neg_helper(skb, k, size);
 }
 
 /**

^ permalink raw reply related

* Re: [PATCH -3.2.y] igb: Fix locking to silence ASSERT_RTNL() warning during suspend/resume
From: Srivatsa S. Bhat @ 2012-03-29 12:31 UTC (permalink / raw)
  To: netdev
  Cc: Greg KH, zheng.z.yan, aaron.f.brown, jeffrey.t.kirsher,
	David S. Miller, stable, srivatsa.bhat,
	linux-kernel@vger.kernel.org, alexander.h.duyck, carolyn.wyborny,
	eric.dumazet, Ben Hutchings
In-Reply-To: <1332777648.3500.93.camel@deadeye>

[ CC'ing netdev mailing list and lkml. I have put relevant info by hand to
  preserve the context, since I didn't find any archives for the stable
  mailing list, to give a link to. ]

On 03/26/2012 09:30 PM, Ben Hutchings wrote:

> On Mon, 2012-03-26 at 17:37 +0530, Srivatsa S. Bhat wrote:
>> From: Zheng Yan <zheng.z.yan@intel.com>
>>
>> [ Upstream commit 749ab2cd127046df79084b6b9165b23491b1db5f ]
>>
>> During suspend/resume (and possibly in other scenarios as well), the
>> ASSERT_RTNL() warning is triggered in netif_set_real_num_tx/rx_queues()
>> functions, something like:

[  374.073765] RTNL: assertion failed at net/core/dev.c (1718)
[  374.073767] Pid: 566, comm: kworker/u:4 Not tainted 3.2.13-0.0.0.2.0f8ab74-trace #1
[  374.073769] RTNL: assertion failed at net/core/dev.c (1718)
[  374.073771] Call Trace:
[  374.073772] Pid: 5271, comm: kworker/u:10 Not tainted 3.2.13-0.0.0.2.0f8ab74-trace #1
[  374.073775] Call Trace:
[  374.073780]  [<ffffffff81385e3e>] netif_set_real_num_tx_queues+0x1ae/0x1d0
[  374.073785]  [<ffffffff81385e3e>] netif_set_real_num_tx_queues+0x1ae/0x1d0
[  374.073792]  [<ffffffffa0323f92>] igb_set_interrupt_capability+0x152/0x1f0 [igb]
[  374.073797]  [<ffffffffa0323f92>] igb_set_interrupt_capability+0x152/0x1f0 [igb]
[  374.073814]  [<ffffffffa0328968>] igb_init_interrupt_scheme+0x28/0x330 [igb]
[  374.073819]  [<ffffffffa0328968>] igb_init_interrupt_scheme+0x28/0x330 [igb]
[  374.073823]  [<ffffffffa0329a9c>] igb_resume+0x9c/0x160 [igb]
[  374.073828]  [<ffffffffa0329a9c>] igb_resume+0x9c/0x160 [igb]
[  374.073831]  [<ffffffff812707b2>] pci_legacy_resume+0x42/0x60
[  374.073835]  [<ffffffff812707b2>] pci_legacy_resume+0x42/0x60
[  374.073837]  [<ffffffff812709c0>] pci_pm_resume+0x90/0xd0
[  374.073840]  [<ffffffff812709c0>] pci_pm_resume+0x90/0xd0
[  374.073843]  [<ffffffff8131b59c>] pm_op+0x10c/0x1e0
[  374.073847]  [<ffffffff8131b59c>] pm_op+0x10c/0x1e0
[  374.073849]  [<ffffffff8131c5ca>] device_resume+0x27a/0x2c0
[  374.073852]  [<ffffffff8131c5ca>] device_resume+0x27a/0x2c0
[  374.073854]  [<ffffffff8131c631>] async_resume+0x21/0x50
[  374.073857]  [<ffffffff8131c631>] async_resume+0x21/0x50
[  374.073860]  [<ffffffff8107e564>] async_run_entry_fn+0x84/0x180
[  374.073864]  [<ffffffff8107e564>] async_run_entry_fn+0x84/0x180
[  374.073867]  [<ffffffff8106ecb1>] process_one_work+0x171/0x350
[  374.073871]  [<ffffffff8106ecb1>] process_one_work+0x171/0x350
[  374.073874]  [<ffffffff8107e4e0>] ? async_schedule+0x20/0x20
[  374.073876]  [<ffffffff8107e4e0>] ? async_schedule+0x20/0x20
[  374.073879]  [<ffffffff81071eeb>] worker_thread+0x18b/0x430
[  374.073881]  [<ffffffff81071eeb>] worker_thread+0x18b/0x430
[  374.073884]  [<ffffffff81071d60>] ? manage_workers+0x120/0x120
[  374.073887]  [<ffffffff81071d60>] ? manage_workers+0x120/0x120
[  374.073890]  [<ffffffff8107683e>] kthread+0x9e/0xb0
[  374.073892]  [<ffffffff8107683e>] kthread+0x9e/0xb0
[  374.073896]  [<ffffffff81451e04>] kernel_thread_helper+0x4/0x10
[  374.073899]  [<ffffffff81451e04>] kernel_thread_helper+0x4/0x10
[  374.073902]  [<ffffffff81447c21>] ? retint_restore_args+0x13/0x13
[  374.073905]  [<ffffffff81447c21>] ? retint_restore_args+0x13/0x13
[  374.073907]  [<ffffffff810767a0>] ? kthread_worker_fn+0x1b0/0x1b0
[  374.073911]  [<ffffffff810767a0>] ? kthread_worker_fn+0x1b0/0x1b0
[  374.073913]  [<ffffffff81451e00>] ? gs_change+0x13/0x13
[  374.073915]  [<ffffffff81451e00>] ? gs_change+0x13/0x13

>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
>> index 222954d..8775087 100644
>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> @@ -6697,7 +6697,18 @@ static int igb_resume(struct pci_dev *pdev)
>>  	pci_enable_wake(pdev, PCI_D3hot, 0);
>>  	pci_enable_wake(pdev, PCI_D3cold, 0);
>>  
>> -	if (igb_init_interrupt_scheme(adapter)) {
>> +	if (!rtnl_is_locked()) {
> 
> This doesn't tell whether this process holds the lock.  So it suppresses
> the warning but doesn't fix the real bug.
> 
> Ben.

>
> You mean to say that this issue needs to be revisited in mainline too?

On 03/27/2012 07:57 PM, Ben Hutchings wrote:

| Oh, yes.  Because you said this was a smaller patch for stable, I didn't
| realise that mainline had this problem too.

> 
>> +		/*
>> +		 * shut up ASSERT_RTNL() warning in
>> +		 * netif_set_real_num_tx/rx_queues.
>> +		 */
>> +		rtnl_lock();
>> +		err = igb_init_interrupt_scheme(adapter);
>> +		rtnl_unlock();
>> +	} else {
>> +		err = igb_init_interrupt_scheme(adapter);
>> +	}
>> +	if (err) {
>>  		dev_err(&pdev->dev, "Unable to allocate memory for queues\n");
>>  		return -ENOMEM;
>>  	}
> 
> 

 
Regards,
Srivatsa S. Bhat

^ permalink raw reply

* Re: [PATCH V3 0/6] net/mlx4_en: DCB QoS support
From: Or Gerlitz @ 2012-03-29 12:33 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S. Miller, netdev, Roland Dreier, Yevgeny Petrilin,
	Oren Duer, Amir Vadai, John Fastabend, Amir Vadai
In-Reply-To: <1332990846.3402.5.camel@edumazet-laptop>

On Thu, Mar 29, 2012 at 5:14 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 29 mars 2012 à 00:21 +0200, Or Gerlitz a écrit :
>
>> For the ratelimit Eric D. provided feedback and it was implemented along his suggestion,
>
>
> My suggestion was exactly the following :
>
> /sys/class/net/eth2/qos/0/ratelimit
> ...
> /sys/class/net/eth2/qos/7/ratelimit
>
>> and now John and Ben have other suggestions,
>
> I dont think so.
> Amir provided in V2 something that was not what I suggested.
>
> /sys/class/net/eth2/ratelimit/tc0
> ...
> /sys/class/net/eth2/ratelimit/tc7
>
>
> And this is where John, Ben were complaining.
> [ Obviously I totally agree with them ]

to be precise they were suggesting to go e.g through DCBNL and not sysfs,
anyway,Amir works to implement this along their suggestion

Or

^ permalink raw reply

* [Q] ipv6: RTM_GETROUTE interpretation of RTA_IIF
From: Shmulik Ladkani @ 2012-03-29 13:03 UTC (permalink / raw)
  To: netdev

Hi,

In IPv4, if the RTA_IIF attribute is specified in an RTM_GETROUTE
message, then a route is searched as if a packet was received on the
specified iif interface - i.e. 'inet_rtm_getroute()' calls
'ip_route_input()'.

However in IPv6, RTA_IIF is not interpreted in the same way:
'inet6_rtm_getroute()' always calls 'ip6_route_output()', regardless the
RTA_IIF attribute.

As a result, in IPv6 there's no way to use RTM_GETROUTE in order to look
for a route as if a packet was received on a specific interface.

I'd like to modify 'inet6_rtm_getroute()' so that RTA_IIF is interpreted
in the same way as in IPv4's 'inet_rtm_getroute()'.

Before I come up with a patch, I'd like to know whether current
interpretation of RTA_IIF in 'inet6_rtm_getroute()' is deliberate.

Regards,
Shmulik

^ permalink raw reply

* Re: [REGRESSION][PATCH] bpf_jit drops the ball on indirect negative mem references
From: Eric Dumazet @ 2012-03-29 13:57 UTC (permalink / raw)
  To: Jan Seiffert; +Cc: netdev, linux-kernel, David S. Miller, Matt Evans
In-Reply-To: <4F744D58.3070009@googlemail.com>

Le jeudi 29 mars 2012 à 13:54 +0200, Jan Seiffert a écrit :

> Ok, to keep the ball rolling here is a V2 with the changes you suggested:
> 
...

> I propose the following patch to fix this situation.
> Lightly tested on x86, but the powerpc asm part is prop. wrong.
> 
> 
> Signed-of-by: Jan Seiffert <kaffeemonster@googlemail.com>
> 

x86 part seems very good at first glance, I'll test it and give my
feedback soon.

Thanks !

^ permalink raw reply

* Re: [Q/RFC] BPF use in broader scope
From: Nuno Martins @ 2012-03-29 14:04 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, eric.dumazet, davem, bhutchings, shemminger,
	Alfredo Matos
In-Reply-To: <20120329074443.GB2098@minipsycho>

On Thu, Mar 29, 2012 at 8:44 AM, Jiri Pirko <jpirko@redhat.com> wrote:
> Hi all.
>
> I came to an idea of using BPF infrastructure currently used in kernel,
> for computing hashes selecting TX ports in team device. Since the same
> data (skb) are alalyzed/used as for socket filtering, BPF seems so be quite
> suitable for this. It would allow userspace daemon to specify various
> kinds of TX selection algorithms.
>
> Here are proposed things to be done:
> 1) introduce in-kernel api for creating sk-unattached filters (I have
>   the patch cooked up already)
>
> 2) extend current BPF machine to allow XOR operation. Not sure if this
>   is doable or what the best of doing this is.
>
> 3) add possibility to pass some data to the machine via
>   pre-filling "Scratch Memory Store". I think this can be done easily
>   moving "u32 mem[BPF_MEMWORDS];" to bpf_func caller and pass it as the
>   second function parameter. That should not break anything.
>
> Then the computed hash can be either stored into Scratch memory or returned
> directly (where ordinary sk filters return len).
>
> Does this seems reasonable? Thoughts, comments?
>

Hi all,

I've also been working on mechanism that requires creating custom BFP
filters, but for PID based packet filtering. It allows attaching
custom filters to a
socket, through setsocketopt, making it possible to detect when packets belongs
 to a target process id (previously identified through kprobes).

The in-kernel api we use, enables registering and unregistering filter
functions,
 which are then stored on a filter function list.

When attaching the custom filter function, it replaces the current
filter, releasing the JIT code if necessary. However, the newly
attached filter function does not get JIT compiled (bpf_jit_compile)
to allow custom functions without having to provide the corresponding
ASM code.

Using the JIT extensions that Jirka mentions in step 3 could also allows to
compile custom.

We are in the process of making the source code available through a
project page dedicated to PID monitoring, and will (hopefully) send a
Q/RFC to this list tomorrow.

> Thanks!

Thanks.

>
> Jirka
>

Nuno Martins

> --
> 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

* [PATCH] Lockd: pass network namespace to creation and destruction routines
From: Stanislav Kinsbursky @ 2012-03-29 14:23 UTC (permalink / raw)
  To: Trond.Myklebust
  Cc: linux-nfs, xemul, neilb, netdev, linux-kernel, jbottomley,
	bfields, davem, devel

These routines are called from locks reclaimer() kernel thread. This thread
works in "init_net" network context and currently relays on presence on lockd
thread and it's per-net resources. Thus lockd_up() and lockd_down() can't use
current network context. So let's pass corrent one into them.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>

---
 fs/lockd/clntlock.c        |   10 +++++-----
 fs/lockd/svc.c             |    7 +++----
 fs/nfsd/nfssvc.c           |    6 +++---
 include/linux/lockd/bind.h |    4 ++--
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
index ba1dc2e..32b3c2b 100644
--- a/fs/lockd/clntlock.c
+++ b/fs/lockd/clntlock.c
@@ -56,7 +56,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
 	u32 nlm_version = (nlm_init->nfs_version == 2) ? 1 : 4;
 	int status;
 
-	status = lockd_up();
+	status = lockd_up(nlm_init->net);
 	if (status < 0)
 		return ERR_PTR(status);
 
@@ -65,7 +65,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
 				   nlm_init->hostname, nlm_init->noresvport,
 				   nlm_init->net);
 	if (host == NULL) {
-		lockd_down();
+		lockd_down(nlm_init->net);
 		return ERR_PTR(-ENOLCK);
 	}
 
@@ -81,7 +81,7 @@ EXPORT_SYMBOL_GPL(nlmclnt_init);
 void nlmclnt_done(struct nlm_host *host)
 {
 	nlmclnt_release_host(host);
-	lockd_down();
+	lockd_down(host->net);
 }
 EXPORT_SYMBOL_GPL(nlmclnt_done);
 
@@ -224,7 +224,7 @@ reclaimer(void *ptr)
 	allow_signal(SIGKILL);
 
 	down_write(&host->h_rwsem);
-	lockd_up();	/* note: this cannot fail as lockd is already running */
+	lockd_up(host->net);	/* note: this cannot fail as lockd is already running */
 
 	dprintk("lockd: reclaiming locks for host %s\n", host->h_name);
 
@@ -275,6 +275,6 @@ restart:
 
 	/* Release host handle after use */
 	nlmclnt_release_host(host);
-	lockd_down();
+	lockd_down(host->net);
 	return 0;
 }
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index b34100e..ce4c80e 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -295,11 +295,10 @@ static void lockd_down_net(struct net *net)
 /*
  * Bring up the lockd process if it's not already up.
  */
-int lockd_up(void)
+int lockd_up(struct net *net)
 {
 	struct svc_serv *serv;
 	int		error = 0;
-	struct net *net = current->nsproxy->net_ns;
 
 	mutex_lock(&nlmsvc_mutex);
 	/*
@@ -377,12 +376,12 @@ EXPORT_SYMBOL_GPL(lockd_up);
  * Decrement the user count and bring down lockd if we're the last.
  */
 void
-lockd_down(void)
+lockd_down(struct net *net)
 {
 	mutex_lock(&nlmsvc_mutex);
 	if (nlmsvc_users) {
 		if (--nlmsvc_users) {
-			lockd_down_net(current->nsproxy->net_ns);
+			lockd_down_net(net);
 			goto out;
 		}
 	} else {
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index fce472f..0f3e35b 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -220,7 +220,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
 	ret = nfsd_init_socks(port);
 	if (ret)
 		goto out_racache;
-	ret = lockd_up();
+	ret = lockd_up(&init_net);
 	if (ret)
 		goto out_racache;
 	ret = nfs4_state_start();
@@ -229,7 +229,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
 	nfsd_up = true;
 	return 0;
 out_lockd:
-	lockd_down();
+	lockd_down(&init_net);
 out_racache:
 	nfsd_racache_shutdown();
 	return ret;
@@ -246,7 +246,7 @@ static void nfsd_shutdown(void)
 	if (!nfsd_up)
 		return;
 	nfs4_state_shutdown();
-	lockd_down();
+	lockd_down(&init_net);
 	nfsd_racache_shutdown();
 	nfsd_up = false;
 }
diff --git a/include/linux/lockd/bind.h b/include/linux/lockd/bind.h
index 11a966e..4d24d64 100644
--- a/include/linux/lockd/bind.h
+++ b/include/linux/lockd/bind.h
@@ -54,7 +54,7 @@ extern void	nlmclnt_done(struct nlm_host *host);
 
 extern int	nlmclnt_proc(struct nlm_host *host, int cmd,
 					struct file_lock *fl);
-extern int	lockd_up(void);
-extern void	lockd_down(void);
+extern int	lockd_up(struct net *net);
+extern void	lockd_down(struct net *net);
 
 #endif /* LINUX_LOCKD_BIND_H */

^ permalink raw reply related

* Re: [PATCH] Lockd: pass network namespace to creation and destruction routines
From: Stanislav Kinsbursky @ 2012-03-29 14:36 UTC (permalink / raw)
  To: Trond.Myklebust@netapp.com
  Cc: linux-nfs@vger.kernel.org, Pavel Emelianov, neilb@suse.de,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	James Bottomley, bfields@fieldses.org, davem@davemloft.net,
	devel@openvz.org
In-Reply-To: <20120329142209.20506.43224.stgit@localhost6.localdomain6>

This patch has a flaw. Thus will be v2.
Sorry.

-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply

* [PATCH v2] Lockd: pass network namespace to creation and destruction routines
From: Stanislav Kinsbursky @ 2012-03-29 14:54 UTC (permalink / raw)
  To: Trond.Myklebust
  Cc: linux-nfs, xemul, neilb, netdev, linux-kernel, jbottomley,
	bfields, davem, devel

v2: dereference of most probably already released nlm_host removed in
nlmclnt_done() and reclaimer().

These routines are called from locks reclaimer() kernel thread. This thread
works in "init_net" network context and currently relays on persence on lockd
thread and it's per-net resources. Thus lockd_up() and lockd_down() can't relay
on current network context. So let's pass corrent one into them.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>

---
 fs/lockd/clntlock.c        |   13 ++++++++-----
 fs/lockd/svc.c             |    7 +++----
 fs/nfsd/nfssvc.c           |    6 +++---
 include/linux/lockd/bind.h |    4 ++--
 4 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
index ba1dc2e..ca0a080 100644
--- a/fs/lockd/clntlock.c
+++ b/fs/lockd/clntlock.c
@@ -56,7 +56,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
 	u32 nlm_version = (nlm_init->nfs_version == 2) ? 1 : 4;
 	int status;
 
-	status = lockd_up();
+	status = lockd_up(nlm_init->net);
 	if (status < 0)
 		return ERR_PTR(status);
 
@@ -65,7 +65,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
 				   nlm_init->hostname, nlm_init->noresvport,
 				   nlm_init->net);
 	if (host == NULL) {
-		lockd_down();
+		lockd_down(nlm_init->net);
 		return ERR_PTR(-ENOLCK);
 	}
 
@@ -80,8 +80,10 @@ EXPORT_SYMBOL_GPL(nlmclnt_init);
  */
 void nlmclnt_done(struct nlm_host *host)
 {
+	struct net *net = host->net;
+
 	nlmclnt_release_host(host);
-	lockd_down();
+	lockd_down(net);
 }
 EXPORT_SYMBOL_GPL(nlmclnt_done);
 
@@ -220,11 +222,12 @@ reclaimer(void *ptr)
 	struct nlm_wait	  *block;
 	struct file_lock *fl, *next;
 	u32 nsmstate;
+	struct net *net = host->net;
 
 	allow_signal(SIGKILL);
 
 	down_write(&host->h_rwsem);
-	lockd_up();	/* note: this cannot fail as lockd is already running */
+	lockd_up(net);	/* note: this cannot fail as lockd is already running */
 
 	dprintk("lockd: reclaiming locks for host %s\n", host->h_name);
 
@@ -275,6 +278,6 @@ restart:
 
 	/* Release host handle after use */
 	nlmclnt_release_host(host);
-	lockd_down();
+	lockd_down(net);
 	return 0;
 }
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index b34100e..ce4c80e 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -295,11 +295,10 @@ static void lockd_down_net(struct net *net)
 /*
  * Bring up the lockd process if it's not already up.
  */
-int lockd_up(void)
+int lockd_up(struct net *net)
 {
 	struct svc_serv *serv;
 	int		error = 0;
-	struct net *net = current->nsproxy->net_ns;
 
 	mutex_lock(&nlmsvc_mutex);
 	/*
@@ -377,12 +376,12 @@ EXPORT_SYMBOL_GPL(lockd_up);
  * Decrement the user count and bring down lockd if we're the last.
  */
 void
-lockd_down(void)
+lockd_down(struct net *net)
 {
 	mutex_lock(&nlmsvc_mutex);
 	if (nlmsvc_users) {
 		if (--nlmsvc_users) {
-			lockd_down_net(current->nsproxy->net_ns);
+			lockd_down_net(net);
 			goto out;
 		}
 	} else {
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index fce472f..0f3e35b 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -220,7 +220,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
 	ret = nfsd_init_socks(port);
 	if (ret)
 		goto out_racache;
-	ret = lockd_up();
+	ret = lockd_up(&init_net);
 	if (ret)
 		goto out_racache;
 	ret = nfs4_state_start();
@@ -229,7 +229,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
 	nfsd_up = true;
 	return 0;
 out_lockd:
-	lockd_down();
+	lockd_down(&init_net);
 out_racache:
 	nfsd_racache_shutdown();
 	return ret;
@@ -246,7 +246,7 @@ static void nfsd_shutdown(void)
 	if (!nfsd_up)
 		return;
 	nfs4_state_shutdown();
-	lockd_down();
+	lockd_down(&init_net);
 	nfsd_racache_shutdown();
 	nfsd_up = false;
 }
diff --git a/include/linux/lockd/bind.h b/include/linux/lockd/bind.h
index 11a966e..4d24d64 100644
--- a/include/linux/lockd/bind.h
+++ b/include/linux/lockd/bind.h
@@ -54,7 +54,7 @@ extern void	nlmclnt_done(struct nlm_host *host);
 
 extern int	nlmclnt_proc(struct nlm_host *host, int cmd,
 					struct file_lock *fl);
-extern int	lockd_up(void);
-extern void	lockd_down(void);
+extern int	lockd_up(struct net *net);
+extern void	lockd_down(struct net *net);
 
 #endif /* LINUX_LOCKD_BIND_H */

^ permalink raw reply related

* [PATCH V4 4/8] net/mlx4_en: sk_prio <=> UP for untagged traffic
From: Amir Vadai @ 2012-03-29 15:03 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Roland Dreier, Yevgeny Petrilin, Oren Duer, Amir Vadai,
	Amir Vadai
In-Reply-To: <1333033418-1669-1-git-send-email-amirv@mellanox.com>

Since vlan egress map is only good for tagged traffic, need to have other
mapping to be used by untagged traffic.
For that, the driver uses sch_mqprio mapping. This mapping could be set by
using tc tool from iproute2 package.
Mapped UP will be used by the HW for QoS purposes, but won't go out on the
wire.

Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   18 ++++++++++++++++++
 drivers/net/ethernet/mellanox/mlx4/en_tx.c     |    2 +-
 2 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 9b456ae..44dbe1f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -45,6 +45,14 @@
 #include "mlx4_en.h"
 #include "en_port.h"
 
+static int mlx4_en_setup_tc(struct net_device *dev, u8 up)
+{
+	if (up != MLX4_EN_NUM_UP)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int mlx4_en_vlan_rx_add_vid(struct net_device *dev, unsigned short vid)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
@@ -1042,6 +1050,7 @@ static const struct net_device_ops mlx4_netdev_ops = {
 	.ndo_poll_controller	= mlx4_en_netpoll,
 #endif
 	.ndo_set_features	= mlx4_en_set_features,
+	.ndo_setup_tc		= mlx4_en_setup_tc,
 };
 
 int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
@@ -1130,6 +1139,15 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	netif_set_real_num_tx_queues(dev, priv->tx_ring_num);
 	netif_set_real_num_rx_queues(dev, priv->rx_ring_num);
 
+	netdev_set_num_tc(dev, MLX4_EN_NUM_UP);
+
+	/* First 9 rings are for UP 0 */
+	netdev_set_tc_queue(dev, 0, MLX4_EN_NUM_TX_RINGS + 1, 0);
+
+	/* Partition Tx queues evenly amongst UP's 1-7 */
+	for (i = 1; i < MLX4_EN_NUM_UP; i++)
+		netdev_set_tc_queue(dev, i, 1, MLX4_EN_NUM_TX_RINGS + i);
+
 	SET_ETHTOOL_OPS(dev, &mlx4_en_ethtool_ops);
 
 	/* Set defualt MAC */
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 94a605a..d9bab53 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -577,7 +577,7 @@ u16 mlx4_en_select_queue(struct net_device *dev, struct sk_buff *skb)
 		return MLX4_EN_NUM_TX_RINGS + (vlan_tag >> 13);
 	}
 
-	return __skb_tx_hash(dev, skb, MLX4_EN_NUM_TX_RINGS);
+	return skb_tx_hash(dev, skb);
 }
 
 static void mlx4_bf_copy(void __iomem *dst, unsigned long *src, unsigned bytecnt)
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH V4 5/8] net/route: export symbol ip_tos2prio
From: Amir Vadai @ 2012-03-29 15:03 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Roland Dreier, Yevgeny Petrilin, Oren Duer, Amir Vadai,
	Amir Vadai
In-Reply-To: <1333033418-1669-1-git-send-email-amirv@mellanox.com>

Need to export this to enable drivers use rt_tos2priority()

Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 net/ipv4/route.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 12ccf88..69e0b86 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -230,7 +230,7 @@ const __u8 ip_tos2prio[16] = {
 	TC_PRIO_INTERACTIVE_BULK,
 	ECN_OR_COST(INTERACTIVE_BULK)
 };
-
+EXPORT_SYMBOL(ip_tos2prio);
 
 /*
  * Route cache.
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH V4 8/8] net/mlx4_en: Set max rate-limit for a TC
From: Amir Vadai @ 2012-03-29 15:03 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Roland Dreier, Yevgeny Petrilin, Oren Duer, Amir Vadai,
	Amir Vadai
In-Reply-To: <1333033418-1669-1-git-send-email-amirv@mellanox.com>

This patch is using the DCB netlink to set rate limit per ETS TC

Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c |   31 ++++++++++++++++++++++++
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    7 +++++
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   |    2 +-
 3 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c b/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
index 5bf0106..0f92b2e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
@@ -100,6 +100,7 @@ static int mlx4_en_config_port_scheduler(struct mlx4_en_priv *priv,
 	__u8 pg[IEEE_8021QAZ_MAX_TCS] = { 0 };
 
 	ets = ets ?: priv->mlx4_en_ieee_ets;
+	ratelimit = ratelimit ?: priv->maxrate->tc_maxrate;
 
 	/* higher TC means higher priority => lower pg */
 	for (i = IEEE_8021QAZ_MAX_TCS - 1; i >= 0; i--) {
@@ -200,9 +201,39 @@ static u8 mlx4_en_dcbnl_setdcbx(struct net_device *dev, u8 mode)
 	return 0;
 }
 
+static int mlx4_en_dcbnl_ieee_getmaxrate(struct net_device *dev,
+				   struct ieee_maxrate *maxrate)
+{
+	struct mlx4_en_priv *priv = netdev_priv(dev);
+
+	if (!priv->maxrate)
+		return -EINVAL;
+
+	memcpy(maxrate, priv->maxrate, sizeof(maxrate));
+
+	return 0;
+}
+
+static int mlx4_en_dcbnl_ieee_setmaxrate(struct net_device *dev,
+		struct ieee_maxrate *maxrate)
+{
+	int err;
+	struct mlx4_en_priv *priv = netdev_priv(dev);
+
+	err = mlx4_en_config_port_scheduler(priv, NULL, maxrate->tc_maxrate);
+	if (err)
+		return err;
+
+	memcpy(priv->maxrate, maxrate, sizeof(priv->maxrate));
+
+	return 0;
+}
+
 const struct dcbnl_rtnl_ops mlx4_en_dcbnl_ops = {
 	.ieee_getets	= mlx4_en_dcbnl_ieee_getets,
 	.ieee_setets	= mlx4_en_dcbnl_ieee_setets,
+	.ieee_getmaxrate = mlx4_en_dcbnl_ieee_getmaxrate,
+	.ieee_setmaxrate = mlx4_en_dcbnl_ieee_setmaxrate,
 	.ieee_getpfc	= mlx4_en_dcbnl_ieee_getpfc,
 	.ieee_setpfc	= mlx4_en_dcbnl_ieee_setpfc,
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 44dbe1f..be7ffbf 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -978,6 +978,7 @@ void mlx4_en_destroy_netdev(struct net_device *dev)
 
 #ifdef CONFIG_MLX4_EN_DCB
 	vfree(priv->mlx4_en_ieee_ets);
+	vfree(priv->maxrate);
 #endif
 
 	free_netdev(dev);
@@ -1103,6 +1104,12 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 			err = -ENOMEM;
 			goto out;
 		}
+
+		priv->maxrate = vzalloc(sizeof(struct ieee_maxrate));
+		if (!priv->maxrate) {
+			err = -ENOMEM;
+			goto out;
+		}
 	}
 #endif
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index fa09792..d1f5ac2 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -420,7 +420,6 @@ struct mlx4_en_frag_info {
 #define MLX4_EN_BW_MAX 100 /* Utilize 100% of the line */
 
 #define MLX4_EN_TC_ETS 7
-
 #endif
 
 struct mlx4_en_priv {
@@ -499,6 +498,7 @@ struct mlx4_en_priv {
 
 #ifdef CONFIG_MLX4_EN_DCB
 	struct ieee_ets *mlx4_en_ieee_ets;
+	struct ieee_maxrate *maxrate;
 #endif
 };
 
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH V4 2/8] net/mlx4_core: set port QoS attributes
From: Amir Vadai @ 2012-03-29 15:03 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Roland Dreier, Yevgeny Petrilin, Oren Duer, Amir Vadai,
	Amir Vadai
In-Reply-To: <1333033418-1669-1-git-send-email-amirv@mellanox.com>

Adding QoS firmware commands:
- mlx4_en_SET_PORT_PRIO2TC - set UP <=> TC
- mlx4_en_SET_PORT_SCHEDULER - set promised BW, max BW and PG number

Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_port.h |    2 +
 drivers/net/ethernet/mellanox/mlx4/mlx4.h    |   21 ++++++++
 drivers/net/ethernet/mellanox/mlx4/port.c    |   67 ++++++++++++++++++++++++++
 include/linux/mlx4/cmd.h                     |    4 ++
 include/linux/mlx4/device.h                  |    3 +
 5 files changed, 97 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_port.h b/drivers/net/ethernet/mellanox/mlx4/en_port.h
index 6934fd7..745090b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_port.h
+++ b/drivers/net/ethernet/mellanox/mlx4/en_port.h
@@ -39,6 +39,8 @@
 #define SET_PORT_PROMISC_SHIFT	31
 #define SET_PORT_MC_PROMISC_SHIFT	30
 
+#define MLX4_EN_NUM_TC		8
+
 #define VLAN_FLTR_SIZE	128
 struct mlx4_set_vlan_fltr_mbox {
 	__be32 entry[VLAN_FLTR_SIZE];
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index 2a0ff2c..98cbb2f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -53,6 +53,27 @@
 #define DRV_VERSION	"1.1"
 #define DRV_RELDATE	"Dec, 2011"
 
+#define MLX4_NUM_UP		8
+#define MLX4_NUM_TC		8
+#define MLX4_RATELIMIT_UNITS 3 /* 100 Mbps */
+#define MLX4_RATELIMIT_KB_TO_HW(_mb) ((_mb) / 100000)
+#define MLX4_RATELIMIT_DEFAULT 0xffff
+
+struct mlx4_set_port_prio2tc_context {
+	u8 prio2tc[4];
+};
+
+struct mlx4_port_scheduler_tc_cfg_be {
+	__be16 pg;
+	__be16 bw_precentage;
+	__be16 max_bw_units; /* 3-100Mbps, 4-1Gbps, other values - reserved */
+	__be16 max_bw_value;
+};
+
+struct mlx4_set_port_scheduler_context {
+	struct mlx4_port_scheduler_tc_cfg_be tc[MLX4_NUM_TC];
+};
+
 enum {
 	MLX4_HCR_BASE		= 0x80680,
 	MLX4_HCR_SIZE		= 0x0001c,
diff --git a/drivers/net/ethernet/mellanox/mlx4/port.c b/drivers/net/ethernet/mellanox/mlx4/port.c
index 77535ff..e619cd5 100644
--- a/drivers/net/ethernet/mellanox/mlx4/port.c
+++ b/drivers/net/ethernet/mellanox/mlx4/port.c
@@ -834,6 +834,73 @@ int mlx4_SET_PORT_qpn_calc(struct mlx4_dev *dev, u8 port, u32 base_qpn,
 }
 EXPORT_SYMBOL(mlx4_SET_PORT_qpn_calc);
 
+int mlx4_SET_PORT_PRIO2TC(struct mlx4_dev *dev, u8 port, u8 *prio2tc)
+{
+	struct mlx4_cmd_mailbox *mailbox;
+	struct mlx4_set_port_prio2tc_context *context;
+	int err;
+	u32 in_mod;
+	int i;
+
+	mailbox = mlx4_alloc_cmd_mailbox(dev);
+	if (IS_ERR(mailbox))
+		return PTR_ERR(mailbox);
+	context = mailbox->buf;
+	memset(context, 0, sizeof *context);
+
+	for (i = 0; i < MLX4_NUM_UP; i += 2)
+		context->prio2tc[i >> 1] = prio2tc[i] << 4 | prio2tc[i + 1];
+
+	in_mod = MLX4_SET_PORT_PRIO2TC << 8 | port;
+	err = mlx4_cmd(dev, mailbox->dma, in_mod, 1, MLX4_CMD_SET_PORT,
+		       MLX4_CMD_TIME_CLASS_B, MLX4_CMD_NATIVE);
+
+	mlx4_free_cmd_mailbox(dev, mailbox);
+	return err;
+}
+EXPORT_SYMBOL(mlx4_SET_PORT_PRIO2TC);
+
+int mlx4_SET_PORT_SCHEDULER(struct mlx4_dev *dev, u8 port, u8 *tc_tx_bw,
+		u8 *pg, u64 *ratelimit)
+{
+	struct mlx4_cmd_mailbox *mailbox;
+	struct mlx4_set_port_scheduler_context *context;
+	int err;
+	u32 in_mod;
+	int i;
+
+	mailbox = mlx4_alloc_cmd_mailbox(dev);
+	if (IS_ERR(mailbox))
+		return PTR_ERR(mailbox);
+	context = mailbox->buf;
+	memset(context, 0, sizeof *context);
+
+	for (i = 0; i < MLX4_NUM_TC; i++) {
+		struct mlx4_port_scheduler_tc_cfg_be *tc = &context->tc[i];
+		u16 r = 0;
+
+		if (ratelimit)
+			r = MLX4_RATELIMIT_KB_TO_HW(ratelimit[i]);
+
+		if (!r)
+			r = MLX4_RATELIMIT_DEFAULT;
+
+		tc->pg = htons(pg[i]);
+		tc->bw_precentage = htons(tc_tx_bw[i]);
+
+		tc->max_bw_units = htons(MLX4_RATELIMIT_UNITS);
+		tc->max_bw_value = htons(r);
+	}
+
+	in_mod = MLX4_SET_PORT_SCHEDULER << 8 | port;
+	err = mlx4_cmd(dev, mailbox->dma, in_mod, 1, MLX4_CMD_SET_PORT,
+		       MLX4_CMD_TIME_CLASS_B, MLX4_CMD_NATIVE);
+
+	mlx4_free_cmd_mailbox(dev, mailbox);
+	return err;
+}
+EXPORT_SYMBOL(mlx4_SET_PORT_SCHEDULER);
+
 int mlx4_SET_MCAST_FLTR_wrapper(struct mlx4_dev *dev, int slave,
 				struct mlx4_vhcr *vhcr,
 				struct mlx4_cmd_mailbox *inbox,
diff --git a/include/linux/mlx4/cmd.h b/include/linux/mlx4/cmd.h
index 9958ff2..1f3860a 100644
--- a/include/linux/mlx4/cmd.h
+++ b/include/linux/mlx4/cmd.h
@@ -150,6 +150,10 @@ enum {
 	/* statistics commands */
 	MLX4_CMD_QUERY_IF_STAT	 = 0X54,
 	MLX4_CMD_SET_IF_STAT	 = 0X55,
+
+	/* set port opcode modifiers */
+	MLX4_SET_PORT_PRIO2TC = 0x8,
+	MLX4_SET_PORT_SCHEDULER  = 0x9,
 };
 
 enum {
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 834c96c..6627136 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -628,6 +628,9 @@ int mlx4_SET_PORT_general(struct mlx4_dev *dev, u8 port, int mtu,
 			  u8 pptx, u8 pfctx, u8 pprx, u8 pfcrx);
 int mlx4_SET_PORT_qpn_calc(struct mlx4_dev *dev, u8 port, u32 base_qpn,
 			   u8 promisc);
+int mlx4_SET_PORT_PRIO2TC(struct mlx4_dev *dev, u8 port, u8 *prio2tc);
+int mlx4_SET_PORT_SCHEDULER(struct mlx4_dev *dev, u8 port, u8 *tc_tx_bw,
+		u8 *pg, u64 *ratelimit);
 int mlx4_find_cached_vlan(struct mlx4_dev *dev, u8 port, u16 vid, int *idx);
 int mlx4_register_vlan(struct mlx4_dev *dev, u8 port, u16 vlan, int *index);
 void mlx4_unregister_vlan(struct mlx4_dev *dev, u8 port, int index);
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH V4 1/8] net/mlx4_en: Force user priority by QP attribute
From: Amir Vadai @ 2012-03-29 15:03 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Roland Dreier, Yevgeny Petrilin, Oren Duer, Amir Vadai,
	Amir Vadai
In-Reply-To: <1333033418-1669-1-git-send-email-amirv@mellanox.com>

Instead of relying on HW to change schedule queue by UP, schedule
queue is fixed for a tx_ring, and UP in WQE is ignored in this aspect.  This
resolves two issues with untagged traffic:
1. untagged traffic has no UP in packet which is needed for QoS. The change
   above allows setting the schedule queue (and by that the UP) of such a stream.
2. BlueFlame uses the same field used by vlan tag. So forcing UP from QPC
   allows using BF for untagged but prioritized traffic.

In old firmware that force UP is not supported, untagged traffic will not subject to
QoS.

Because UP is set by QP, need to always have a tx ring per UP, even if pfcrx
module paramter is false.

Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_main.c      |    2 +-
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c    |    3 ++-
 drivers/net/ethernet/mellanox/mlx4/en_resources.c |    6 +++++-
 drivers/net/ethernet/mellanox/mlx4/en_rx.c        |    4 ++--
 drivers/net/ethernet/mellanox/mlx4/en_tx.c        |   12 ++++--------
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h      |    6 +++---
 include/linux/mlx4/qp.h                           |    3 ++-
 7 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_main.c b/drivers/net/ethernet/mellanox/mlx4/en_main.c
index 2097a7d..346fdb2 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_main.c
@@ -114,7 +114,7 @@ static int mlx4_en_get_profile(struct mlx4_en_dev *mdev)
 		params->prof[i].tx_ring_size = MLX4_EN_DEF_TX_RING_SIZE;
 		params->prof[i].rx_ring_size = MLX4_EN_DEF_RX_RING_SIZE;
 		params->prof[i].tx_ring_num = MLX4_EN_NUM_TX_RINGS +
-			(!!pfcrx) * MLX4_EN_NUM_PPP_RINGS;
+			MLX4_EN_NUM_PPP_RINGS;
 		params->prof[i].rss_rings = 0;
 	}
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 31b455a..2322622 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -650,7 +650,8 @@ int mlx4_en_start_port(struct net_device *dev)
 
 		/* Configure ring */
 		tx_ring = &priv->tx_ring[i];
-		err = mlx4_en_activate_tx_ring(priv, tx_ring, cq->mcq.cqn);
+		err = mlx4_en_activate_tx_ring(priv, tx_ring, cq->mcq.cqn,
+				max(0, i - MLX4_EN_NUM_TX_RINGS));
 		if (err) {
 			en_err(priv, "Failed allocating Tx ring\n");
 			mlx4_en_deactivate_cq(priv, cq);
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_resources.c b/drivers/net/ethernet/mellanox/mlx4/en_resources.c
index bcbc54c..10c24c7 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_resources.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_resources.c
@@ -39,7 +39,7 @@
 
 void mlx4_en_fill_qp_context(struct mlx4_en_priv *priv, int size, int stride,
 			     int is_tx, int rss, int qpn, int cqn,
-			     struct mlx4_qp_context *context)
+			     int user_prio, struct mlx4_qp_context *context)
 {
 	struct mlx4_en_dev *mdev = priv->mdev;
 
@@ -57,6 +57,10 @@ void mlx4_en_fill_qp_context(struct mlx4_en_priv *priv, int size, int stride,
 	context->local_qpn = cpu_to_be32(qpn);
 	context->pri_path.ackto = 1 & 0x07;
 	context->pri_path.sched_queue = 0x83 | (priv->port - 1) << 6;
+	if (user_prio >= 0) {
+		context->pri_path.sched_queue |= user_prio << 3;
+		context->pri_path.feup = 1 << 6;
+	}
 	context->pri_path.counter_index = 0xff;
 	context->cqn_send = cpu_to_be32(cqn);
 	context->cqn_recv = cpu_to_be32(cqn);
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 9adbd53..d49a7ac 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -823,7 +823,7 @@ static int mlx4_en_config_rss_qp(struct mlx4_en_priv *priv, int qpn,
 
 	memset(context, 0, sizeof *context);
 	mlx4_en_fill_qp_context(priv, ring->actual_size, ring->stride, 0, 0,
-				qpn, ring->cqn, context);
+				qpn, ring->cqn, -1, context);
 	context->db_rec_addr = cpu_to_be64(ring->wqres.db.dma);
 
 	/* Cancel FCS removal if FW allows */
@@ -890,7 +890,7 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
 	}
 	rss_map->indir_qp.event = mlx4_en_sqp_event;
 	mlx4_en_fill_qp_context(priv, 0, 0, 0, 1, priv->base_qpn,
-				priv->rx_ring[0].cqn, &context);
+				priv->rx_ring[0].cqn, -1, &context);
 
 	if (!priv->prof->rss_rings || priv->prof->rss_rings > priv->rx_ring_num)
 		rss_rings = priv->rx_ring_num;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 1796824..94a605a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -156,7 +156,7 @@ void mlx4_en_destroy_tx_ring(struct mlx4_en_priv *priv,
 
 int mlx4_en_activate_tx_ring(struct mlx4_en_priv *priv,
 			     struct mlx4_en_tx_ring *ring,
-			     int cq)
+			     int cq, int user_prio)
 {
 	struct mlx4_en_dev *mdev = priv->mdev;
 	int err;
@@ -174,7 +174,7 @@ int mlx4_en_activate_tx_ring(struct mlx4_en_priv *priv,
 	ring->doorbell_qpn = ring->qp.qpn << 8;
 
 	mlx4_en_fill_qp_context(priv, ring->size, ring->stride, 1, 0, ring->qpn,
-				ring->cqn, &ring->context);
+				ring->cqn, user_prio, &ring->context);
 	if (ring->bf_enabled)
 		ring->context.usr_page = cpu_to_be32(ring->bf.uar->index);
 
@@ -570,18 +570,14 @@ static void build_inline_wqe(struct mlx4_en_tx_desc *tx_desc, struct sk_buff *sk
 
 u16 mlx4_en_select_queue(struct net_device *dev, struct sk_buff *skb)
 {
-	struct mlx4_en_priv *priv = netdev_priv(dev);
 	u16 vlan_tag = 0;
 
-	/* If we support per priority flow control and the packet contains
-	 * a vlan tag, send the packet to the TX ring assigned to that priority
-	 */
-	if (priv->prof->rx_ppp && vlan_tx_tag_present(skb)) {
+	if (vlan_tx_tag_present(skb)) {
 		vlan_tag = vlan_tx_tag_get(skb);
 		return MLX4_EN_NUM_TX_RINGS + (vlan_tag >> 13);
 	}
 
-	return skb_tx_hash(dev, skb);
+	return __skb_tx_hash(dev, skb, MLX4_EN_NUM_TX_RINGS);
 }
 
 static void mlx4_bf_copy(void __iomem *dst, unsigned long *src, unsigned bytecnt)
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 9e2b911..5bd7c2a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -521,7 +521,7 @@ int mlx4_en_create_tx_ring(struct mlx4_en_priv *priv, struct mlx4_en_tx_ring *ri
 void mlx4_en_destroy_tx_ring(struct mlx4_en_priv *priv, struct mlx4_en_tx_ring *ring);
 int mlx4_en_activate_tx_ring(struct mlx4_en_priv *priv,
 			     struct mlx4_en_tx_ring *ring,
-			     int cq);
+			     int cq, int user_prio);
 void mlx4_en_deactivate_tx_ring(struct mlx4_en_priv *priv,
 				struct mlx4_en_tx_ring *ring);
 
@@ -539,8 +539,8 @@ int mlx4_en_process_rx_cq(struct net_device *dev,
 			  int budget);
 int mlx4_en_poll_rx_cq(struct napi_struct *napi, int budget);
 void mlx4_en_fill_qp_context(struct mlx4_en_priv *priv, int size, int stride,
-			     int is_tx, int rss, int qpn, int cqn,
-			     struct mlx4_qp_context *context);
+		int is_tx, int rss, int qpn, int cqn, int user_prio,
+		struct mlx4_qp_context *context);
 void mlx4_en_sqp_event(struct mlx4_qp *qp, enum mlx4_event event);
 int mlx4_en_map_buffer(struct mlx4_buf *buf);
 void mlx4_en_unmap_buffer(struct mlx4_buf *buf);
diff --git a/include/linux/mlx4/qp.h b/include/linux/mlx4/qp.h
index 091f9e7..96005d7 100644
--- a/include/linux/mlx4/qp.h
+++ b/include/linux/mlx4/qp.h
@@ -139,7 +139,8 @@ struct mlx4_qp_path {
 	u8			rgid[16];
 	u8			sched_queue;
 	u8			vlan_index;
-	u8			reserved3[2];
+	u8			feup;
+	u8			reserved3;
 	u8			reserved4[2];
 	u8			dmac[6];
 };
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH V4 7/8] net/dcb: Add an optional max rate attribute
From: Amir Vadai @ 2012-03-29 15:03 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Roland Dreier, Yevgeny Petrilin, Oren Duer, Amir Vadai,
	Amir Vadai
In-Reply-To: <1333033418-1669-1-git-send-email-amirv@mellanox.com>

Although not specified in 8021Qaz spec, it could be useful to enable drivers
setting a rate limit for an ETS TC.
This patch add this optional attribute to DCB netlink.
To use it, drivers should implement and register the callbacks ieee_setmaxrate
and ieee_getmaxrate.
Units are 64 bits long and in units of Kbps to enable it to be used both by
slow and very fast networks

Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 include/linux/dcbnl.h |    5 +++++
 include/net/dcbnl.h   |    2 ++
 net/dcb/dcbnl.c       |   18 ++++++++++++++++++
 3 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/include/linux/dcbnl.h b/include/linux/dcbnl.h
index 65a2562..ec8e372 100644
--- a/include/linux/dcbnl.h
+++ b/include/linux/dcbnl.h
@@ -67,6 +67,10 @@ struct ieee_ets {
 	__u8	reco_prio_tc[IEEE_8021QAZ_MAX_TCS];
 };
 
+struct ieee_maxrate {
+	__u64	tc_maxrate[IEEE_8021QAZ_MAX_TCS];
+};
+
 /* This structure contains the IEEE 802.1Qaz PFC managed object
  *
  * @pfc_cap: Indicates the number of traffic classes on the local device
@@ -321,6 +325,7 @@ enum ieee_attrs {
 	DCB_ATTR_IEEE_PEER_ETS,
 	DCB_ATTR_IEEE_PEER_PFC,
 	DCB_ATTR_IEEE_PEER_APP,
+	DCB_ATTR_IEEE_MAXRATE,
 	__DCB_ATTR_IEEE_MAX
 };
 #define DCB_ATTR_IEEE_MAX (__DCB_ATTR_IEEE_MAX - 1)
diff --git a/include/net/dcbnl.h b/include/net/dcbnl.h
index f55c980..fc5d5dc 100644
--- a/include/net/dcbnl.h
+++ b/include/net/dcbnl.h
@@ -48,6 +48,8 @@ struct dcbnl_rtnl_ops {
 	/* IEEE 802.1Qaz std */
 	int (*ieee_getets) (struct net_device *, struct ieee_ets *);
 	int (*ieee_setets) (struct net_device *, struct ieee_ets *);
+	int (*ieee_getmaxrate) (struct net_device *, struct ieee_maxrate *);
+	int (*ieee_setmaxrate) (struct net_device *, struct ieee_maxrate *);
 	int (*ieee_getpfc) (struct net_device *, struct ieee_pfc *);
 	int (*ieee_setpfc) (struct net_device *, struct ieee_pfc *);
 	int (*ieee_getapp) (struct net_device *, struct dcb_app *);
diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index d860530..77dbc1d 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -178,6 +178,8 @@ static const struct nla_policy dcbnl_ieee_policy[DCB_ATTR_IEEE_MAX + 1] = {
 	[DCB_ATTR_IEEE_ETS]	    = {.len = sizeof(struct ieee_ets)},
 	[DCB_ATTR_IEEE_PFC]	    = {.len = sizeof(struct ieee_pfc)},
 	[DCB_ATTR_IEEE_APP_TABLE]   = {.type = NLA_NESTED},
+	[DCB_ATTR_IEEE_MAXRATE]   = {.len = sizeof(struct ieee_maxrate)},
+
 };
 
 static const struct nla_policy dcbnl_ieee_app[DCB_ATTR_IEEE_APP_MAX + 1] = {
@@ -1243,6 +1245,14 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
 			NLA_PUT(skb, DCB_ATTR_IEEE_ETS, sizeof(ets), &ets);
 	}
 
+	if (ops->ieee_getmaxrate) {
+		struct ieee_maxrate maxrate;
+		err = ops->ieee_getmaxrate(netdev, &maxrate);
+		if (!err)
+			NLA_PUT(skb, DCB_ATTR_IEEE_MAXRATE, sizeof(maxrate),
+					&maxrate);
+	}
+
 	if (ops->ieee_getpfc) {
 		struct ieee_pfc pfc;
 		err = ops->ieee_getpfc(netdev, &pfc);
@@ -1589,6 +1599,14 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlattr **tb,
 			goto err;
 	}
 
+	if (ieee[DCB_ATTR_IEEE_MAXRATE] && ops->ieee_setmaxrate) {
+		struct ieee_maxrate *maxrate =
+			nla_data(ieee[DCB_ATTR_IEEE_MAXRATE]);
+		err = ops->ieee_setmaxrate(netdev, maxrate);
+		if (err)
+			goto err;
+	}
+
 	if (ieee[DCB_ATTR_IEEE_PFC] && ops->ieee_setpfc) {
 		struct ieee_pfc *pfc = nla_data(ieee[DCB_ATTR_IEEE_PFC]);
 		err = ops->ieee_setpfc(netdev, pfc);
-- 
1.7.8.2

^ 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