Netdev List
 help / color / mirror / Atom feed
* [PATCH 00/11] Add support for software nodes to gpiolib
From: Dmitry Torokhov @ 2019-09-11  7:52 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andy Shevchenko, Mika Westerberg, linux-kernel, linux-gpio,
	Andrew Lunn, Andrzej Hajda, Bartosz Golaszewski, Daniel Vetter,
	David Airlie, David S. Miller, Florian Fainelli, Heiner Kallweit,
	Jernej Skrabec, Jonas Karlman, Laurent Pinchart, Neil Armstrong,
	Russell King, dri-devel, linux-acpi, netdev

This series attempts to add support for software nodes to gpiolib, using
software node references that were introduced recently. This allows us
to convert more drivers to the generic device properties and drop
support for custom platform data:

static const struct software_node gpio_bank_b_node = {
|-------.name = "B",
};

static const struct property_entry simone_key_enter_props[] = {
|-------PROPERTY_ENTRY_U32("linux,code", KEY_ENTER),
|-------PROPERTY_ENTRY_STRING("label", "enter"),
|-------PROPERTY_ENTRY_REF("gpios", &gpio_bank_b_node, 123, GPIO_ACTIVE_LOW),
|-------{ }
};

If we agree in principle, I would like to have the very first 3 patches
in an immutable branch off maybe -rc8 so that it can be pulled into
individual subsystems so that patches switching various drivers to
fwnode_gpiod_get_index() could be applied.

Thanks,
Dmitry

Dmitry Torokhov (11):
  gpiolib: of: add a fallback for wlf,reset GPIO name
  gpiolib: introduce devm_fwnode_gpiod_get_index()
  gpiolib: introduce fwnode_gpiod_get_index()
  net: phylink: switch to using fwnode_gpiod_get_index()
  net: mdio: switch to using fwnode_gpiod_get_index()
  drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()
  gpliolib: make fwnode_get_named_gpiod() static
  gpiolib: of: tease apart of_find_gpio()
  gpiolib: of: tease apart acpi_find_gpio()
  gpiolib: consolidate fwnode GPIO lookups
  gpiolib: add support for software nodes

 drivers/gpio/Makefile              |   1 +
 drivers/gpio/gpiolib-acpi.c        | 153 ++++++++++++++----------
 drivers/gpio/gpiolib-acpi.h        |  21 ++--
 drivers/gpio/gpiolib-devres.c      |  33 ++----
 drivers/gpio/gpiolib-of.c          | 159 ++++++++++++++-----------
 drivers/gpio/gpiolib-of.h          |  26 ++--
 drivers/gpio/gpiolib-swnode.c      |  92 +++++++++++++++
 drivers/gpio/gpiolib-swnode.h      |  13 ++
 drivers/gpio/gpiolib.c             | 184 ++++++++++++++++-------------
 drivers/gpu/drm/bridge/ti-tfp410.c |   4 +-
 drivers/net/phy/mdio_bus.c         |   4 +-
 drivers/net/phy/phylink.c          |   4 +-
 include/linux/gpio/consumer.h      |  53 ++++++---
 13 files changed, 471 insertions(+), 276 deletions(-)
 create mode 100644 drivers/gpio/gpiolib-swnode.c
 create mode 100644 drivers/gpio/gpiolib-swnode.h

-- 
2.23.0.162.g0b9fbb3734-goog


^ permalink raw reply

* Re: [PATCH] net: Remove the source address setting in connect() for UDP
From: David Miller @ 2019-09-11  7:57 UTC (permalink / raw)
  To: enkechen; +Cc: kuznet, yoshfuji, netdev, linux-kernel, xe-linux-external
In-Reply-To: <324B00C3-4526-4026-809B-299634E49368@cisco.com>

From: "Enke Chen (enkechen)" <enkechen@cisco.com>
Date: Tue, 10 Sep 2019 23:55:59 +0000

> Do you still have concerns about backward compatibility of the fix?

I'm not convinced by your arguments and I am also completely swamped at
LPC2019 running the networking track this week.

^ permalink raw reply

* Re: [PATCH V2 net-next 0/7] net: hns3: add a feature & bugfixes & cleanups
From: David Miller @ 2019-09-11  8:09 UTC (permalink / raw)
  To: tanhuazhong
  Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
	jakub.kicinski
In-Reply-To: <1568169639-43658-1-git-send-email-tanhuazhong@huawei.com>

From: Huazhong Tan <tanhuazhong@huawei.com>
Date: Wed, 11 Sep 2019 10:40:32 +0800

> This patch-set includes a VF feature, bugfixes and cleanups for the HNS3
> ethernet controller driver.

Series applied.

^ permalink raw reply

* Re: [PATCH] wimax: i2400: fix memory leak
From: David Miller @ 2019-09-11  8:10 UTC (permalink / raw)
  To: navid.emamdoost
  Cc: emamd001, smccaman, kjlu, inaky.perez-gonzalez, linux-wimax,
	netdev, linux-kernel
In-Reply-To: <20190910230210.19012-1-navid.emamdoost@gmail.com>

From: Navid Emamdoost <navid.emamdoost@gmail.com>
Date: Tue, 10 Sep 2019 18:01:40 -0500

> In i2400m_op_rfkill_sw_toggle cmd buffer should be released along with
> skb response.
> 
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>

Applied.

Good thing nobody uses wimax.

^ permalink raw reply

* [PATCH iproute2-next] rdma: Check comm string before print in print_comm()
From: Leon Romanovsky @ 2019-09-11  8:12 UTC (permalink / raw)
  To: David Ahern
  Cc: Mark Zhang, netdev, RDMA mailing list, Stephen Hemminger,
	Leon Romanovsky

From: Mark Zhang <markz@mellanox.com>

Broken kernels (not-upstream) can provide wrong empty "comm" field.
It causes to segfault while printing in JSON format.

Fixes: 8ecac46a60ff ("rdma: Add QP resource tracking information")
Signed-off-by: Mark Zhang <markz@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 rdma/res.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/rdma/res.c b/rdma/res.c
index 97a7b964..6003006e 100644
--- a/rdma/res.c
+++ b/rdma/res.c
@@ -161,6 +161,9 @@ void print_comm(struct rd *rd, const char *str, struct nlattr **nla_line)
 {
 	char tmp[18];
 
+	if (!str)
+		return;
+
 	if (rd->json_output) {
 		/* Don't beatify output in JSON format */
 		jsonw_string_field(rd->jw, "comm", str);
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] net: qrtr: fix memort leak in qrtr_tun_write_iter
From: David Miller @ 2019-09-11  8:13 UTC (permalink / raw)
  To: navid.emamdoost; +Cc: emamd001, smccaman, kjlu, netdev, linux-kernel
In-Reply-To: <20190911003748.26841-1-navid.emamdoost@gmail.com>

From: Navid Emamdoost <navid.emamdoost@gmail.com>
Date: Tue, 10 Sep 2019 19:37:45 -0500

> In qrtr_tun_write_iter the allocated kbuf should be release in case of
> error happening.
> 
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>

Shouldn't it also be freed in case of success too?

^ permalink raw reply

* Re: [PATCH v2 net] net: sonic: replace dev_kfree_skb in sonic_send_packet
From: David Miller @ 2019-09-11  8:14 UTC (permalink / raw)
  To: maowenan; +Cc: tsbogend, netdev, linux-kernel, kernel-janitors
In-Reply-To: <20190911013623.36897-1-maowenan@huawei.com>

From: Mao Wenan <maowenan@huawei.com>
Date: Wed, 11 Sep 2019 09:36:23 +0800

> sonic_send_packet will be processed in irq or non-irq 
> context, so it would better use dev_kfree_skb_any
> instead of dev_kfree_skb.
> 
> Fixes: d9fb9f384292 ("*sonic/natsemi/ns83829: Move the National Semi-conductor drivers")
> Signed-off-by: Mao Wenan <maowenan@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 5/5] sctp: add spt_pathcpthld in struct sctp_paddrthlds
From: Xin Long @ 2019-09-11  8:14 UTC (permalink / raw)
  To: David Miller
  Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, Neil Horman
In-Reply-To: <20190910.192755.717621354475214603.davem@davemloft.net>

On Wed, Sep 11, 2019 at 1:27 AM David Miller <davem@davemloft.net> wrote:
>
> From: Xin Long <lucien.xin@gmail.com>
> Date: Mon,  9 Sep 2019 15:56:51 +0800
>
> > diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> > index a15cc28..dfd81e1 100644
> > --- a/include/uapi/linux/sctp.h
> > +++ b/include/uapi/linux/sctp.h
> > @@ -1069,6 +1069,7 @@ struct sctp_paddrthlds {
> >       struct sockaddr_storage spt_address;
> >       __u16 spt_pathmaxrxt;
> >       __u16 spt_pathpfthld;
> > +     __u16 spt_pathcpthld;
> >  };
> >
> >  /*
>
> As pointed out you can't add things to this structure without breaking existing
> binaries.
we had the same problem when adding:
spp_ipv6_flowlabel and spp_dscp into struct sctp_paddrparams. in:

commit 0b0dce7a36fb9f1a9dd8245ea82d3a268c6943fe
Author: Xin Long <lucien.xin@gmail.com>
Date:   Mon Jul 2 18:21:13 2018 +0800

    sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams

the solution was:

        if (optlen == sizeof(params)) {
                if (copy_from_user(&params, optval, optlen))
                        return -EFAULT;
        } else if (optlen == ALIGN(offsetof(struct sctp_paddrparams,
                                            spp_ipv6_flowlabel), 4)) {
                if (copy_from_user(&params, optval, optlen))
                        return -EFAULT;
                if (params.spp_flags & (SPP_DSCP | SPP_IPV6_FLOWLABEL))
                        return -EINVAL;
        } else {
                return -EINVAL;
        }

I will do the same for this patch. Thanks.

^ permalink raw reply

* Re: [PATCH bpf-next v10 2/4] bpf: new helper to obtain namespace data from current task New bpf helper bpf_get_current_pidns_info.
From: Eric W. Biederman @ 2019-09-11  8:16 UTC (permalink / raw)
  To: Al Viro
  Cc: Yonghong Song, Carlos Antonio Neira Bustos,
	netdev@vger.kernel.org, brouer@redhat.com, bpf@vger.kernel.org
In-Reply-To: <20190910231506.GL1131@ZenIV.linux.org.uk>

Al Viro <viro@zeniv.linux.org.uk> writes:

> On Tue, Sep 10, 2019 at 10:35:09PM +0000, Yonghong Song wrote:
>> 
>> Carlos,
>> 
>> Discussed with Eric today for what is the best way to get
>> the device number for a namespace. The following patch seems
>> a reasonable start although Eric would like to see
>> how the helper is used in order to decide whether the
>> interface looks right.
>> 
>> commit bb00fc36d5d263047a8bceb3e51e969d7fbce7db (HEAD -> fs2)
>> Author: Yonghong Song <yhs@fb.com>
>> Date:   Mon Sep 9 21:50:51 2019 -0700
>> 
>>      nsfs: add an interface function ns_get_inum_dev()
>> 
>>      This patch added an interface function
>>      ns_get_inum_dev(). Given a ns_common structure,
>>      the function returns the inode and device
>>      numbers. The function will be used later
>>      by a newly added bpf helper.
>> 
>>      Signed-off-by: Yonghong Song <yhs@fb.com>
>> 
>> diff --git a/fs/nsfs.c b/fs/nsfs.c
>> index a0431642c6b5..a603c6fc3f54 100644
>> --- a/fs/nsfs.c
>> +++ b/fs/nsfs.c
>> @@ -245,6 +245,14 @@ struct file *proc_ns_fget(int fd)
>>          return ERR_PTR(-EINVAL);
>>   }
>> 
>> +/* Get the device number for the current task pidns.
>> + */
>> +void ns_get_inum_dev(struct ns_common *ns, u32 *inum, dev_t *dev)
>> +{
>> +       *inum = ns->inum;
>> +       *dev = nsfs_mnt->mnt_sb->s_dev;
>> +}
>
> Umm...  Where would it get the device number once we get (hell knows
> what for) multiple nsfs instances?  I still don't understand what
> would that be about, TBH...  Is it really per-userns?  Or something
> else entirely?  Eric, could you give some context?

My goal is not to paint things into a corner, with future changes.
Right now it is possible to stat a namespace file descriptor and
get a device and inode number.  Then compare that. 

I don't want people using the inode number in nsfd as some magic
namespace id.

We have had times in the past where there was more than one superblock
and thus more than one device number.  Further if userspace ever uses
this heavily there may be times in the future where for
checkpoint/restart purposes we will want multiple nsfd's so we can
preserve the inode number accross a migration.

Realistically there will probably just some kind of hotplug notification
to userspace to say we have hotplugged your operatining system as
a migration notification.

Now the halway discussion did not quite capture everything I was trying
to say but it at least got to the right ballpark.

The helper in fs/nsfs.c should be:

bool ns_match(const struct ns_common *ns, dev_t dev, ino_t ino)
{
        return ((ns->inum == ino) && (nsfs_mnt->mnt_sb->s_dev == dev));
}

That way if/when there are multiple inodes identifying the same
namespace the bpf programs don't need to change.

Up farther in the stack it should be something like:

> BPF_CALL_2(bpf_current_pidns_match, dev_t *dev, ino_t *ino)
> {
>         return ns_match(&task_active_pid_ns(current)->ns, *dev, *ino);
> }
> 
> const struct bpf_func_proto bpf_current_pidns_match_proto = {
> 	.func		= bpf_current_pins_match,
> 	.gpl_only	= true,
> 	.ret_type	= RET_INTEGER
> 	.arg1_type	= ARG_PTR_TO_DEVICE_NUMBER,
> 	.arg2_type	= ARG_PTR_TO_INODE_NUMBER,
> };

That allows comparing what the bpf came up with with whatever value
userspace generated by stating the file descriptor.


That is the least bad suggestion I currently have for that
functionality.  It really would be better to not have that filter in the
bpf program itself but in the infrastructure that binds a program to a
set of tasks.

The problem with this approach is whatever device/inode you have when
the namespace they refer to exits there is the possibility that the
inode will be reused.  So your filter will eventually start matching on
the wrong thing.

Eric

^ permalink raw reply

* Re: [PATCH v7 5/7] nfc: pn533: add UART phy driver
From: David Miller @ 2019-09-11  8:17 UTC (permalink / raw)
  To: poeschel
  Cc: gregkh, tglx, kstewart, swinslow, allison, linux-kernel, netdev,
	johan, Claudiu.Beznea
In-Reply-To: <20190910093359.2110-1-poeschel@lemonage.de>

From: Lars Poeschel <poeschel@lemonage.de>
Date: Tue, 10 Sep 2019 11:33:50 +0200

> +static int pn532_uart_send_ack(struct pn533 *dev, gfp_t flags)
> +{
> +	struct pn532_uart_phy *pn532 = dev->phy;
> +	/* spec 7.1.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
> +	static const u8 ack[PN533_STD_FRAME_ACK_SIZE] = {
> +			0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
> +	int err;

Reverse christmas tree ordering for the local variables please.

> +static int pn532_uart_rx_is_frame(struct sk_buff *skb)
> +{
> +	int i;
> +	u16 frame_len;
> +	struct pn533_std_frame *std;
> +	struct pn533_ext_frame *ext;

Likewise.

^ permalink raw reply

* Re: [PATCH bpf-next v10 2/4] bpf: new helper to obtain namespace data from current task New bpf helper bpf_get_current_pidns_info.
From: Eric W. Biederman @ 2019-09-11  8:17 UTC (permalink / raw)
  To: Carlos Antonio Neira Bustos
  Cc: Yonghong Song, Al Viro, netdev@vger.kernel.org, brouer@redhat.com,
	bpf@vger.kernel.org
In-Reply-To: <20190911043225.GA22183@frodo.byteswizards.com>

Carlos Antonio Neira Bustos <cneirabustos@gmail.com> writes:

> On Tue, Sep 10, 2019 at 10:35:09PM +0000, Yonghong Song wrote:
> Thanks a lot Yonghong.
> I'll include this patch when submitting changes for version 11 of
> this patch.

Please see my reply to Al.

Eric



^ permalink raw reply

* Re: [PATCH v7 6/7] nfc: pn533: Add autopoll capability
From: David Miller @ 2019-09-11  8:17 UTC (permalink / raw)
  To: poeschel
  Cc: allison, keescook, opensource, swinslow, gregkh, gustavo, tglx,
	kstewart, netdev, linux-kernel, johan
In-Reply-To: <20190910093415.2186-1-poeschel@lemonage.de>

From: Lars Poeschel <poeschel@lemonage.de>
Date: Tue, 10 Sep 2019 11:34:12 +0200

> +static int pn533_autopoll_complete(struct pn533 *dev, void *arg,
> +			       struct sk_buff *resp)
> +{
> +	u8 nbtg;
> +	int rc;
> +	struct pn532_autopoll_resp *apr;
> +	struct nfc_target nfc_tgt;

Need reverse christmas tree here.

> @@ -1534,6 +1655,7 @@ static int pn533_start_poll(struct nfc_dev *nfc_dev,
>  	struct pn533_poll_modulations *cur_mod;
>  	u8 rand_mod;
>  	int rc;
> +	struct sk_buff *skb;

Likewise.

^ permalink raw reply

* Re: [PATCH net-next 0/6] net: stmmac: Improvements for -next
From: David Miller @ 2019-09-11  8:21 UTC (permalink / raw)
  To: Jose.Abreu
  Cc: netdev, Joao.Pinto, peppe.cavallaro, alexandre.torgue,
	mcoquelin.stm32, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1568126224.git.joabreu@synopsys.com>

From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Tue, 10 Sep 2019 16:41:21 +0200

> Misc patches for -next. It includes:
>  - Two fixes for features in -next only
>  - New features support for GMAC cores (which includes GMAC4 and GMAC5)

Series applied, but what exactly does "ARP offload" even do?

^ permalink raw reply

* Re: [net-next 11/14] ixgbe: Prevent u8 wrapping of ITR value to something less than 10us
From: David Miller @ 2019-09-11  8:24 UTC (permalink / raw)
  To: jeffrey.t.kirsher
  Cc: alexander.h.duyck, netdev, nhorman, sassmann, gleventhal,
	andrewx.bowers
In-Reply-To: <20190910163434.2449-12-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 10 Sep 2019 09:34:31 -0700

> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> There were a couple cases where the ITR value generated via the adaptive
> ITR scheme could exceed 126. This resulted in the value becoming either 0
> or something less than 10. Switching back and forth between a value less
> than 10 and a value greater than 10 can cause issues as certain hardware
> features such as RSC to not function well when the ITR value has dropped
> that low.
> 
> Reported-by: Gregg Leventhal <gleventhal@janestreet.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Please remove this from the series, add the Fixes: tag, and submit for 'net'
since Alexander says it is -stable material.

Thanks.

^ permalink raw reply

* Re: [pull request][net-next 0/3] Mellanox, mlx5 build cleanup 2019-09-10
From: David Miller @ 2019-09-11  8:27 UTC (permalink / raw)
  To: saeedm; +Cc: netdev
In-Reply-To: <20190910214542.8433-1-saeedm@mellanox.com>

From: Saeed Mahameed <saeedm@mellanox.com>
Date: Tue, 10 Sep 2019 21:45:57 +0000

> This series provides three build warnings cleanup patches for mlx5,
> Originally i wanted to wait a bit more and attach more patches to this
> series, but apparently this can't wait since already 3 different patches
> for the same fix were submitted this week :).
> 
> For more information please see tag log below.
> 
> Please pull and let me know if there is any problem.

Pulled, thanks.

^ permalink raw reply

* Re: [PATCH] lib/Kconfig: fix OBJAGG in lib/ menu structure
From: David Miller @ 2019-09-11  8:30 UTC (permalink / raw)
  To: rdunlap; +Cc: netdev, linux-kernel, jiri, idosch
In-Reply-To: <34674398-54dc-a4d1-6052-67ad1a3b2fe9@infradead.org>

From: Randy Dunlap <rdunlap@infradead.org>
Date: Mon, 9 Sep 2019 14:54:21 -0700

> From: Randy Dunlap <rdunlap@infradead.org>
> 
> Keep the "Library routines" menu intact by moving OBJAGG into it.
> Otherwise OBJAGG is displayed/presented as an orphan in the
> various config menus.
> 
> Fixes: 0a020d416d0a ("lib: introduce initial implementation of object aggregation manager")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>

Applied, thanks Randy.

^ permalink raw reply

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
From: Dan Carpenter @ 2019-09-11  8:30 UTC (permalink / raw)
  To: maowenan
  Cc: vyasevich, nhorman, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors
In-Reply-To: <53556c87-a351-4314-cbd9-49a39d0b41aa@huawei.com>

On Wed, Sep 11, 2019 at 09:30:47AM +0800, maowenan wrote:
> 
> 
> On 2019/9/11 3:22, Dan Carpenter wrote:
> > On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
> >> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> >>> There are more parentheses in if clause when call sctp_get_port_local
> >>> in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> >>> do cleanup.
> >>>
> >>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >>> ---
> >>>  net/sctp/socket.c | 3 +--
> >>>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> >>> index 9d1f83b10c0a..766b68b55ebe 100644
> >>> --- a/net/sctp/socket.c
> >>> +++ b/net/sctp/socket.c
> >>> @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
> >>>  	 * detection.
> >>>  	 */
> >>>  	addr->v4.sin_port = htons(snum);
> >>> -	if ((ret = sctp_get_port_local(sk, addr))) {
> >>> +	if (sctp_get_port_local(sk, addr))
> >>>  		return -EADDRINUSE;
> >>
> >> sctp_get_port_local() returns a long which is either 0,1 or a pointer
> >> casted to long.  It's not documented what it means and neither of the
> >> callers use the return since commit 62208f12451f ("net: sctp: simplify
> >> sctp_get_port").
> > 
> > Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
> > socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
> > because before the code assumed that a pointer casted to an int was the
> > same as a pointer casted to a long.
> 
> commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
> cleanup is ok?

Yeah.  It's fine, I was just confused why we weren't preserving the
error code and then I saw that we didn't return errors at all and got
confused.

regards,
dan carpenter


^ permalink raw reply

* Re: [PATCH net-next 5/5] sctp: add spt_pathcpthld in struct sctp_paddrthlds
From: Xin Long @ 2019-09-11  8:51 UTC (permalink / raw)
  To: David Laight
  Cc: network dev, linux-sctp@vger.kernel.org, Marcelo Ricardo Leitner,
	Neil Horman, davem@davemloft.net
In-Reply-To: <9fc7ca1598e641cda3914840a4416aab@AcuMS.aculab.com>

On Tue, Sep 10, 2019 at 9:19 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Xin Long
> > Sent: 09 September 2019 08:57
> > Section 7.2 of rfc7829: "Peer Address Thresholds (SCTP_PEER_ADDR_THLDS)
> > Socket Option" extends 'struct sctp_paddrthlds' with 'spt_pathcpthld'
> > added to allow a user to change ps_retrans per sock/asoc/transport, as
> > other 2 paddrthlds: pf_retrans, pathmaxrxt.
> >
> > Note that ps_retrans is not allowed to be greater than pf_retrans.
> >
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> >  include/uapi/linux/sctp.h |  1 +
> >  net/sctp/socket.c         | 10 ++++++++++
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> > index a15cc28..dfd81e1 100644
> > --- a/include/uapi/linux/sctp.h
> > +++ b/include/uapi/linux/sctp.h
> > @@ -1069,6 +1069,7 @@ struct sctp_paddrthlds {
> >       struct sockaddr_storage spt_address;
> >       __u16 spt_pathmaxrxt;
> >       __u16 spt_pathpfthld;
> > +     __u16 spt_pathcpthld;
> >  };
> >
> >  /*
> > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > index 5e2098b..5b9774d 100644
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -3954,6 +3954,9 @@ static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
>
> This code does:
>         if (optlen < sizeof(struct sctp_paddrthlds))
>                 return -EINVAL;
here will become:

        if (optlen >= sizeof(struct sctp_paddrthlds)) {
                optlen = sizeof(struct sctp_paddrthlds);
        } else if (optlen >= ALIGN(offsetof(struct sctp_paddrthlds,
                                            spt_pathcpthld), 4))
                optlen = ALIGN(offsetof(struct sctp_paddrthlds,
                                        spt_pathcpthld), 4);
                val.spt_pathcpthld = 0xffff;
        else {
                return -EINVAL;
        }

        if (copy_from_user(&val, (struct sctp_paddrthlds __user *)optval,
                           optlen))
                return -EFAULT;

in sctp_getsockopt_paddr_thresholds():

        if (len >= sizeof(struct sctp_paddrthlds))
                len = sizeof(struct sctp_paddrthlds);
        else if (len >= ALIGN(offsetof(struct sctp_paddrthlds,
                                       spt_pathcpthld), 4))
                len = ALIGN(offsetof(struct sctp_paddrthlds,
                                     spt_pathcpthld), 4);
        else
                return -EINVAL;

        if (copy_from_user(&val, (struct sctp_paddrthlds __user *)optval, len))
                return -EFAULT;

>
> So adding an extra field breaks existing application binaries
> that use this option.
>
> I've not checked the other patches or similar fubar.
>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

^ permalink raw reply

* RE: [PATCH net-next 0/6] net: stmmac: Improvements for -next
From: Jose Abreu @ 2019-09-11  8:59 UTC (permalink / raw)
  To: David Miller, Jose.Abreu@synopsys.com
  Cc: netdev@vger.kernel.org, Joao.Pinto@synopsys.com,
	peppe.cavallaro@st.com, alexandre.torgue@st.com,
	mcoquelin.stm32@gmail.com,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190911.102155.148817974369878410.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Sep/11/2019, 09:21:55 (UTC+00:00)

> From: Jose Abreu <Jose.Abreu@synopsys.com>
> Date: Tue, 10 Sep 2019 16:41:21 +0200
> 
> > Misc patches for -next. It includes:
> >  - Two fixes for features in -next only
> >  - New features support for GMAC cores (which includes GMAC4 and GMAC5)
> 
> Series applied, but what exactly does "ARP offload" even do?

ARP Offload allows the IP to reply to ARP_REQUEST packets automatically 
without passing by the application.

As net doesn't support this offloading I'm currently using this feature 
to test endianness issues in the IP and check if MAC Address is 
correctly configured, the logic is as follows:
 - MAC is set in loopback mode and ARP offload is activated
 - selftests create a dummy ARP_REQUEST packet and send it out
 - IP will detect the ARP_REQUEST packet and generate an ARP_REPLY 
packet
 - As MAC is in loopback mode then selftests will receive the ARP_REPLY 
packet
 - selftests logic will check if ARP_REPLY packet is correct (i.e. MAC 
address and packet type)

This way if this test fails it probably indicates that MAC address of IP 
is not correctly configured or that endianness of the IP was changed 
from default setting (which is LE).

By default the feature is off because user may not want to reply to 
ARP_REQUEST and I'm more using it as a diagnose facility. Let me know if 
you agree with this approach.

---
Thanks,
Jose Miguel Abreu

^ permalink raw reply

* Re: linux-next: Signed-off-by missing for commit in the net-next tree
From: Oleksij Rempel @ 2019-09-11  9:02 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: David Miller, Networking, Linux Next Mailing List,
	Linux Kernel Mailing List, The j1939 authors, Bastian Stender,
	Elenita Hinds, Kurt Van Dijck, kbuild test robot, Maxime Jayat,
	Robin van der Gracht, Marc Kleine-Budde
In-Reply-To: <20190911004103.3480fa40@canb.auug.org.au>

Hi Stephen,

On Wed, Sep 11, 2019 at 12:41:03AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Commit
> 
>   9d71dd0c7009 ("can: add support of SAE J1939 protocol")
> 
> is missing a Signed-off-by from its author.
> 
> [Not sure if I should complain about this one ...]

Here is the original pull request message for this patch series:
"The final patch is the collective effort of many entities (The j1939
authors: Oliver Hartkopp, Bastian Stender, Elenita Hinds, kbuild test
robot, Kurt Van Dijck, Maxime Jayat, Robin van der Gracht, Oleksij
Rempel, Marc Kleine-Budde). It adds support of SAE J1939 protocol to the
CAN networking stack."
https://www.mail-archive.com/netdev@vger.kernel.org/msg313476.html

Since the patch can be hardly assigned to one author we deiced to use
address of CAN mailing list.

Best regards,
Oleksij Rempel
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* RE: [PATCH net-next 5/5] sctp: add spt_pathcpthld in struct sctp_paddrthlds
From: David Laight @ 2019-09-11  9:03 UTC (permalink / raw)
  To: 'Xin Long'
  Cc: network dev, linux-sctp@vger.kernel.org, Marcelo Ricardo Leitner,
	Neil Horman, davem@davemloft.net
In-Reply-To: <CADvbK_d_Emw0K2Uq4P9OanRBr52tNjMsAOiJNi0TGsuWt6+81A@mail.gmail.com>

From: Xin Long [mailto:lucien.xin@gmail.com]
> Sent: 11 September 2019 09:52
> On Tue, Sep 10, 2019 at 9:19 PM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Xin Long
> > > Sent: 09 September 2019 08:57
> > > Section 7.2 of rfc7829: "Peer Address Thresholds (SCTP_PEER_ADDR_THLDS)
> > > Socket Option" extends 'struct sctp_paddrthlds' with 'spt_pathcpthld'
> > > added to allow a user to change ps_retrans per sock/asoc/transport, as
> > > other 2 paddrthlds: pf_retrans, pathmaxrxt.
> > >
> > > Note that ps_retrans is not allowed to be greater than pf_retrans.
> > >
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > >  include/uapi/linux/sctp.h |  1 +
> > >  net/sctp/socket.c         | 10 ++++++++++
> > >  2 files changed, 11 insertions(+)
> > >
> > > diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> > > index a15cc28..dfd81e1 100644
> > > --- a/include/uapi/linux/sctp.h
> > > +++ b/include/uapi/linux/sctp.h
> > > @@ -1069,6 +1069,7 @@ struct sctp_paddrthlds {
> > >       struct sockaddr_storage spt_address;
> > >       __u16 spt_pathmaxrxt;
> > >       __u16 spt_pathpfthld;
> > > +     __u16 spt_pathcpthld;
> > >  };
> > >
> > >  /*
> > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > index 5e2098b..5b9774d 100644
> > > --- a/net/sctp/socket.c
> > > +++ b/net/sctp/socket.c
> > > @@ -3954,6 +3954,9 @@ static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
> >
> > This code does:
> >         if (optlen < sizeof(struct sctp_paddrthlds))
> >                 return -EINVAL;
> here will become:
> 
>         if (optlen >= sizeof(struct sctp_paddrthlds)) {
>                 optlen = sizeof(struct sctp_paddrthlds);
>         } else if (optlen >= ALIGN(offsetof(struct sctp_paddrthlds,
>                                             spt_pathcpthld), 4))
>                 optlen = ALIGN(offsetof(struct sctp_paddrthlds,
>                                         spt_pathcpthld), 4);
>                 val.spt_pathcpthld = 0xffff;
>         else {
>                 return -EINVAL;
>         }

Hmmm...
If the kernel has to default 'val.spt_pathcpthld = 0xffff'
then recompiling an existing application with the new uapi
header is going to lead to very unexpected behaviour.

The best you can hope for is that the application memset the
structure to zero.
But more likely it is 'random' on-stack data.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* RE: [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10
From: Jose Abreu @ 2019-09-11  9:15 UTC (permalink / raw)
  To: Florian Fainelli, Jose Abreu, Thierry Reding
  Cc: David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
	Jon Hunter, Bitan Biswas, netdev@vger.kernel.org,
	linux-tegra@vger.kernel.org
In-Reply-To: <c8d419d3-6cf6-e260-a2e2-6a339c6c321b@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sep/10/2019, 20:01:01 (UTC+00:00)

> On 9/10/19 1:35 AM, Jose Abreu wrote:
> > From: Thierry Reding <thierry.reding@gmail.com>
> > Date: Sep/09/2019, 20:13:29 (UTC+00:00)
> > 
> >> On Mon, Sep 09, 2019 at 04:05:52PM +0000, Jose Abreu wrote:
> >>> From: Thierry Reding <thierry.reding@gmail.com>
> >>> Date: Sep/09/2019, 16:25:46 (UTC+00:00)
> >>>
> >>>> @@ -79,6 +79,10 @@ static void dwmac4_dma_init_rx_chan(void __iomem *ioaddr,
> >>>>  	value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
> >>>>  	writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
> >>>>  
> >>>> +	if (dma_cfg->eame)
> >>>
> >>> There is no need for this check. If EAME is not enabled then upper 32 
> >>> bits will be zero.
> >>
> >> The idea here was to potentially guard against this register not being
> >> available on some revisions. Having the check here would avoid access to
> >> the register if the device doesn't support enhanced addressing.
> > 
> > I see your point but I don't think there will be any problems unless you 
> > have some strange system that doesn't handle the write accesses to 
> > unimplemented features properly ...
> 
> Is not it then just safer to not do the write to a register that you do
> not know how the implementation is going to respond to with one of a
> target abort, timeout, decoding error, just dead lock?

I don't think any of these will ever happen. Notice that this is already 
been done for a long time in some registers that may not exist in some 
random HW config and there is also the point that this is a write 
operation so Slave Error would only get triggered if we did a read.

> Also, would it make sense to consider adding an #ifdef
> CONFIG_PHYS_ADDR_T_64BIT plus the conditional check so that you can be
> slightly more optimal in the hot-path here?

Well, this is not hot-path. It's only done in HW open sequence. The 
hot-path would be set_{rx/tx}_tail_ptr() but that's 32 bits only. 

---
Thanks,
Jose Miguel Abreu

^ permalink raw reply

* Re: [PATCH 1/7] net/dsa: configure autoneg for CPU port
From: Robert Beckett @ 2019-09-11  9:16 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn; +Cc: netdev, Vivien Didelot, David S. Miller
In-Reply-To: <aa0459e0-64ee-de84-fc38-3c9364301275@gmail.com>

On Tue, 2019-09-10 at 11:29 -0700, Florian Fainelli wrote:
> On 9/10/19 11:26 AM, Andrew Lunn wrote:
> > On Tue, Sep 10, 2019 at 04:41:47PM +0100, Robert Beckett wrote:
> > > This enables us to negoatiate pause frame transmission to
> > > prioritise
> > > packet delivery over throughput.
> > 
> > I don't think we can unconditionally enable this. It is a big
> > behaviour change, and it is likely to break running systems. It has
> > affects on QoS, packet prioritisation, etc.
> > 
> > I think there needs to be a configuration knob. But unfortunately,
> > i
> > don't know of a good place to put this knob. The switch CPU port is
> > not visible in any way.
> 
> Broadcast storm suppression is to be solved at ingress, not on the
> CPU
> port, once this lands on the CPU port, it's game over already.

It is not just for broadcast storm protection. The original issue that
made me look in to all of this turned out to be rx descritor ring
buffer exhaustion due to the CPU not being able to keep up with packet
reception.

Although the simple repro case for it is a broadcast storm, this could
happen with many legitimate small packets, and the correct way to
handle it seems to be pause frames, though I am not traditionally a
network programmer, so my knowledge may be incorrect. Please advise if
you know of a better way to handle that.

Fundamentally, with a phy to phy CPU connection, the CPU MAC may well
wish to enable pause frames for various reasons, so we should strive to
handle that I think.



^ permalink raw reply

* Re: [PATCH net-next 5/5] sctp: add spt_pathcpthld in struct sctp_paddrthlds
From: Xin Long @ 2019-09-11  9:21 UTC (permalink / raw)
  To: David Laight
  Cc: network dev, linux-sctp@vger.kernel.org, Marcelo Ricardo Leitner,
	Neil Horman, davem@davemloft.net
In-Reply-To: <1e5c3163e6c649b09137eeb62d193d87@AcuMS.aculab.com>

On Wed, Sep 11, 2019 at 5:03 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Xin Long [mailto:lucien.xin@gmail.com]
> > Sent: 11 September 2019 09:52
> > On Tue, Sep 10, 2019 at 9:19 PM David Laight <David.Laight@aculab.com> wrote:
> > >
> > > From: Xin Long
> > > > Sent: 09 September 2019 08:57
> > > > Section 7.2 of rfc7829: "Peer Address Thresholds (SCTP_PEER_ADDR_THLDS)
> > > > Socket Option" extends 'struct sctp_paddrthlds' with 'spt_pathcpthld'
> > > > added to allow a user to change ps_retrans per sock/asoc/transport, as
> > > > other 2 paddrthlds: pf_retrans, pathmaxrxt.
> > > >
> > > > Note that ps_retrans is not allowed to be greater than pf_retrans.
> > > >
> > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > ---
> > > >  include/uapi/linux/sctp.h |  1 +
> > > >  net/sctp/socket.c         | 10 ++++++++++
> > > >  2 files changed, 11 insertions(+)
> > > >
> > > > diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> > > > index a15cc28..dfd81e1 100644
> > > > --- a/include/uapi/linux/sctp.h
> > > > +++ b/include/uapi/linux/sctp.h
> > > > @@ -1069,6 +1069,7 @@ struct sctp_paddrthlds {
> > > >       struct sockaddr_storage spt_address;
> > > >       __u16 spt_pathmaxrxt;
> > > >       __u16 spt_pathpfthld;
> > > > +     __u16 spt_pathcpthld;
> > > >  };
> > > >
> > > >  /*
> > > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > > index 5e2098b..5b9774d 100644
> > > > --- a/net/sctp/socket.c
> > > > +++ b/net/sctp/socket.c
> > > > @@ -3954,6 +3954,9 @@ static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
> > >
> > > This code does:
> > >         if (optlen < sizeof(struct sctp_paddrthlds))
> > >                 return -EINVAL;
> > here will become:
> >
> >         if (optlen >= sizeof(struct sctp_paddrthlds)) {
> >                 optlen = sizeof(struct sctp_paddrthlds);
> >         } else if (optlen >= ALIGN(offsetof(struct sctp_paddrthlds,
> >                                             spt_pathcpthld), 4))
> >                 optlen = ALIGN(offsetof(struct sctp_paddrthlds,
> >                                         spt_pathcpthld), 4);
> >                 val.spt_pathcpthld = 0xffff;
> >         else {
> >                 return -EINVAL;
> >         }
>
> Hmmm...
> If the kernel has to default 'val.spt_pathcpthld = 0xffff'
> then recompiling an existing application with the new uapi
> header is going to lead to very unexpected behaviour.
>
> The best you can hope for is that the application memset the
> structure to zero.
> But more likely it is 'random' on-stack data.
0xffff is a value to disable the feature 'Primary Path Switchover'.
you're right that user might set it to zero unexpectly with their
old application rebuilt.

A safer way is to introduce "sysctl net.sctp.ps_retrans", it won't
matter if users set spt_pathcpthld properly when they're not aware
of this feature, like "sysctl net.sctp.pF_retrans". Looks better?

>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
From: Andy Shevchenko @ 2019-09-11  9:25 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linus Walleij, Mika Westerberg, linux-kernel, linux-gpio,
	Andrew Lunn, David S. Miller, Florian Fainelli, Heiner Kallweit,
	Russell King, netdev
In-Reply-To: <20190911075215.78047-5-dmitry.torokhov@gmail.com>

On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote:
> Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use
> the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit
> works with arbitrary firmware node.

I'm wondering if it's possible to step forward and replace
fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and
in other cases in this series.

-- 
With Best Regards,
Andy Shevchenko



^ 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