* [RFC patch net-next 1/3] net: dsa: ksz9477: port mirror sniffing limited to one port
From: Arun Ramadoss @ 2022-04-27 16:23 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Paolo Abeni, Jakub Kicinski, David S. Miller, Vladimir Oltean,
Florian Fainelli, Vivien Didelot, Andrew Lunn, UNGLinuxDriver,
Woojung Huh
In-Reply-To: <20220427162343.18092-1-arun.ramadoss@microchip.com>
This patch limits the sniffing to only one port during the mirror add.
And during the mirror_del it checks for all the ports using the sniff,
if and only if no other ports are referring, sniffing is disabled.
The code is updated based on the review comments of LAN937x port mirror
patch.
https://patchwork.kernel.org/project/netdevbpf/patch/20210422094257.1641396-8-prasanna.vengateshan@microchip.com/
Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
drivers/net/dsa/microchip/ksz9477.c | 38 ++++++++++++++++++++++++++---
1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 4f617fee9a4e..90ce789107eb 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -990,14 +990,32 @@ static int ksz9477_port_mirror_add(struct dsa_switch *ds, int port,
bool ingress, struct netlink_ext_ack *extack)
{
struct ksz_device *dev = ds->priv;
+ u8 data;
+ int p;
+
+ /* Limit to one sniffer port
+ * Check if any of the port is already set for sniffing
+ * If yes, instruct the user to remove the previous entry & exit
+ */
+ for (p = 0; p < dev->port_cnt; p++) {
+ /* Skip the current sniffing port */
+ if (p == mirror->to_local_port)
+ continue;
+
+ ksz_pread8(dev, p, P_MIRROR_CTRL, &data);
+
+ if (data & PORT_MIRROR_SNIFFER) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Sniffer port is already configured, delete existing rules & retry");
+ return -EBUSY;
+ }
+ }
if (ingress)
ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_RX, true);
else
ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_TX, true);
- ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_SNIFFER, false);
-
/* configure mirror port */
ksz_port_cfg(dev, mirror->to_local_port, P_MIRROR_CTRL,
PORT_MIRROR_SNIFFER, true);
@@ -1011,16 +1029,28 @@ static void ksz9477_port_mirror_del(struct dsa_switch *ds, int port,
struct dsa_mall_mirror_tc_entry *mirror)
{
struct ksz_device *dev = ds->priv;
+ bool in_use = false;
u8 data;
+ int p;
if (mirror->ingress)
ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_RX, false);
else
ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_TX, false);
- ksz_pread8(dev, port, P_MIRROR_CTRL, &data);
- if (!(data & (PORT_MIRROR_RX | PORT_MIRROR_TX)))
+ /* Check if any of the port is still referring to sniffer port */
+ for (p = 0; p < dev->port_cnt; p++) {
+ ksz_pread8(dev, p, P_MIRROR_CTRL, &data);
+
+ if ((data & (PORT_MIRROR_RX | PORT_MIRROR_TX))) {
+ in_use = true;
+ break;
+ }
+ }
+
+ /* delete sniffing if there are no other mirroring rule exist */
+ if (!in_use)
ksz_port_cfg(dev, mirror->to_local_port, P_MIRROR_CTRL,
PORT_MIRROR_SNIFFER, false);
}
--
2.33.0
^ permalink raw reply related
* [RFC patch net-next 0/3] net: dsa: ksz: generic port mirror function for ksz9477 based switch
From: Arun Ramadoss @ 2022-04-27 16:23 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Paolo Abeni, Jakub Kicinski, David S. Miller, Vladimir Oltean,
Florian Fainelli, Vivien Didelot, Andrew Lunn, UNGLinuxDriver,
Woojung Huh
This patch series updates the ksz9477 port_mirror_add and port_mirror_del as
per the LAN937x patch submission. It allows only one port to be sniffing port.
Then moves the function to ksz_common.c file, to have it referenced by LAN937x
based switch. And add new header file ksz_reg.h which has common register
address between them.
Arun Ramadoss (3):
net: dsa: ksz9477: port mirror sniffing limited to one port
net: dsa: ksz: remove duplicate ksz_cfg and ksz_port_cfg
net: dsa: ksz: moved ksz9477 port mirror to ksz_common.c
drivers/net/dsa/microchip/ksz8795.c | 12 -----
drivers/net/dsa/microchip/ksz9477.c | 56 +------------------
drivers/net/dsa/microchip/ksz9477_reg.h | 15 ------
drivers/net/dsa/microchip/ksz_common.c | 72 +++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz_common.h | 18 +++++++
drivers/net/dsa/microchip/ksz_reg.h | 29 ++++++++++
6 files changed, 121 insertions(+), 81 deletions(-)
create mode 100644 drivers/net/dsa/microchip/ksz_reg.h
base-commit: 03fa8fc93e443e6caa485cc741328a1386c63630
--
2.33.0
^ permalink raw reply
* Re: [PATCH net] drivers: net: can: Fix deadlock in grcan_close()
From: Oliver Hartkopp @ 2022-04-27 16:25 UTC (permalink / raw)
To: Andreas Larsson, Duoming Zhou, linux-kernel
Cc: wg, mkl, davem, kuba, pabeni, linux-can, netdev
In-Reply-To: <2f5295c1-d787-84a5-1b3e-813f96dd4ae2@gaisler.com>
On 27.04.22 14:47, Andreas Larsson wrote:
> On 2022-04-26 21:12, Oliver Hartkopp wrote:
>> On 25.04.22 06:24, Duoming Zhou wrote:
>>> There are deadlocks caused by del_timer_sync(&priv->hang_timer)
>>> and del_timer_sync(&priv->rr_timer) in grcan_close(), one of
>>> the deadlocks are shown below:
>>>
>>> (Thread 1) | (Thread 2)
>>> | grcan_reset_timer()
>>> grcan_close() | mod_timer()
>>> spin_lock_irqsave() //(1) | (wait a time)
>>> ... | grcan_initiate_running_reset()
>>> del_timer_sync() | spin_lock_irqsave() //(2)
>>> (wait timer to stop) | ...
>>>
>>> We hold priv->lock in position (1) of thread 1 and use
>>> del_timer_sync() to wait timer to stop, but timer handler
>>> also need priv->lock in position (2) of thread 2.
>>> As a result, grcan_close() will block forever.
>>>
>>> This patch extracts del_timer_sync() from the protection of
>>> spin_lock_irqsave(), which could let timer handler to obtain
>>> the needed lock.
>>>
>>> Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
>>> ---
>>> drivers/net/can/grcan.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
>>> index d0c5a7a60da..1189057b5d6 100644
>>> --- a/drivers/net/can/grcan.c
>>> +++ b/drivers/net/can/grcan.c
>>> @@ -1102,8 +1102,10 @@ static int grcan_close(struct net_device *dev)
>>> priv->closing = true;
>>> if (priv->need_txbug_workaround) {
>>> + spin_unlock_irqrestore(&priv->lock, flags);
>>> del_timer_sync(&priv->hang_timer);
>>> del_timer_sync(&priv->rr_timer);
>>> + spin_lock_irqsave(&priv->lock, flags);
>>
>> It looks weird to unlock and re-lock the operations like this. This
>> breaks the intended locking for the closing process.
>>
>> Isn't there any possibility to e.g. move that entire if-section before
>> the lock?
>
> All functions wishing to start the timers both check priv->closing and
> then, if false, start the timer within the priv->lock spinlock. Given
> that, it should be ok that del_timer_sync is not done within the
> spinlock as therefore no one can restart any timers after priv->closing
> has been set to true.
>
> It looks a bit weird, but setting priv->closing to true needs to happen
> within the priv->lock spinlock protection and needs to happen before
> del_timer_sync to avoid a race between grcan_close and someone starting
> the timer.
>
> Reviewed-by: Andreas Larsson <andreas@gaisler.com>
>
Thanks Andreas!
Best regards,
Oliver
^ permalink raw reply
* Re: [PATCH net 3/7] tcp: resalt the secret every 10 seconds
From: Willy Tarreau @ 2022-04-27 16:21 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
Yossi Gilad, Amit Klein, linux-kernel
In-Reply-To: <20220427085621.5f2d1759@hermes.local>
Hi Stephen,
On Wed, Apr 27, 2022 at 08:56:21AM -0700, Stephen Hemminger wrote:
> On Wed, 27 Apr 2022 08:52:29 +0200
> Willy Tarreau <w@1wt.eu> wrote:
>
> > From: Eric Dumazet <edumazet@google.com>
> >
> > In order to limit the ability for an observer to recognize the source
> > ports sequence used to contact a set of destinations, we should
> > periodically shuffle the secret. 10 seconds looks effective enough
> > without causing particular issues.
> >
> > Cc: Moshe Kol <moshe.kol@mail.huji.ac.il>
> > Cc: Yossi Gilad <yossi.gilad@mail.huji.ac.il>
> > Cc: Amit Klein <aksecurity@gmail.com>
> > Tested-by: Willy Tarreau <w@1wt.eu>
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > ---
> > net/core/secure_seq.c | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
> > index 2cdd43a63f64..200ab4686275 100644
> > --- a/net/core/secure_seq.c
> > +++ b/net/core/secure_seq.c
> > @@ -22,6 +22,8 @@
> > static siphash_aligned_key_t net_secret;
> > static siphash_aligned_key_t ts_secret;
> >
>
> Rather than hard coding, why not have a sysctl knob for this?
> That way the tinfoil types can set it smaller.
It's a legit question. First I think that there's no good value; before
it used to be infinite, and now we're trying to figure a reasonable value
that make the attack impractical without going too close to the risk of
occasionally failing to establish a connection. I'm really not convinced
that there's any benefit in fiddling with that, except for breaking one's
stack by resalting too often and complaining about stupid network issues
with ACK or RST being sent in response to a SYN.
And stupidly, dividing jiffies by a constant known at build time is
slightly cheaper than dividing by a variable. I know it's a detail but
we tried hard to limit the accumulation of details here :-/
Just my two cents,
Willy
^ permalink raw reply
* [PATCH] MAINTAINERS: Update BNXT entry with firmware files
From: Florian Fainelli @ 2022-04-27 16:21 UTC (permalink / raw)
To: netdev; +Cc: michael.chan, zajec5, andy, Florian Fainelli
There appears to be a maintainer gap for BNXT TEE firmware files which
causes some patches to be missed. Update the entry for the BNXT Ethernet
controller with its companion firmware files.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index f585242da63d..0316d0c9a908 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3927,7 +3927,9 @@ BROADCOM BNXT_EN 50 GIGABIT ETHERNET DRIVER
M: Michael Chan <michael.chan@broadcom.com>
L: netdev@vger.kernel.org
S: Supported
+F: drivers/firmware/broadcom/tee_bnxt_fw.c
F: drivers/net/ethernet/broadcom/bnxt/
+F: linux/firmware/broadcom/tee_bnxt_fw.h
BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER
M: Arend van Spriel <aspriel@gmail.com>
--
2.25.1
^ permalink raw reply related
* Re: [PATCH net-next 08/14] eth: bgnet: remove a copy of the NAPI_POLL_WEIGHT define
From: Florian Fainelli @ 2022-04-27 16:09 UTC (permalink / raw)
To: Jakub Kicinski, davem, pabeni; +Cc: netdev, rafal, bcm-kernel-feedback-list
In-Reply-To: <20220427154111.529975-9-kuba@kernel.org>
On 4/27/22 08:41, Jakub Kicinski wrote:
> Defining local versions of NAPI_POLL_WEIGHT with the same
> values in the drivers just makes refactoring harder.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Looks fine, however this is a new subject prefix, do you mind using:
net: bgmac: remove a copy of the NAPI_POLL_WEIGHT define
to be somewhat consistent with past submission? With that:
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* [PATCH net-next v5 18/18] team: Remove use of list iterator variable for list_for_each_entry_from()
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
In preparation to limit the scope of the list iterator variable to the
list traversal loop, use a dedicated pointer to iterate through the
list [1].
Since that variable should not be used past the loop iteration, a
separate variable is used to 'remember the current location within the
loop'.
To either continue iterating from that position or skip the iteration
(if the previous iteration was complete) list_prepare_entry() is used.
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
drivers/net/team/team.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index b07dde6f0abf..688c4393f099 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -2425,17 +2425,17 @@ static int team_nl_send_options_get(struct team *team, u32 portid, u32 seq,
int flags, team_nl_send_func_t *send_func,
struct list_head *sel_opt_inst_list)
{
+ struct team_option_inst *opt_inst, *tmp = NULL;
struct nlattr *option_list;
struct nlmsghdr *nlh;
void *hdr;
- struct team_option_inst *opt_inst;
int err;
struct sk_buff *skb = NULL;
bool incomplete;
int i;
- opt_inst = list_first_entry(sel_opt_inst_list,
- struct team_option_inst, tmp_list);
+ tmp = list_first_entry(sel_opt_inst_list,
+ struct team_option_inst, tmp_list);
start_again:
err = __send_and_alloc_skb(&skb, team, portid, send_func);
@@ -2456,7 +2456,9 @@ static int team_nl_send_options_get(struct team *team, u32 portid, u32 seq,
goto nla_put_failure;
i = 0;
+ opt_inst = list_prepare_entry(tmp, sel_opt_inst_list, tmp_list);
incomplete = false;
+ tmp = NULL;
list_for_each_entry_from(opt_inst, sel_opt_inst_list, tmp_list) {
err = team_nl_fill_one_option_get(skb, team, opt_inst);
if (err) {
@@ -2464,6 +2466,7 @@ static int team_nl_send_options_get(struct team *team, u32 portid, u32 seq,
if (!i)
goto errout;
incomplete = true;
+ tmp = opt_inst;
break;
}
goto errout;
@@ -2707,14 +2710,14 @@ static int team_nl_send_port_list_get(struct team *team, u32 portid, u32 seq,
struct nlattr *port_list;
struct nlmsghdr *nlh;
void *hdr;
- struct team_port *port;
+ struct team_port *port, *tmp = NULL;
int err;
struct sk_buff *skb = NULL;
bool incomplete;
int i;
- port = list_first_entry_or_null(&team->port_list,
- struct team_port, list);
+ tmp = list_first_entry_or_null(&team->port_list,
+ struct team_port, list);
start_again:
err = __send_and_alloc_skb(&skb, team, portid, send_func);
@@ -2744,7 +2747,9 @@ static int team_nl_send_port_list_get(struct team *team, u32 portid, u32 seq,
err = team_nl_fill_one_port_get(skb, one_port);
if (err)
goto errout;
- } else if (port) {
+ } else {
+ port = list_prepare_entry(tmp, &team->port_list, list);
+ tmp = NULL;
list_for_each_entry_from(port, &team->port_list, list) {
err = team_nl_fill_one_port_get(skb, port);
if (err) {
@@ -2752,6 +2757,7 @@ static int team_nl_send_port_list_get(struct team *team, u32 portid, u32 seq,
if (!i)
goto errout;
incomplete = true;
+ tmp = port;
break;
}
goto errout;
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 17/18] ipvlan: Remove usage of list iterator variable for the loop body
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
In preparation to limit the scope of the list iterator variable to the
list traversal loop, use a dedicated pointer to iterate through the
list [1].
Since that variable should not be used past the loop iteration, a
separate variable is used to 'remember the current location within the
loop'.
To either continue iterating from that position or start a new
iteration (if the previous iteration was complete) list_prepare_entry()
is used.
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
drivers/net/ipvlan/ipvlan_main.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 696e245f6d00..063d7c30e944 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -9,7 +9,7 @@
static int ipvlan_set_port_mode(struct ipvl_port *port, u16 nval,
struct netlink_ext_ack *extack)
{
- struct ipvl_dev *ipvlan;
+ struct ipvl_dev *ipvlan, *tmp = NULL;
unsigned int flags;
int err;
@@ -26,8 +26,10 @@ static int ipvlan_set_port_mode(struct ipvl_port *port, u16 nval,
flags & ~IFF_NOARP,
extack);
}
- if (unlikely(err))
+ if (unlikely(err)) {
+ tmp = ipvlan;
goto fail;
+ }
}
if (nval == IPVLAN_MODE_L3S) {
/* New mode is L3S */
@@ -43,6 +45,7 @@ static int ipvlan_set_port_mode(struct ipvl_port *port, u16 nval,
return 0;
fail:
+ ipvlan = list_prepare_entry(tmp, &port->ipvlans, pnode);
/* Undo the flags changes that have been done so far. */
list_for_each_entry_continue_reverse(ipvlan, &port->ipvlans, pnode) {
flags = ipvlan->dev->flags;
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 10/18] qed: Replace usage of found with dedicated list iterator variable
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.
To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].
This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
drivers/net/ethernet/qlogic/qed/qed_iwarp.c | 26 ++++++++++-----------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
index 1d1d4caad680..198c9321bf51 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
@@ -1630,38 +1630,36 @@ static struct qed_iwarp_listener *
qed_iwarp_get_listener(struct qed_hwfn *p_hwfn,
struct qed_iwarp_cm_info *cm_info)
{
- struct qed_iwarp_listener *listener = NULL;
+ struct qed_iwarp_listener *listener = NULL, *iter;
static const u32 ip_zero[4] = { 0, 0, 0, 0 };
- bool found = false;
- list_for_each_entry(listener,
+ list_for_each_entry(iter,
&p_hwfn->p_rdma_info->iwarp.listen_list,
list_entry) {
- if (listener->port == cm_info->local_port) {
- if (!memcmp(listener->ip_addr,
+ if (iter->port == cm_info->local_port) {
+ if (!memcmp(iter->ip_addr,
ip_zero, sizeof(ip_zero))) {
- found = true;
+ listener = iter;
break;
}
- if (!memcmp(listener->ip_addr,
+ if (!memcmp(iter->ip_addr,
cm_info->local_ip,
sizeof(cm_info->local_ip)) &&
- (listener->vlan == cm_info->vlan)) {
- found = true;
+ iter->vlan == cm_info->vlan) {
+ listener = iter;
break;
}
}
}
- if (found) {
+ if (listener)
DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "listener found = %p\n",
listener);
- return listener;
- }
+ else
+ DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "listener not found\n");
- DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "listener not found\n");
- return NULL;
+ return listener;
}
static int
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 11/18] qed: Remove usage of list iterator variable after the loop
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.
Since "found" and "p_ent" need to be equal, "found" should be used
consistently to limit the scope of "p_ent" to the list traversal in
the future.
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
drivers/net/ethernet/qlogic/qed/qed_spq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_spq.c b/drivers/net/ethernet/qlogic/qed/qed_spq.c
index d01b9245f811..cbaa2abed660 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_spq.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_spq.c
@@ -934,10 +934,10 @@ int qed_spq_completion(struct qed_hwfn *p_hwfn,
u8 fw_return_code,
union event_ring_data *p_data)
{
+ struct qed_spq_entry *found = NULL;
struct qed_spq *p_spq;
- struct qed_spq_entry *p_ent = NULL;
+ struct qed_spq_entry *p_ent;
struct qed_spq_entry *tmp;
- struct qed_spq_entry *found = NULL;
if (!p_hwfn)
return -EINVAL;
@@ -980,7 +980,7 @@ int qed_spq_completion(struct qed_hwfn *p_hwfn,
DP_VERBOSE(p_hwfn, QED_MSG_SPQ,
"Complete EQE [echo %04x]: func %p cookie %p)\n",
le16_to_cpu(echo),
- p_ent->comp_cb.function, p_ent->comp_cb.cookie);
+ found->comp_cb.function, found->comp_cb.cookie);
if (found->comp_cb.function)
found->comp_cb.function(p_hwfn, found->comp_cb.cookie, p_data,
fw_return_code);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] libbpf: fix returnvar.cocci warnings
From: Alexei Starovoitov @ 2022-04-27 16:07 UTC (permalink / raw)
To: kernel test robot, Julia Lawall
Cc: Andrii Nakryiko, kbuild-all, Linux Memory Management List,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Network Development, bpf,
LKML
In-Reply-To: <Ymj5AJtiBx0UjEdT@8276d8ba1d54>
On Wed, Apr 27, 2022 at 1:04 AM kernel test robot <lkp@intel.com> wrote:
>
> From: kernel test robot <lkp@intel.com>
>
> tools/lib/bpf/relo_core.c:1064:8-11: Unneeded variable: "len". Return "0" on line 1086
>
>
> Remove unneeded variable used to store return value.
>
> Generated by: scripts/coccinelle/misc/returnvar.cocci
>
> Fixes: b58af63aab11 ("libbpf: Refactor CO-RE relo human description formatting routine")
> CC: Andrii Nakryiko <andrii@kernel.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: kernel test robot <lkp@intel.com>
> ---
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head: f02ac5c95dfd45d2f50ecc68d79177de326c668c
> commit: b58af63aab11e4ae00fe96de9505759cfdde8ee9 [6746/7265] libbpf: Refactor CO-RE relo human description formatting routine
> :::::: branch date: 2 hours ago
> :::::: commit date: 9 hours ago
>
> tools/lib/bpf/relo_core.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> --- a/tools/lib/bpf/relo_core.c
> +++ b/tools/lib/bpf/relo_core.c
> @@ -1061,7 +1061,7 @@ static int bpf_core_format_spec(char *bu
> const struct btf_enum *e;
> const char *s;
> __u32 type_id;
> - int i, len = 0;
> + int i;
>
> #define append_buf(fmt, args...) \
> ({ \
> @@ -1083,7 +1083,7 @@ static int bpf_core_format_spec(char *bu
> type_id, btf_kind_str(t), str_is_empty(s) ? "<anon>" : s);
>
> if (core_relo_is_type_based(spec->relo_kind))
> - return len;
> + return 0;
cocci is wrong.
It missed append_buf() macro.
Please fix cocci so we don't have to manually deal with
broken patches like this one.
^ permalink raw reply
* [PATCH net-next v5 12/18] net: qede: Replace usage of found with dedicated list iterator variable
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.
To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].
This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
drivers/net/ethernet/qlogic/qede/qede_rdma.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_rdma.c b/drivers/net/ethernet/qlogic/qede/qede_rdma.c
index 6304514a6f2c..2eb03ffe2484 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_rdma.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_rdma.c
@@ -246,18 +246,17 @@ static void qede_rdma_change_mtu(struct qede_dev *edev)
static struct qede_rdma_event_work *
qede_rdma_get_free_event_node(struct qede_dev *edev)
{
- struct qede_rdma_event_work *event_node = NULL;
- bool found = false;
+ struct qede_rdma_event_work *event_node = NULL, *iter;
- list_for_each_entry(event_node, &edev->rdma_info.rdma_event_list,
+ list_for_each_entry(iter, &edev->rdma_info.rdma_event_list,
list) {
- if (!work_pending(&event_node->work)) {
- found = true;
+ if (!work_pending(&iter->work)) {
+ event_node = iter;
break;
}
}
- if (!found) {
+ if (!event_node) {
event_node = kzalloc(sizeof(*event_node), GFP_ATOMIC);
if (!event_node) {
DP_NOTICE(edev,
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 13/18] net: qede: Remove check of list iterator against head past the loop body
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
When list_for_each_entry() completes the iteration over the whole list
without breaking the loop, the iterator value will be a bogus pointer
computed based on the head element.
While it is safe to use the pointer to determine if it was computed
based on the head element, either with list_entry_is_head() or
&pos->member == head, using the iterator variable after the loop should
be avoided.
In preparation to limit the scope of a list iterator to the list
traversal loop, use a dedicated pointer to point to the found element [1].
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
drivers/net/ethernet/qlogic/qede/qede_filter.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index 3010833ddde3..3d167e37e654 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
@@ -829,18 +829,21 @@ int qede_configure_vlan_filters(struct qede_dev *edev)
int qede_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
{
struct qede_dev *edev = netdev_priv(dev);
- struct qede_vlan *vlan;
+ struct qede_vlan *vlan = NULL;
+ struct qede_vlan *iter;
int rc = 0;
DP_VERBOSE(edev, NETIF_MSG_IFDOWN, "Removing vlan 0x%04x\n", vid);
/* Find whether entry exists */
__qede_lock(edev);
- list_for_each_entry(vlan, &edev->vlan_list, list)
- if (vlan->vid == vid)
+ list_for_each_entry(iter, &edev->vlan_list, list)
+ if (iter->vid == vid) {
+ vlan = iter;
break;
+ }
- if (list_entry_is_head(vlan, &edev->vlan_list, list)) {
+ if (!vlan) {
DP_VERBOSE(edev, (NETIF_MSG_IFUP | NETIF_MSG_IFDOWN),
"Vlan isn't configured\n");
goto out;
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 15/18] net: netcp: Remove usage of list iterator for list_add() after loop body
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
In preparation to limit the scope of a list iterator to the list
traversal loop, use a dedicated pointer pointing to the location
where the element should be inserted [1].
Before, the code implicitly used the head when no element was found
when using &next->list. The new 'pos' variable is set to the list head
by default and overwritten if the list exits early, marking the
insertion point for list_add().
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
drivers/net/ethernet/ti/netcp_core.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 16507bff652a..f25104b5a31b 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -471,6 +471,7 @@ struct netcp_hook_list {
int netcp_register_txhook(struct netcp_intf *netcp_priv, int order,
netcp_hook_rtn *hook_rtn, void *hook_data)
{
+ struct list_head *pos = &netcp_priv->txhook_list_head;
struct netcp_hook_list *entry;
struct netcp_hook_list *next;
unsigned long flags;
@@ -485,10 +486,12 @@ int netcp_register_txhook(struct netcp_intf *netcp_priv, int order,
spin_lock_irqsave(&netcp_priv->lock, flags);
list_for_each_entry(next, &netcp_priv->txhook_list_head, list) {
- if (next->order > order)
+ if (next->order > order) {
+ pos = &next->list;
break;
+ }
}
- __list_add(&entry->list, next->list.prev, &next->list);
+ list_add_tail(&entry->list, pos);
spin_unlock_irqrestore(&netcp_priv->lock, flags);
return 0;
@@ -520,6 +523,7 @@ EXPORT_SYMBOL_GPL(netcp_unregister_txhook);
int netcp_register_rxhook(struct netcp_intf *netcp_priv, int order,
netcp_hook_rtn *hook_rtn, void *hook_data)
{
+ struct list_head *pos = &netcp_priv->rxhook_list_head;
struct netcp_hook_list *entry;
struct netcp_hook_list *next;
unsigned long flags;
@@ -534,10 +538,12 @@ int netcp_register_rxhook(struct netcp_intf *netcp_priv, int order,
spin_lock_irqsave(&netcp_priv->lock, flags);
list_for_each_entry(next, &netcp_priv->rxhook_list_head, list) {
- if (next->order > order)
+ if (next->order > order) {
+ pos = &next->list;
break;
+ }
}
- __list_add(&entry->list, next->list.prev, &next->list);
+ list_add_tail(&entry->list, pos);
spin_unlock_irqrestore(&netcp_priv->lock, flags);
return 0;
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 16/18] ps3_gelic: Replace usage of found with dedicated list iterator variable
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.
To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].
This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
.../net/ethernet/toshiba/ps3_gelic_wireless.c | 30 +++++++++----------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c b/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c
index dc14a66583ff..c8a016c902cd 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c
@@ -1495,14 +1495,14 @@ static int gelic_wl_start_scan(struct gelic_wl_info *wl, int always_scan,
*/
static void gelic_wl_scan_complete_event(struct gelic_wl_info *wl)
{
+ struct gelic_wl_scan_info *target = NULL, *iter, *tmp;
struct gelic_eurus_cmd *cmd = NULL;
- struct gelic_wl_scan_info *target, *tmp;
struct gelic_wl_scan_info *oldest = NULL;
struct gelic_eurus_scan_info *scan_info;
unsigned int scan_info_size;
union iwreq_data data;
unsigned long this_time = jiffies;
- unsigned int data_len, i, found, r;
+ unsigned int data_len, i, r;
void *buf;
pr_debug("%s:start\n", __func__);
@@ -1539,14 +1539,14 @@ static void gelic_wl_scan_complete_event(struct gelic_wl_info *wl)
wl->scan_stat = GELIC_WL_SCAN_STAT_GOT_LIST;
/* mark all entries are old */
- list_for_each_entry_safe(target, tmp, &wl->network_list, list) {
- target->valid = 0;
+ list_for_each_entry_safe(iter, tmp, &wl->network_list, list) {
+ iter->valid = 0;
/* expire too old entries */
- if (time_before(target->last_scanned + wl->scan_age,
+ if (time_before(iter->last_scanned + wl->scan_age,
this_time)) {
- kfree(target->hwinfo);
- target->hwinfo = NULL;
- list_move_tail(&target->list, &wl->network_free_list);
+ kfree(iter->hwinfo);
+ iter->hwinfo = NULL;
+ list_move_tail(&iter->list, &wl->network_free_list);
}
}
@@ -1569,22 +1569,22 @@ static void gelic_wl_scan_complete_event(struct gelic_wl_info *wl)
continue;
}
- found = 0;
+ target = NULL;
oldest = NULL;
- list_for_each_entry(target, &wl->network_list, list) {
- if (ether_addr_equal(&target->hwinfo->bssid[2],
+ list_for_each_entry(iter, &wl->network_list, list) {
+ if (ether_addr_equal(&iter->hwinfo->bssid[2],
&scan_info->bssid[2])) {
- found = 1;
+ target = iter;
pr_debug("%s: same BBS found scanned list\n",
__func__);
break;
}
if (!oldest ||
- (target->last_scanned < oldest->last_scanned))
- oldest = target;
+ (iter->last_scanned < oldest->last_scanned))
+ oldest = iter;
}
- if (!found) {
+ if (!target) {
/* not found in the list */
if (list_empty(&wl->network_free_list)) {
/* expire oldest */
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 14/18] sfc: Remove usage of list iterator for list_add() after the loop body
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
In preparation to limit the scope of a list iterator to the list
traversal loop, use a dedicated pointer pointing to the location
where the element should be inserted [1].
Before, the code implicitly used the head when no element was found
when using &new->list. The new 'pos' variable is set to the list head
by default and overwritten if the list exits early, marking the
insertion point for list_add().
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
drivers/net/ethernet/sfc/rx_common.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/sfc/rx_common.c b/drivers/net/ethernet/sfc/rx_common.c
index fa8b9aacca11..74c056210e0b 100644
--- a/drivers/net/ethernet/sfc/rx_common.c
+++ b/drivers/net/ethernet/sfc/rx_common.c
@@ -560,14 +560,17 @@ struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx)
{
struct list_head *head = &efx->rss_context.list;
struct efx_rss_context *ctx, *new;
+ struct list_head *pos = head;
u32 id = 1; /* Don't use zero, that refers to the master RSS context */
WARN_ON(!mutex_is_locked(&efx->rss_lock));
/* Search for first gap in the numbering */
list_for_each_entry(ctx, head, list) {
- if (ctx->user_id != id)
+ if (ctx->user_id != id) {
+ pos = &ctx->list;
break;
+ }
id++;
/* Check for wrap. If this happens, we have nearly 2^32
* allocated RSS contexts, which seems unlikely.
@@ -585,7 +588,7 @@ struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx)
/* Insert the new entry into the gap */
new->user_id = id;
- list_add_tail(&new->list, &ctx->list);
+ list_add_tail(&new->list, pos);
return new;
}
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 09/18] qed: Use dedicated list iterator variable
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.
To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable [1].
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
drivers/net/ethernet/qlogic/qed/qed_dev.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index 672480c9d195..e920e7dcf66a 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -174,7 +174,7 @@ int qed_db_recovery_add(struct qed_dev *cdev,
int qed_db_recovery_del(struct qed_dev *cdev,
void __iomem *db_addr, void *db_data)
{
- struct qed_db_recovery_entry *db_entry = NULL;
+ struct qed_db_recovery_entry *db_entry = NULL, *iter;
struct qed_hwfn *p_hwfn;
int rc = -EINVAL;
@@ -190,12 +190,13 @@ int qed_db_recovery_del(struct qed_dev *cdev,
/* Protect the list */
spin_lock_bh(&p_hwfn->db_recovery_info.lock);
- list_for_each_entry(db_entry,
+ list_for_each_entry(iter,
&p_hwfn->db_recovery_info.list, list_entry) {
/* search according to db_data addr since db_addr is not unique (roce) */
- if (db_entry->db_data == db_data) {
- qed_db_recovery_dp_entry(p_hwfn, db_entry, "Deleting");
- list_del(&db_entry->list_entry);
+ if (iter->db_data == db_data) {
+ qed_db_recovery_dp_entry(p_hwfn, iter, "Deleting");
+ list_del(&iter->list_entry);
+ db_entry = iter;
rc = 0;
break;
}
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 08/18] net: sparx5: Replace usage of found with dedicated list iterator variable
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.
To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].
This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
.../microchip/sparx5/sparx5_mactable.c | 25 +++++++++----------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
index a5837dbe0c7e..bb8d9ce79ac2 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
@@ -362,8 +362,7 @@ static void sparx5_mact_handle_entry(struct sparx5 *sparx5,
unsigned char mac[ETH_ALEN],
u16 vid, u32 cfg2)
{
- struct sparx5_mact_entry *mact_entry;
- bool found = false;
+ struct sparx5_mact_entry *mact_entry = NULL, *iter;
u16 port;
if (LRN_MAC_ACCESS_CFG_2_MAC_ENTRY_ADDR_TYPE_GET(cfg2) !=
@@ -378,28 +377,28 @@ static void sparx5_mact_handle_entry(struct sparx5 *sparx5,
return;
mutex_lock(&sparx5->mact_lock);
- list_for_each_entry(mact_entry, &sparx5->mact_entries, list) {
- if (mact_entry->vid == vid &&
- ether_addr_equal(mac, mact_entry->mac)) {
- found = true;
- mact_entry->flags |= MAC_ENT_ALIVE;
- if (mact_entry->port != port) {
+ list_for_each_entry(iter, &sparx5->mact_entries, list) {
+ if (iter->vid == vid &&
+ ether_addr_equal(mac, iter->mac)) {
+ iter->flags |= MAC_ENT_ALIVE;
+ if (iter->port != port) {
dev_warn(sparx5->dev, "Entry move: %d -> %d\n",
- mact_entry->port, port);
- mact_entry->port = port;
- mact_entry->flags |= MAC_ENT_MOVED;
+ iter->port, port);
+ iter->port = port;
+ iter->flags |= MAC_ENT_MOVED;
}
/* Entry handled */
+ mact_entry = iter;
break;
}
}
mutex_unlock(&sparx5->mact_lock);
- if (found && !(mact_entry->flags & MAC_ENT_MOVED))
+ if (mact_entry && !(mact_entry->flags & MAC_ENT_MOVED))
/* Present, not moved */
return;
- if (!found) {
+ if (!mact_entry) {
/* Entry not found - now add */
mact_entry = alloc_mact_entry(sparx5, mac, vid, port);
if (!mact_entry)
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 07/18] net: dsa: Replace usage of found with dedicated list iterator variable
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.
To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].
This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 89c6c86e746f..645522c4dd4a 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -112,22 +112,21 @@ const struct dsa_device_ops *dsa_find_tagger_by_name(const char *buf)
const struct dsa_device_ops *dsa_tag_driver_get(int tag_protocol)
{
- struct dsa_tag_driver *dsa_tag_driver;
+ struct dsa_tag_driver *dsa_tag_driver = NULL, *iter;
const struct dsa_device_ops *ops;
- bool found = false;
request_module("%s%d", DSA_TAG_DRIVER_ALIAS, tag_protocol);
mutex_lock(&dsa_tag_drivers_lock);
- list_for_each_entry(dsa_tag_driver, &dsa_tag_drivers_list, list) {
- ops = dsa_tag_driver->ops;
+ list_for_each_entry(iter, &dsa_tag_drivers_list, list) {
+ ops = iter->ops;
if (ops->proto == tag_protocol) {
- found = true;
+ dsa_tag_driver = iter;
break;
}
}
- if (found) {
+ if (dsa_tag_driver) {
if (!try_module_get(dsa_tag_driver->owner))
ops = ERR_PTR(-ENOPROTOOPT);
} else {
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 06/18] net: dsa: mv88e6xxx: refactor mv88e6xxx_port_vlan()
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.,
Vladimir Oltean
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To avoid bugs and speculative execution exploits due to type-confused
pointers at the end of a list_for_each_entry() loop, one measure is to
restrict code to not use the iterator variable outside the loop block.
In the case of mv88e6xxx_port_vlan(), this isn't a problem, as we never
let the loops exit through "natural causes" anyway, by using a "found"
variable and then using the last "dp" iterator prior to the break, which
is a safe thing to do.
Nonetheless, with the expected new syntax, this pattern will no longer
be possible.
Profit off of the occasion and break the two port finding methods into
smaller sub-functions. Somehow, returning a copy of the iterator pointer
is still accepted.
This change makes it redundant to have a "bool found", since the "dp"
from mv88e6xxx_port_vlan() now holds NULL if we haven't found what we
were looking for.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 54 ++++++++++++++++++--------------
1 file changed, 31 insertions(+), 23 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b3aa0e5bc842..1f35e89053e6 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1378,42 +1378,50 @@ static int mv88e6xxx_set_mac_eee(struct dsa_switch *ds, int port,
return 0;
}
+static struct dsa_port *mv88e6xxx_find_port(struct dsa_switch_tree *dst,
+ int sw_index, int port)
+{
+ struct dsa_port *dp;
+
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->ds->index == sw_index && dp->index == port)
+ return dp;
+
+ return NULL;
+}
+
+static struct dsa_port *
+mv88e6xxx_find_port_by_bridge_num(struct dsa_switch_tree *dst,
+ unsigned int bridge_num)
+{
+ struct dsa_port *dp;
+
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_bridge_num_get(dp) == bridge_num)
+ return dp;
+
+ return NULL;
+}
+
/* Mask of the local ports allowed to receive frames from a given fabric port */
static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
{
struct dsa_switch *ds = chip->ds;
struct dsa_switch_tree *dst = ds->dst;
struct dsa_port *dp, *other_dp;
- bool found = false;
u16 pvlan;
- /* dev is a physical switch */
if (dev <= dst->last_switch) {
- list_for_each_entry(dp, &dst->ports, list) {
- if (dp->ds->index == dev && dp->index == port) {
- /* dp might be a DSA link or a user port, so it
- * might or might not have a bridge.
- * Use the "found" variable for both cases.
- */
- found = true;
- break;
- }
- }
- /* dev is a virtual bridge */
+ /* dev is a physical switch */
+ dp = mv88e6xxx_find_port(dst, dev, port);
} else {
- list_for_each_entry(dp, &dst->ports, list) {
- unsigned int bridge_num = dsa_port_bridge_num_get(dp);
-
- if (bridge_num + dst->last_switch != dev)
- continue;
-
- found = true;
- break;
- }
+ /* dev is a virtual bridge */
+ dp = mv88e6xxx_find_port_by_bridge_num(dst,
+ dev - dst->last_switch);
}
/* Prevent frames from unknown switch or virtual bridge */
- if (!found)
+ if (!dp)
return 0;
/* Frames from DSA links and CPU ports can egress any local port */
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 01/18] connector: Replace usage of found with dedicated list iterator variable
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.
To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].
This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
drivers/connector/cn_queue.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
index 996f025eb63c..ed77599b0b25 100644
--- a/drivers/connector/cn_queue.c
+++ b/drivers/connector/cn_queue.c
@@ -92,20 +92,19 @@ int cn_queue_add_callback(struct cn_queue_dev *dev, const char *name,
void cn_queue_del_callback(struct cn_queue_dev *dev, const struct cb_id *id)
{
- struct cn_callback_entry *cbq, *n;
- int found = 0;
+ struct cn_callback_entry *cbq = NULL, *iter, *n;
spin_lock_bh(&dev->queue_lock);
- list_for_each_entry_safe(cbq, n, &dev->queue_list, callback_entry) {
- if (cn_cb_equal(&cbq->id.id, id)) {
- list_del(&cbq->callback_entry);
- found = 1;
+ list_for_each_entry_safe(iter, n, &dev->queue_list, callback_entry) {
+ if (cn_cb_equal(&iter->id.id, id)) {
+ list_del(&iter->callback_entry);
+ cbq = iter;
break;
}
}
spin_unlock_bh(&dev->queue_lock);
- if (found)
+ if (cbq)
cn_queue_release_callback(cbq);
}
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 03/18] net: dsa: sja1105: reorder sja1105_first_entry_longer_than with memory allocation
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.,
Vladimir Oltean
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
From: Vladimir Oltean <vladimir.oltean@nxp.com>
sja1105_first_entry_longer_than() does not make use of the full struct
sja1105_gate_entry *e, just of e->interval which is set from the passed
entry_time.
This means that if there is a gate conflict, we have allocated e for
nothing, just to free it later. Reorder the memory allocation and the
function call, to avoid that and simplify the error unwind path.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_vl.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index 369be2ac3587..e5ea8eb9ec4e 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -36,7 +36,11 @@ static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg,
{
struct sja1105_gate_entry *e;
struct list_head *pos;
- int rc;
+
+ pos = sja1105_first_entry_longer_than(&gating_cfg->entries,
+ entry_time, extack);
+ if (IS_ERR(pos))
+ return PTR_ERR(pos);
e = kzalloc(sizeof(*e), GFP_KERNEL);
if (!e)
@@ -45,22 +49,11 @@ static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg,
e->rule = rule;
e->gate_state = gate_state;
e->interval = entry_time;
-
- pos = sja1105_first_entry_longer_than(&gating_cfg->entries,
- e->interval, extack);
- if (IS_ERR(pos)) {
- rc = PTR_ERR(pos);
- goto err;
- }
-
list_add(&e->list, pos->prev);
gating_cfg->num_entries++;
return 0;
-err:
- kfree(e);
- return rc;
}
/* The gate entries contain absolute times in their e->interval field. Convert
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 04/18] net: dsa: sja1105: use list_add_tail(pos) instead of list_add(pos->prev)
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.,
Vladimir Oltean
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
From: Vladimir Oltean <vladimir.oltean@nxp.com>
When passed a non-head list element, list_add_tail() actually adds the
new element to its left, which is what we want. Despite the slightly
confusing name, use the dedicated function which does the same thing as
the open-coded list_add(pos->prev).
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_vl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index e5ea8eb9ec4e..7fe9b18f1cbd 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -49,7 +49,7 @@ static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg,
e->rule = rule;
e->gate_state = gate_state;
e->interval = entry_time;
- list_add(&e->list, pos->prev);
+ list_add_tail(&e->list, pos);
gating_cfg->num_entries++;
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 05/18] net: dsa: mv88e6xxx: remove redundant check in mv88e6xxx_port_vlan()
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.,
Vladimir Oltean
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
From: Vladimir Oltean <vladimir.oltean@nxp.com>
We know that "dev > dst->last_switch" in the "else" block.
In other words, that "dev - dst->last_switch" is > 0.
dsa_port_bridge_num_get(dp) can be 0, but the check
"if (bridge_num + dst->last_switch != dev) continue", rewritten as
"if (bridge_num != dev - dst->last_switch) continue", aka
"if (bridge_num != something which cannot be 0) continue",
makes it redundant to have the extra "if (!bridge_num) continue" logic,
since a bridge_num of zero would have been skipped anyway.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 64f4fdd02902..b3aa0e5bc842 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1404,9 +1404,6 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
list_for_each_entry(dp, &dst->ports, list) {
unsigned int bridge_num = dsa_port_bridge_num_get(dp);
- if (!bridge_num)
- continue;
-
if (bridge_num + dst->last_switch != dev)
continue;
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 02/18] net: dsa: sja1105: remove use of iterator after list_for_each_entry() loop
From: Jakob Koschel @ 2022-04-27 16:06 UTC (permalink / raw)
To: David S. Miller
Cc: Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Ariel Elior, Manish Chopra, Edward Cree,
Martin Habets, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Casper Andersson, Jakob Koschel,
Arnd Bergmann, Jason Gunthorpe, Christophe JAILLET,
Colin Ian King, Eric Dumazet, Xu Wang, netdev, linux-kernel,
linux-arm-kernel, linuxppc-dev, bpf, Mike Rapoport,
Brian Johannesmeyer, Cristiano Giuffrida, Bos, H.J.,
Vladimir Oltean
In-Reply-To: <20220427160635.420492-1-jakobkoschel@gmail.com>
From: Vladimir Oltean <vladimir.oltean@nxp.com>
The link below explains that there is a desire to syntactically change
list_for_each_entry() and list_for_each() such that it becomes
impossible to use the iterator variable outside the scope of the loop.
Although sja1105_insert_gate_entry() makes legitimate use of the
iterator pointer when it breaks out, the pattern it uses may become
illegal, so it needs to change.
It is deemed acceptable to use a copy of the loop iterator, and
sja1105_insert_gate_entry() only needs to know the list_head element
before which the list insertion should be made. So let's profit from the
occasion and refactor the list iteration to a dedicated function.
An additional benefit is given by the fact that with the helper function
in place, we no longer need to special-case the empty list, since it is
equivalent to not having found any gating entry larger than the
specified interval in the list. We just need to insert at the tail of
that list (list_add vs list_add_tail on an empty list does the same
thing).
Link: https://patchwork.kernel.org/project/netdevbpf/patch/20220407102900.3086255-3-jakobkoschel@gmail.com/#24810127
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_vl.c | 46 ++++++++++++++++++----------
1 file changed, 29 insertions(+), 17 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index b7e95d60a6e4..369be2ac3587 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -7,6 +7,27 @@
#define SJA1105_SIZE_VL_STATUS 8
+static struct list_head *
+sja1105_first_entry_longer_than(struct list_head *entries,
+ s64 interval,
+ struct netlink_ext_ack *extack)
+{
+ struct sja1105_gate_entry *p;
+
+ list_for_each_entry(p, entries, list) {
+ if (p->interval == interval) {
+ NL_SET_ERR_MSG_MOD(extack, "Gate conflict");
+ return ERR_PTR(-EBUSY);
+ }
+
+ if (interval < p->interval)
+ return &p->list;
+ }
+
+ /* Empty list, or specified interval is largest within the list */
+ return entries;
+}
+
/* Insert into the global gate list, sorted by gate action time. */
static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg,
struct sja1105_rule *rule,
@@ -14,6 +35,7 @@ static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg,
struct netlink_ext_ack *extack)
{
struct sja1105_gate_entry *e;
+ struct list_head *pos;
int rc;
e = kzalloc(sizeof(*e), GFP_KERNEL);
@@ -24,25 +46,15 @@ static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg,
e->gate_state = gate_state;
e->interval = entry_time;
- if (list_empty(&gating_cfg->entries)) {
- list_add(&e->list, &gating_cfg->entries);
- } else {
- struct sja1105_gate_entry *p;
-
- list_for_each_entry(p, &gating_cfg->entries, list) {
- if (p->interval == e->interval) {
- NL_SET_ERR_MSG_MOD(extack,
- "Gate conflict");
- rc = -EBUSY;
- goto err;
- }
-
- if (e->interval < p->interval)
- break;
- }
- list_add(&e->list, p->list.prev);
+ pos = sja1105_first_entry_longer_than(&gating_cfg->entries,
+ e->interval, extack);
+ if (IS_ERR(pos)) {
+ rc = PTR_ERR(pos);
+ goto err;
}
+ list_add(&e->list, pos->prev);
+
gating_cfg->num_entries++;
return 0;
--
2.25.1
^ permalink raw reply related
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