* [PATCH net 2/2] r8152: avoid using napi_disable after netif_napi_del.
From: Hayes Wang @ 2019-08-23 8:53 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, jslaby, Hayes Wang
In-Reply-To: <1394712342-15778-314-Taiwan-albertk@realtek.com>
Exchange netif_napi_del() and unregister_netdev() in rtl8152_disconnect()
to avoid using napi_disable() after netif_napi_del().
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 690a24d1ef82..29390eda5251 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -5364,8 +5364,8 @@ static void rtl8152_disconnect(struct usb_interface *intf)
if (tp) {
rtl_set_unplug(tp);
- netif_napi_del(&tp->napi);
unregister_netdev(tp->netdev);
+ netif_napi_del(&tp->napi);
cancel_delayed_work_sync(&tp->hw_phy_work);
tp->rtl_ops.unload(tp);
free_netdev(tp->netdev);
--
2.21.0
^ permalink raw reply related
* [PATCH net 1/2] Revert "r8152: napi hangup fix after disconnect"
From: Hayes Wang @ 2019-08-23 8:53 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, jslaby, Hayes Wang
In-Reply-To: <1394712342-15778-314-Taiwan-albertk@realtek.com>
This reverts commit 0ee1f4734967af8321ecebaf9c74221ace34f2d5.
This conflicts with commit ffa9fec30ca0 ("r8152: set
RTL8152_UNPLUG only for real disconnection").
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0cc03a9ff545..690a24d1ef82 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -4018,8 +4018,7 @@ static int rtl8152_close(struct net_device *netdev)
#ifdef CONFIG_PM_SLEEP
unregister_pm_notifier(&tp->pm_notifier);
#endif
- if (!test_bit(RTL8152_UNPLUG, &tp->flags))
- napi_disable(&tp->napi);
+ napi_disable(&tp->napi);
clear_bit(WORK_ENABLE, &tp->flags);
usb_kill_urb(tp->intr_urb);
cancel_delayed_work_sync(&tp->schedule);
--
2.21.0
^ permalink raw reply related
* [PATCH net 0/2] r8152: fix side effect
From: Hayes Wang @ 2019-08-23 8:53 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, jslaby, Hayes Wang
The commit 0ee1f4734967 ("r8152: napi hangup fix after disconnect")
add a check to avoid using napi_disable after netif_napi_del. However,
the commit ffa9fec30ca0 ("r8152: set RTL8152_UNPLUG only for real
disconnection") let the check useless.
Therefore, I revert commit 0ee1f4734967 ("r8152: napi hangup fix
after disconnect") first, and add another patch to fix it.
Hayes Wang (2):
Revert "r8152: napi hangup fix after disconnect"
r8152: avoid using napi_disable after netif_napi_del.
drivers/net/usb/r8152.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH] wimax/i2400m: fix calculation of index, remove sizeof
From: Colin King @ 2019-08-23 8:52 UTC (permalink / raw)
To: Inaky Perez-Gonzalez, linux-wimax, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The subtraction of the two pointers is automatically scaled by the
size of the size of the object the pointers point to, so the division
by sizeof(*i2400m->barker) is incorrect. Fix this by removing the
division. Also make index an unsigned int to clean up a checkpatch
warning.
Addresses-Coverity: ("Extra sizeof expression")
Fixes: aba3792ac2d7 ("wimax/i2400m: rework bootrom initialization to be more flexible")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wimax/i2400m/fw.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
index 489cba9b284d..599a703af6eb 100644
--- a/drivers/net/wimax/i2400m/fw.c
+++ b/drivers/net/wimax/i2400m/fw.c
@@ -399,8 +399,7 @@ int i2400m_is_boot_barker(struct i2400m *i2400m,
* associated with the device. */
if (i2400m->barker
&& !memcmp(buf, i2400m->barker, sizeof(i2400m->barker->data))) {
- unsigned index = (i2400m->barker - i2400m_barker_db)
- / sizeof(*i2400m->barker);
+ unsigned int index = i2400m->barker - i2400m_barker_db;
d_printf(2, dev, "boot barker cache-confirmed #%u/%08x\n",
index, le32_to_cpu(i2400m->barker->data[0]));
return 0;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net 0/9] rxrpc: Fix use of skb_cow_data()
From: David Howells @ 2019-08-23 8:52 UTC (permalink / raw)
To: David Miller; +Cc: dhowells, netdev, linux-afs, linux-kernel
In-Reply-To: <20190822.121207.731320146177703787.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote:
> Why don't you just do an skb_unshare() at the beginning when you know that
> you'll need to do that?
I was trying to defer any copying to process context rather than doing it in
softirq context to spend less time in softirq context - plus that way I can
use GFP_NOIO (kafs) or GFP_KERNEL (direct AF_RXRPC socket) rather than
GFP_ATOMIC if the api supports it.
I don't remember now why I used skb_cow_data() rather than skb_unshare() - but
it was probably because the former leaves the sk_buff object itself intact,
whereas the latter replaces it. I can switch to using skb_unshare() instead.
Question for you: how likely is a newly received buffer, through a UDP socket,
to be 'cloned'?
David
^ permalink raw reply
* Re: [PATCH] brcmfmac: replace strncpy() by strscpy()
From: Arend Van Spriel @ 2019-08-23 8:18 UTC (permalink / raw)
To: Xulin Sun, kvalo
Cc: stefan.wahren, linux-kernel, netdev, brcm80211-dev-list,
brcm80211-dev-list.pdl, linux-wireless, franky.lin,
hante.meuleman, chi-hsien.lin, wright.feng, davem, stanley.hsu
In-Reply-To: <20190823074708.20081-1-xulin.sun@windriver.com>
On 8/23/2019 9:47 AM, Xulin Sun wrote:
> The strncpy() may truncate the copied string,
> replace it by the safer strscpy().
>
> To avoid below compile warning with gcc 8.2:
>
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:In function 'brcmf_vndr_ie':
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:4227:2:
> warning: 'strncpy' output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation]
> strncpy(iebuf, add_del_cmd, VNDR_IE_CMD_LEN - 1);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Xulin Sun <xulin.sun@windriver.com>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
^ permalink raw reply
* RE: [PATCH v2 0/2] Simplify mtty driver and mdev core
From: Parav Pandit @ 2019-08-23 8:14 UTC (permalink / raw)
To: Jiri Pirko
Cc: Alex Williamson, Jiri Pirko, David S . Miller, Kirti Wankhede,
Cornelia Huck, kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
cjia, netdev@vger.kernel.org
In-Reply-To: <20190823081221.GG2276@nanopsycho.orion>
Hi Alex,
> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Friday, August 23, 2019 1:42 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>; Jiri Pirko
> <jiri@mellanox.com>; David S . Miller <davem@davemloft.net>; Kirti
> Wankhede <kwankhede@nvidia.com>; Cornelia Huck <cohuck@redhat.com>;
> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; cjia <cjia@nvidia.com>;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
>
> Thu, Aug 22, 2019 at 03:33:30PM CEST, parav@mellanox.com wrote:
> >
> >
> >> -----Original Message-----
> >> From: Jiri Pirko <jiri@resnulli.us>
> >> Sent: Thursday, August 22, 2019 5:50 PM
> >> To: Parav Pandit <parav@mellanox.com>
> >> Cc: Alex Williamson <alex.williamson@redhat.com>; Jiri Pirko
> >> <jiri@mellanox.com>; David S . Miller <davem@davemloft.net>; Kirti
> >> Wankhede <kwankhede@nvidia.com>; Cornelia Huck
> <cohuck@redhat.com>;
> >> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; cjia
> >> <cjia@nvidia.com>; netdev@vger.kernel.org
> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
> >>
> >> Thu, Aug 22, 2019 at 12:04:02PM CEST, parav@mellanox.com wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: Jiri Pirko <jiri@resnulli.us>
> >> >> Sent: Thursday, August 22, 2019 3:28 PM
> >> >> To: Parav Pandit <parav@mellanox.com>
> >> >> Cc: Alex Williamson <alex.williamson@redhat.com>; Jiri Pirko
> >> >> <jiri@mellanox.com>; David S . Miller <davem@davemloft.net>; Kirti
> >> >> Wankhede <kwankhede@nvidia.com>; Cornelia Huck
> >> <cohuck@redhat.com>;
> >> >> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; cjia
> >> >> <cjia@nvidia.com>; netdev@vger.kernel.org
> >> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
> >> >>
> >> >> Thu, Aug 22, 2019 at 11:42:13AM CEST, parav@mellanox.com wrote:
> >> >> >
> >> >> >
> >> >> >> -----Original Message-----
> >> >> >> From: Jiri Pirko <jiri@resnulli.us>
> >> >> >> Sent: Thursday, August 22, 2019 2:59 PM
> >> >> >> To: Parav Pandit <parav@mellanox.com>
> >> >> >> Cc: Alex Williamson <alex.williamson@redhat.com>; Jiri Pirko
> >> >> >> <jiri@mellanox.com>; David S . Miller <davem@davemloft.net>;
> >> >> >> Kirti Wankhede <kwankhede@nvidia.com>; Cornelia Huck
> >> >> <cohuck@redhat.com>;
> >> >> >> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; cjia
> >> >> >> <cjia@nvidia.com>; netdev@vger.kernel.org
> >> >> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
> >> >> >>
> >> >> >> Wed, Aug 21, 2019 at 08:23:17AM CEST, parav@mellanox.com wrote:
> >> >> >> >
> >> >> >> >
> >> >> >> >> -----Original Message-----
> >> >> >> >> From: Alex Williamson <alex.williamson@redhat.com>
> >> >> >> >> Sent: Wednesday, August 21, 2019 10:56 AM
> >> >> >> >> To: Parav Pandit <parav@mellanox.com>
> >> >> >> >> Cc: Jiri Pirko <jiri@mellanox.com>; David S . Miller
> >> >> >> >> <davem@davemloft.net>; Kirti Wankhede
> >> >> >> >> <kwankhede@nvidia.com>; Cornelia Huck <cohuck@redhat.com>;
> >> >> >> >> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; cjia
> >> >> >> >> <cjia@nvidia.com>; netdev@vger.kernel.org
> >> >> >> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev
> >> >> >> >> core
> >> >> >> >>
> >> >> >> >> > > > > Just an example of the alias, not proposing how it's set.
> >> >> >> >> > > > > In fact, proposing that the user does not set it,
> >> >> >> >> > > > > mdev-core provides one
> >> >> >> >> > > automatically.
> >> >> >> >> > > > >
> >> >> >> >> > > > > > > Since there seems to be some prefix overhead, as
> >> >> >> >> > > > > > > I ask about above in how many characters we
> >> >> >> >> > > > > > > actually have to work with in IFNAMESZ, maybe we
> >> >> >> >> > > > > > > start with
> >> >> >> >> > > > > > > 8 characters (matching your "index" namespace)
> >> >> >> >> > > > > > > and expand as necessary for
> >> >> >> disambiguation.
> >> >> >> >> > > > > > > If we can eliminate overhead in IFNAMESZ, let's
> >> >> >> >> > > > > > > start with
> >> 12.
> >> >> >> >> > > > > > > Thanks,
> >> >> >> >> > > > > > >
> >> >> >> >> > > > > > If user is going to choose the alias, why does it
> >> >> >> >> > > > > > have to be limited to
> >> >> >> >> sha1?
> >> >> >> >> > > > > > Or you just told it as an example?
> >> >> >> >> > > > > >
> >> >> >> >> > > > > > It can be an alpha-numeric string.
> >> >> >> >> > > > >
> >> >> >> >> > > > > No, I'm proposing a different solution where
> >> >> >> >> > > > > mdev-core creates an alias based on an abbreviated
> >> >> >> >> > > > > sha1. The user does not provide the
> >> >> >> >> alias.
> >> >> >> >> > > > >
> >> >> >> >> > > > > > Instead of mdev imposing number of characters on
> >> >> >> >> > > > > > the alias, it should be best
> >> >> >> >> > > > > left to the user.
> >> >> >> >> > > > > > Because in future if netdev improves on the naming
> >> >> >> >> > > > > > scheme, mdev will be
> >> >> >> >> > > > > limiting it, which is not right.
> >> >> >> >> > > > > > So not restricting alias size seems right to me.
> >> >> >> >> > > > > > User configuring mdev for networking devices in a
> >> >> >> >> > > > > > given kernel knows what
> >> >> >> >> > > > > user is doing.
> >> >> >> >> > > > > > So user can choose alias name size as it finds suitable.
> >> >> >> >> > > > >
> >> >> >> >> > > > > That's not what I'm proposing, please read again.
> >> >> >> >> > > > > Thanks,
> >> >> >> >> > > >
> >> >> >> >> > > > I understood your point. But mdev doesn't know how
> >> >> >> >> > > > user is going to use
> >> >> >> >> > > udev/systemd to name the netdev.
> >> >> >> >> > > > So even if mdev chose to pick 12 characters, it could
> >> >> >> >> > > > result in
> >> >> collision.
> >> >> >> >> > > > Hence the proposal to provide the alias by the user,
> >> >> >> >> > > > as user know the best
> >> >> >> >> > > policy for its use case in the environment its using.
> >> >> >> >> > > > So 12 character sha1 method will still work by user.
> >> >> >> >> > >
> >> >> >> >> > > Haven't you already provided examples where certain
> >> >> >> >> > > drivers or subsystems have unique netdev prefixes? If
> >> >> >> >> > > mdev provides a unique alias within the subsystem,
> >> >> >> >> > > couldn't we simply define a netdev prefix for the mdev
> >> >> >> >> > > subsystem and avoid all other collisions? I'm not in
> >> >> >> >> > > favor of the user providing both a uuid and an
> >> >> >> >> > > alias/instance. Thanks,
> >> >> >> >> > >
> >> >> >> >> > For a given prefix, say ens2f0, can two UUID->sha1 first 9
> >> >> >> >> > characters have
> >> >> >> >> collision?
> >> >> >> >>
> >> >> >> >> I think it would be a mistake to waste so many chars on a
> >> >> >> >> prefix, but
> >> >> >> >> 9 characters of sha1 likely wouldn't have a collision before
> >> >> >> >> we have 10s of thousands of devices. Thanks,
> >> >> >> >>
> >> >> >> >> Alex
> >> >> >> >
> >> >> >> >Jiri, Dave,
> >> >> >> >Are you ok with it for devlink/netdev part?
> >> >> >> >Mdev core will create an alias from a UUID.
> >> >> >> >
> >> >> >> >This will be supplied during devlink port attr set such as,
> >> >> >> >
> >> >> >> >devlink_port_attrs_mdev_set(struct devlink_port *port, const
> >> >> >> >char *mdev_alias);
> >> >> >> >
> >> >> >> >This alias is used to generate representor netdev's phys_port_name.
> >> >> >> >This alias from the mdev device's sysfs will be used by the
> >> >> >> >udev/systemd to
> >> >> >> generate predicable netdev's name.
> >> >> >> >Example: enm<mdev_alias_first_12_chars>
> >> >> >>
> >> >> >> What happens in unlikely case of 2 UUIDs collide?
> >> >> >>
> >> >> >Since users sees two devices with same phys_port_name, user
> >> >> >should destroy
> >> >> recently created mdev and recreate mdev with different UUID?
> >> >>
> >> >> Driver should make sure phys port name wont collide,
> >> >So when mdev creation is initiated, mdev core calculates the alias
> >> >and if there
> >> is any other mdev with same alias exist, it returns -EEXIST error
> >> before progressing further.
> >> >This way user will get to know upfront in event of collision before
> >> >the mdev
> >> device gets created.
> >> >How about that?
> >>
> >> Sounds fine to me. Now the question is how many chars do we want to have.
> >>
> >12 characters from Alex's suggestion similar to git?
>
> Ok.
>
Can you please confirm this scheme looks good now? I like to get patches started.
> >
> >> >
> >> >
> >> >> in this case that it does
> >> >> not provide 2 same attrs for 2 different ports.
> >> >> Hmm, so the order of creation matters. That is not good.
> >> >>
> >> >> >>
> >> >> >> >I took Ethernet mdev as an example.
> >> >> >> >New prefix 'm' stands for mediated device.
> >> >> >> >Remaining 12 characters are first 12 chars of the mdev alias.
> >> >> >>
> >> >> >> Does this resolve the identification of devlink port representor?
> >> >> >Not sure if I understood your question correctly, attemping to
> >> >> >answer
> >> below.
> >> >> >phys_port_name of devlink port is defined by the first 12
> >> >> >characters of mdev
> >> >> alias.
> >> >> >> I assume you want to use the same 12(or so) chars, don't you?
> >> >> >Mdev's netdev will also use the same mdev alias from the sysfs to
> >> >> >rename
> >> >> netdev name from ethX to enm<mdev_alias>, where en=Etherenet,
> >> m=mdev.
> >> >> >
> >> >> >So yes, same 12 characters are use for mdev's netdev and mdev
> >> >> >devlink port's
> >> >> phys_port_name.
> >> >> >
> >> >> >Is that what are you asking?
> >> >>
> >> >> Yes. Then you have 3 chars to handle the rest of the name (pci, pf)...
^ permalink raw reply
* Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
From: Jiri Pirko @ 2019-08-23 8:12 UTC (permalink / raw)
To: Parav Pandit
Cc: Alex Williamson, Jiri Pirko, David S . Miller, Kirti Wankhede,
Cornelia Huck, kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
cjia, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB4866F9650CF73FC671972127D1A50@AM0PR05MB4866.eurprd05.prod.outlook.com>
Thu, Aug 22, 2019 at 03:33:30PM CEST, parav@mellanox.com wrote:
>
>
>> -----Original Message-----
>> From: Jiri Pirko <jiri@resnulli.us>
>> Sent: Thursday, August 22, 2019 5:50 PM
>> To: Parav Pandit <parav@mellanox.com>
>> Cc: Alex Williamson <alex.williamson@redhat.com>; Jiri Pirko
>> <jiri@mellanox.com>; David S . Miller <davem@davemloft.net>; Kirti
>> Wankhede <kwankhede@nvidia.com>; Cornelia Huck <cohuck@redhat.com>;
>> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; cjia <cjia@nvidia.com>;
>> netdev@vger.kernel.org
>> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
>>
>> Thu, Aug 22, 2019 at 12:04:02PM CEST, parav@mellanox.com wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: Jiri Pirko <jiri@resnulli.us>
>> >> Sent: Thursday, August 22, 2019 3:28 PM
>> >> To: Parav Pandit <parav@mellanox.com>
>> >> Cc: Alex Williamson <alex.williamson@redhat.com>; Jiri Pirko
>> >> <jiri@mellanox.com>; David S . Miller <davem@davemloft.net>; Kirti
>> >> Wankhede <kwankhede@nvidia.com>; Cornelia Huck
>> <cohuck@redhat.com>;
>> >> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; cjia
>> >> <cjia@nvidia.com>; netdev@vger.kernel.org
>> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
>> >>
>> >> Thu, Aug 22, 2019 at 11:42:13AM CEST, parav@mellanox.com wrote:
>> >> >
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Jiri Pirko <jiri@resnulli.us>
>> >> >> Sent: Thursday, August 22, 2019 2:59 PM
>> >> >> To: Parav Pandit <parav@mellanox.com>
>> >> >> Cc: Alex Williamson <alex.williamson@redhat.com>; Jiri Pirko
>> >> >> <jiri@mellanox.com>; David S . Miller <davem@davemloft.net>; Kirti
>> >> >> Wankhede <kwankhede@nvidia.com>; Cornelia Huck
>> >> <cohuck@redhat.com>;
>> >> >> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; cjia
>> >> >> <cjia@nvidia.com>; netdev@vger.kernel.org
>> >> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
>> >> >>
>> >> >> Wed, Aug 21, 2019 at 08:23:17AM CEST, parav@mellanox.com wrote:
>> >> >> >
>> >> >> >
>> >> >> >> -----Original Message-----
>> >> >> >> From: Alex Williamson <alex.williamson@redhat.com>
>> >> >> >> Sent: Wednesday, August 21, 2019 10:56 AM
>> >> >> >> To: Parav Pandit <parav@mellanox.com>
>> >> >> >> Cc: Jiri Pirko <jiri@mellanox.com>; David S . Miller
>> >> >> >> <davem@davemloft.net>; Kirti Wankhede <kwankhede@nvidia.com>;
>> >> >> >> Cornelia Huck <cohuck@redhat.com>; kvm@vger.kernel.org;
>> >> >> >> linux-kernel@vger.kernel.org; cjia <cjia@nvidia.com>;
>> >> >> >> netdev@vger.kernel.org
>> >> >> >> Subject: Re: [PATCH v2 0/2] Simplify mtty driver and mdev core
>> >> >> >>
>> >> >> >> > > > > Just an example of the alias, not proposing how it's set.
>> >> >> >> > > > > In fact, proposing that the user does not set it,
>> >> >> >> > > > > mdev-core provides one
>> >> >> >> > > automatically.
>> >> >> >> > > > >
>> >> >> >> > > > > > > Since there seems to be some prefix overhead, as I
>> >> >> >> > > > > > > ask about above in how many characters we actually
>> >> >> >> > > > > > > have to work with in IFNAMESZ, maybe we start with
>> >> >> >> > > > > > > 8 characters (matching your "index" namespace) and
>> >> >> >> > > > > > > expand as necessary for
>> >> >> disambiguation.
>> >> >> >> > > > > > > If we can eliminate overhead in IFNAMESZ, let's start with
>> 12.
>> >> >> >> > > > > > > Thanks,
>> >> >> >> > > > > > >
>> >> >> >> > > > > > If user is going to choose the alias, why does it
>> >> >> >> > > > > > have to be limited to
>> >> >> >> sha1?
>> >> >> >> > > > > > Or you just told it as an example?
>> >> >> >> > > > > >
>> >> >> >> > > > > > It can be an alpha-numeric string.
>> >> >> >> > > > >
>> >> >> >> > > > > No, I'm proposing a different solution where mdev-core
>> >> >> >> > > > > creates an alias based on an abbreviated sha1. The
>> >> >> >> > > > > user does not provide the
>> >> >> >> alias.
>> >> >> >> > > > >
>> >> >> >> > > > > > Instead of mdev imposing number of characters on the
>> >> >> >> > > > > > alias, it should be best
>> >> >> >> > > > > left to the user.
>> >> >> >> > > > > > Because in future if netdev improves on the naming
>> >> >> >> > > > > > scheme, mdev will be
>> >> >> >> > > > > limiting it, which is not right.
>> >> >> >> > > > > > So not restricting alias size seems right to me.
>> >> >> >> > > > > > User configuring mdev for networking devices in a
>> >> >> >> > > > > > given kernel knows what
>> >> >> >> > > > > user is doing.
>> >> >> >> > > > > > So user can choose alias name size as it finds suitable.
>> >> >> >> > > > >
>> >> >> >> > > > > That's not what I'm proposing, please read again.
>> >> >> >> > > > > Thanks,
>> >> >> >> > > >
>> >> >> >> > > > I understood your point. But mdev doesn't know how user
>> >> >> >> > > > is going to use
>> >> >> >> > > udev/systemd to name the netdev.
>> >> >> >> > > > So even if mdev chose to pick 12 characters, it could
>> >> >> >> > > > result in
>> >> collision.
>> >> >> >> > > > Hence the proposal to provide the alias by the user, as
>> >> >> >> > > > user know the best
>> >> >> >> > > policy for its use case in the environment its using.
>> >> >> >> > > > So 12 character sha1 method will still work by user.
>> >> >> >> > >
>> >> >> >> > > Haven't you already provided examples where certain drivers
>> >> >> >> > > or subsystems have unique netdev prefixes? If mdev
>> >> >> >> > > provides a unique alias within the subsystem, couldn't we
>> >> >> >> > > simply define a netdev prefix for the mdev subsystem and
>> >> >> >> > > avoid all other collisions? I'm not in favor of the user
>> >> >> >> > > providing both a uuid and an alias/instance. Thanks,
>> >> >> >> > >
>> >> >> >> > For a given prefix, say ens2f0, can two UUID->sha1 first 9
>> >> >> >> > characters have
>> >> >> >> collision?
>> >> >> >>
>> >> >> >> I think it would be a mistake to waste so many chars on a
>> >> >> >> prefix, but
>> >> >> >> 9 characters of sha1 likely wouldn't have a collision before we
>> >> >> >> have 10s of thousands of devices. Thanks,
>> >> >> >>
>> >> >> >> Alex
>> >> >> >
>> >> >> >Jiri, Dave,
>> >> >> >Are you ok with it for devlink/netdev part?
>> >> >> >Mdev core will create an alias from a UUID.
>> >> >> >
>> >> >> >This will be supplied during devlink port attr set such as,
>> >> >> >
>> >> >> >devlink_port_attrs_mdev_set(struct devlink_port *port, const char
>> >> >> >*mdev_alias);
>> >> >> >
>> >> >> >This alias is used to generate representor netdev's phys_port_name.
>> >> >> >This alias from the mdev device's sysfs will be used by the
>> >> >> >udev/systemd to
>> >> >> generate predicable netdev's name.
>> >> >> >Example: enm<mdev_alias_first_12_chars>
>> >> >>
>> >> >> What happens in unlikely case of 2 UUIDs collide?
>> >> >>
>> >> >Since users sees two devices with same phys_port_name, user should
>> >> >destroy
>> >> recently created mdev and recreate mdev with different UUID?
>> >>
>> >> Driver should make sure phys port name wont collide,
>> >So when mdev creation is initiated, mdev core calculates the alias and if there
>> is any other mdev with same alias exist, it returns -EEXIST error before
>> progressing further.
>> >This way user will get to know upfront in event of collision before the mdev
>> device gets created.
>> >How about that?
>>
>> Sounds fine to me. Now the question is how many chars do we want to have.
>>
>12 characters from Alex's suggestion similar to git?
Ok.
>
>> >
>> >
>> >> in this case that it does
>> >> not provide 2 same attrs for 2 different ports.
>> >> Hmm, so the order of creation matters. That is not good.
>> >>
>> >> >>
>> >> >> >I took Ethernet mdev as an example.
>> >> >> >New prefix 'm' stands for mediated device.
>> >> >> >Remaining 12 characters are first 12 chars of the mdev alias.
>> >> >>
>> >> >> Does this resolve the identification of devlink port representor?
>> >> >Not sure if I understood your question correctly, attemping to answer
>> below.
>> >> >phys_port_name of devlink port is defined by the first 12 characters
>> >> >of mdev
>> >> alias.
>> >> >> I assume you want to use the same 12(or so) chars, don't you?
>> >> >Mdev's netdev will also use the same mdev alias from the sysfs to
>> >> >rename
>> >> netdev name from ethX to enm<mdev_alias>, where en=Etherenet,
>> m=mdev.
>> >> >
>> >> >So yes, same 12 characters are use for mdev's netdev and mdev
>> >> >devlink port's
>> >> phys_port_name.
>> >> >
>> >> >Is that what are you asking?
>> >>
>> >> Yes. Then you have 3 chars to handle the rest of the name (pci, pf)...
^ permalink raw reply
* [PATCH] brcmfmac: replace strncpy() by strscpy()
From: Xulin Sun @ 2019-08-23 7:47 UTC (permalink / raw)
To: kvalo
Cc: stefan.wahren, xulin.sun, linux-kernel, netdev,
brcm80211-dev-list, brcm80211-dev-list.pdl, linux-wireless,
arend.vanspriel, franky.lin, hante.meuleman, chi-hsien.lin,
wright.feng, davem, stanley.hsu
The strncpy() may truncate the copied string,
replace it by the safer strscpy().
To avoid below compile warning with gcc 8.2:
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:In function 'brcmf_vndr_ie':
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:4227:2:
warning: 'strncpy' output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation]
strncpy(iebuf, add_del_cmd, VNDR_IE_CMD_LEN - 1);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Xulin Sun <xulin.sun@windriver.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index b6d0df354b36..7ad60374fa96 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4226,9 +4226,7 @@ brcmf_parse_vndr_ies(const u8 *vndr_ie_buf, u32 vndr_ie_len,
static u32
brcmf_vndr_ie(u8 *iebuf, s32 pktflag, u8 *ie_ptr, u32 ie_len, s8 *add_del_cmd)
{
-
- strncpy(iebuf, add_del_cmd, VNDR_IE_CMD_LEN - 1);
- iebuf[VNDR_IE_CMD_LEN - 1] = '\0';
+ strscpy(iebuf, add_del_cmd, VNDR_IE_CMD_LEN);
put_unaligned_le32(1, &iebuf[VNDR_IE_COUNT_OFFSET]);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v4 0/2] r8152: save EEE
From: Hayes Wang @ 2019-08-23 7:33 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
In-Reply-To: <1394712342-15778-304-Taiwan-albertk@realtek.com>
v4:
For patch #2, remove redundant calling of "ocp_reg_write(tp, OCP_EEE_ADV, 0)".
v3:
For patch #2, fix the mistake caused by copying and pasting.
v2:
Adjust patch #1. The EEE has been disabled in the beginning of
r8153_hw_phy_cfg() and r8153b_hw_phy_cfg(), so only check if
it is necessary to enable EEE.
Add the patch #2 for the helper function.
v1:
Saving the settings of EEE to avoid they become the default settings
after reset_resume().
Hayes Wang (2):
r8152: saving the settings of EEE
r8152: add a helper function about setting EEE
drivers/net/usb/r8152.c | 182 +++++++++++++++++++++-------------------
1 file changed, 95 insertions(+), 87 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH net-next v4 2/2] r8152: add a helper function about setting EEE
From: Hayes Wang @ 2019-08-23 7:33 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
In-Reply-To: <1394712342-15778-311-Taiwan-albertk@realtek.com>
Add a helper function "rtl_eee_enable" for setting EEE. Besides, I
move r8153_eee_en() and r8153b_eee_en(). And, I remove r8152b_enable_eee(),
r8153_set_eee(), and r8153b_set_eee().
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 168 ++++++++++++++++++----------------------
1 file changed, 77 insertions(+), 91 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index a7aa48bee732..17f0e9e98697 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3202,14 +3202,75 @@ static void r8152_eee_en(struct r8152 *tp, bool enable)
ocp_reg_write(tp, OCP_EEE_CONFIG3, config3);
}
-static void r8152b_enable_eee(struct r8152 *tp)
+static void r8153_eee_en(struct r8152 *tp, bool enable)
{
- if (tp->eee_en) {
- r8152_eee_en(tp, true);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, tp->eee_adv);
+ u32 ocp_data;
+ u16 config;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
+ config = ocp_reg_read(tp, OCP_EEE_CFG);
+
+ if (enable) {
+ ocp_data |= EEE_RX_EN | EEE_TX_EN;
+ config |= EEE10_EN;
} else {
- r8152_eee_en(tp, false);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ ocp_data &= ~(EEE_RX_EN | EEE_TX_EN);
+ config &= ~EEE10_EN;
+ }
+
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEE_CR, ocp_data);
+ ocp_reg_write(tp, OCP_EEE_CFG, config);
+}
+
+static void r8153b_eee_en(struct r8152 *tp, bool enable)
+{
+ r8153_eee_en(tp, enable);
+
+ if (enable)
+ r8153b_ups_flags_w1w0(tp, UPS_FLAGS_EN_EEE, 0);
+ else
+ r8153b_ups_flags_w1w0(tp, 0, UPS_FLAGS_EN_EEE);
+}
+
+static void rtl_eee_enable(struct r8152 *tp, bool enable)
+{
+ switch (tp->version) {
+ case RTL_VER_01:
+ case RTL_VER_02:
+ case RTL_VER_07:
+ if (enable) {
+ r8152_eee_en(tp, true);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV,
+ tp->eee_adv);
+ } else {
+ r8152_eee_en(tp, false);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ }
+ break;
+ case RTL_VER_03:
+ case RTL_VER_04:
+ case RTL_VER_05:
+ case RTL_VER_06:
+ if (enable) {
+ r8153_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ } else {
+ r8153_eee_en(tp, false);
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
+ }
+ break;
+ case RTL_VER_08:
+ case RTL_VER_09:
+ if (enable) {
+ r8153b_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ } else {
+ r8153b_eee_en(tp, false);
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
+ }
+ break;
+ default:
+ break;
}
}
@@ -3231,7 +3292,7 @@ static void rtl8152_disable(struct r8152 *tp)
static void r8152b_hw_phy_cfg(struct r8152 *tp)
{
- r8152b_enable_eee(tp);
+ rtl_eee_enable(tp, tp->eee_en);
r8152_aldps_en(tp, true);
r8152b_enable_fc(tp);
@@ -3425,36 +3486,6 @@ static void r8153b_aldps_en(struct r8152 *tp, bool enable)
r8153b_ups_flags_w1w0(tp, 0, UPS_FLAGS_EN_ALDPS);
}
-static void r8153_eee_en(struct r8152 *tp, bool enable)
-{
- u32 ocp_data;
- u16 config;
-
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
- config = ocp_reg_read(tp, OCP_EEE_CFG);
-
- if (enable) {
- ocp_data |= EEE_RX_EN | EEE_TX_EN;
- config |= EEE10_EN;
- } else {
- ocp_data &= ~(EEE_RX_EN | EEE_TX_EN);
- config &= ~EEE10_EN;
- }
-
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEE_CR, ocp_data);
- ocp_reg_write(tp, OCP_EEE_CFG, config);
-}
-
-static void r8153b_eee_en(struct r8152 *tp, bool enable)
-{
- r8153_eee_en(tp, enable);
-
- if (enable)
- r8153b_ups_flags_w1w0(tp, UPS_FLAGS_EN_EEE, 0);
- else
- r8153b_ups_flags_w1w0(tp, 0, UPS_FLAGS_EN_EEE);
-}
-
static void r8153b_enable_fc(struct r8152 *tp)
{
r8152b_enable_fc(tp);
@@ -3470,8 +3501,7 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
r8153_aldps_en(tp, false);
/* disable EEE before updating the PHY parameters */
- r8153_eee_en(tp, false);
- ocp_reg_write(tp, OCP_EEE_ADV, 0);
+ rtl_eee_enable(tp, false);
if (tp->version == RTL_VER_03) {
data = ocp_reg_read(tp, OCP_EEE_CFG);
@@ -3502,10 +3532,8 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
sram_write(tp, SRAM_10M_AMP1, 0x00af);
sram_write(tp, SRAM_10M_AMP2, 0x0208);
- if (tp->eee_en) {
- r8153_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
- }
+ if (tp->eee_en)
+ rtl_eee_enable(tp, true);
r8153_aldps_en(tp, true);
r8152b_enable_fc(tp);
@@ -3545,8 +3573,7 @@ static void r8153b_hw_phy_cfg(struct r8152 *tp)
r8153b_aldps_en(tp, false);
/* disable EEE before updating the PHY parameters */
- r8153b_eee_en(tp, false);
- ocp_reg_write(tp, OCP_EEE_ADV, 0);
+ rtl_eee_enable(tp, false);
r8153b_green_en(tp, test_bit(GREEN_ETHERNET, &tp->flags));
@@ -3608,10 +3635,8 @@ static void r8153b_hw_phy_cfg(struct r8152 *tp)
r8153b_ups_flags_w1w0(tp, ups_flags, 0);
- if (tp->eee_en) {
- r8153b_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
- }
+ if (tp->eee_en)
+ rtl_eee_enable(tp, true);
r8153b_aldps_en(tp, true);
r8153b_enable_fc(tp);
@@ -4930,12 +4955,7 @@ static int r8152_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
tp->eee_en = eee->eee_enabled;
tp->eee_adv = val;
- r8152_eee_en(tp, eee->eee_enabled);
-
- if (eee->eee_enabled)
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val);
- else
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ rtl_eee_enable(tp, tp->eee_en);
return 0;
}
@@ -4963,40 +4983,6 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
return 0;
}
-static int r8153_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
-{
- u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
-
- tp->eee_en = eee->eee_enabled;
- tp->eee_adv = val;
-
- r8153_eee_en(tp, eee->eee_enabled);
-
- if (eee->eee_enabled)
- ocp_reg_write(tp, OCP_EEE_ADV, val);
- else
- ocp_reg_write(tp, OCP_EEE_ADV, 0);
-
- return 0;
-}
-
-static int r8153b_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
-{
- u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
-
- tp->eee_en = eee->eee_enabled;
- tp->eee_adv = val;
-
- r8153b_eee_en(tp, eee->eee_enabled);
-
- if (eee->eee_enabled)
- ocp_reg_write(tp, OCP_EEE_ADV, val);
- else
- ocp_reg_write(tp, OCP_EEE_ADV, 0);
-
- return 0;
-}
-
static int
rtl_ethtool_get_eee(struct net_device *net, struct ethtool_eee *edata)
{
@@ -5382,7 +5368,7 @@ static int rtl_ops_init(struct r8152 *tp)
ops->down = rtl8153_down;
ops->unload = rtl8153_unload;
ops->eee_get = r8153_get_eee;
- ops->eee_set = r8153_set_eee;
+ ops->eee_set = r8152_set_eee;
ops->in_nway = rtl8153_in_nway;
ops->hw_phy_cfg = r8153_hw_phy_cfg;
ops->autosuspend_en = rtl8153_runtime_enable;
@@ -5400,7 +5386,7 @@ static int rtl_ops_init(struct r8152 *tp)
ops->down = rtl8153b_down;
ops->unload = rtl8153b_unload;
ops->eee_get = r8153_get_eee;
- ops->eee_set = r8153b_set_eee;
+ ops->eee_set = r8152_set_eee;
ops->in_nway = rtl8153_in_nway;
ops->hw_phy_cfg = r8153b_hw_phy_cfg;
ops->autosuspend_en = rtl8153b_runtime_enable;
--
2.21.0
^ permalink raw reply related
* [PATCH net-next v4 1/2] r8152: saving the settings of EEE
From: Hayes Wang @ 2019-08-23 7:33 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
In-Reply-To: <1394712342-15778-311-Taiwan-albertk@realtek.com>
Saving the settings of EEE to avoid they become the default settings
after reset_resume().
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 80 +++++++++++++++++++++++++----------------
1 file changed, 50 insertions(+), 30 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 1aa61610f0bb..a7aa48bee732 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -751,6 +751,7 @@ struct r8152 {
atomic_t rx_count;
+ bool eee_en;
int intr_interval;
u32 saved_wolopts;
u32 msg_enable;
@@ -762,6 +763,7 @@ struct r8152 {
u16 ocp_base;
u16 speed;
+ u16 eee_adv;
u8 *intr_buff;
u8 version;
u8 duplex;
@@ -3202,8 +3204,13 @@ static void r8152_eee_en(struct r8152 *tp, bool enable)
static void r8152b_enable_eee(struct r8152 *tp)
{
- r8152_eee_en(tp, true);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, MDIO_EEE_100TX);
+ if (tp->eee_en) {
+ r8152_eee_en(tp, true);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, tp->eee_adv);
+ } else {
+ r8152_eee_en(tp, false);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ }
}
static void r8152b_enable_fc(struct r8152 *tp)
@@ -3495,8 +3502,10 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
sram_write(tp, SRAM_10M_AMP1, 0x00af);
sram_write(tp, SRAM_10M_AMP2, 0x0208);
- r8153_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, MDIO_EEE_1000T | MDIO_EEE_100TX);
+ if (tp->eee_en) {
+ r8153_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ }
r8153_aldps_en(tp, true);
r8152b_enable_fc(tp);
@@ -3599,8 +3608,10 @@ static void r8153b_hw_phy_cfg(struct r8152 *tp)
r8153b_ups_flags_w1w0(tp, ups_flags, 0);
- r8153b_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, MDIO_EEE_1000T | MDIO_EEE_100TX);
+ if (tp->eee_en) {
+ r8153b_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ }
r8153b_aldps_en(tp, true);
r8153b_enable_fc(tp);
@@ -4891,7 +4902,7 @@ static void rtl8152_get_strings(struct net_device *dev, u32 stringset, u8 *data)
static int r8152_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
- u32 ocp_data, lp, adv, supported = 0;
+ u32 lp, adv, supported = 0;
u16 val;
val = r8152_mmd_read(tp, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
@@ -4903,13 +4914,10 @@ static int r8152_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
val = r8152_mmd_read(tp, MDIO_MMD_AN, MDIO_AN_EEE_LPABLE);
lp = mmd_eee_adv_to_ethtool_adv_t(val);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
- ocp_data &= EEE_RX_EN | EEE_TX_EN;
-
- eee->eee_enabled = !!ocp_data;
+ eee->eee_enabled = tp->eee_en;
eee->eee_active = !!(supported & adv & lp);
eee->supported = supported;
- eee->advertised = adv;
+ eee->advertised = tp->eee_adv;
eee->lp_advertised = lp;
return 0;
@@ -4919,19 +4927,22 @@ static int r8152_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
- r8152_eee_en(tp, eee->eee_enabled);
+ tp->eee_en = eee->eee_enabled;
+ tp->eee_adv = val;
- if (!eee->eee_enabled)
- val = 0;
+ r8152_eee_en(tp, eee->eee_enabled);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val);
+ if (eee->eee_enabled)
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val);
+ else
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
return 0;
}
static int r8153_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
- u32 ocp_data, lp, adv, supported = 0;
+ u32 lp, adv, supported = 0;
u16 val;
val = ocp_reg_read(tp, OCP_EEE_ABLE);
@@ -4943,13 +4954,10 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
val = ocp_reg_read(tp, OCP_EEE_LPABLE);
lp = mmd_eee_adv_to_ethtool_adv_t(val);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
- ocp_data &= EEE_RX_EN | EEE_TX_EN;
-
- eee->eee_enabled = !!ocp_data;
+ eee->eee_enabled = tp->eee_en;
eee->eee_active = !!(supported & adv & lp);
eee->supported = supported;
- eee->advertised = adv;
+ eee->advertised = tp->eee_adv;
eee->lp_advertised = lp;
return 0;
@@ -4959,12 +4967,15 @@ static int r8153_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
- r8153_eee_en(tp, eee->eee_enabled);
+ tp->eee_en = eee->eee_enabled;
+ tp->eee_adv = val;
- if (!eee->eee_enabled)
- val = 0;
+ r8153_eee_en(tp, eee->eee_enabled);
- ocp_reg_write(tp, OCP_EEE_ADV, val);
+ if (eee->eee_enabled)
+ ocp_reg_write(tp, OCP_EEE_ADV, val);
+ else
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
return 0;
}
@@ -4973,12 +4984,15 @@ static int r8153b_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
- r8153b_eee_en(tp, eee->eee_enabled);
+ tp->eee_en = eee->eee_enabled;
+ tp->eee_adv = val;
- if (!eee->eee_enabled)
- val = 0;
+ r8153b_eee_en(tp, eee->eee_enabled);
- ocp_reg_write(tp, OCP_EEE_ADV, val);
+ if (eee->eee_enabled)
+ ocp_reg_write(tp, OCP_EEE_ADV, val);
+ else
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
return 0;
}
@@ -5353,6 +5367,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8152b_hw_phy_cfg;
ops->autosuspend_en = rtl_runtime_suspend_enable;
tp->rx_buf_sz = 16 * 1024;
+ tp->eee_en = true;
+ tp->eee_adv = MDIO_EEE_100TX;
break;
case RTL_VER_03:
@@ -5371,6 +5387,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8153_hw_phy_cfg;
ops->autosuspend_en = rtl8153_runtime_enable;
tp->rx_buf_sz = 32 * 1024;
+ tp->eee_en = true;
+ tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
break;
case RTL_VER_08:
@@ -5387,6 +5405,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8153b_hw_phy_cfg;
ops->autosuspend_en = rtl8153b_runtime_enable;
tp->rx_buf_sz = 32 * 1024;
+ tp->eee_en = true;
+ tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
break;
default:
--
2.21.0
^ permalink raw reply related
* [PATCH net-next v3 2/2] r8152: add a helper function about setting EEE
From: Hayes Wang @ 2019-08-23 7:19 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
In-Reply-To: <1394712342-15778-308-Taiwan-albertk@realtek.com>
Add a helper funtion "rtl_eee_enable" for setting EEE. Besides, I
move r8153_eee_en() and r8153b_eee_en(). And, I remove r8152b_enable_eee(),
r8153_set_eee(), and r8153b_set_eee().
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 166 +++++++++++++++++++---------------------
1 file changed, 77 insertions(+), 89 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index a7aa48bee732..a003591c3078 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3202,14 +3202,75 @@ static void r8152_eee_en(struct r8152 *tp, bool enable)
ocp_reg_write(tp, OCP_EEE_CONFIG3, config3);
}
-static void r8152b_enable_eee(struct r8152 *tp)
+static void r8153_eee_en(struct r8152 *tp, bool enable)
{
- if (tp->eee_en) {
- r8152_eee_en(tp, true);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, tp->eee_adv);
+ u32 ocp_data;
+ u16 config;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
+ config = ocp_reg_read(tp, OCP_EEE_CFG);
+
+ if (enable) {
+ ocp_data |= EEE_RX_EN | EEE_TX_EN;
+ config |= EEE10_EN;
} else {
- r8152_eee_en(tp, false);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ ocp_data &= ~(EEE_RX_EN | EEE_TX_EN);
+ config &= ~EEE10_EN;
+ }
+
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEE_CR, ocp_data);
+ ocp_reg_write(tp, OCP_EEE_CFG, config);
+}
+
+static void r8153b_eee_en(struct r8152 *tp, bool enable)
+{
+ r8153_eee_en(tp, enable);
+
+ if (enable)
+ r8153b_ups_flags_w1w0(tp, UPS_FLAGS_EN_EEE, 0);
+ else
+ r8153b_ups_flags_w1w0(tp, 0, UPS_FLAGS_EN_EEE);
+}
+
+static void rtl_eee_enable(struct r8152 *tp, bool enable)
+{
+ switch (tp->version) {
+ case RTL_VER_01:
+ case RTL_VER_02:
+ case RTL_VER_07:
+ if (enable) {
+ r8152_eee_en(tp, true);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV,
+ tp->eee_adv);
+ } else {
+ r8152_eee_en(tp, false);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ }
+ break;
+ case RTL_VER_03:
+ case RTL_VER_04:
+ case RTL_VER_05:
+ case RTL_VER_06:
+ if (enable) {
+ r8153_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ } else {
+ r8153_eee_en(tp, false);
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
+ }
+ break;
+ case RTL_VER_08:
+ case RTL_VER_09:
+ if (enable) {
+ r8153b_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ } else {
+ r8153b_eee_en(tp, false);
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
+ }
+ break;
+ default:
+ break;
}
}
@@ -3231,7 +3292,7 @@ static void rtl8152_disable(struct r8152 *tp)
static void r8152b_hw_phy_cfg(struct r8152 *tp)
{
- r8152b_enable_eee(tp);
+ rtl_eee_enable(tp, tp->eee_en);
r8152_aldps_en(tp, true);
r8152b_enable_fc(tp);
@@ -3425,36 +3486,6 @@ static void r8153b_aldps_en(struct r8152 *tp, bool enable)
r8153b_ups_flags_w1w0(tp, 0, UPS_FLAGS_EN_ALDPS);
}
-static void r8153_eee_en(struct r8152 *tp, bool enable)
-{
- u32 ocp_data;
- u16 config;
-
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
- config = ocp_reg_read(tp, OCP_EEE_CFG);
-
- if (enable) {
- ocp_data |= EEE_RX_EN | EEE_TX_EN;
- config |= EEE10_EN;
- } else {
- ocp_data &= ~(EEE_RX_EN | EEE_TX_EN);
- config &= ~EEE10_EN;
- }
-
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEE_CR, ocp_data);
- ocp_reg_write(tp, OCP_EEE_CFG, config);
-}
-
-static void r8153b_eee_en(struct r8152 *tp, bool enable)
-{
- r8153_eee_en(tp, enable);
-
- if (enable)
- r8153b_ups_flags_w1w0(tp, UPS_FLAGS_EN_EEE, 0);
- else
- r8153b_ups_flags_w1w0(tp, 0, UPS_FLAGS_EN_EEE);
-}
-
static void r8153b_enable_fc(struct r8152 *tp)
{
r8152b_enable_fc(tp);
@@ -3470,7 +3501,7 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
r8153_aldps_en(tp, false);
/* disable EEE before updating the PHY parameters */
- r8153_eee_en(tp, false);
+ rtl_eee_enable(tp, false);
ocp_reg_write(tp, OCP_EEE_ADV, 0);
if (tp->version == RTL_VER_03) {
@@ -3502,10 +3533,8 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
sram_write(tp, SRAM_10M_AMP1, 0x00af);
sram_write(tp, SRAM_10M_AMP2, 0x0208);
- if (tp->eee_en) {
- r8153_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
- }
+ if (tp->eee_en)
+ rtl_eee_enable(tp, true);
r8153_aldps_en(tp, true);
r8152b_enable_fc(tp);
@@ -3545,7 +3574,7 @@ static void r8153b_hw_phy_cfg(struct r8152 *tp)
r8153b_aldps_en(tp, false);
/* disable EEE before updating the PHY parameters */
- r8153b_eee_en(tp, false);
+ rtl_eee_enable(tp, false);
ocp_reg_write(tp, OCP_EEE_ADV, 0);
r8153b_green_en(tp, test_bit(GREEN_ETHERNET, &tp->flags));
@@ -3608,10 +3637,8 @@ static void r8153b_hw_phy_cfg(struct r8152 *tp)
r8153b_ups_flags_w1w0(tp, ups_flags, 0);
- if (tp->eee_en) {
- r8153b_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
- }
+ if (tp->eee_en)
+ rtl_eee_enable(tp, true);
r8153b_aldps_en(tp, true);
r8153b_enable_fc(tp);
@@ -4930,12 +4957,7 @@ static int r8152_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
tp->eee_en = eee->eee_enabled;
tp->eee_adv = val;
- r8152_eee_en(tp, eee->eee_enabled);
-
- if (eee->eee_enabled)
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val);
- else
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ rtl_eee_enable(tp, tp->eee_en);
return 0;
}
@@ -4963,40 +4985,6 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
return 0;
}
-static int r8153_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
-{
- u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
-
- tp->eee_en = eee->eee_enabled;
- tp->eee_adv = val;
-
- r8153_eee_en(tp, eee->eee_enabled);
-
- if (eee->eee_enabled)
- ocp_reg_write(tp, OCP_EEE_ADV, val);
- else
- ocp_reg_write(tp, OCP_EEE_ADV, 0);
-
- return 0;
-}
-
-static int r8153b_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
-{
- u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
-
- tp->eee_en = eee->eee_enabled;
- tp->eee_adv = val;
-
- r8153b_eee_en(tp, eee->eee_enabled);
-
- if (eee->eee_enabled)
- ocp_reg_write(tp, OCP_EEE_ADV, val);
- else
- ocp_reg_write(tp, OCP_EEE_ADV, 0);
-
- return 0;
-}
-
static int
rtl_ethtool_get_eee(struct net_device *net, struct ethtool_eee *edata)
{
@@ -5382,7 +5370,7 @@ static int rtl_ops_init(struct r8152 *tp)
ops->down = rtl8153_down;
ops->unload = rtl8153_unload;
ops->eee_get = r8153_get_eee;
- ops->eee_set = r8153_set_eee;
+ ops->eee_set = r8152_set_eee;
ops->in_nway = rtl8153_in_nway;
ops->hw_phy_cfg = r8153_hw_phy_cfg;
ops->autosuspend_en = rtl8153_runtime_enable;
@@ -5400,7 +5388,7 @@ static int rtl_ops_init(struct r8152 *tp)
ops->down = rtl8153b_down;
ops->unload = rtl8153b_unload;
ops->eee_get = r8153_get_eee;
- ops->eee_set = r8153b_set_eee;
+ ops->eee_set = r8152_set_eee;
ops->in_nway = rtl8153_in_nway;
ops->hw_phy_cfg = r8153b_hw_phy_cfg;
ops->autosuspend_en = rtl8153b_runtime_enable;
--
2.21.0
^ permalink raw reply related
* [PATCH net-next v3 1/2] r8152: saving the settings of EEE
From: Hayes Wang @ 2019-08-23 7:19 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
In-Reply-To: <1394712342-15778-308-Taiwan-albertk@realtek.com>
Saving the settings of EEE to avoid they become the default settings
after reset_resume().
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 80 +++++++++++++++++++++++++----------------
1 file changed, 50 insertions(+), 30 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 1aa61610f0bb..a7aa48bee732 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -751,6 +751,7 @@ struct r8152 {
atomic_t rx_count;
+ bool eee_en;
int intr_interval;
u32 saved_wolopts;
u32 msg_enable;
@@ -762,6 +763,7 @@ struct r8152 {
u16 ocp_base;
u16 speed;
+ u16 eee_adv;
u8 *intr_buff;
u8 version;
u8 duplex;
@@ -3202,8 +3204,13 @@ static void r8152_eee_en(struct r8152 *tp, bool enable)
static void r8152b_enable_eee(struct r8152 *tp)
{
- r8152_eee_en(tp, true);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, MDIO_EEE_100TX);
+ if (tp->eee_en) {
+ r8152_eee_en(tp, true);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, tp->eee_adv);
+ } else {
+ r8152_eee_en(tp, false);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ }
}
static void r8152b_enable_fc(struct r8152 *tp)
@@ -3495,8 +3502,10 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
sram_write(tp, SRAM_10M_AMP1, 0x00af);
sram_write(tp, SRAM_10M_AMP2, 0x0208);
- r8153_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, MDIO_EEE_1000T | MDIO_EEE_100TX);
+ if (tp->eee_en) {
+ r8153_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ }
r8153_aldps_en(tp, true);
r8152b_enable_fc(tp);
@@ -3599,8 +3608,10 @@ static void r8153b_hw_phy_cfg(struct r8152 *tp)
r8153b_ups_flags_w1w0(tp, ups_flags, 0);
- r8153b_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, MDIO_EEE_1000T | MDIO_EEE_100TX);
+ if (tp->eee_en) {
+ r8153b_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ }
r8153b_aldps_en(tp, true);
r8153b_enable_fc(tp);
@@ -4891,7 +4902,7 @@ static void rtl8152_get_strings(struct net_device *dev, u32 stringset, u8 *data)
static int r8152_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
- u32 ocp_data, lp, adv, supported = 0;
+ u32 lp, adv, supported = 0;
u16 val;
val = r8152_mmd_read(tp, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
@@ -4903,13 +4914,10 @@ static int r8152_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
val = r8152_mmd_read(tp, MDIO_MMD_AN, MDIO_AN_EEE_LPABLE);
lp = mmd_eee_adv_to_ethtool_adv_t(val);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
- ocp_data &= EEE_RX_EN | EEE_TX_EN;
-
- eee->eee_enabled = !!ocp_data;
+ eee->eee_enabled = tp->eee_en;
eee->eee_active = !!(supported & adv & lp);
eee->supported = supported;
- eee->advertised = adv;
+ eee->advertised = tp->eee_adv;
eee->lp_advertised = lp;
return 0;
@@ -4919,19 +4927,22 @@ static int r8152_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
- r8152_eee_en(tp, eee->eee_enabled);
+ tp->eee_en = eee->eee_enabled;
+ tp->eee_adv = val;
- if (!eee->eee_enabled)
- val = 0;
+ r8152_eee_en(tp, eee->eee_enabled);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val);
+ if (eee->eee_enabled)
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val);
+ else
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
return 0;
}
static int r8153_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
- u32 ocp_data, lp, adv, supported = 0;
+ u32 lp, adv, supported = 0;
u16 val;
val = ocp_reg_read(tp, OCP_EEE_ABLE);
@@ -4943,13 +4954,10 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
val = ocp_reg_read(tp, OCP_EEE_LPABLE);
lp = mmd_eee_adv_to_ethtool_adv_t(val);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
- ocp_data &= EEE_RX_EN | EEE_TX_EN;
-
- eee->eee_enabled = !!ocp_data;
+ eee->eee_enabled = tp->eee_en;
eee->eee_active = !!(supported & adv & lp);
eee->supported = supported;
- eee->advertised = adv;
+ eee->advertised = tp->eee_adv;
eee->lp_advertised = lp;
return 0;
@@ -4959,12 +4967,15 @@ static int r8153_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
- r8153_eee_en(tp, eee->eee_enabled);
+ tp->eee_en = eee->eee_enabled;
+ tp->eee_adv = val;
- if (!eee->eee_enabled)
- val = 0;
+ r8153_eee_en(tp, eee->eee_enabled);
- ocp_reg_write(tp, OCP_EEE_ADV, val);
+ if (eee->eee_enabled)
+ ocp_reg_write(tp, OCP_EEE_ADV, val);
+ else
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
return 0;
}
@@ -4973,12 +4984,15 @@ static int r8153b_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
- r8153b_eee_en(tp, eee->eee_enabled);
+ tp->eee_en = eee->eee_enabled;
+ tp->eee_adv = val;
- if (!eee->eee_enabled)
- val = 0;
+ r8153b_eee_en(tp, eee->eee_enabled);
- ocp_reg_write(tp, OCP_EEE_ADV, val);
+ if (eee->eee_enabled)
+ ocp_reg_write(tp, OCP_EEE_ADV, val);
+ else
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
return 0;
}
@@ -5353,6 +5367,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8152b_hw_phy_cfg;
ops->autosuspend_en = rtl_runtime_suspend_enable;
tp->rx_buf_sz = 16 * 1024;
+ tp->eee_en = true;
+ tp->eee_adv = MDIO_EEE_100TX;
break;
case RTL_VER_03:
@@ -5371,6 +5387,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8153_hw_phy_cfg;
ops->autosuspend_en = rtl8153_runtime_enable;
tp->rx_buf_sz = 32 * 1024;
+ tp->eee_en = true;
+ tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
break;
case RTL_VER_08:
@@ -5387,6 +5405,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8153b_hw_phy_cfg;
ops->autosuspend_en = rtl8153b_runtime_enable;
tp->rx_buf_sz = 32 * 1024;
+ tp->eee_en = true;
+ tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
break;
default:
--
2.21.0
^ permalink raw reply related
* [PATCH net-next v3 0/2] r8152: save EEE
From: Hayes Wang @ 2019-08-23 7:18 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
In-Reply-To: <1394712342-15778-304-Taiwan-albertk@realtek.com>
v3:
For patch #2, fix the mistake caused by copying and pasting.
v2:
Adjust patch #1. The EEE has been disabled in the beginning of
r8153_hw_phy_cfg() and r8153b_hw_phy_cfg(), so only check if
it is necessary to enable EEE.
Add the patch #2 for the helper function.
v1:
Saving the settings of EEE to avoid they become the default settings
after reset_resume().
Hayes Wang (2):
r8152: saving the settings of EEE
r8152: add a helper function about setting EEE
drivers/net/usb/r8152.c | 182 +++++++++++++++++++++-------------------
1 file changed, 95 insertions(+), 87 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH net-next v2 0/2] Save EEE
From: Hayes Wang @ 2019-08-23 7:04 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
In-Reply-To: <1394712342-15778-304-Taiwan-albertk@realtek.com>
v2:
Adjust patch #1. The EEE has been disabled in the beginning of
r8153_hw_phy_cfg() and r8153b_hw_phy_cfg(), so only check if
it is necessary to enable EEE.
Add the patch #2 for the helper function.
v1:
Saving the settings of EEE to avoid they become the default settings
after reset_resume().
Hayes Wang (2):
r8152: saving the settings of EEE
r8152: add a helper function about setting EEE
drivers/net/usb/r8152.c | 182 +++++++++++++++++++++-------------------
1 file changed, 95 insertions(+), 87 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH net-next v2 2/2] r8152: add a helper function about setting EEE
From: Hayes Wang @ 2019-08-23 7:04 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
In-Reply-To: <1394712342-15778-305-Taiwan-albertk@realtek.com>
Add a helper funtcion "rtl_eee_enable" for setting EEE. Besides, I
move r8153_eee_en() and r8153b_eee_en(). And, I remove r8152b_enable_eee(),
r8153_set_eee(), and r8153b_set_eee().
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 166 +++++++++++++++++++---------------------
1 file changed, 77 insertions(+), 89 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index a7aa48bee732..220079a8882f 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3202,14 +3202,75 @@ static void r8152_eee_en(struct r8152 *tp, bool enable)
ocp_reg_write(tp, OCP_EEE_CONFIG3, config3);
}
-static void r8152b_enable_eee(struct r8152 *tp)
+static void r8153_eee_en(struct r8152 *tp, bool enable)
{
- if (tp->eee_en) {
- r8152_eee_en(tp, true);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, tp->eee_adv);
+ u32 ocp_data;
+ u16 config;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
+ config = ocp_reg_read(tp, OCP_EEE_CFG);
+
+ if (enable) {
+ ocp_data |= EEE_RX_EN | EEE_TX_EN;
+ config |= EEE10_EN;
} else {
- r8152_eee_en(tp, false);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ ocp_data &= ~(EEE_RX_EN | EEE_TX_EN);
+ config &= ~EEE10_EN;
+ }
+
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEE_CR, ocp_data);
+ ocp_reg_write(tp, OCP_EEE_CFG, config);
+}
+
+static void r8153b_eee_en(struct r8152 *tp, bool enable)
+{
+ r8153_eee_en(tp, enable);
+
+ if (enable)
+ r8153b_ups_flags_w1w0(tp, UPS_FLAGS_EN_EEE, 0);
+ else
+ r8153b_ups_flags_w1w0(tp, 0, UPS_FLAGS_EN_EEE);
+}
+
+static void rtl_eee_enable(struct r8152 *tp, bool enable)
+{
+ switch (tp->version) {
+ case RTL_VER_01:
+ case RTL_VER_02:
+ case RTL_VER_07:
+ if (enable) {
+ r8152_eee_en(tp, true);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV,
+ tp->eee_adv);
+ } else {
+ r8152_eee_en(tp, false);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ }
+ break;
+ case RTL_VER_03:
+ case RTL_VER_04:
+ case RTL_VER_05:
+ case RTL_VER_06:
+ if (enable) {
+ r8153_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ } else {
+ r8153_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
+ }
+ break;
+ case RTL_VER_08:
+ case RTL_VER_09:
+ if (enable) {
+ r8153b_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ } else {
+ r8153b_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
+ }
+ break;
+ default:
+ break;
}
}
@@ -3231,7 +3292,7 @@ static void rtl8152_disable(struct r8152 *tp)
static void r8152b_hw_phy_cfg(struct r8152 *tp)
{
- r8152b_enable_eee(tp);
+ rtl_eee_enable(tp, tp->eee_en);
r8152_aldps_en(tp, true);
r8152b_enable_fc(tp);
@@ -3425,36 +3486,6 @@ static void r8153b_aldps_en(struct r8152 *tp, bool enable)
r8153b_ups_flags_w1w0(tp, 0, UPS_FLAGS_EN_ALDPS);
}
-static void r8153_eee_en(struct r8152 *tp, bool enable)
-{
- u32 ocp_data;
- u16 config;
-
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
- config = ocp_reg_read(tp, OCP_EEE_CFG);
-
- if (enable) {
- ocp_data |= EEE_RX_EN | EEE_TX_EN;
- config |= EEE10_EN;
- } else {
- ocp_data &= ~(EEE_RX_EN | EEE_TX_EN);
- config &= ~EEE10_EN;
- }
-
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEE_CR, ocp_data);
- ocp_reg_write(tp, OCP_EEE_CFG, config);
-}
-
-static void r8153b_eee_en(struct r8152 *tp, bool enable)
-{
- r8153_eee_en(tp, enable);
-
- if (enable)
- r8153b_ups_flags_w1w0(tp, UPS_FLAGS_EN_EEE, 0);
- else
- r8153b_ups_flags_w1w0(tp, 0, UPS_FLAGS_EN_EEE);
-}
-
static void r8153b_enable_fc(struct r8152 *tp)
{
r8152b_enable_fc(tp);
@@ -3470,7 +3501,7 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
r8153_aldps_en(tp, false);
/* disable EEE before updating the PHY parameters */
- r8153_eee_en(tp, false);
+ rtl_eee_enable(tp, false);
ocp_reg_write(tp, OCP_EEE_ADV, 0);
if (tp->version == RTL_VER_03) {
@@ -3502,10 +3533,8 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
sram_write(tp, SRAM_10M_AMP1, 0x00af);
sram_write(tp, SRAM_10M_AMP2, 0x0208);
- if (tp->eee_en) {
- r8153_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
- }
+ if (tp->eee_en)
+ rtl_eee_enable(tp, true);
r8153_aldps_en(tp, true);
r8152b_enable_fc(tp);
@@ -3545,7 +3574,7 @@ static void r8153b_hw_phy_cfg(struct r8152 *tp)
r8153b_aldps_en(tp, false);
/* disable EEE before updating the PHY parameters */
- r8153b_eee_en(tp, false);
+ rtl_eee_enable(tp, false);
ocp_reg_write(tp, OCP_EEE_ADV, 0);
r8153b_green_en(tp, test_bit(GREEN_ETHERNET, &tp->flags));
@@ -3608,10 +3637,8 @@ static void r8153b_hw_phy_cfg(struct r8152 *tp)
r8153b_ups_flags_w1w0(tp, ups_flags, 0);
- if (tp->eee_en) {
- r8153b_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
- }
+ if (tp->eee_en)
+ rtl_eee_enable(tp, true);
r8153b_aldps_en(tp, true);
r8153b_enable_fc(tp);
@@ -4930,12 +4957,7 @@ static int r8152_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
tp->eee_en = eee->eee_enabled;
tp->eee_adv = val;
- r8152_eee_en(tp, eee->eee_enabled);
-
- if (eee->eee_enabled)
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val);
- else
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ rtl_eee_enable(tp, tp->eee_en);
return 0;
}
@@ -4963,40 +4985,6 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
return 0;
}
-static int r8153_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
-{
- u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
-
- tp->eee_en = eee->eee_enabled;
- tp->eee_adv = val;
-
- r8153_eee_en(tp, eee->eee_enabled);
-
- if (eee->eee_enabled)
- ocp_reg_write(tp, OCP_EEE_ADV, val);
- else
- ocp_reg_write(tp, OCP_EEE_ADV, 0);
-
- return 0;
-}
-
-static int r8153b_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
-{
- u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
-
- tp->eee_en = eee->eee_enabled;
- tp->eee_adv = val;
-
- r8153b_eee_en(tp, eee->eee_enabled);
-
- if (eee->eee_enabled)
- ocp_reg_write(tp, OCP_EEE_ADV, val);
- else
- ocp_reg_write(tp, OCP_EEE_ADV, 0);
-
- return 0;
-}
-
static int
rtl_ethtool_get_eee(struct net_device *net, struct ethtool_eee *edata)
{
@@ -5382,7 +5370,7 @@ static int rtl_ops_init(struct r8152 *tp)
ops->down = rtl8153_down;
ops->unload = rtl8153_unload;
ops->eee_get = r8153_get_eee;
- ops->eee_set = r8153_set_eee;
+ ops->eee_set = r8152_set_eee;
ops->in_nway = rtl8153_in_nway;
ops->hw_phy_cfg = r8153_hw_phy_cfg;
ops->autosuspend_en = rtl8153_runtime_enable;
@@ -5400,7 +5388,7 @@ static int rtl_ops_init(struct r8152 *tp)
ops->down = rtl8153b_down;
ops->unload = rtl8153b_unload;
ops->eee_get = r8153_get_eee;
- ops->eee_set = r8153b_set_eee;
+ ops->eee_set = r8152_set_eee;
ops->in_nway = rtl8153_in_nway;
ops->hw_phy_cfg = r8153b_hw_phy_cfg;
ops->autosuspend_en = rtl8153b_runtime_enable;
--
2.21.0
^ permalink raw reply related
* [PATCH net-next v2 1/2] r8152: saving the settings of EEE
From: Hayes Wang @ 2019-08-23 7:04 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
In-Reply-To: <1394712342-15778-305-Taiwan-albertk@realtek.com>
Saving the settings of EEE to avoid they become the default settings
after reset_resume().
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 80 +++++++++++++++++++++++++----------------
1 file changed, 50 insertions(+), 30 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 1aa61610f0bb..a7aa48bee732 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -751,6 +751,7 @@ struct r8152 {
atomic_t rx_count;
+ bool eee_en;
int intr_interval;
u32 saved_wolopts;
u32 msg_enable;
@@ -762,6 +763,7 @@ struct r8152 {
u16 ocp_base;
u16 speed;
+ u16 eee_adv;
u8 *intr_buff;
u8 version;
u8 duplex;
@@ -3202,8 +3204,13 @@ static void r8152_eee_en(struct r8152 *tp, bool enable)
static void r8152b_enable_eee(struct r8152 *tp)
{
- r8152_eee_en(tp, true);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, MDIO_EEE_100TX);
+ if (tp->eee_en) {
+ r8152_eee_en(tp, true);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, tp->eee_adv);
+ } else {
+ r8152_eee_en(tp, false);
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+ }
}
static void r8152b_enable_fc(struct r8152 *tp)
@@ -3495,8 +3502,10 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
sram_write(tp, SRAM_10M_AMP1, 0x00af);
sram_write(tp, SRAM_10M_AMP2, 0x0208);
- r8153_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, MDIO_EEE_1000T | MDIO_EEE_100TX);
+ if (tp->eee_en) {
+ r8153_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ }
r8153_aldps_en(tp, true);
r8152b_enable_fc(tp);
@@ -3599,8 +3608,10 @@ static void r8153b_hw_phy_cfg(struct r8152 *tp)
r8153b_ups_flags_w1w0(tp, ups_flags, 0);
- r8153b_eee_en(tp, true);
- ocp_reg_write(tp, OCP_EEE_ADV, MDIO_EEE_1000T | MDIO_EEE_100TX);
+ if (tp->eee_en) {
+ r8153b_eee_en(tp, true);
+ ocp_reg_write(tp, OCP_EEE_ADV, tp->eee_adv);
+ }
r8153b_aldps_en(tp, true);
r8153b_enable_fc(tp);
@@ -4891,7 +4902,7 @@ static void rtl8152_get_strings(struct net_device *dev, u32 stringset, u8 *data)
static int r8152_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
- u32 ocp_data, lp, adv, supported = 0;
+ u32 lp, adv, supported = 0;
u16 val;
val = r8152_mmd_read(tp, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
@@ -4903,13 +4914,10 @@ static int r8152_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
val = r8152_mmd_read(tp, MDIO_MMD_AN, MDIO_AN_EEE_LPABLE);
lp = mmd_eee_adv_to_ethtool_adv_t(val);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
- ocp_data &= EEE_RX_EN | EEE_TX_EN;
-
- eee->eee_enabled = !!ocp_data;
+ eee->eee_enabled = tp->eee_en;
eee->eee_active = !!(supported & adv & lp);
eee->supported = supported;
- eee->advertised = adv;
+ eee->advertised = tp->eee_adv;
eee->lp_advertised = lp;
return 0;
@@ -4919,19 +4927,22 @@ static int r8152_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
- r8152_eee_en(tp, eee->eee_enabled);
+ tp->eee_en = eee->eee_enabled;
+ tp->eee_adv = val;
- if (!eee->eee_enabled)
- val = 0;
+ r8152_eee_en(tp, eee->eee_enabled);
- r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val);
+ if (eee->eee_enabled)
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val);
+ else
+ r8152_mmd_write(tp, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
return 0;
}
static int r8153_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
- u32 ocp_data, lp, adv, supported = 0;
+ u32 lp, adv, supported = 0;
u16 val;
val = ocp_reg_read(tp, OCP_EEE_ABLE);
@@ -4943,13 +4954,10 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_eee *eee)
val = ocp_reg_read(tp, OCP_EEE_LPABLE);
lp = mmd_eee_adv_to_ethtool_adv_t(val);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEE_CR);
- ocp_data &= EEE_RX_EN | EEE_TX_EN;
-
- eee->eee_enabled = !!ocp_data;
+ eee->eee_enabled = tp->eee_en;
eee->eee_active = !!(supported & adv & lp);
eee->supported = supported;
- eee->advertised = adv;
+ eee->advertised = tp->eee_adv;
eee->lp_advertised = lp;
return 0;
@@ -4959,12 +4967,15 @@ static int r8153_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
- r8153_eee_en(tp, eee->eee_enabled);
+ tp->eee_en = eee->eee_enabled;
+ tp->eee_adv = val;
- if (!eee->eee_enabled)
- val = 0;
+ r8153_eee_en(tp, eee->eee_enabled);
- ocp_reg_write(tp, OCP_EEE_ADV, val);
+ if (eee->eee_enabled)
+ ocp_reg_write(tp, OCP_EEE_ADV, val);
+ else
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
return 0;
}
@@ -4973,12 +4984,15 @@ static int r8153b_set_eee(struct r8152 *tp, struct ethtool_eee *eee)
{
u16 val = ethtool_adv_to_mmd_eee_adv_t(eee->advertised);
- r8153b_eee_en(tp, eee->eee_enabled);
+ tp->eee_en = eee->eee_enabled;
+ tp->eee_adv = val;
- if (!eee->eee_enabled)
- val = 0;
+ r8153b_eee_en(tp, eee->eee_enabled);
- ocp_reg_write(tp, OCP_EEE_ADV, val);
+ if (eee->eee_enabled)
+ ocp_reg_write(tp, OCP_EEE_ADV, val);
+ else
+ ocp_reg_write(tp, OCP_EEE_ADV, 0);
return 0;
}
@@ -5353,6 +5367,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8152b_hw_phy_cfg;
ops->autosuspend_en = rtl_runtime_suspend_enable;
tp->rx_buf_sz = 16 * 1024;
+ tp->eee_en = true;
+ tp->eee_adv = MDIO_EEE_100TX;
break;
case RTL_VER_03:
@@ -5371,6 +5387,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8153_hw_phy_cfg;
ops->autosuspend_en = rtl8153_runtime_enable;
tp->rx_buf_sz = 32 * 1024;
+ tp->eee_en = true;
+ tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
break;
case RTL_VER_08:
@@ -5387,6 +5405,8 @@ static int rtl_ops_init(struct r8152 *tp)
ops->hw_phy_cfg = r8153b_hw_phy_cfg;
ops->autosuspend_en = rtl8153b_runtime_enable;
tp->rx_buf_sz = 32 * 1024;
+ tp->eee_en = true;
+ tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
break;
default:
--
2.21.0
^ permalink raw reply related
* Re: [PATCH net v2] openvswitch: Fix conntrack cache with timeout
From: Pravin Shelar @ 2019-08-23 6:53 UTC (permalink / raw)
To: Yi-Hung Wei; +Cc: Linux Kernel Network Developers
In-Reply-To: <1566505070-38748-1-git-send-email-yihung.wei@gmail.com>
On Thu, Aug 22, 2019 at 1:28 PM Yi-Hung Wei <yihung.wei@gmail.com> wrote:
>
> This patch addresses a conntrack cache issue with timeout policy.
> Currently, we do not check if the timeout extension is set properly in the
> cached conntrack entry. Thus, after packet recirculate from conntrack
> action, the timeout policy is not applied properly. This patch fixes the
> aforementioned issue.
>
> Fixes: 06bd2bdf19d2 ("openvswitch: Add timeout support to ct action")
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
> ---
> v1->v2: Fix rcu dereference issue reported by kbuild test robot.
> ---
> net/openvswitch/conntrack.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 848c6eb55064..4d7896135e73 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -67,6 +67,7 @@ struct ovs_conntrack_info {
> struct md_mark mark;
> struct md_labels labels;
> char timeout[CTNL_TIMEOUT_NAME_MAX];
> + struct nf_ct_timeout *nf_ct_timeout;
> #if IS_ENABLED(CONFIG_NF_NAT)
> struct nf_nat_range2 range; /* Only present for SRC NAT and DST NAT. */
> #endif
> @@ -697,6 +698,14 @@ static bool skb_nfct_cached(struct net *net,
> if (help && rcu_access_pointer(help->helper) != info->helper)
> return false;
> }
> + if (info->nf_ct_timeout) {
> + struct nf_conn_timeout *timeout_ext;
> +
> + timeout_ext = nf_ct_timeout_find(ct);
> + if (!timeout_ext || info->nf_ct_timeout !=
> + rcu_dereference(timeout_ext->timeout))
> + return false;
> + }
> /* Force conntrack entry direction to the current packet? */
> if (info->force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) {
> /* Delete the conntrack entry if confirmed, else just release
> @@ -1657,6 +1666,10 @@ int ovs_ct_copy_action(struct net *net, const struct nlattr *attr,
> ct_info.timeout))
> pr_info_ratelimited("Failed to associated timeout "
> "policy `%s'\n", ct_info.timeout);
> + else
> + ct_info.nf_ct_timeout = rcu_dereference(
> + nf_ct_timeout_find(ct_info.ct)->timeout);
Is this dereference safe from NULL pointer?
^ permalink raw reply
* Re: [PATCH net-next] net/rds: Fix info leak in rds6_inc_info_copy()
From: santosh.shilimkar @ 2019-08-23 6:32 UTC (permalink / raw)
To: Ka-Cheong Poon, netdev; +Cc: davem, rds-devel
In-Reply-To: <1566443904-12671-1-git-send-email-ka-cheong.poon@oracle.com>
On 8/21/19 8:18 PM, Ka-Cheong Poon wrote:
> The rds6_inc_info_copy() function has a couple struct members which
> are leaking stack information. The ->tos field should hold actual
> information and the ->flags field needs to be zeroed out.
>
> Fixes: 3eb450367d08 ("rds: add type of service(tos) infrastructure")
> Fixes: b7ff8b1036f0 ("rds: Extend RDS API for IPv6 support")
> Reported-by: 黄ID蝴蝶 <butterflyhuangxx@gmail.com>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
> ---
Thanks for getting this out of the list.
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
^ permalink raw reply
* Re: [RFC bpf-next 0/5] Convert iproute2 to use libbpf (WIP)
From: Andrii Nakryiko @ 2019-08-23 6:31 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Toke Høiland-Jørgensen, Stephen Hemminger,
Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
David Miller, Jesper Dangaard Brouer, Networking, bpf
In-Reply-To: <43e8c177-cc9c-ca0b-1622-e30a7a1281b7@iogearbox.net>
On Thu, Aug 22, 2019 at 1:33 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 8/22/19 9:49 AM, Andrii Nakryiko wrote:
> > On Wed, Aug 21, 2019 at 2:07 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
> >>> On Tue, Aug 20, 2019 at 4:47 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >>>>
> >>>> iproute2 uses its own bpf loader to load eBPF programs, which has
> >>>> evolved separately from libbpf. Since we are now standardising on
> >>>> libbpf, this becomes a problem as iproute2 is slowly accumulating
> >>>> feature incompatibilities with libbpf-based loaders. In particular,
> >>>> iproute2 has its own (expanded) version of the map definition struct,
> >>>> which makes it difficult to write programs that can be loaded with both
> >>>> custom loaders and iproute2.
> >>>>
> >>>> This series seeks to address this by converting iproute2 to using libbpf
> >>>> for all its bpf needs. This version is an early proof-of-concept RFC, to
> >>>> get some feedback on whether people think this is the right direction.
> >>>>
> >>>> What this series does is the following:
> >>>>
> >>>> - Updates the libbpf map definition struct to match that of iproute2
> >>>> (patch 1).
> >>>
> >>> Thanks for taking a stab at unifying libbpf and iproute2 loaders. I'm
> >>> totally in support of making iproute2 use libbpf to load/initialize
> >>> BPF programs. But I'm against adding iproute2-specific fields to
> >>> libbpf's bpf_map_def definitions to support this.
> >>>
> >>> I've proposed the plan of extending libbpf's supported features so
> >>> that it can be used to load iproute2-style BPF programs earlier,
> >>> please see discussions in [0] and [1].
> >>
> >> Yeah, I've seen that discussion, and agree that longer term this is
> >> probably a better way to do map-in-map definitions.
> >>
> >> However, I view your proposal as complementary to this series: we'll
> >> probably also want the BTF-based definition to work with iproute2, and
> >> that means iproute2 needs to be ported to libbpf. But iproute2 needs to
> >> be backwards compatible with the format it supports now, and, well, this
> >> series is the simplest way to achieve that IMO :)
> >
> > Ok, I understand that. But I'd still want to avoid adding extra cruft
> > to libbpf just for backwards-compatibility with *exact* iproute2
> > format. Libbpf as a whole is trying to move away from relying on
> > binary bpf_map_def and into using BTF-defined map definitions, and
> > this patch series is a step backwards in that regard, that adds,
> > essentially, already outdated stuff that we'll need to support forever
> > (I mean those extra fields in bpf_map_def, that will stay there
> > forever).
>
> Agree, adding these extensions for libbpf would be a step backwards
> compared to using BTF defined map defs.
>
> > We've discussed one way to deal with it, IMO, in a cleaner way. It can
> > be done in few steps:
> >
> > 1. I originally wanted BTF-defined map definitions to ignore unknown
> > fields. It shouldn't be a default mode, but it should be supported
> > (and of course is very easy to add). So let's add that and let libbpf
> > ignore unknown stuff.
> >
> > 2. Then to let iproute2 loader deal with backwards-compatibility for
> > libbpf-incompatible bpf_elf_map, we need to "pass-through" all those
> > fields so that users of libbpf (iproute2 loader, in this case) can
> > make use of it. The easiest and cleanest way to do this is to expose
> > BTF ID of a type describing each map entry and let iproute2 process
> > that in whichever way it sees fit.
> >
> > Luckily, bpf_elf_map is compatible in `type` field, which will let
> > libbpf recognize bpf_elf_map as map definition. All the rest setup
> > will be done by iproute2, by processing BTF of bpf_elf_map, which will
> > let it set up map sizes, flags and do all of its map-in-map magic.
> >
> > The only additions to libbpf in this case would be a new `__u32
> > bpf_map__btf_id(struct bpf_map* map);` API.
> >
> > I haven't written any code and haven't 100% checked that this will
> > cover everything, but I think we should try. This will allow to let
> > users of libbpf do custom stuff with map definitions without having to
> > put all this extra logic into libbpf itself, which I think is
> > desirable outcome.
>
> Sounds reasonable in general, but all this still has the issue that we're
> assuming that BTF is /always/ present. Existing object files that would load
> just fine /today/ but do not have BTF attached won't be handled here. Wouldn't
> it be more straight forward to allow passing callbacks to the libbpf loader
> such that if the map section is not found to be bpf_map_def compatible, we
> rely on external user aka callback to parse the ELF section, handle any
> non-default libbpf behavior like pinning/retrieving from BPF fs, populate
> related internal libbpf map data structures and pass control back to libbpf
Having all those special callbacks feels a bit too narrow-focused. I
do agree that we need to provide enough flexibility to allow
non-standard BPF loaders like iproute2 to adjust and/or extend some of
BPF initialization logic, though. I'm just unsure if adding more and
more callbacks is the right approach.
Let's think slightly beyond iproute2, because iproute2 and libbpf use
the same ELF section name ("maps") for non-BTF-defined map defs, which
makes it easy to fall into the trap of designing too specific
solution. Let's take, say, BCC. BCC uses one section per each map.
And, unlike, iproute2, ELF section names don't coincide. So what if
BCC were to use libbpf as an underlying BPF loader, while preserving
its layout? This callback for "maps" section won't work at all.
BTW, I realize BCC might not be the best example here, given
on-the-fly complication nature of its programs and extensive usage of
macro, which provide quite a lot of flexibility in changing ELF
layout, if necessary, without changing user programs, but bear with
me, let's pretend we can't change some of those aspects easily. The
point I'm trying to make is that iproute2 and libbpf cases are
examples of almost compatible ELF layouts, and if we try to solve just
such case, we might end up inventing too specific solution.
What seems like a bit more flexible and generic solution is to make
libbpf API granular enough to allow users to adjust/add extra maps,
programs, relocations, whatever is necessary, before libbpf does final
loading or some other checks that would fail for normal libbpf
programs, but are ok for custom BPF programs. So instead of having
callbacks, we have API for each step, where custom loader can skip
and/or adjust behavior of each of them, while also implement their own
steps.
E.g., today's API is essentially three steps:
1. open and parse ELF: collect relos, programs, map definitions
2. load: create maps from collected defs, do program/global data/CO-RE
relocs, load and verify BPF programs
3. attach programs one by one.
Between step 1 and 2 user has flexibility to create more maps, set up
map-in-map, etc. Between 2 and 3 you can fill in global data, fill in
tail call maps, etc. That's already pretty flexible. But we can tune
and break apart those steps even further, if necessary.
E.g., for iproute2 maps, I think the biggest problem is that we can't
create a custom map dynamically, but still allow BPF instructions
relocations against that map. Plus, of course, a name clash of "maps"
ELF section with incompatible map definitions. So how about:
1. for open call, tell it to not parse "maps" section at all. We can
model that as an extra option to override map definitions ELF section
name. E.g., so that some applications can put their map defs into
"my_fancy_map_def_section" ELF section, for instance. If that section
is empty, though, libbpf will just skip step of collecting
bpf_map_defs (it can still do BTF-defined maps, btw), allowing gradual
migration.
2. provide API to add dynamically created (e.g., by iproute2 loader)
maps that are "relocatable to", before program relocations happen.
We'll need to figure out best interface here. Internally relocations
refer to maps as section index + offset and translate that to map
index. If we keep relos (internally) just as section index + offset,
it will be simple and consistent interface to add external maps that
can be relocated against, IMO. Map index is inconvenient in this case.
BTW, we'd need to answer some of those questions regardless, even for
callback-based solution. There are a bunch of details to be worked
out, of course, and I don't have exact answer to all of them right
now, but I think it's a worthwhile exercise to try to answer them and
see how API would look like.
As an aside, taking a step back and thinking about this whole API
design thing, it occurred to me that ELF is just one way to specify
programs, maps, relocations, global data, etc. But it doesn't have to
be just ELF, right? What if we have a use case where we have
"on-the-fly" creation of BPF program, with dynamically added maps,
dynamically generated BPF programs, etc. E.g., think about bpftrace
generating BPF object/program on-the-fly from their DSL language. Or
some pcap to BPF translator for firewall rules, etc, etc. It might be
inconvenient and unnecessary to generate ELF and then pass it to
libbpf to load it. Instead it could be more convenient to create an
empty bpf_object (bpf_object__new) and then use programmatic APIs to
add a bunch of maps (some sort of bpf_object__add_map) and progs
(bpf_object__add_program) with relocations against those maps, and let
libbpf do all the low-level stuff (relos, CO-RE, etc). And provide
high-level API for bpf_map, bpf_program, etc.
How API would look like to support this? Today's bpf_object__open +
bpf__object__load could be just a higher-level wrappers on top of
those APIs, constructing all the entities from standardized ELF
layout. But there still is low-level API to construct same bpf_object
construct dynamically. That seems flexible and powerful and not tied
to any particular use case, but of course requires a bit of thought
about best API.
Sorry for the wall of text :) Callback-based solutions always seem
convoluted to me, as well as hard to follow and quite often too
limited. This topic is not the easiest one to discuss over email, as
well, maybe we should chat about this while at LPC?
> loader afterwards. (Similar callback with prog section name handling for the
> case where tail call maps get automatically populated.)
I'm not sure I completely understand why we need this prog section
name callback. Can you elaborate what problem does it solve that can't
be solved with existing API?
>
> Thanks,
> Daniel
^ permalink raw reply
* Re: [PATCH net-next,v5, 0/6] Add software backchannel and mlx5e HV VHCA stats
From: David Miller @ 2019-08-23 6:11 UTC (permalink / raw)
To: saeedm
Cc: haiyangz, kys, sthemmin, lorenzo.pieralisi, linux-kernel, eranbe,
netdev, linux-pci, leon, sashal, bhelgaas, linux-hyperv
In-Reply-To: <f7a0ce8822e197ace496a348a14ac6939313d8f6.camel@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Fri, 23 Aug 2019 05:29:48 +0000
> On Thu, 2019-08-22 at 15:39 -0700, David Miller wrote:
>> From: Haiyang Zhang <haiyangz@microsoft.com>
>> Date: Thu, 22 Aug 2019 22:37:13 +0000
>>
>> > The v5 is pretty much the same as v4, except Eran had a fix to
>> patch #3 in response to
>> > Leon Romanovsky <leon@kernel.org>.
>>
>> Well you now have to send me a patch relative to v4 in order to fix
>> that.
>>
>> When I say "applied", the series is in my tree and is therefore
>> permanent.
>> It is therefore never appropriate to then post a new version of the
>> series.
>
> Dave, I think you didn't reply back to v4 that the series was applied.
> So that might have created some confusion for Haiyang.
I thought I did, sorry, my bad.
^ permalink raw reply
* Re: [PATCH net v3] ixgbe: fix double clean of tx descriptors with xdp
From: Björn Töpel @ 2019-08-23 6:10 UTC (permalink / raw)
To: William Tu, Alexander Duyck
Cc: Ilya Maximets, Netdev, LKML, bpf, David S. Miller,
Magnus Karlsson, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, Jeff Kirsher, intel-wired-lan, Eelco Chaudron
In-Reply-To: <CALDO+SYhU4krmBO8d4hsDGm+BuUAR4qMv=WzVa=jAx27+g9KnA@mail.gmail.com>
On 2019-08-22 19:32, William Tu wrote:
> On Thu, Aug 22, 2019 at 10:21 AM Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>>
>> On Thu, Aug 22, 2019 at 10:12 AM Ilya Maximets <i.maximets@samsung.com> wrote:
>>>
>>> Tx code doesn't clear the descriptors' status after cleaning.
>>> So, if the budget is larger than number of used elems in a ring, some
>>> descriptors will be accounted twice and xsk_umem_complete_tx will move
>>> prod_tail far beyond the prod_head breaking the completion queue ring.
>>>
>>> Fix that by limiting the number of descriptors to clean by the number
>>> of used descriptors in the tx ring.
>>>
>>> 'ixgbe_clean_xdp_tx_irq()' function refactored to look more like
>>> 'ixgbe_xsk_clean_tx_ring()' since we're allowed to directly use
>>> 'next_to_clean' and 'next_to_use' indexes.
>>>
>>> Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")
>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>> ---
>>>
>>> Version 3:
>>> * Reverted some refactoring made for v2.
>>> * Eliminated 'budget' for tx clean.
>>> * prefetch returned.
>>>
>>> Version 2:
>>> * 'ixgbe_clean_xdp_tx_irq()' refactored to look more like
>>> 'ixgbe_xsk_clean_tx_ring()'.
>>>
>>> drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 29 ++++++++------------
>>> 1 file changed, 11 insertions(+), 18 deletions(-)
>>
>> Thanks for addressing my concerns.
>>
>> Acked-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
>
> Thanks.
>
> Tested-by: William Tu <u9012063@gmail.com>
>
Will, thanks for testing! For this patch, did you notice any performance
degradation?
Cheers,
Björn
^ permalink raw reply
* Re: [PATCH v2 net] Add genphy_c45_config_aneg() function to phy-c45.c
From: Heiner Kallweit @ 2019-08-23 6:00 UTC (permalink / raw)
To: David Miller, marco.hartmann
Cc: andrew, f.fainelli, netdev, linux-kernel, christian.herber
In-Reply-To: <20190822.161520.1087789793326068678.davem@davemloft.net>
On 23.08.2019 01:15, David Miller wrote:
> From: Marco Hartmann <marco.hartmann@nxp.com>
> Date: Wed, 21 Aug 2019 11:00:46 +0000
>
>> Commit 34786005eca3 ("net: phy: prevent PHYs w/o Clause 22 regs from calling
>> genphy_config_aneg") introduced a check that aborts phy_config_aneg()
>> if the phy is a C45 phy.
>> This causes phy_state_machine() to call phy_error() so that the phy
>> ends up in PHY_HALTED state.
>>
>> Instead of returning -EOPNOTSUPP, call genphy_c45_config_aneg()
>> (analogous to the C22 case) so that the state machine can run
>> correctly.
>>
>> genphy_c45_config_aneg() closely resembles mv3310_config_aneg()
>> in drivers/net/phy/marvell10g.c, excluding vendor specific
>> configurations for 1000BaseT.
>>
>> Fixes: 22b56e827093 ("net: phy: replace genphy_10g_driver with genphy_c45_driver")
>>
>> Signed-off-by: Marco Hartmann <marco.hartmann@nxp.com>
>
> Andrew, Heiner, et al. where are we with this patch?
>
For me this patch would be ok, even though this generic config_aneg
doesn't support 1000BaseT.
1. The whole genphy_c45 driver doesn't make sense w/o a config_aneg
callback implementation.
2. It can serve as a temporary fallback for new C45 PHY's that don't
have a dedicated driver yet.
3. We may have C45 PHYs not supporting 1000BaseT (e.g. T1).
Andrew?
Heiner
^ permalink raw reply
* Re: [net-next 4/8] net/mlx5e: Add device out of buffer counter
From: Saeed Mahameed @ 2019-08-23 6:00 UTC (permalink / raw)
To: jakub.kicinski@netronome.com
Cc: davem@davemloft.net, netdev@vger.kernel.org, Moshe Shemesh
In-Reply-To: <20190822183324.79b74f7b@cakuba.netronome.com>
On Thu, 2019-08-22 at 18:33 -0700, Jakub Kicinski wrote:
> On Thu, 22 Aug 2019 23:35:52 +0000, Saeed Mahameed wrote:
> > From: Moshe Shemesh <moshe@mellanox.com>
> >
> > Added the following packets drop counter:
> > Device out of buffer - counts packets which were dropped due to
> > full
> > device internal receive queue.
> > This counter will be shown on ethtool as a new counter called
> > dev_out_of_buffer.
> > The counter is read from FW by command QUERY_VNIC_ENV.
> >
> > Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>
> Sounds like rx_fifo_errors, no? Doesn't rx_fifo_errors count RX
> overruns?
No, that is port buffer you are looking for and we got that fully
covered in mlx5. this is different.
This new counter is deep into the HW data path pipeline and it covers
very rare and complex scenarios that got only recently introduced with
swichdev mode and "some" lately added tunnels offloads that are routed
between VFs/PFs.
Normally the HW is lossless once the packet passes port buffers into
the data plane pipeline, let's call that "fast lane", BUT for sriov
configurations with switchdev mode enabled and some special hand
crafted tc tunnel offloads that requires hairpin between VFs/PFs, the
hw might decide to send some traffic to a "service lane" which is still
fast path but unlike the "fast lane" it handles traffic through "HW
internal" receive and send queues (just like we do with hairpin) that
might drop packets. the whole thing is transparent to driver and it is
HW implementation specific.
Thanks,
Saeed.
^ 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