* Re: [PATCH] mlx5: ensure 0 is returned when vport is zero
From: Leon Romanovsky @ 2017-08-18 16:02 UTC (permalink / raw)
To: Colin King
Cc: Saeed Mahameed, Matan Barak, netdev, linux-rdma, kernel-janitors,
linux-kernel
In-Reply-To: <20170818134925.16604-1-colin.king@canonical.com>
[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]
On Fri, Aug 18, 2017 at 02:49:25PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently, if vport is zero then then an uninialized return status
> in err is returned. Since the only return status at the end of the
> function esw_add_uc_addr is zero for the current set of return paths
> we may as well just return 0 rather than err to fix this issue.
>
> Detected by CoverityScan, CID#1452698 ("Uninitialized scalar variable")
>
> Fixes: eeb66cdb6826 ("net/mlx5: Separate between E-Switch and MPFS")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> index 6d9fb6ac6e9b..c77f4c0c7769 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> @@ -401,7 +401,7 @@ static int esw_add_uc_addr(struct mlx5_eswitch *esw, struct vport_addr *vaddr)
> esw_debug(esw->dev, "\tADDED UC MAC: vport[%d] %pM fr(%p)\n",
> vport, mac, vaddr->flow_rule);
>
> - return err;
> + return 0;
> }
>
I personally prefer initialization of "err" to zero, but this solution
is fine enough too.
Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* RE: [PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev
From: Salil Mehta @ 2017-08-18 16:10 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem@davemloft.net, Zhuangyuzeng (Yisen), lipeng (Y),
mehta.salil.lnk@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
Linuxarm
In-Reply-To: <20170818160157.GB3258@lunn.ch>
Hi Andrew
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Friday, August 18, 2017 5:02 PM
> To: Salil Mehta
> Cc: davem@davemloft.net; Zhuangyuzeng (Yisen); lipeng (Y);
> mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH net-next] net: hns3: Add support to change MTU in
> hardware & netdev
>
> > for example,
> > file: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> > static int init_one(struct pci_dev *pdev, const struct pci_device_id
> *ent)
> >
> > netdev->priv_flags |= IFF_UNICAST_FLT;
> >
> > + /* MTU range: 81 - 9600 */
> > + netdev->min_mtu = 81;
> > + netdev->max_mtu = MAX_MTU;
>
> In this cause, the driver is not using the default values. So it sets
> them.
>
> Anyway, try it. After your alloc_etherdev_mqs(), print the value of
> min_mtu. It should already be set to MIN_ETH_MTU.
I understand your point. In this case, I would like to keep the
range being set by the driver just to be more explicit.
So for now keep this initialization in the driver?
Thanks
Salil
>
> > I see. IMHO HNS3 is currently limited by maximum buffer per
> descriptor
> > which is 64k. I am sure such frames would get dropped in the hardware
> > itself and which I guess should be more preferable than dropping in
> > driver since it saves you some precious cpu cycles?
>
> If you hardware handles this, then you don't need to do anything.
Fine. Thanks!
Salil
>
> Andrew
^ permalink raw reply
* Re: [iproute PATCH v2 0/2] Covscan: Shell script fixes
From: Stephen Hemminger @ 2017-08-18 16:12 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170817170932.24900-1-phil@nwl.cc>
On Thu, 17 Aug 2017 19:09:30 +0200
Phil Sutter <phil@nwl.cc> wrote:
> This series collects patches from v1 which deal with programming
> mistakes in shell scripts.
>
> No changes to the actual patches, just splitting into smaller series.
>
> Phil Sutter (2):
> examples: Some shell fixes to cbq.init
> ifcfg: Quote left-hand side of [ ] expression
>
> examples/cbq.init-v0.7.3 | 24 ++++++++++++------------
> ip/ifcfg | 2 +-
> 2 files changed, 13 insertions(+), 13 deletions(-)
>
Applied.
^ permalink raw reply
* Re: [iproute PATCH v2 0/3] Covscan: Fixes for obvious programming mistakes
From: Stephen Hemminger @ 2017-08-18 16:15 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170817170932.24812-1-phil@nwl.cc>
On Thu, 17 Aug 2017 19:09:29 +0200
Phil Sutter <phil@nwl.cc> wrote:
> This series collects those patches from v1 which are clear programming
> flaws.
>
> No changes to the actual patches, just splitting into smaller series.
>
> Phil Sutter (3):
> iproute_lwtunnel: csum_mode value checking was ineffective
> iproute_lwtunnel: Argument to strerror must be positive
> tipc/node: Fix socket fd check in cmd_node_get_addr()
>
> ip/iproute_lwtunnel.c | 9 +++++----
> tipc/node.c | 3 ++-
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
Accepted.
^ permalink raw reply
* pull request: bluetooth-next 2017-08-18
From: Johan Hedberg @ 2017-08-18 16:16 UTC (permalink / raw)
To: davem; +Cc: linux-bluetooth, netdev
[-- Attachment #1: Type: text/plain, Size: 2456 bytes --]
Hi Dave,
Here's one more bluetooth-next pull request for the 4.14 kernel:
- Multiple fixes for Broadcom controllers
- Fixes to the bluecard HCI driver
- New USB ID for Realtek RTL8723BE controller
- Fix static analyzer warning with kfree
Please let me know if there are any issues pulling. Thanks.
Johan
---
The following changes since commit 1afec92be0d7b516a79f4c548a81d17460c8ba73:
Merge branch 'sctp-remove-typedefs-from-structures-part-5' (2017-08-06 21:33:43 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git for-upstream
for you to fetch changes up to 01d5e44ace8a20fc51e0d530f98acb3c365345a5:
Bluetooth: hci_bcm: Handle empty packet after firmware loading (2017-08-17 22:51:50 +0300)
----------------------------------------------------------------
Colin Ian King (1):
Bluetooth: kfree tmp rather than an alias to it
Dmitry Tunin (1):
Bluetooth: Add support of 13d3:3494 RTL8723BE device
Loic Poulain (2):
Bluetooth: hci_bcm: Add serdev support
dt-bindings: net: bluetooth: Add broadcom-bluetooth
Marcel Holtmann (4):
Bluetooth: btusb: Add workaround for Broadcom devices without product id
Bluetooth: hci_bcm: Use operation speed of 4Mbps only for ACPI devices
Bluetooth: btbcm: Consolidate the controller information commands
Bluetooth: hci_bcm: Handle empty packet after firmware loading
Ondrej Zary (3):
Bluetooth: bluecard: Always enable LEDs (fix for Anycom CF-300)
Bluetooth: bluecard: fix LED behavior
Bluetooth: bluecard: blink LED during continuous activity
Pavel Machek (1):
Bluetooth: document config options
Sukumar Ghorai (1):
Bluetooth: btusb: driver to enable the usb-wakeup feature
.../devicetree/bindings/net/broadcom-bluetooth.txt | 35 +++++++
drivers/bluetooth/Kconfig | 1 +
drivers/bluetooth/bluecard_cs.c | 58 ++++++------
drivers/bluetooth/btbcm.c | 69 ++++++++------
drivers/bluetooth/btusb.c | 24 +++++
drivers/bluetooth/hci_bcm.c | 103 ++++++++++++++++++++-
net/bluetooth/Kconfig | 12 +++
net/bluetooth/selftest.c | 2 +-
8 files changed, 238 insertions(+), 66 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [iproute PATCH v2 0/2] Covscan: Fix potential file descriptor leaks
From: Stephen Hemminger @ 2017-08-18 16:18 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170817170931.24224-1-phil@nwl.cc>
On Thu, 17 Aug 2017 19:09:29 +0200
Phil Sutter <phil@nwl.cc> wrote:
> This series collects patches from v1 which deal with potential file
> descriptor leaks.
>
> No changes to the actual patches, just splitting into smaller series.
>
> Phil Sutter (2):
> ss: Don't leak fd in tcp_show_netlink_file()
> tc/em_ipset: Don't leak sockfd on error path
>
> misc/ss.c | 32 ++++++++++++++++++++------------
> tc/em_ipset.c | 1 +
> 2 files changed, 21 insertions(+), 12 deletions(-)
>
Applied
^ permalink raw reply
* Re: [PATCH v1 net] TCP_USER_TIMEOUT and tcp_keepalive should conform to RFC5482
From: Eric Dumazet @ 2017-08-08 9:59 UTC (permalink / raw)
To: Rao Shoaib; +Cc: davem, kuznet, netdev
In-Reply-To: <20170807181614.GA16700@caduceus5>
On Mon, 2017-08-07 at 11:16 -0700, Rao Shoaib wrote:
> Change from version 0: Rationale behind the change:
>
> The man page for tcp(7) states
>
> when used with the TCP keepalive (SO_KEEPALIVE) option, TCP_USER_TIMEOUT will
> override keepalive to determine when to close a connection due to keepalive
> failure.
>
> This is ambigious at best. user expectation is most likely that the connection
> will be reset after TCP_USER_TIMEOUT milliseconds of inactivity.
>
> The code however waits for the keepalive to kick-in (default 2hrs) and than
> after one failure resets the conenction.
>
> What is the rationale for that ? The same effect can be obtained by simply
> changing the value of tcp_keep_alive_probes.
>
> Since the TCP_USER_TIMEOUT option was added based on RFC 5482 we need to follow
> the RFC. Which states
>
> 4.2 TCP keep-Alives:
> Some TCP implementations, such as those in BSD systems, use a
> different abort policy for TCP keep-alives than for user data. Thus,
> the TCP keep-alive mechanism might abort a connection that would
> otherwise have survived the transient period without connectivity.
> Therefore, if a connection that enables keep-alives is also using the
> TCP User Timeout Option, then the keep-alive timer MUST be set to a
> value larger than that of the adopted USER TIMEOUT.
>
> This patch enforces the MUST and also dis-associates user timeout from keep
> alive. A man page patch will be submitted separately.
>
> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
> ---
> net/ipv4/tcp.c | 10 ++++++++--
> net/ipv4/tcp_timer.c | 9 +--------
> 2 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 71ce33d..f2af44d 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2628,7 +2628,9 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
> break;
>
> case TCP_KEEPIDLE:
> - if (val < 1 || val > MAX_TCP_KEEPIDLE)
> + /* Per RFC5482 keepalive_time must be > user_timeout */
> + if (val < 1 || val > MAX_TCP_KEEPIDLE ||
> + ((val * HZ) <= icsk->icsk_user_timeout))
> err = -EINVAL;
> else {
> tp->keepalive_time = val * HZ;
> @@ -2724,8 +2726,12 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
> case TCP_USER_TIMEOUT:
> /* Cap the max time in ms TCP will retry or probe the window
> * before giving up and aborting (ETIMEDOUT) a connection.
> + * Per RFC5482 TCP user timeout must be < keepalive_time.
> + * If the default value changes later -- all bets are off.
> */
> - if (val < 0)
> + if (val < 0 || (tp->keepalive_time &&
> + tp->keepalive_time <= msecs_to_jiffies(val)) ||
> + net->ipv4.sysctl_tcp_keepalive_time <= msecs_to_jiffies(val))
When TCP_USER_TIMEOUT socket option is attempted, maybe keepalive option
was not used.
Yet your new tests assume it is engaged.
It might break some usages.
> err = -EINVAL;
> else
> icsk->icsk_user_timeout = msecs_to_jiffies(val);
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index c0feeee..d39fe60 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -664,14 +664,7 @@ static void tcp_keepalive_timer (unsigned long data)
> elapsed = keepalive_time_elapsed(tp);
>
> if (elapsed >= keepalive_time_when(tp)) {
> - /* If the TCP_USER_TIMEOUT option is enabled, use that
> - * to determine when to timeout instead.
> - */
> - if ((icsk->icsk_user_timeout != 0 &&
> - elapsed >= icsk->icsk_user_timeout &&
> - icsk->icsk_probes_out > 0) ||
> - (icsk->icsk_user_timeout == 0 &&
> - icsk->icsk_probes_out >= keepalive_probes(tp))) {
> + if (icsk->icsk_probes_out >= keepalive_probes(tp)) {
> tcp_send_active_reset(sk, GFP_ATOMIC);
> tcp_write_err(sk);
> goto out;
^ permalink raw reply
* Re: [iproute PATCH v2 5/5] tipc/bearer: Prevent NULL pointer dereference
From: Stephen Hemminger @ 2017-08-18 16:24 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170817170931.24545-6-phil@nwl.cc>
On Thu, 17 Aug 2017 19:09:31 +0200
Phil Sutter <phil@nwl.cc> wrote:
> - opt = get_opt(opts, "media");
> - if (strcmp(opt->val, "udp") == 0) {
> + if ((opt = get_opt(opts, "media")) &&
> + strcmp(opt->val, "udp") == 0) {
Please don't merge assignment and comparison unless necessary for other reasons.
opt = get_opt(opts, "media");
if (opt && strcmp(opt->val, "udp") == 0) {
^ permalink raw reply
* Re: [iproute PATCH v2 3/5] tc/q_netem: Don't dereference possibly NULL pointer
From: Stephen Hemminger @ 2017-08-18 16:25 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170817170931.24545-4-phil@nwl.cc>
On Thu, 17 Aug 2017 19:09:29 +0200
Phil Sutter <phil@nwl.cc> wrote:
> @@ -546,6 +546,8 @@ static int netem_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
> if (opt == NULL)
> return 0;
>
> + len = RTA_PAYLOAD(opt) - sizeof(qopt);
> +
> if (len < 0) {
Dont add blank line between computation and conditional.
Having them together reads better.
^ permalink raw reply
* Re: [iproute PATCH v2 2/5] nstat: Fix for potential NULL pointer dereference
From: Stephen Hemminger @ 2017-08-18 16:28 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170817170931.24545-3-phil@nwl.cc>
On Thu, 17 Aug 2017 19:09:28 +0200
Phil Sutter <phil@nwl.cc> wrote:
> If the string at 'p' contains neither space not newline, 'p' will become
> NULL. Make sure this isn't the case before dereferencing it.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
Why not fix the parsing code instead. Other places here call abort().
^ permalink raw reply
* RE: [iproute PATCH v2 1/7] ipntable: Make sure filter.name is NULL-terminated
From: David Laight @ 2017-08-18 16:32 UTC (permalink / raw)
To: 'Phil Sutter'; +Cc: Stephen Hemminger, netdev@vger.kernel.org
In-Reply-To: <20170818105224.GG10864@orbyte.nwl.cc>
From: Phil Sutter
> Sent: 18 August 2017 11:52
> On Fri, Aug 18, 2017 at 09:19:16AM +0000, David Laight wrote:
> > From: Phil Sutter
> > > Sent: 17 August 2017 18:09
> > > To: Stephen Hemminger
> > > Cc: netdev@vger.kernel.org
> > > Subject: [iproute PATCH v2 1/7] ipntable: Make sure filter.name is NULL-terminated
> > >
> > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > ---
> > > ip/ipntable.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/ip/ipntable.c b/ip/ipntable.c
> > > index 879626ee4f491..7be1f04d33d90 100644
> > > --- a/ip/ipntable.c
> > > +++ b/ip/ipntable.c
> > > @@ -633,7 +633,8 @@ static int ipntable_show(int argc, char **argv)
> > > } else if (strcmp(*argv, "name") == 0) {
> > > NEXT_ARG();
> > >
> > > - strncpy(filter.name, *argv, sizeof(filter.name));
> > > + strncpy(filter.name, *argv, sizeof(filter.name) - 1);
> > > + filter.name[sizeof(filter.name) - 1] = '\0';
> >
> > Why not check for overflow instead?
> > if (filter.name[sizeof(filter.name) - 1])
> > usage("filer name too long");
>
> sizeof(filter.name) is 1024, which is maybe a bit over the top for
> something a user would input. So I found a better way avoiding all this
> at once: I made filter.name a const char *, then just assigned *argv to
> it. This should be safe since rtnl_dump_filter() and therefore
> print_ntable() callback is called from inside ipntable_show() so *argv
> is not accessed outside of it's scope.
>
> What do you think?
There isn't a scope problem, *argv is program data (written to unusable
stack space by the kernel during exec.
If the filter is done in userpace it is ok, but I'd have thought
it would be passed to kernel later on?
David
^ permalink raw reply
* Re: [iproute PATCH v2 7/7] lib/ll_map: Make sure im->name is NULL-terminated
From: Stephen Hemminger @ 2017-08-18 16:33 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170817170932.24659-8-phil@nwl.cc>
On Thu, 17 Aug 2017 19:09:32 +0200
Phil Sutter <phil@nwl.cc> wrote:
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> lib/ll_map.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/ll_map.c b/lib/ll_map.c
> index 4e4556c9ac80b..4d06eb69f138a 100644
> --- a/lib/ll_map.c
> +++ b/lib/ll_map.c
> @@ -120,11 +120,11 @@ int ll_remember_index(const struct sockaddr_nl *who,
> return 0;
> }
>
> - im = malloc(sizeof(*im));
> + im = calloc(1, sizeof(*im));
> if (im == NULL)
> return 0;
> im->index = ifi->ifi_index;
> - strcpy(im->name, ifname);
> + strncpy(im->name, ifname, IFNAMSIZ - 1);
> im->type = ifi->ifi_type;
> im->flags = ifi->ifi_flags;
>
This is not really necessary. kernel won't return
an ifname with a length >= IFNAMSIZ.
If you wanted to future proof it, why not use variable size allocation
--- a/lib/ll_map.c
+++ b/lib/ll_map.c
@@ -30,7 +30,7 @@ struct ll_cache {
unsigned flags;
unsigned index;
unsigned short type;
- char name[IFNAMSIZ];
+ char name[];
};
#define IDXMAP_SIZE 1024
@@ -120,7 +120,7 @@ int ll_remember_index(const struct sockaddr_nl *who,
return 0;
}
- im = malloc(sizeof(*im));
+ im = malloc(sizeof(*im) + strlen(ifname) + 1);
if (im == NULL)
return 0;
im->index = ifi->ifi_index;
^ permalink raw reply
* RE: [iproute PATCH v2 4/5] tc/tc_filter: Make sure filter name is not empty
From: David Laight @ 2017-08-18 16:34 UTC (permalink / raw)
To: 'Phil Sutter'; +Cc: Stephen Hemminger, netdev@vger.kernel.org
In-Reply-To: <20170818111613.GH10864@orbyte.nwl.cc>
From: Phil Sutter
> Sent: 18 August 2017 12:16
> On Fri, Aug 18, 2017 at 09:30:35AM +0000, David Laight wrote:
> > From: Phil Sutter
> > > Sent: 17 August 2017 18:10
> > > The later check for 'k[0] != 0' requires a non-empty filter name,
> > > otherwise NULL pointer dereference in 'q' might happen.
> > >
> > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > ---
> > > tc/tc_filter.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/tc/tc_filter.c b/tc/tc_filter.c
> > > index b13fb9185d4fd..a799edb35886d 100644
> > > --- a/tc/tc_filter.c
> > > +++ b/tc/tc_filter.c
> > > @@ -412,6 +412,9 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
> > > usage();
> > > return 0;
> > > } else {
> > > + if (!strlen(*argv))
> > > + invarg("invalid filter name", *argv);
> >
> > That is nearly as bad as:
> > p[strlen(p)] = 0;
>
> Hey, it's not impossible! I could call tc like so:
>
> | # tc filter get protocol ip prio 1 ""
You missed the point. Just check **argv there is no need to
determine the length just to check it is non-zero.
David
^ permalink raw reply
* Re: [iproute PATCH v2 4/7] lib/inet_proto: Make sure destination buffers are NULL-terminated
From: Stephen Hemminger @ 2017-08-18 16:37 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20170817170932.24659-5-phil@nwl.cc>
On Thu, 17 Aug 2017 19:09:29 +0200
Phil Sutter <phil@nwl.cc> wrote:
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> lib/inet_proto.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/lib/inet_proto.c b/lib/inet_proto.c
> index ceda082b12a2e..87ed4769fc3da 100644
> --- a/lib/inet_proto.c
> +++ b/lib/inet_proto.c
> @@ -35,8 +35,10 @@ const char *inet_proto_n2a(int proto, char *buf, int len)
> pe = getprotobynumber(proto);
> if (pe) {
> icache = proto;
> - strncpy(ncache, pe->p_name, 16);
> - strncpy(buf, pe->p_name, len);
> + strncpy(ncache, pe->p_name, 15);
> + ncache[15] = '\0';
> + strncpy(buf, pe->p_name, len - 1);
> + buf[len] = '\0';
> return buf;
> }
> snprintf(buf, len, "ipproto-%d", proto);
> @@ -62,7 +64,8 @@ int inet_proto_a2n(const char *buf)
> pe = getprotobyname(buf);
> if (pe) {
> icache = pe->p_proto;
> - strncpy(ncache, pe->p_name, 16);
> + strncpy(ncache, pe->p_name, 15);
> + ncache[15] = '\0';
> return pe->p_proto;
> }
> return -1;
Depending on proto name to be 15 characters or less is a silly
choice. Why not use strdup() and do it right?
^ permalink raw reply
* RE: [iproute PATCH v2 2/2] ifcfg: Quote left-hand side of [ ] expression
From: David Laight @ 2017-08-18 16:38 UTC (permalink / raw)
To: 'Phil Sutter'; +Cc: Stephen Hemminger, netdev@vger.kernel.org
In-Reply-To: <20170818112414.GJ10864@orbyte.nwl.cc>
From: Phil Sutter
> Sent: 18 August 2017 12:24
...
> > > -if [ `ip link ls $dev | grep -c MULTICAST` -ge 1 ]; then
> > > +if [ "`ip link ls $dev | grep -c MULTICAST`" -ge 1 ]; then
> >
> > You could drag all these scripts into the 1990's by using $(...)
> > instead of `...`.
>
> That's a different kettle of fish IMO, using $() doesn't change the
> situation this addresses:
>
> | $ [ $(echo foo bar) -eq 0 ] && echo ok
> | bash: [: too many arguments
> | $ [ "$(echo foo bar)" -eq 0 ] && echo ok
> | bash: [: foo bar: integer expression expected
I didn't say it would.
IFS=
set -o noglob
would though - not that I'd suggest it here.
protecting the 'eval' is somewhat harder.
David
^ permalink raw reply
* Re: [PATCH net v2] datagram: When peeking datagrams with offset < 0 don't skip empty skbs
From: Matthew Dawson @ 2017-08-18 16:39 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, Macieira, Thiago, willemdebruijn.kernel
In-Reply-To: <1503065118.3344.18.camel@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 6103 bytes --]
On Friday, August 18, 2017 10:05:18 AM EDT Paolo Abeni wrote:
> On Thu, 2017-08-17 at 22:11 -0400, Matthew Dawson wrote:
> > Due to commit e6afc8ace6dd5cef5e812f26c72579da8806f5ac ("udp: remove
> > headers from UDP packets before queueing"), when udp packets are being
> > peeked the requested extra offset is always 0 as there is no need to skip
> > the udp header. However, when the offset is 0 and the next skb is
> > of length 0, it is only returned once. The behaviour can be seen with
> > the following python script:
> >
> > from socket import *;
> > f=socket(AF_INET6, SOCK_DGRAM | SOCK_NONBLOCK, 0);
> > g=socket(AF_INET6, SOCK_DGRAM | SOCK_NONBLOCK, 0);
> > f.bind(('::', 0));
> > addr=('::1', f.getsockname()[1]);
> > g.sendto(b'', addr)
> > g.sendto(b'b', addr)
> > print(f.recvfrom(10, MSG_PEEK));
> > print(f.recvfrom(10, MSG_PEEK));
> >
> > Where the expected output should be the empty string twice.
> >
> > Instead, make sk_peek_offset return negative values, and pass those values
> > to __skb_try_recv_datagram/__skb_try_recv_from_queue. If the passed
> > offset
> > to __skb_try_recv_from_queue is negative, the checked skb is never
> > skipped.
> > __skb_try_recv_from_queue will then ensure the offset is reset back to 0
> > if a peek is requested without an offset, unless no packets are found.
> >
> > Also simplify the if condition in __skb_try_recv_from_queue. If _off is
> > greater then 0, and off is greater then or equal to skb->len, then
> > (_off || skb->len) must always be true assuming skb->len >= 0 is always
> > true.
> >
> > Also remove a redundant check around a call to sk_peek_offset in
> > af_unix.c,
> > as it double checked if MSG_PEEK was set in the flags.
> >
> > V2:
> > - Moved the negative fixup into __skb_try_recv_from_queue, and remove now
> >
> > redundant checks
> >
> > - Fix peeking in udp{,v6}_recvmsg to report the right value when the
> >
> > offset is 0
> >
> > Signed-off-by: Matthew Dawson <matthew@mjdsystems.ca>
> > ---
> >
> > include/net/sock.h | 4 +---
> > net/core/datagram.c | 12 +++++++++---
> > net/ipv4/udp.c | 3 ++-
> > net/ipv6/udp.c | 3 ++-
> > net/unix/af_unix.c | 5 +----
> > 5 files changed, 15 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index 7c0632c7e870..aeeec62992ca 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -507,9 +507,7 @@ int sk_set_peek_off(struct sock *sk, int val);
> >
> > static inline int sk_peek_offset(struct sock *sk, int flags)
> > {
> >
> > if (unlikely(flags & MSG_PEEK)) {
> >
> > - s32 off = READ_ONCE(sk->sk_peek_off);
> > - if (off >= 0)
> > - return off;
> > + return READ_ONCE(sk->sk_peek_off);
> >
> > }
> >
> > return 0;
> >
> > diff --git a/net/core/datagram.c b/net/core/datagram.c
> > index ee5647bd91b3..4b558503bef5 100644
> > --- a/net/core/datagram.c
> > +++ b/net/core/datagram.c
> > @@ -169,14 +169,20 @@ struct sk_buff *__skb_try_recv_from_queue(struct
> > sock *sk,>
> > int *peeked, int *off, int *err,
> > struct sk_buff **last)
> >
> > {
> >
> > + bool peek_at_off = false;
> >
> > struct sk_buff *skb;
> >
> > - int _off = *off;
> > + int _off = 0;
> > +
> > + if (flags & MSG_PEEK && *off >= 0) {
> > + peek_at_off = true;
> > + _off = *off;
> > + }
>
> I think that unlikely() will fit the above condition
Sounds good.
>
> > *last = queue->prev;
> > skb_queue_walk(queue, skb) {
> >
> > if (flags & MSG_PEEK) {
> >
> > - if (_off >= skb->len && (skb->len || _off ||
> > - skb->peeked)) {
> > + if (peek_at_off && _off >= skb->len &&
> > + (_off || skb->peeked)) {
> >
> > _off -= skb->len;
> > continue;
> >
> > }
> >
> > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > index a7c804f73990..cd1d044a7fa5 100644
> > --- a/net/ipv4/udp.c
> > +++ b/net/ipv4/udp.c
> > @@ -1574,7 +1574,8 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg,
> > size_t len, int noblock,>
> > return ip_recv_error(sk, msg, len, addr_len);
> >
> > try_again:
> > - peeking = off = sk_peek_offset(sk, flags);
> > + peeking = flags & MSG_PEEK;
> > + off = sk_peek_offset(sk, flags);
> >
> > skb = __skb_recv_udp(sk, flags, noblock, &peeked, &off, &err);
> > if (!skb)
> >
> > return err;
> >
> > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> > index 578142b7ca3e..20039c8501eb 100644
> > --- a/net/ipv6/udp.c
> > +++ b/net/ipv6/udp.c
> > @@ -362,7 +362,8 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg,
> > size_t len,>
> > return ipv6_recv_rxpmtu(sk, msg, len, addr_len);
> >
> > try_again:
> > - peeking = off = sk_peek_offset(sk, flags);
> > + peeking = flags & MSG_PEEK;
> > + off = sk_peek_offset(sk, flags);
> >
> > skb = __skb_recv_udp(sk, flags, noblock, &peeked, &off, &err);
> > if (!skb)
> >
> > return err;
> >
> > diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> > index 7b52a380d710..be8982b4f8c0 100644
> > --- a/net/unix/af_unix.c
> > +++ b/net/unix/af_unix.c
> > @@ -2304,10 +2304,7 @@ static int unix_stream_read_generic(struct
> > unix_stream_read_state *state,>
> > */
> >
> > mutex_lock(&u->iolock);
> >
> > - if (flags & MSG_PEEK)
> > - skip = sk_peek_offset(sk, flags);
> > - else
> > - skip = 0;
> > + skip = max(sk_peek_offset(sk, flags), 0);
> >
> > do {
> >
> > int chunk;
>
> later we have:
>
> chunk = min_t(unsigned int, unix_skb_len(skb) - skip, size);
>
> without any call to __skb_try_recv_from_queue(), so we will get
> bad/unexpected values from the above assignment when 'skip' is
> negative.
The assignment to skip should ensure it is never less then zero, thanks to the
max(sk...(), 0). Thus that shouldn't be an issue?
>
> Overall I still think that adding/using an explicit MSG_PEEK_OFF bit
> would produce a simpler code, but is just a personal preference.
I don't mind either way, that just seemed to be the preference I saw from the
discussion around the patch. I think either way will work, so whatever the
list prefers I'm happy with.
--
Matthew
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [iproute PATCH v2 1/7] ipntable: Make sure filter.name is NULL-terminated
From: Phil Sutter @ 2017-08-18 16:52 UTC (permalink / raw)
To: David Laight; +Cc: Stephen Hemminger, netdev@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD005ADE9@AcuExch.aculab.com>
On Fri, Aug 18, 2017 at 04:32:47PM +0000, David Laight wrote:
> From: Phil Sutter
> > Sent: 18 August 2017 11:52
> > On Fri, Aug 18, 2017 at 09:19:16AM +0000, David Laight wrote:
> > > From: Phil Sutter
> > > > Sent: 17 August 2017 18:09
> > > > To: Stephen Hemminger
> > > > Cc: netdev@vger.kernel.org
> > > > Subject: [iproute PATCH v2 1/7] ipntable: Make sure filter.name is NULL-terminated
> > > >
> > > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > > ---
> > > > ip/ipntable.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/ip/ipntable.c b/ip/ipntable.c
> > > > index 879626ee4f491..7be1f04d33d90 100644
> > > > --- a/ip/ipntable.c
> > > > +++ b/ip/ipntable.c
> > > > @@ -633,7 +633,8 @@ static int ipntable_show(int argc, char **argv)
> > > > } else if (strcmp(*argv, "name") == 0) {
> > > > NEXT_ARG();
> > > >
> > > > - strncpy(filter.name, *argv, sizeof(filter.name));
> > > > + strncpy(filter.name, *argv, sizeof(filter.name) - 1);
> > > > + filter.name[sizeof(filter.name) - 1] = '\0';
> > >
> > > Why not check for overflow instead?
> > > if (filter.name[sizeof(filter.name) - 1])
> > > usage("filer name too long");
> >
> > sizeof(filter.name) is 1024, which is maybe a bit over the top for
> > something a user would input. So I found a better way avoiding all this
> > at once: I made filter.name a const char *, then just assigned *argv to
> > it. This should be safe since rtnl_dump_filter() and therefore
> > print_ntable() callback is called from inside ipntable_show() so *argv
> > is not accessed outside of it's scope.
> >
> > What do you think?
>
> There isn't a scope problem, *argv is program data (written to unusable
> stack space by the kernel during exec.
Ah, thanks for the info!
> If the filter is done in userpace it is ok, but I'd have thought
> it would be passed to kernel later on?
No, it just calls sends RTM_GETNEIGHTBL to kernel and throws away
anything that doesn't match the filter. So filtering happens in
user space exclusively.
Thanks, Phil
^ permalink raw reply
* Re: [iproute PATCH v2 4/7] lib/inet_proto: Make sure destination buffers are NULL-terminated
From: Phil Sutter @ 2017-08-18 16:55 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170818093734.705f6742@xeon-e3>
On Fri, Aug 18, 2017 at 09:37:33AM -0700, Stephen Hemminger wrote:
> On Thu, 17 Aug 2017 19:09:29 +0200
> Phil Sutter <phil@nwl.cc> wrote:
>
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> > lib/inet_proto.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/inet_proto.c b/lib/inet_proto.c
> > index ceda082b12a2e..87ed4769fc3da 100644
> > --- a/lib/inet_proto.c
> > +++ b/lib/inet_proto.c
> > @@ -35,8 +35,10 @@ const char *inet_proto_n2a(int proto, char *buf, int len)
> > pe = getprotobynumber(proto);
> > if (pe) {
> > icache = proto;
> > - strncpy(ncache, pe->p_name, 16);
> > - strncpy(buf, pe->p_name, len);
> > + strncpy(ncache, pe->p_name, 15);
> > + ncache[15] = '\0';
> > + strncpy(buf, pe->p_name, len - 1);
> > + buf[len] = '\0';
> > return buf;
> > }
> > snprintf(buf, len, "ipproto-%d", proto);
> > @@ -62,7 +64,8 @@ int inet_proto_a2n(const char *buf)
> > pe = getprotobyname(buf);
> > if (pe) {
> > icache = pe->p_proto;
> > - strncpy(ncache, pe->p_name, 16);
> > + strncpy(ncache, pe->p_name, 15);
> > + ncache[15] = '\0';
> > return pe->p_proto;
> > }
> > return -1;
>
> Depending on proto name to be 15 characters or less is a silly
> choice. Why not use strdup() and do it right?
Yes, I was puzzled by that as well. Luckily the longest proto name in my
/etc/protocols is rsvp-e2e-ignore which is 15 chars long. I'll change
the patch to use dynamic allocation.
Thanks, Phil
^ permalink raw reply
* Re: [iproute PATCH v2 4/5] tc/tc_filter: Make sure filter name is not empty
From: Phil Sutter @ 2017-08-18 16:57 UTC (permalink / raw)
To: David Laight; +Cc: Stephen Hemminger, netdev@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD005AE0E@AcuExch.aculab.com>
On Fri, Aug 18, 2017 at 04:34:44PM +0000, David Laight wrote:
> From: Phil Sutter
> > Sent: 18 August 2017 12:16
> > On Fri, Aug 18, 2017 at 09:30:35AM +0000, David Laight wrote:
> > > From: Phil Sutter
> > > > Sent: 17 August 2017 18:10
> > > > The later check for 'k[0] != 0' requires a non-empty filter name,
> > > > otherwise NULL pointer dereference in 'q' might happen.
> > > >
> > > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > > ---
> > > > tc/tc_filter.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/tc/tc_filter.c b/tc/tc_filter.c
> > > > index b13fb9185d4fd..a799edb35886d 100644
> > > > --- a/tc/tc_filter.c
> > > > +++ b/tc/tc_filter.c
> > > > @@ -412,6 +412,9 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
> > > > usage();
> > > > return 0;
> > > > } else {
> > > > + if (!strlen(*argv))
> > > > + invarg("invalid filter name", *argv);
> > >
> > > That is nearly as bad as:
> > > p[strlen(p)] = 0;
> >
> > Hey, it's not impossible! I could call tc like so:
> >
> > | # tc filter get protocol ip prio 1 ""
>
> You missed the point. Just check **argv there is no need to
> determine the length just to check it is non-zero.
Oh, hehe. Thanks for the short-cut!
Thanks, PHil
^ permalink raw reply
* Re: [PATCH v3 4/4] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY
From: Chen-Yu Tsai @ 2017-08-18 16:57 UTC (permalink / raw)
To: Corentin Labbe
Cc: Rob Herring, Mark Rutland, Russell King, Maxime Ripard,
Chen-Yu Tsai, Giuseppe Cavallaro, Alexandre Torgue, devicetree,
linux-arm-kernel, linux-kernel, netdev
In-Reply-To: <20170818122118.4925-5-clabbe.montjoie@gmail.com>
On Fri, Aug 18, 2017 at 8:21 PM, Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
> This patch add documentation about the MDIO switch used on sun8i-h3-emac
> for integrated PHY.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> .../devicetree/bindings/net/dwmac-sun8i.txt | 112 +++++++++++++++++++--
> 1 file changed, 105 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> index 725f3b187886..631084122532 100644
> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> @@ -39,7 +39,7 @@ Optional properties for the following compatibles:
> - allwinner,leds-active-low: EPHY LEDs are active low
>
> Required child node of emac:
> -- mdio bus node: should be named mdio
> +- mdio bus node: should be named mdio (or mdio_parent in case of H3)
The node would still be named "mdio". You are confusing the node name
with the label. But you don't really need the mdio node for the H3.
You could say here that it is not needed for the H3. See below.
>
> Required properties of the mdio node:
> - #address-cells: shall be 1
> @@ -48,14 +48,21 @@ Required properties of the mdio node:
> The device node referenced by "phy" or "phy-handle" should be a child node
> of the mdio node. See phy.txt for the generic PHY bindings.
>
> -Required properties of the phy node with the following compatibles:
> +Required mdio-mux nodes for the following compatibles:
The following compatibles require an mdio-mux node:
> + - "allwinner,sun8i-h3-emac",
Drop the comma. It's already a list.
> +- a mdio-mux node with compatible = "allwinner,sun8i-h3-mdio-switch"
Required properties for the mdio-mux node:
- compatible: "allwinner,sun8i-h3-mdio-switch"
> + - mdio-parent-bus: The parent bus is "mdio_parent"
This is optional in the mdio-mux binding. Since you have the mdio-mux under
the EMAC node, it should be obvious that it is connected to the EMAC. The
parent phandle is more useful for muxes that are separate from the controller,
such as an external GPIO-controlled mux. So you could just drop it.
> + - two child mdio, one for the integrated mdio, one for the external mdio
> +
> +Required properties of the integrated phy node with the following compatibles:
The following compatibles require a PHY node representing the integrated
PHY, under the integrated MDIO bus node if an mdio-mux node is used:
> - "allwinner,sun8i-h3-emac",
> - "allwinner,sun8i-v3s-emac":
Required properties of the integrated PHY node:
> - clocks: a phandle to the reference clock for the EPHY
> - resets: a phandle to the reset control for the EPHY
> +- phy-is-integrated
> +- Should be a child of the integrated mdio
>
> -Example:
> -
> +Example with integrated PHY:
> emac: ethernet@1c0b000 {
> compatible = "allwinner,sun8i-h3-emac";
> syscon = <&syscon>;
> @@ -75,10 +82,101 @@ emac: ethernet@1c0b000 {
> mdio: mdio {
> #address-cells = <1>;
> #size-cells = <0>;
> - int_mii_phy: ethernet-phy@1 {
> + };
> + mdio-mux {
Drop the mdio node as mentioned above. And also have spaces between
blocks to be consistent in styling. It's also easier to read.
> + compatible = "allwinner,sun8i-h3-mdio-switch";
> + mdio-parent-bus = <&mdio>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + int_mdio: mdio@1 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
> + phy-is-integrated
> + };
> + };
> + ext_mdio: mdio@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +};
> +
> +Example with external PHY:
> +emac: ethernet@1c0b000 {
> + compatible = "allwinner,sun8i-h3-emac";
> + syscon = <&syscon>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "macirq";
> + resets = <&ccu RST_BUS_EMAC>;
> + reset-names = "stmmaceth";
> + clocks = <&ccu CLK_BUS_EMAC>;
> + clock-names = "stmmaceth";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy-handle = <&ext_rgmii_phy>;
> + phy-mode = "rgmii";
> + allwinner,leds-active-low;
You don't need this when using the external PHY.
Regards
ChenYu
> + mdio: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + mdio-mux {
> + compatible = "allwinner,sun8i-h3-mdio-switch";
> + mdio-parent-bus = <&mdio>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + int_mdio: mdio@1 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
> + phy-is-integrated
> + };
> + };
> + ext_mdio: mdio@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ext_rgmii_phy: ethernet-phy@1 {
> + reg = <1>;
> + };
> + };
> +};
> +
> +Example with SoC without integrated PHY
> +
> +emac: ethernet@1c0b000 {
> + compatible = "allwinner,sun8i-a83t-emac";
> + syscon = <&syscon>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "macirq";
> + resets = <&ccu RST_BUS_EMAC>;
> + reset-names = "stmmaceth";
> + clocks = <&ccu CLK_BUS_EMAC>;
> + clock-names = "stmmaceth";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy-handle = <&ext_rgmii_phy>;
> + phy-mode = "rgmii";
> + mdio: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ext_rgmii_phy: ethernet-phy@1 {
> reg = <1>;
> - clocks = <&ccu CLK_BUS_EPHY>;
> - resets = <&ccu RST_BUS_EPHY>;
> };
> };
> };
> --
> 2.13.0
>
^ permalink raw reply
* [PATCH V2 net-next] net: hns3: Add support to change MTU in HNS3 hardware
From: Salil Mehta @ 2017-08-18 16:57 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil.lnk, netdev,
linux-kernel, linux-rdma, linuxarm
This patch adds the following support to the HNS3 driver:
1. Support to change the Maximum Transmission Unit of a
of a port in the HNS NIC hardware .
2. Initializes the supported MTU range for the netdevice.
Signed-off-by: lipeng <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
PATCH V2: Addresses comments given by Andrew Lunn
1. https://lkml.org/lkml/2017/8/18/282
PATCH V1: Initial Submit
---
.../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 38 ++++++++++++++++++++++
.../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h | 1 +
2 files changed, 39 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
index e731f87..d905ea1 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
@@ -1278,11 +1278,46 @@ static int hns3_ndo_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan,
return ret;
}
+static int hns3_nic_change_mtu(struct net_device *netdev, int new_mtu)
+{
+ struct hns3_nic_priv *priv = netdev_priv(netdev);
+ struct hnae3_handle *h = priv->ae_handle;
+ bool if_running = netif_running(netdev);
+ int ret;
+
+ if (!h->ae_algo->ops->set_mtu)
+ return -ENOTSUPP;
+
+ /* if this was called with netdev up then bring netdevice down */
+ if (if_running) {
+ (void)hns3_nic_net_stop(netdev);
+ msleep(100);
+ }
+
+ ret = h->ae_algo->ops->set_mtu(h, new_mtu);
+ if (ret) {
+ netdev_err(netdev, "failed to change MTU in hardware %d\n",
+ ret);
+ return ret;
+ }
+
+ /* if the netdev was running earlier, bring it up again */
+ if (if_running) {
+ if (hns3_nic_net_open(netdev)) {
+ netdev_err(netdev, "MTU, couldnt up netdev again\n");
+ ret = -EINVAL;
+ }
+ }
+
+ return ret;
+}
+
static const struct net_device_ops hns3_nic_netdev_ops = {
.ndo_open = hns3_nic_net_open,
.ndo_stop = hns3_nic_net_stop,
.ndo_start_xmit = hns3_nic_net_xmit,
.ndo_set_mac_address = hns3_nic_net_set_mac_address,
+ .ndo_change_mtu = hns3_nic_change_mtu,
.ndo_set_features = hns3_nic_set_features,
.ndo_get_stats64 = hns3_nic_get_stats64,
.ndo_setup_tc = hns3_nic_setup_tc,
@@ -2752,6 +2787,9 @@ static int hns3_client_init(struct hnae3_handle *handle)
goto out_reg_netdev_fail;
}
+ /* MTU range: (ETH_MIN_MTU(kernel default) - 9706) */
+ netdev->max_mtu = HNS3_MAX_MTU - (ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN);
+
return ret;
out_reg_netdev_fail:
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
index a6e8f15..7e87461 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
@@ -76,6 +76,7 @@ enum hns3_nic_state {
#define HNS3_RING_NAME_LEN 16
#define HNS3_BUFFER_SIZE_2048 2048
#define HNS3_RING_MAX_PENDING 32768
+#define HNS3_MAX_MTU 9728
#define HNS3_BD_SIZE_512_TYPE 0
#define HNS3_BD_SIZE_1024_TYPE 1
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3 2/4] net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated
From: Chen-Yu Tsai @ 2017-08-18 16:58 UTC (permalink / raw)
To: Corentin Labbe
Cc: Rob Herring, Mark Rutland, Russell King, Maxime Ripard,
Chen-Yu Tsai, Giuseppe Cavallaro, Alexandre Torgue, devicetree,
linux-arm-kernel, linux-kernel, netdev
In-Reply-To: <20170818122118.4925-3-clabbe.montjoie@gmail.com>
On Fri, Aug 18, 2017 at 8:21 PM, Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
> The current way to find if the phy is internal is to compare DT phy-mode
> and emac_variant/internal_phy.
> But it will negate a possible future SoC where an external PHY use the
> same phy mode than the internal one.
>
> This patch adds a new way to find if the PHY is internal, via
> the phy-is-integrated property.
>
> Since the internal_phy variable does not need anymore to contain the xMII mode
> used by the internal PHY, it is still used for knowing the presence of an
> internal PHY, so it is modified to a boolean soc_has_internal_phy.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Acked-by: Chen-Yu Tsai <wens@csie.org>
^ permalink raw reply
* Re: [patch net] net: sched: fix p_filter_chain check in tcf_chain_flush
From: Cong Wang @ 2017-08-18 16:59 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
mlxsw
In-Reply-To: <20170818081043.2130-1-jiri@resnulli.us>
On Fri, Aug 18, 2017 at 1:10 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> The dereference before check is wrong and leads to an oops when
> p_filter_chain is NULL. The check needs to be done on the pointer to
> prevent NULL dereference.
>
> Fixes: f93e1cdcf42c ("net/sched: fix filter flushing")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* RE: [PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev
From: Salil Mehta @ 2017-08-18 17:01 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem@davemloft.net, Zhuangyuzeng (Yisen), lipeng (Y),
mehta.salil.lnk@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
Linuxarm
In-Reply-To: <20170818160157.GB3258@lunn.ch>
Hi Andrew,
> > -----Original Message-----
> > From: Andrew Lunn [mailto:andrew@lunn.ch]
> > Sent: Friday, August 18, 2017 5:02 PM
> > To: Salil Mehta
> > Cc: davem@davemloft.net; Zhuangyuzeng (Yisen); lipeng (Y);
> > mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> > Subject: Re: [PATCH net-next] net: hns3: Add support to change MTU in
> > hardware & netdev
> >
> > > for example,
> > > file: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> > > static int init_one(struct pci_dev *pdev, const struct
> pci_device_id
> > *ent)
> > >
> > > netdev->priv_flags |= IFF_UNICAST_FLT;
> > >
> > > + /* MTU range: 81 - 9600 */
> > > + netdev->min_mtu = 81;
> > > + netdev->max_mtu = MAX_MTU;
> >
> > In this cause, the driver is not using the default values. So it sets
> > them.
> >
> > Anyway, try it. After your alloc_etherdev_mqs(), print the value of
> > min_mtu. It should already be set to MIN_ETH_MTU.
> I understand your point. In this case, I would like to keep the
> range being set by the driver just to be more explicit.
> So for now keep this initialization in the driver?
Removed the min_mtu initialization from the driver code and added a
comment over it to be explicit. Please have a look at the V2 patch.
Thanks
Salil
^ permalink raw reply
* Re: [PATCH v3 3/4] net: stmmac: register parent MDIO node for sun8i-h3-emac
From: Chen-Yu Tsai @ 2017-08-18 17:05 UTC (permalink / raw)
To: Corentin Labbe
Cc: Rob Herring, Mark Rutland, Russell King, Maxime Ripard,
Chen-Yu Tsai, Giuseppe Cavallaro, Alexandre Torgue, devicetree,
linux-arm-kernel, linux-kernel, netdev
In-Reply-To: <20170818122118.4925-4-clabbe.montjoie@gmail.com>
On Fri, Aug 18, 2017 at 8:21 PM, Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
> In case of a MDIO switch, the registered MDIO node should be
> the parent of the PHY. Otherwise of_phy_connect will fail.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index a366b3747eeb..ca3cc99d8960 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -312,10 +312,12 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
> static const struct of_device_id need_mdio_ids[] = {
> { .compatible = "snps,dwc-qos-ethernet-4.10" },
> { .compatible = "allwinner,sun8i-a83t-emac" },
> - { .compatible = "allwinner,sun8i-h3-emac" },
> { .compatible = "allwinner,sun8i-v3s-emac" },
> { .compatible = "allwinner,sun50i-a64-emac" },
> };
> + static const struct of_device_id need_mdio_mux_ids[] = {
> + { .compatible = "allwinner,sun8i-h3-emac" },
> + };
>
> /* If phy-handle property is passed from DT, use it as the PHY */
> plat->phy_node = of_parse_phandle(np, "phy-handle", 0);
> @@ -332,7 +334,13 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
> mdio = false;
> }
>
> - if (of_match_node(need_mdio_ids, np)) {
> + /*
> + * In case of a MDIO switch/mux, the registered MDIO node should be
> + * the parent of the PHY. Otherwise of_phy_connect will fail.
> + */
> + if (of_match_node(need_mdio_mux_ids, np)) {
> + plat->mdio_node = of_get_parent(plat->phy_node);
Extra space before of_get_parent.
Also this is going to fail horribly if a fixed link is used.
ChenYu
> + } else if (of_match_node(need_mdio_ids, np)) {
> plat->mdio_node = of_get_child_by_name(np, "mdio");
> } else {
> /**
> --
> 2.13.0
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox