Netdev List
 help / color / mirror / Atom feed
* Re: Page allocator bottleneck
From: Aaron Lu @ 2017-09-18  7:44 UTC (permalink / raw)
  To: Tariq Toukan
  Cc: Jesper Dangaard Brouer, David Miller, Mel Gorman, Eric Dumazet,
	Alexei Starovoitov, Saeed Mahameed, Eran Ben Elisha,
	Linux Kernel Network Developers, Andrew Morton, Michal Hocko,
	linux-mm, Dave Hansen
In-Reply-To: <20170918073447.GB4107@intel.com>

On Mon, Sep 18, 2017 at 03:34:47PM +0800, Aaron Lu wrote:
> On Sun, Sep 17, 2017 at 07:16:15PM +0300, Tariq Toukan wrote:
> > 
> > It's nice to have the option to dynamically play with the parameter.
> > But maybe we should also think of changing the default fraction guaranteed
> > to the PCP, so that unaware admins of networking servers would also benefit.
> 
> I collected some performance data with will-it-scale/page_fault1 process
> mode on different machines with different pcp->batch sizes, starting
> from the default 31(calculated by zone_batchsize(), 31 is the standard
> value for any zone that has more than 1/2MiB memory), then incremented
> by 31 upwards till 527. PCP's upper limit is 6*batch.
> 
> An image is plotted and attached: batch_full.png(full here means the
> number of process started equals to CPU number).

To be clear: X-axis is the value of batch size(31, 62, 93, ..., 527),
Y-axis is the value of per_process_ops, generated by will-it-scale,
higher is better.

> 
> From the image:
> - For EX machines, they all see throughput increase with increased batch
>   size and peaked at around batch_size=310, then fall;
> - For EP machines, Haswell-EP and Broadwell-EP also see throughput
>   increase with increased batch size and peaked at batch_size=279, then
>   fall, batch_size=310 also delivers pretty good result. Skylake-EP is
>   quite different in that it doesn't see any obvious throughput increase
>   after batch_size=93, though the trend is still increasing, but in a very
>   small way and finally peaked at batch_size=403, then fall.
>   Ivybridge EP behaves much like desktop ones.
> - For Desktop machines, they do not see any obvious changes with
>   increased batch_size.
> 
> So the default batch size(31) doesn't deliver good enough result, we
> probbaly should change the default value.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCHv2 iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough
From: Michal Kubecek @ 2017-09-18  7:55 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, Phil Sutter
In-Reply-To: <1505296780-8444-2-git-send-email-liuhangbin@gmail.com>

On Wed, Sep 13, 2017 at 05:59:39PM +0800, Hangbin Liu wrote:
> With commit 72b365e8e0fd ("libnetlink: Double the dump buffer size")
> we doubled the buffer size to support more VFs. But the VFs number is
> increasing all the time. Some customers even use more than 200 VFs now.
> 
> We could not double it everytime when the buffer is not enough. Let's just
> not hard code the buffer size and malloc the correct number when running.
> 
> Introduce function rtnl_recvmsg() to always return a newly allocated buffer.
> The caller need to free it after using.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  lib/libnetlink.c | 112 ++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 78 insertions(+), 34 deletions(-)
> 
> diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> index be7ac86..e3fa7cf 100644
> --- a/lib/libnetlink.c
> +++ b/lib/libnetlink.c
> @@ -402,6 +402,62 @@ static void rtnl_dump_error(const struct rtnl_handle *rth,
>  	}
>  }
>  
> +static int rtnl_recvmsg(int fd, struct msghdr *msg, char **answer)
> +{
> +	struct iovec *iov;
> +	int len = -1, buf_len = 32768;
> +	char *bufp, *buf = NULL;
> +
> +	int flag = MSG_PEEK | MSG_TRUNC;
> +
> +realloc:
> +	bufp = realloc(buf, buf_len);
> +
> +	if (bufp == NULL) {
> +		fprintf(stderr, "malloc error: not enough buffer\n");
> +		free(buf);
> +		return -ENOMEM;
> +	}
> +	buf = bufp;
> +	iov = msg->msg_iov;
> +	iov->iov_base = buf;
> +	iov->iov_len = buf_len;
> +
> +recv:
> +	len = recvmsg(fd, msg, flag);
> +
> +	if (len < 0) {
> +		if (errno == EINTR || errno == EAGAIN)
> +			goto recv;
> +		fprintf(stderr, "netlink receive error %s (%d)\n",
> +			strerror(errno), errno);

free(buf);

> +		return len;

Maybe we should return -errno (saved before calling fprintf()) to be
consistent.

> +	}
> +
> +	if (len == 0) {
> +		fprintf(stderr, "EOF on netlink\n");

free(buf);

> +		return -ENODATA;
> +	}
> +
> +	if (len > buf_len) {
> +		buf_len = len;
> +		flag = 0;
> +		goto realloc;
> +	}
> +
> +	if (flag != 0) {
> +		flag = 0;
> +		goto recv;
> +	}

This means that even if the default buffer size is sufficient (which
should be most of the time) we make the kernel copy the message to
userspace again. Perhaps we could just call recvmsg() with zero length
to discard the message from the queue in this case. But it's not really
a big problem, I guess.

> +
> +	if (answer)
> +		*answer = buf;
> +	else
> +		free(buf);
> +
> +	return len;
> +}
> +
>  int rtnl_dump_filter_l(struct rtnl_handle *rth,
>  		       const struct rtnl_dump_filter_arg *arg)
>  {
> @@ -413,31 +469,18 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
>  		.msg_iov = &iov,
>  		.msg_iovlen = 1,
>  	};
> -	char buf[32768];
> +	char *buf;
>  	int dump_intr = 0;
>  
> -	iov.iov_base = buf;
>  	while (1) {
>  		int status;
>  		const struct rtnl_dump_filter_arg *a;
>  		int found_done = 0;
>  		int msglen = 0;
>  
> -		iov.iov_len = sizeof(buf);
> -		status = recvmsg(rth->fd, &msg, 0);
> -
> -		if (status < 0) {
> -			if (errno == EINTR || errno == EAGAIN)
> -				continue;
> -			fprintf(stderr, "netlink receive error %s (%d)\n",
> -				strerror(errno), errno);
> -			return -1;
> -		}
> -
> -		if (status == 0) {
> -			fprintf(stderr, "EOF on netlink\n");
> -			return -1;
> -		}
> +		status = rtnl_recvmsg(rth->fd, &msg, &buf);
> +		if (status < 0)
> +			return status;
>  
>  		if (rth->dump_fp)
>  			fwrite(buf, 1, NLMSG_ALIGN(status), rth->dump_fp);
> @@ -462,8 +505,10 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
>  
>  				if (h->nlmsg_type == NLMSG_DONE) {
>  					err = rtnl_dump_done(h);
> -					if (err < 0)
> +					if (err < 0) {
> +						free(buf);
>  						return -1;
> +					}
>  
>  					found_done = 1;
>  					break; /* process next filter */
> @@ -471,19 +516,23 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
>  
>  				if (h->nlmsg_type == NLMSG_ERROR) {
>  					rtnl_dump_error(rth, h);
> +					free(buf);
>  					return -1;
>  				}
>  
>  				if (!rth->dump_fp) {
>  					err = a->filter(&nladdr, h, a->arg1);
> -					if (err < 0)
> +					if (err < 0) {
> +						free(buf);
>  						return err;
> +					}
>  				}
>  
>  skip_it:
>  				h = NLMSG_NEXT(h, msglen);
>  			}
>  		}
> +		free(buf);

We only free the last buffer returned by rtnl_recvmsg() this way. IMHO
this free(buf) should be moved inside the loop.

>  
>  		if (found_done) {
>  			if (dump_intr)
> @@ -543,7 +592,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
>  		.msg_iov = &iov,
>  		.msg_iovlen = 1,
>  	};
> -	char   buf[32768] = {};
> +	char *buf;
>  
>  	n->nlmsg_seq = seq = ++rtnl->seq;
>  
> @@ -556,22 +605,12 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
>  		return -1;
>  	}
>  
> -	iov.iov_base = buf;
>  	while (1) {
> -		iov.iov_len = sizeof(buf);
> -		status = recvmsg(rtnl->fd, &msg, 0);
> +		status = rtnl_recvmsg(rtnl->fd, &msg, &buf);
> +
> +		if (status < 0)
> +			return status;
>  
> -		if (status < 0) {
> -			if (errno == EINTR || errno == EAGAIN)
> -				continue;
> -			fprintf(stderr, "netlink receive error %s (%d)\n",
> -				strerror(errno), errno);
> -			return -1;
> -		}
> -		if (status == 0) {
> -			fprintf(stderr, "EOF on netlink\n");
> -			return -1;
> -		}
>  		if (msg.msg_namelen != sizeof(nladdr)) {
>  			fprintf(stderr,
>  				"sender address length == %d\n",
> @@ -585,6 +624,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
>  			if (l < 0 || len > status) {
>  				if (msg.msg_flags & MSG_TRUNC) {
>  					fprintf(stderr, "Truncated message\n");
> +					free(buf);
>  					return -1;
>  				}
>  				fprintf(stderr,
> @@ -611,6 +651,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
>  					if (answer)
>  						memcpy(answer, h,
>  						       MIN(maxlen, h->nlmsg_len));
> +					free(buf);
>  					return 0;
>  				}
>  
> @@ -619,12 +660,14 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
>  					rtnl_talk_error(h, err, errfn);
>  
>  				errno = -err->error;
> +				free(buf);
>  				return -1;
>  			}
>  
>  			if (answer) {
>  				memcpy(answer, h,
>  				       MIN(maxlen, h->nlmsg_len));
> +				free(buf);
>  				return 0;
>  			}
>  
> @@ -633,6 +676,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
>  			status -= NLMSG_ALIGN(len);
>  			h = (struct nlmsghdr *)((char *)h + NLMSG_ALIGN(len));
>  		}
> +		free(buf);

Same as above.

>  
>  		if (msg.msg_flags & MSG_TRUNC) {
>  			fprintf(stderr, "Message truncated\n");
> -- 
> 2.5.5
> 

^ permalink raw reply

* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-09-18  8:02 UTC (permalink / raw)
  To: Vinicius Costa Gomes
  Cc: netdev, jhs, xiyou.wangcong, jiri, intel-wired-lan, andre.guedes,
	ivan.briano, jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <20170901012625.14838-1-vinicius.gomes@intel.com>

On Thu, Aug 31, 2017 at 06:26:20PM -0700, Vinicius Costa Gomes wrote:
>  * Time-aware shaper (802.1Qbv):

I just posted a working alternative showing how to handle 802.1Qbv and
many other Ethernet field buses.
 
>    The idea we are currently exploring is to add a "time-aware", priority based
>    qdisc, that also exposes the Tx queues available and provides a mechanism for
>    mapping priority <-> traffic class <-> Tx queues in a similar fashion as
>    mqprio. We are calling this qdisc 'taprio', and its 'tc' cmd line would be:
> 
>    $ $ tc qdisc add dev ens4 parent root handle 100 taprio num_tc 4    \
>      	   map 2 2 1 0 3 3 3 3 3 3 3 3 3 3 3 3                         \
> 	   queues 0 1 2 3                                              \
>      	   sched-file gates.sched [base-time <interval>]               \
>            [cycle-time <interval>] [extension-time <interval>]
> 
>    <file> is multi-line, with each line being of the following format:
>    <cmd> <gate mask> <interval in nanoseconds>
> 
>    Qbv only defines one <cmd>: "S" for 'SetGates'
> 
>    For example:
> 
>    S 0x01 300
>    S 0x03 500
> 
>    This means that there are two intervals, the first will have the gate
>    for traffic class 0 open for 300 nanoseconds, the second will have
>    both traffic classes open for 500 nanoseconds.

The idea of the schedule file will not work in practice.  Consider the
fact that the application wants to deliver time critical data in a
particular slot.  How can it find out a) what the time slots are and
b) when the next slot is scheduled?  With this Qdisc, it cannot do
this, AFAICT.  The admin might delete the file after configuring the
Qdisc!

Using the SO_TXTIME option, the application has total control over the
scheduling.  The great advantages of this approach is that we can
support any possible combination of periodic or aperiodic scheduling
and we can support any priority scheme user space dreams up.

For example, one can imaging running two or more loops that only
occasionally collide.  When they do collide, which packet should be
sent first?  Just let user space decide.

Thanks,
Richard

^ permalink raw reply

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Marc Haber @ 2017-09-18  8:02 UTC (permalink / raw)
  To: Cole Robinson
  Cc: Josef Bacik, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <10124256-647f-99a5-aa29-8073438a21df@redhat.com>

On Sun, Sep 17, 2017 at 09:17:13AM -0400, Cole Robinson wrote:
> On 09/15/2017 01:51 PM, Josef Bacik wrote:
> > Finally got access to a box to run this down myself.  This patch on top of the other patches fixes the problem for me, could you verify it works for you?  Thanks,
> > 
> 
> Yup I can confirm that patch fixes things when applied on top of the
> previous 3 patches. Thanks! Please tag those patches for stable releases
> if appropriate, this is affecting a decent amount of libvirt users

I can also confirm that these four patches fix things for me (on
Debian) as well. Thanks!

I would love to have this in one of Greg's next 4.13 releases.

Greetings
Marc

-- 
-----------------------------------------------------------------------------
Marc Haber         | "I don't trust Computers. They | Mailadresse im Header
Leimen, Germany    |  lose things."    Winona Ryder | Fon: *49 6224 1600402
Nordisch by Nature |  How to make an American Quilt | Fax: *49 6224 1600421

^ permalink raw reply

* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-09-18  8:12 UTC (permalink / raw)
  To: Vinicius Costa Gomes
  Cc: netdev, jhs, xiyou.wangcong, jiri, intel-wired-lan, andre.guedes,
	ivan.briano, jesus.sanchez-palencia, boon.leong.ong, henrik, tglx
In-Reply-To: <20170901012625.14838-1-vinicius.gomes@intel.com>

On Thu, Aug 31, 2017 at 06:26:20PM -0700, Vinicius Costa Gomes wrote:
> This patchset is an RFC on a proposal of how the Traffic Control subsystem can
> be used to offload the configuration of traffic shapers into network devices
> that provide support for them in HW. Our goal here is to start upstreaming
> support for features related to the Time-Sensitive Networking (TSN) set of
> standards into the kernel.

Just for the record, here is my score card showing the current status
of TSN support in Linux.  Comments and corrections are more welcome.

Thanks,
Richard


 | FEATURE                                        | STANDARD            | STATUS                       |
 |------------------------------------------------+---------------------+------------------------------|
 | Synchronization                                | 802.1AS-2011        | Implemented in               |
 |                                                |                     | - Linux kernel PHC subsystem |
 |                                                |                     | - linuxptp (userspace)       |
 |------------------------------------------------+---------------------+------------------------------|
 | Forwarding and Queuing Enhancements            | 802.1Q-2014 sec. 34 | RFC posted (this thread)     |
 | for Time-Sensitive Streams (FQTSS)             |                     |                              |
 |------------------------------------------------+---------------------+------------------------------|
 | Stream Reservation Protocol (SRP)              | 802.1Q-2014 sec. 35 | in Open-AVB [1]              |
 |------------------------------------------------+---------------------+------------------------------|
 | Audio Video Transport Protocol (AVTP)          | IEEE 1722-2011      | DNE                          |
 |------------------------------------------------+---------------------+------------------------------|
 | Audio/Video Device Discovery, Enumeration,     | IEEE 1722.1-2013    | jdksavdecc-c [2]             |
 | Connection Management and Control (AVDECC)     |                     |                              |
 | AVDECC Connection Management Protocol (ACMP)   |                     |                              |
 | AVDECC Enumeration and Control Protocol (AECP) |                     |                              |
 | MAC Address Acquisition Protocol (MAAP)        |                     | in Open-AVB                  |
 |------------------------------------------------+---------------------+------------------------------|
 | Frame Preemption                               | P802.1Qbu           | DNE                          |
 | Scheduled Traffic                              | P802.1Qbv           | RFC posted (SO_TXTIME)       |
 | SRP Enhancements and Performance Improvements  | P802.1Qcc           | DNE                          |

 DNE = Does Not Exist (to my knowledge)

1. https://github.com/Avnu/OpenAvnu

   (DISCLAIMER from the website:)

   It is planned to eventually include the various packet encapsulation types,
   protocol discovery daemons, libraries to convert media clocks to AVB clocks
   and vice versa, and drivers.

   This repository does not include all components required to build a full
   production AVB/TSN system (e.g. a turnkey solution to stream stored or live audio
   or video content). Some simple example applications are provided which
   illustrate the flow - but a professional Audio/Video system requires a full media stack
   - including audio and video inputs and outputs, media processing elements, and
   various graphical user interfaces. Various companies provide such integrated
   solutions.

2. https://github.com/jdkoftinoff/jdksavdecc-c

^ permalink raw reply

* Re: [pktgen script v2 0/2] Add a pktgen sample script of NUMA awareness
From: Jesper Dangaard Brouer @ 2017-09-18  9:06 UTC (permalink / raw)
  To: Robert Hoo; +Cc: davem, tariqt, kyle.leet, netdev, robert.hu, brouer
In-Reply-To: <1505651798-106642-1-git-send-email-robert.hu@linux.intel.com>

On Sun, 17 Sep 2017 20:36:36 +0800 Robert Hoo <robert.hu@linux.intel.com> wrote:

> Change log
> v2:
>  Rebased to https://github.com/netoptimizer/network-testing/tree/master/pktgen

Hi Robert,

Thank you for submitting this against my git tree[1]. I skimmed the
patches and they looked okay.  I'll give them a test run, before I
accept them into my tree.

Later I'll synchronize my pktgen scripts/git-tree with the kernel via
regular patches against DaveM's net-next tree[2] (and I'll try to
remember to give you author credit).

[1] https://github.com/netoptimizer/network-testing/tree/master/pktgen
[2] https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/samples/pktgen
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: Page allocator bottleneck
From: Tariq Toukan @ 2017-09-18  9:16 UTC (permalink / raw)
  To: Mel Gorman, Tariq Toukan
  Cc: David Miller, Jesper Dangaard Brouer, Eric Dumazet,
	Alexei Starovoitov, Saeed Mahameed, Eran Ben Elisha,
	Linux Kernel Network Developers, Andrew Morton, Michal Hocko,
	linux-mm
In-Reply-To: <20170915102320.zqceocmvvkyybekj@techsingularity.net>



On 15/09/2017 1:23 PM, Mel Gorman wrote:
> On Thu, Sep 14, 2017 at 07:49:31PM +0300, Tariq Toukan wrote:
>> Insights: Major degradation between #1 and #2, not getting any
>> close to linerate! Degradation is fixed between #2 and #3. This is
>> because page allocator cannot stand the higher allocation rate. In
>> #2, we also see that the addition of rings (cores) reduces BW (!!),
>> as result of increasing congestion over shared resources.
>> 
> 
> Unfortunately, no surprises there.
> 
>> Congestion in this case is very clear. When monitored in perf top: 
>> 85.58% [kernel] [k] queued_spin_lock_slowpath
>> 
> 
> While it's not proven, the most likely candidate is the zone lock
> and that should be confirmed using a call-graph profile. If so, then
> the suggestion to tune to the size of the per-cpu allocator would
> mitigate the problem.
> 
Indeed, I tuned the per-cpu allocator and bottleneck is released.

>> I think that page allocator issues should be discussed separately: 
>> 1) Rate: Increase the allocation rate on a single core. 2)
>> Scalability: Reduce congestion and sync overhead between cores.
>> 
>> This is clearly the current bottleneck in the network stack receive
>> flow.
>> 
>> I know about some efforts that were made in the past two years. For
>> example the ones from Jesper et al.: - Page-pool (not accepted
>> AFAIK).
> 
> Indeed not and it would also need driver conversion.
> 
>> - Page-allocation bulking.
> 
> Prototypes exist but it's pointless without the pool or driver 
> conversion so it's in the back burner for the moment.
> 

As I already mentioned in another reply (to Jesper), this would
perfectly fit with our Striding RQ feature, as we have large descriptors
that serve several packets, requiring the allocation of several pages at
once. I'd gladly move to using the bulking API.

>> - Optimize order-0 allocations in Per-Cpu-Pages.
>> 
> 
> This had a prototype that was reverted as it must be able to cope
> with both irq and noirq contexts.
Yeah, I remember that I tested and reported the issue.

Unfortunately I never found the time to
> revisit it but a split there to handle both would mitigate the
> problem. Probably not enough to actually reach line speed though so
> tuning of the per-cpu allocator sizes would still be needed. I don't
> know when I'll get the chance to revisit it. I'm travelling all next
> week and am mostly occupied with other work at the moment that is
> consuming all my concentration.
> 
>> I am not an mm expert, but wanted to raise the issue again, to
>> combine the efforts and hear from you guys about status and
>> possible directions.
> 
> The recent effort to reduce overhead from stats will help mitigate
> the problem.
I should get more familiar with these stats, check how costly they are, 
and whether they can be turned off in Kconfig.

> Finishing the page pool, the bulk allocator and converting drivers 
> would be the most likely successful path forward but it's currently
> stalled as everyone that was previously involved is too busy.
> 
I think we should consider changing the default allocation of PCP 
fraction as well, or implement some smart dynamic heuristic.
This turned on to have significant effect over networking performance.

Many thanks Mel!

Regards,
Tariq

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH net-next 09/11] bnxt_en: bnxt: add TC flower filter offload support
From: Jiri Pirko @ 2017-09-18  9:52 UTC (permalink / raw)
  To: Sathya Perla; +Cc: Michael Chan, David Miller, Netdev
In-Reply-To: <CAKvpyk1iV4z_f7cUA8DL1nU772hPtiZC4MtjA9t=S5J=8EcD-A@mail.gmail.com>

Mon, Sep 18, 2017 at 11:43:22AM CEST, sathya.perla@broadcom.com wrote:
>On Mon, Sep 11, 2017 at 7:06 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Mon, Aug 28, 2017 at 07:40:33PM CEST, michael.chan@broadcom.com wrote:
>>>From: Sathya Perla <sathya.perla@broadcom.com>
>>>
>>>This patch adds support for offloading TC based flow
>>>rules and actions for the 'flower' classifier in the bnxt_en driver.
>>>It includes logic to parse flow rules and actions received from the
>>>TC subsystem, store them and issue the corresponding
>>>hwrm_cfa_flow_alloc/free FW cmds. L2/IPv4/IPv6 flows and drop,
>>>redir, vlan push/pop actions are supported in this patch.
>>>
>>>In this patch the hwrm_cfa_flow_xxx routines are just stubs.
>>>The code for these routines is introduced in the next patch for easier
>>>review. Also, the code to query the TC/flower action stats will
>>>be introduced in a subsequent patch.
>>
>> Hi.
>>
>> You are missing checks for the offload. Please see nfp as an example:
>> Function nfp_flower_setup_tc:
>>
>>             !is_classid_clsact_ingress(cls_flower->common.classid) ||
>>             cls_flower->common.chain_index)
>
>Thanks for catching this...will send a patch.
>
>>
>> Do you support both ingress and egress or ingress only?
>
>Ingress only for flower offload.

Cool, the check would resolve it. Thanks!

^ permalink raw reply

* Re: [PATCH net-next] net: remove useless comments in dst.c
From: Sergei Shtylyov @ 2017-09-18 10:24 UTC (permalink / raw)
  To: Duan Jiong, netdev; +Cc: weiwan
In-Reply-To: <CALttK1QuA3tSdfSUPoYtoQV5reJdsVOicLX_9iN1TkwY6De6HQ@mail.gmail.com>

Hello!

On 9/18/2017 9:00 AM, Duan Jiong wrote:

> dst gc related code has been removed in commit
> 5b7c9a8ff828, so those comments are no longer

    Please also specify the commit's summary line enclosed in ("").

> useful.
> 
> Signed-off-by: Duan Jiong <jduan@fiberhome.com>
[...]

MBR, Sergei

^ permalink raw reply

* [PATCH] sunrpc: remove redundant initialization of sock
From: Colin King @ 2017-09-18 11:21 UTC (permalink / raw)
  To: J . Bruce Fields, Jeff Layton, Trond Myklebust, Anna Schumaker,
	David S . Miller, linux-nfs
  Cc: kernel-janitors, netdev

From: Colin Ian King <colin.king@canonical.com>

sock is being initialized and then being almost immediately updated
hence the initialized value is not being used and is redundant. Remove
the initialization. Cleans up clang warning:

warning: Value stored to 'sock' during its initialization is never read

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 net/sunrpc/xprtsock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 9b5de31aa429..c1841f234a71 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2203,7 +2203,7 @@ static void xs_udp_setup_socket(struct work_struct *work)
 	struct sock_xprt *transport =
 		container_of(work, struct sock_xprt, connect_worker.work);
 	struct rpc_xprt *xprt = &transport->xprt;
-	struct socket *sock = transport->sock;
+	struct socket *sock;
 	int status = -EIO;
 
 	sock = xs_create_sock(xprt, transport,
-- 
2.14.1


^ permalink raw reply related

* [PATCH] net_sched: use explicit size of struct tcmsg, remove need to declare tcm
From: Colin King @ 2017-09-18 11:40 UTC (permalink / raw)
  To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Pointer tcm is being initialized and is never read, it is only being used
to determine the size of struct tcmsg.  Clean this up by removing
variable tcm and explicitly using the sizeof struct tcmsg rather than *tcm.
Cleans up clang warning:

warning: Value stored to 'tcm' during its initialization is never read

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 net/sched/sch_api.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index c6deb74e3d2f..aa82116ed10c 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1500,7 +1500,6 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 	int s_idx, s_q_idx;
 	struct net_device *dev;
 	const struct nlmsghdr *nlh = cb->nlh;
-	struct tcmsg *tcm = nlmsg_data(nlh);
 	struct nlattr *tca[TCA_MAX + 1];
 	int err;
 
@@ -1510,7 +1509,7 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 	idx = 0;
 	ASSERT_RTNL();
 
-	err = nlmsg_parse(nlh, sizeof(*tcm), tca, TCA_MAX, NULL, NULL);
+	err = nlmsg_parse(nlh, sizeof(struct tcmsg), tca, TCA_MAX, NULL, NULL);
 	if (err < 0)
 		return err;
 
-- 
2.14.1

^ permalink raw reply related

* [net PATCH] bnxt_en: check for ingress qdisc in flower offload
From: Sathya Perla @ 2017-09-18 11:35 UTC (permalink / raw)
  To: netdev; +Cc: michael.chan

Check for ingress-only qdisc for flower offload, as other qdiscs
are not supported for flower offload.

Suggested-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Sathya Perla <sathya.perla@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index ccd699f..7dd3d13 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -750,6 +750,10 @@ int bnxt_tc_setup_flower(struct bnxt *bp, u16 src_fid,
 {
 	int rc = 0;
 
+	if (!is_classid_clsact_ingress(cls_flower->common.classid) ||
+	    cls_flower->common.chain_index)
+		return -EOPNOTSUPP;
+
 	switch (cls_flower->command) {
 	case TC_CLSFLOWER_REPLACE:
 		rc = bnxt_tc_add_flow(bp, src_fid, cls_flower);
-- 
2.7.4

^ permalink raw reply related

* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Henrik Austad @ 2017-09-18 11:46 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Vinicius Costa Gomes, netdev, jhs, xiyou.wangcong, jiri,
	intel-wired-lan, andre.guedes, ivan.briano,
	jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <20170918080214.yrejz67wwnp2pjzf@localhost>

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

On Mon, Sep 18, 2017 at 10:02:14AM +0200, Richard Cochran wrote:
> On Thu, Aug 31, 2017 at 06:26:20PM -0700, Vinicius Costa Gomes wrote:
> >  * Time-aware shaper (802.1Qbv):
> 
> I just posted a working alternative showing how to handle 802.1Qbv and
> many other Ethernet field buses.

Yes, I saw them, grabbing them for testing now - thanks!

> >    The idea we are currently exploring is to add a "time-aware", priority based
> >    qdisc, that also exposes the Tx queues available and provides a mechanism for
> >    mapping priority <-> traffic class <-> Tx queues in a similar fashion as
> >    mqprio. We are calling this qdisc 'taprio', and its 'tc' cmd line would be:
> > 
> >    $ $ tc qdisc add dev ens4 parent root handle 100 taprio num_tc 4    \
> >      	   map 2 2 1 0 3 3 3 3 3 3 3 3 3 3 3 3                         \
> > 	   queues 0 1 2 3                                              \
> >      	   sched-file gates.sched [base-time <interval>]               \
> >            [cycle-time <interval>] [extension-time <interval>]
> > 
> >    <file> is multi-line, with each line being of the following format:
> >    <cmd> <gate mask> <interval in nanoseconds>
> > 
> >    Qbv only defines one <cmd>: "S" for 'SetGates'
> > 
> >    For example:
> > 
> >    S 0x01 300
> >    S 0x03 500
> > 
> >    This means that there are two intervals, the first will have the gate
> >    for traffic class 0 open for 300 nanoseconds, the second will have
> >    both traffic classes open for 500 nanoseconds.
> 
> The idea of the schedule file will not work in practice.  Consider the
> fact that the application wants to deliver time critical data in a
> particular slot.  How can it find out a) what the time slots are and
> b) when the next slot is scheduled?  With this Qdisc, it cannot do
> this, AFAICT.  The admin might delete the file after configuring the
> Qdisc!
> 
> Using the SO_TXTIME option, the application has total control over the
> scheduling.  The great advantages of this approach is that we can
> support any possible combination of periodic or aperiodic scheduling
> and we can support any priority scheme user space dreams up.

Using SO_TXTIME makes a lot of sense. TSN has a presentation_time, which 
you can use to deduce the time it should be transmitted (Class A has a 2ms 
latency guarantee, B has 50), but given how TSN uses the timestamp, it will 
wrap every 4.3 seconds, using SO_TXTIME allows you to schedule transmission 
at a much later time. It should also lessen the dependency on a specific 
protocol, which is also good.

> For example, one can imaging running two or more loops that only
> occasionally collide.  When they do collide, which packet should be
> sent first?  Just let user space decide.

If 2 userspace apps send to the same Tx-queue with the same priority, would 
it not make sense to just do FIFO? For all practical purposes, they have 
the same importance (same SO_PRIORITY, same SO_TXTIME). If the priority 
differs, then they would be directed to different queues, where one queue 
will take presedence anyway.

How far into the future would it make sense to schedule packets anyway?

I'll have a look at the other series you just posted!

-- 
Henrik Austad

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [net PATCH] bnxt_en: check for ingress qdisc in flower offload
From: Jiri Pirko @ 2017-09-18 11:56 UTC (permalink / raw)
  To: Sathya Perla; +Cc: netdev, michael.chan
In-Reply-To: <1505734537-6695-1-git-send-email-sathya.perla@broadcom.com>

Mon, Sep 18, 2017 at 01:35:37PM CEST, sathya.perla@broadcom.com wrote:
>Check for ingress-only qdisc for flower offload, as other qdiscs
>are not supported for flower offload.
>
>Suggested-by: Jiri Pirko <jiri@resnulli.us>
>Signed-off-by: Sathya Perla <sathya.perla@broadcom.com>

Reviewed-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* RE: [PATCH] net_sched: use explicit size of struct tcmsg, remove need to declare tcm
From: David Laight @ 2017-09-18 12:18 UTC (permalink / raw)
  To: 'Colin King', Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	David S . Miller, netdev@vger.kernel.org
  Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20170918114038.29741-1-colin.king@canonical.com>

From: Colin King
> Sent: 18 September 2017 12:41
> Pointer tcm is being initialized and is never read, it is only being used
> to determine the size of struct tcmsg.  Clean this up by removing
> variable tcm and explicitly using the sizeof struct tcmsg rather than *tcm.
> Cleans up clang warning:
> 
> warning: Value stored to 'tcm' during its initialization is never read
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  net/sched/sch_api.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index c6deb74e3d2f..aa82116ed10c 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1500,7 +1500,6 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
>  	int s_idx, s_q_idx;
>  	struct net_device *dev;
>  	const struct nlmsghdr *nlh = cb->nlh;
> -	struct tcmsg *tcm = nlmsg_data(nlh);
>  	struct nlattr *tca[TCA_MAX + 1];
>  	int err;
> 
> @@ -1510,7 +1509,7 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
>  	idx = 0;
>  	ASSERT_RTNL();
> 
> -	err = nlmsg_parse(nlh, sizeof(*tcm), tca, TCA_MAX, NULL, NULL);
> +	err = nlmsg_parse(nlh, sizeof(struct tcmsg), tca, TCA_MAX, NULL, NULL);

Would sizeof(*nlmsg_data(nlh)) be cleaner??

	David


^ permalink raw reply

* Re: [PATCH v06 35/36] uapi linux/tls.h: don't include <net/tcp.h> in user space
From: Mikko Rapeli @ 2017-09-18 12:57 UTC (permalink / raw)
  To: Dmitry V. Levin
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Dave Watson, Ilya Lesokhin,
	Aviad Yehezkel, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170904161510.GA16001-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>

On Mon, Sep 04, 2017 at 07:15:11PM +0300, Dmitry V. Levin wrote:
> On Wed, Aug 09, 2017 at 02:25:54AM +0300, Dmitry V. Levin wrote:
> > On Sun, Aug 06, 2017 at 06:44:26PM +0200, Mikko Rapeli wrote:
> > > It is not needed and not part of uapi headers, but causes
> > > user space compilation error:
> > > 
> > > fatal error: net/tcp.h: No such file or directory
> > >  #include <net/tcp.h>
> > >                      ^
> > > 
> > > Signed-off-by: Mikko Rapeli <mikko.rapeli-X3B1VOXEql0@public.gmane.org>
> > > Cc: Dave Watson <davejwatson-b10kYP2dOMg@public.gmane.org>
> > > Cc: Ilya Lesokhin <ilyal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > Cc: Aviad Yehezkel <aviadye-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > ---
> > >  include/uapi/linux/tls.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/include/uapi/linux/tls.h b/include/uapi/linux/tls.h
> > > index cc1d21db35d8..d87c698623f2 100644
> > > --- a/include/uapi/linux/tls.h
> > > +++ b/include/uapi/linux/tls.h
> > > @@ -37,7 +37,9 @@
> > >  #include <asm/byteorder.h>
> > >  #include <linux/socket.h>
> > >  #include <linux/tcp.h>
> > > +#ifdef __KERNEL__
> > >  #include <net/tcp.h>
> > > +#endif
> > 
> > Let's move it to include/net/tls.h instead.
> 
> So everybody ignored this and *new* uapi header was released
> in a totally unusable form along with v4.13.

Should issues like these be filed into bugzilla instead?

Maybe emails are easier to ignore than bugzilla tickets.

-Mikko

^ permalink raw reply

* [PATCH net] net: phy: Kconfig: Fix PHY infrastructure menu in menuconfig
From: Jerome Brunet @ 2017-09-18 12:59 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli
  Cc: Jerome Brunet, netdev, linux-kernel, Russell King

Since the integration of PHYLINK, the configuration option which
used to be under the PHY infrastructure menu in menuconfig ended
up one level up (the network device driver section)

By placing PHYLINK option right after PHYLIB entry, it broke the
way Kconfig used to build the menu. See kconfig-language.txt, section
"Menu structure", 2nd method.

This is fixed by placing the PHYLINK option just before PHYLIB.

Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/Kconfig | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index a9d16a3af514..cd931cf9dcc2 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -160,15 +160,6 @@ config MDIO_XGENE
 
 endif
 
-menuconfig PHYLIB
-	tristate "PHY Device support and infrastructure"
-	depends on NETDEVICES
-	select MDIO_DEVICE
-	help
-	  Ethernet controllers are usually attached to PHY
-	  devices.  This option provides infrastructure for
-	  managing PHY devices.
-
 config PHYLINK
 	tristate
 	depends on NETDEVICES
@@ -179,6 +170,15 @@ config PHYLINK
 	  configuration links, PHYs, and Serdes links with MAC level
 	  autonegotiation modes.
 
+menuconfig PHYLIB
+	tristate "PHY Device support and infrastructure"
+	depends on NETDEVICES
+	select MDIO_DEVICE
+	help
+	  Ethernet controllers are usually attached to PHY
+	  devices.  This option provides infrastructure for
+	  managing PHY devices.
+
 if PHYLIB
 
 config SWPHY
-- 
2.13.5

^ permalink raw reply related

* Re: [PATCH net-next v2 0/7] korina: performance fixes and cleanup
From: Roman Yeryomin @ 2017-09-18 13:02 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev
In-Reply-To: <649cd233-d9b8-478b-e742-5da469df8f26@gmail.com>

On 17 September 2017 at 23:09, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
> On 09/17/2017 10:23 AM, Roman Yeryomin wrote:
>> Changes from v1:
>> - use GRO instead of increasing ring size
>> - use NAPI_POLL_WEIGHT instead of defining own NAPI_WEIGHT
>> - optimize rx descriptor flags processing
>
> net-next is closed at the moment, but these look like reasonable
> changes, I would just replace patch 7 with a patch that entirely drops
> the driver specific version since that does not serve any purpose in the
> context of an in-kernel driver.

OK

> Some nice clean-ups that you should also consider for future changes:
>
> - reduce the duplication of tests/conditions in korina_send_packet(), a
> lot of them are testing for the same things and setting the same
> descriptor bits

Already doing that :)

> - move korina_tx() to a NAPI context instead of working from hard
> interrupt context
>
> - get rid of the MIPS dma_cache_* calls and instead properly use the
> DMA-API to allocate descriptors and invalidate/write-back skb->data

OK

>>
>> Roman Yeryomin (7):
>>   net: korina: don't use overflow and underflow interrupts
>>   net: korina: optimize rx descriptor flags processing
>>   net: korina: use NAPI_POLL_WEIGHT
>>   net: korina: use GRO
>>   net: korina: whitespace cleanup
>>   net: korina: update authors
>>   net: korina: bump version
>>
>>  drivers/net/ethernet/korina.c | 230 ++++++++++++++----------------------------
>>  1 file changed, 78 insertions(+), 152 deletions(-)
>>
>
> --
> Florian

^ permalink raw reply

* [PATCH] bpf: devmap: pass on return value of bpf_map_precharge_memlock
From: Tobias Klauser @ 2017-09-18 13:03 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: John Fastabend, netdev

If bpf_map_precharge_memlock in dev_map_alloc, -ENOMEM is returned
regardless of the actual error produced by bpf_map_precharge_memlock.
Fix it by passing on the error returned by bpf_map_precharge_memlock.

Also return -EINVAL instead of -ENOMEM if the page count overflow check
fails.

This makes dev_map_alloc match the behavior of other bpf maps' alloc
functions wrt. return values.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 kernel/bpf/devmap.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 959c9a07f318..e093d9a2c4dd 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -75,8 +75,8 @@ static u64 dev_map_bitmap_size(const union bpf_attr *attr)
 static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
 {
 	struct bpf_dtab *dtab;
+	int err = -EINVAL;
 	u64 cost;
-	int err;
 
 	/* check sanity of attributes */
 	if (attr->max_entries == 0 || attr->key_size != 4 ||
@@ -108,6 +108,8 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
 	if (err)
 		goto free_dtab;
 
+	err = -ENOMEM;
+
 	/* A per cpu bitfield with a bit per possible net device */
 	dtab->flush_needed = __alloc_percpu(dev_map_bitmap_size(attr),
 					    __alignof__(unsigned long));
@@ -128,7 +130,7 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
 free_dtab:
 	free_percpu(dtab->flush_needed);
 	kfree(dtab);
-	return ERR_PTR(-ENOMEM);
+	return ERR_PTR(err);
 }
 
 static void dev_map_free(struct bpf_map *map)
-- 
2.13.0

^ permalink raw reply related

* [PATCH net 0/3] net: mvpp2: various fixes
From: Antoine Tenart @ 2017-09-18 13:04 UTC (permalink / raw)
  To: davem
  Cc: Antoine Tenart, andrew, gregory.clement, thomas.petazzoni,
	miquel.raynal, nadavh, linux, linux-kernel, mw, stefanc, netdev

Hi all,

This series contains various fixes for the Marvell PPv2 driver. Please
have a thorough look at patch 1/3 ("net: mvpp2: fix the dma_mask and
coherent_dma_mask settings for PPv2.2") as I'm not 100% sure about the
fix implementation.

Thanks!
Antoine

Antoine Tenart (1):
  net: mvpp2: fix the dma_mask and coherent_dma_mask settings for PPv2.2

Stefan Chulski (1):
  net: mvpp2: fix parsing fragmentation detection

Yan Markman (1):
  net: mvpp2: fix port list indexing

 drivers/net/ethernet/marvell/mvpp2.c | 44 ++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 9 deletions(-)

-- 
2.13.5

^ permalink raw reply

* [PATCH net 1/3] net: mvpp2: fix the dma_mask and coherent_dma_mask settings for PPv2.2
From: Antoine Tenart @ 2017-09-18 13:04 UTC (permalink / raw)
  To: davem
  Cc: Antoine Tenart, andrew, gregory.clement, thomas.petazzoni,
	miquel.raynal, nadavh, linux, linux-kernel, mw, stefanc, netdev
In-Reply-To: <20170918130408.23114-1-antoine.tenart@free-electrons.com>

The dev->dma_mask usually points to dev->coherent_dma_mask. This is an
issue as setting both of them will override the other. This is
problematic here as the PPv2 driver uses a 32-bit-mask for coherent
accesses (txq, rxq, bm) and a 40-bit mask for all other accesses due to
an hardware limitation.

This can lead to a memory remap for all dma_map_single() calls when
dealing with memory above 4GB.

Fixes: 2067e0a13cfe ("net: mvpp2: set dma mask and coherent dma mask on PPv2.2")
Reported-by: Stefan Chulski <stefanc@marvell.com>
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index dd0ee2691c86..7024d4dbb461 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7969,9 +7969,25 @@ static int mvpp2_probe(struct platform_device *pdev)
 	priv->tclk = clk_get_rate(priv->pp_clk);
 
 	if (priv->hw_version == MVPP22) {
+		/* If dma_mask points to coherent_dma_mask, setting both will
+		 * override the value of the other. This is problematic as the
+		 * PPv2 driver uses a 32-bit-mask for coherent accesses (txq,
+		 * rxq, bm) and a 40-bit mask for all other accesses.
+		 */
+		if (pdev->dev.dma_mask == &pdev->dev.coherent_dma_mask) {
+			pdev->dev.dma_mask = devm_kzalloc(&pdev->dev,
+							  sizeof(*pdev->dev.dma_mask),
+							  GFP_KERNEL);
+			if (!pdev->dev.dma_mask) {
+				err = -ENOMEM;
+				goto err_mg_clk;
+			}
+		}
+
 		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(40));
 		if (err)
 			goto err_mg_clk;
+
 		/* Sadly, the BM pools all share the same register to
 		 * store the high 32 bits of their address. So they
 		 * must all have the same high 32 bits, which forces
-- 
2.13.5

^ permalink raw reply related

* [PATCH net 2/3] net: mvpp2: fix parsing fragmentation detection
From: Antoine Tenart @ 2017-09-18 13:04 UTC (permalink / raw)
  To: davem
  Cc: Stefan Chulski, andrew, gregory.clement, thomas.petazzoni,
	miquel.raynal, nadavh, linux, linux-kernel, mw, netdev,
	Antoine Tenart
In-Reply-To: <20170918130408.23114-1-antoine.tenart@free-electrons.com>

From: Stefan Chulski <stefanc@marvell.com>

Parsing fragmentation detection failed due to wrong configured
parser TCAM entry's. Some traffic was marked as fragmented in RX
descriptor, even it wasn't IP fragmented. The hardware also failed to
calculate checksums which lead to use software checksum and caused
performance degradation.

Fixes: 3f518509dedc ("ethernet: Add new driver for Marvell Armada 375 network unit")
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 7024d4dbb461..56d474414cfa 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -676,6 +676,7 @@ enum mvpp2_tag_type {
 #define MVPP2_PRS_RI_L3_MCAST			BIT(15)
 #define MVPP2_PRS_RI_L3_BCAST			(BIT(15) | BIT(16))
 #define MVPP2_PRS_RI_IP_FRAG_MASK		0x20000
+#define MVPP2_PRS_RI_IP_FRAG_TRUE		BIT(17)
 #define MVPP2_PRS_RI_UDF3_MASK			0x300000
 #define MVPP2_PRS_RI_UDF3_RX_SPECIAL		BIT(21)
 #define MVPP2_PRS_RI_L4_PROTO_MASK		0x1c00000
@@ -2315,7 +2316,7 @@ static int mvpp2_prs_ip4_proto(struct mvpp2 *priv, unsigned short proto,
 	    (proto != IPPROTO_IGMP))
 		return -EINVAL;
 
-	/* Fragmented packet */
+	/* Not fragmented packet */
 	tid = mvpp2_prs_tcam_first_free(priv, MVPP2_PE_FIRST_FREE_TID,
 					MVPP2_PE_LAST_FREE_TID);
 	if (tid < 0)
@@ -2334,8 +2335,12 @@ static int mvpp2_prs_ip4_proto(struct mvpp2 *priv, unsigned short proto,
 				  MVPP2_PRS_SRAM_OP_SEL_UDF_ADD);
 	mvpp2_prs_sram_ai_update(&pe, MVPP2_PRS_IPV4_DIP_AI_BIT,
 				 MVPP2_PRS_IPV4_DIP_AI_BIT);
-	mvpp2_prs_sram_ri_update(&pe, ri | MVPP2_PRS_RI_IP_FRAG_MASK,
-				 ri_mask | MVPP2_PRS_RI_IP_FRAG_MASK);
+	mvpp2_prs_sram_ri_update(&pe, ri, ri_mask | MVPP2_PRS_RI_IP_FRAG_MASK);
+
+	mvpp2_prs_tcam_data_byte_set(&pe, 2, 0x00,
+				     MVPP2_PRS_TCAM_PROTO_MASK_L);
+	mvpp2_prs_tcam_data_byte_set(&pe, 3, 0x00,
+				     MVPP2_PRS_TCAM_PROTO_MASK);
 
 	mvpp2_prs_tcam_data_byte_set(&pe, 5, proto, MVPP2_PRS_TCAM_PROTO_MASK);
 	mvpp2_prs_tcam_ai_update(&pe, 0, MVPP2_PRS_IPV4_DIP_AI_BIT);
@@ -2346,7 +2351,7 @@ static int mvpp2_prs_ip4_proto(struct mvpp2 *priv, unsigned short proto,
 	mvpp2_prs_shadow_set(priv, pe.index, MVPP2_PRS_LU_IP4);
 	mvpp2_prs_hw_write(priv, &pe);
 
-	/* Not fragmented packet */
+	/* Fragmented packet */
 	tid = mvpp2_prs_tcam_first_free(priv, MVPP2_PE_FIRST_FREE_TID,
 					MVPP2_PE_LAST_FREE_TID);
 	if (tid < 0)
@@ -2358,8 +2363,11 @@ static int mvpp2_prs_ip4_proto(struct mvpp2 *priv, unsigned short proto,
 	pe.sram.word[MVPP2_PRS_SRAM_RI_CTRL_WORD] = 0x0;
 	mvpp2_prs_sram_ri_update(&pe, ri, ri_mask);
 
-	mvpp2_prs_tcam_data_byte_set(&pe, 2, 0x00, MVPP2_PRS_TCAM_PROTO_MASK_L);
-	mvpp2_prs_tcam_data_byte_set(&pe, 3, 0x00, MVPP2_PRS_TCAM_PROTO_MASK);
+	mvpp2_prs_sram_ri_update(&pe, ri | MVPP2_PRS_RI_IP_FRAG_TRUE,
+				 ri_mask | MVPP2_PRS_RI_IP_FRAG_MASK);
+
+	mvpp2_prs_tcam_data_byte_set(&pe, 2, 0x00, 0x0);
+	mvpp2_prs_tcam_data_byte_set(&pe, 3, 0x00, 0x0);
 
 	/* Update shadow table and hw entry */
 	mvpp2_prs_shadow_set(priv, pe.index, MVPP2_PRS_LU_IP4);
-- 
2.13.5

^ permalink raw reply related

* [PATCH net 3/3] net: mvpp2: fix port list indexing
From: Antoine Tenart @ 2017-09-18 13:04 UTC (permalink / raw)
  To: davem
  Cc: Yan Markman, andrew, gregory.clement, thomas.petazzoni,
	miquel.raynal, nadavh, linux, linux-kernel, mw, stefanc, netdev,
	Antoine Tenart
In-Reply-To: <20170918130408.23114-1-antoine.tenart@free-electrons.com>

From: Yan Markman <ymarkman@marvell.com>

The private port_list array has a list of pointers to mvpp2_port
instances. This list is allocated given the number of ports enabled in
the device tree, but the pointers are set using the port-id property. If
on a single port is enabled, the port_list array will be of size 1, but
when registering the port, if its id is not 0 the driver will crash.
Other crashes were encountered in various situations.

This fixes the issue by using an index not equal to the value of the
port-id property.

Fixes: 3f518509dedc ("ethernet: Add new driver for Marvell Armada 375 network unit")
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 56d474414cfa..e7889464e97e 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7504,7 +7504,7 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
 /* Ports initialization */
 static int mvpp2_port_probe(struct platform_device *pdev,
 			    struct device_node *port_node,
-			    struct mvpp2 *priv)
+			    struct mvpp2 *priv, int index)
 {
 	struct device_node *phy_node;
 	struct phy *comphy;
@@ -7678,7 +7678,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	}
 	netdev_info(dev, "Using %s mac address %pM\n", mac_from, dev->dev_addr);
 
-	priv->port_list[id] = port;
+	priv->port_list[index] = port;
 	return 0;
 
 err_free_port_pcpu:
@@ -8029,10 +8029,12 @@ static int mvpp2_probe(struct platform_device *pdev)
 	}
 
 	/* Initialize ports */
+	i = 0;
 	for_each_available_child_of_node(dn, port_node) {
-		err = mvpp2_port_probe(pdev, port_node, priv);
+		err = mvpp2_port_probe(pdev, port_node, priv, i);
 		if (err < 0)
 			goto err_mg_clk;
+		i++;
 	}
 
 	platform_set_drvdata(pdev, priv);
-- 
2.13.5

^ permalink raw reply related

* Re: [PATCH] bpf: devmap: pass on return value of bpf_map_precharge_memlock
From: Daniel Borkmann @ 2017-09-18 13:14 UTC (permalink / raw)
  To: Tobias Klauser, Alexei Starovoitov; +Cc: John Fastabend, netdev
In-Reply-To: <20170918130346.10833-1-tklauser@distanz.ch>

On 09/18/2017 03:03 PM, Tobias Klauser wrote:
> If bpf_map_precharge_memlock in dev_map_alloc, -ENOMEM is returned
> regardless of the actual error produced by bpf_map_precharge_memlock.
> Fix it by passing on the error returned by bpf_map_precharge_memlock.
>
> Also return -EINVAL instead of -ENOMEM if the page count overflow check
> fails.
>
> This makes dev_map_alloc match the behavior of other bpf maps' alloc
> functions wrt. return values.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>

(This would then need to go via net tree.)

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

^ permalink raw reply

* Re: [PATCH net-next v2 0/7] korina: performance fixes and cleanup
From: Roman Yeryomin @ 2017-09-18 13:23 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev
In-Reply-To: <CACiydb+Au=X4wvw_ToLOw-g=y504kby4y5NwG9czFE8Hs8vnoQ@mail.gmail.com>

On 18 September 2017 at 16:02, Roman Yeryomin <leroi.lists@gmail.com> wrote:
> On 17 September 2017 at 23:09, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>>
>> On 09/17/2017 10:23 AM, Roman Yeryomin wrote:
>>> Changes from v1:
>>> - use GRO instead of increasing ring size
>>> - use NAPI_POLL_WEIGHT instead of defining own NAPI_WEIGHT
>>> - optimize rx descriptor flags processing
>>
>> net-next is closed at the moment, but these look like reasonable
>> changes, I would just replace patch 7 with a patch that entirely drops
>> the driver specific version since that does not serve any purpose in the
>> context of an in-kernel driver.
>
> OK
>

Oh, wait, forgot we've been here already.
What about ethtool_drvinfo?
Or you mean drop all ethtool helpers?

Regards,
Roman

^ permalink raw reply


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