* Re: [PATCH v2] net: dsa: mv88e6xxx: Fix port_hidden_wait to account for port_base_addr
From: Andrew Lunn @ 2022-04-24 19:26 UTC (permalink / raw)
To: Nathan Rossi
Cc: netdev, linux-kernel, Marek Behun, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S. Miller,
Jakub Kicinski, Paolo Abeni
In-Reply-To: <20220424153143.323338-1-nathan@nathanrossi.com>
On Sun, Apr 24, 2022 at 03:31:43PM +0000, Nathan Rossi wrote:
> The other port_hidden functions rely on the port_read/port_write
> functions to access the hidden control port. These functions apply the
> offset for port_base_addr where applicable. Update port_hidden_wait to
> use the port_wait_bit so that port_base_addr offsets are accounted for
> when waiting for the busy bit to change.
>
> Without the offset the port_hidden_wait function would timeout on
> devices that have a non-zero port_base_addr (e.g. MV88E6141), however
> devices that have a zero port_base_addr would operate correctly (e.g.
> MV88E6390).
>
> Fixes: ea89098ef9a5 ("net: dsa: mv88x6xxx: mv88e6390 errata")
That is further back than needed. And due to the code moving around
and getting renamed, you are added extra burden on those doing the
back port for no actual gain.
Please verify what i suggested, 609070133aff1 is better and then
repost.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH v2] net: dsa: mv88e6xxx: Fix port_hidden_wait to account for port_base_addr
From: Marek Behún @ 2022-04-24 19:33 UTC (permalink / raw)
To: Andrew Lunn
Cc: Nathan Rossi, netdev, linux-kernel, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S. Miller,
Jakub Kicinski, Paolo Abeni
In-Reply-To: <YmWkgkILCrBP5hRG@lunn.ch>
On Sun, 24 Apr 2022 21:26:58 +0200
Andrew Lunn <andrew@lunn.ch> wrote:
> On Sun, Apr 24, 2022 at 03:31:43PM +0000, Nathan Rossi wrote:
> > The other port_hidden functions rely on the port_read/port_write
> > functions to access the hidden control port. These functions apply the
> > offset for port_base_addr where applicable. Update port_hidden_wait to
> > use the port_wait_bit so that port_base_addr offsets are accounted for
> > when waiting for the busy bit to change.
> >
> > Without the offset the port_hidden_wait function would timeout on
> > devices that have a non-zero port_base_addr (e.g. MV88E6141), however
> > devices that have a zero port_base_addr would operate correctly (e.g.
> > MV88E6390).
> >
> > Fixes: ea89098ef9a5 ("net: dsa: mv88x6xxx: mv88e6390 errata")
>
> That is further back than needed. And due to the code moving around
> and getting renamed, you are added extra burden on those doing the
> back port for no actual gain.
>
> Please verify what i suggested, 609070133aff1 is better and then
> repost.
The bug was introduced by ea89098ef9a5.
609070133aff1 is only requirement for this fix, but Fixes tag should reference
the commit which introduced the bug, afaik.
So it should be
Fixes: ea89098ef9a5 ("net: dsa: mv88x6xxx: mv88e6390 errata")
Cc: stable@vger.kernel.org # 609070133aff ("net: dsa: mv88e6xxx: update code operating on hidden registers")
Marek
^ permalink raw reply
* Re: [PATCH v2 net] tcp: md5: incorrect tcp_header_len for incoming connections
From: Eric Dumazet @ 2022-04-24 19:36 UTC (permalink / raw)
To: Francesco Ruggeri
Cc: Paolo Abeni, Jakub Kicinski, David Ahern, Hideaki YOSHIFUJI,
David Miller, LKML, netdev
In-Reply-To: <20220421005026.686A45EC01F2@us226.sjc.aristanetworks.com>
On Wed, Apr 20, 2022 at 5:50 PM Francesco Ruggeri <fruggeri@arista.com> wrote:
>
> In tcp_create_openreq_child we adjust tcp_header_len for md5 using the
> remote address in newsk. But that address is still 0 in newsk at this
> point, and it is only set later by the callers (tcp_v[46]_syn_recv_sock).
> Use the address from the request socket instead.
>
> v2: Added "Fixes:" line.
>
> Fixes: cfb6eeb4c860 ("[TCP]: MD5 Signature Option (RFC2385) support.")
> Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
> ---
> net/ipv4/tcp_minisocks.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index 6366df7aaf2a..6854bb1fb32b 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -531,7 +531,7 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
> newtp->tsoffset = treq->ts_off;
> #ifdef CONFIG_TCP_MD5SIG
> newtp->md5sig_info = NULL; /*XXX*/
> - if (newtp->af_specific->md5_lookup(sk, newsk))
> + if (treq->af_specific->req_md5_lookup(sk, req_to_sk(req)))
Wait a minute.
Are you sure treq->af_specific is initialized at this point ?
I should have tested this one liner patch really :/
I think that for syncookies, treq->af_specific is not initialized,
because we do not go through
tcp_conn_request() helper, but instead use cookie_tcp_reqsk_alloc()
Before your patch treq->af_specific was only used during SYNACK
generation, which does not happen in syncookie more while receiving
the third packet.
I will test something like this patch. We could move the init after
cookie_tcp_reqsk_alloc() has been called, but I prefer using the same
construct than tcp_conn_request()
diff --git a/include/net/tcp.h b/include/net/tcp.h
index be712fb9ddd71b2b320356677f3aa1c6759e2698..9987b3fba9f202632916cc439af9d17f1e68bcd3
100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -480,6 +480,7 @@ int __cookie_v4_check(const struct iphdr *iph,
const struct tcphdr *th,
u32 cookie);
struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb);
struct request_sock *cookie_tcp_reqsk_alloc(const struct request_sock_ops *ops,
+ const struct
tcp_request_sock_ops *af_ops,
struct sock *sk, struct
sk_buff *skb);
#ifdef CONFIG_SYN_COOKIES
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 2cb3b852d14861231ac47f0b3e4daeb57682ffd2..f191bfa996d71f11ef786a0a3bcb8f737622d37a
100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -281,6 +281,7 @@ bool cookie_ecn_ok(const struct
tcp_options_received *tcp_opt,
EXPORT_SYMBOL(cookie_ecn_ok);
struct request_sock *cookie_tcp_reqsk_alloc(const struct request_sock_ops *ops,
+ const struct
tcp_request_sock_ops *af_ops,
struct sock *sk,
struct sk_buff *skb)
{
@@ -297,6 +298,8 @@ struct request_sock *cookie_tcp_reqsk_alloc(const
struct request_sock_ops *ops,
return NULL;
treq = tcp_rsk(req);
+ /* treq->af_specific might be used to perform TCP_MD5 lookup */
+ treq->af_specific = af_ops;
treq->syn_tos = TCP_SKB_CB(skb)->ip_dsfield;
#if IS_ENABLED(CONFIG_MPTCP)
treq->is_mptcp = sk_is_mptcp(sk);
@@ -364,7 +367,8 @@ struct sock *cookie_v4_check(struct sock *sk,
struct sk_buff *skb)
goto out;
ret = NULL;
- req = cookie_tcp_reqsk_alloc(&tcp_request_sock_ops, sk, skb);
+ req = cookie_tcp_reqsk_alloc(&tcp_request_sock_ops,
+ &tcp_request_sock_ipv4_ops, sk, skb);
if (!req)
goto out;
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index d1b61d00368e1f58725e9997f74a0b144901277e..9cc123f000fbcfbeff7728bfee5339d6dd6470f9
100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -170,7 +170,8 @@ struct sock *cookie_v6_check(struct sock *sk,
struct sk_buff *skb)
goto out;
ret = NULL;
- req = cookie_tcp_reqsk_alloc(&tcp6_request_sock_ops, sk, skb);
+ req = cookie_tcp_reqsk_alloc(&tcp6_request_sock_ops,
+ &tcp_request_sock_ipv6_ops, sk, skb);
if (!req)
goto out;
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
From: Alaa Mohamed @ 2022-04-24 19:49 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
In-Reply-To: <7c8367b6-95c7-ea39-fafe-72495f343625@blackwall.org>
On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
> On 24/04/2022 15:09, Alaa Mohamed wrote:
>> Add extack support to .ndo_fdb_del in netdevice.h and
>> all related methods.
>>
>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>> ---
>> changes in V3:
>> fix errors reported by checkpatch.pl
>> ---
>> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
>> drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
>> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
>> drivers/net/macvlan.c | 2 +-
>> drivers/net/vxlan/vxlan_core.c | 2 +-
>> include/linux/netdevice.h | 2 +-
>> net/bridge/br_fdb.c | 2 +-
>> net/bridge/br_private.h | 2 +-
>> net/core/rtnetlink.c | 4 ++--
>> 9 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
>> index d768925785ca..7b55d8d94803 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],
>> static int
>> ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
>> struct net_device *dev, const unsigned char *addr,
>> - __always_unused u16 vid)
>> + __always_unused u16 vid, struct netlink_ext_ack *extack)
>> {
>> int err;
>> -
>> +
> What's changed here?
In the previous version, I removed the blank line after "int err;" and
you said I shouldn't so I added blank line.
>
>> if (ndm->ndm_state & NUD_PERMANENT) {
>> netdev_err(dev, "FDB only supports static addresses\n");
>> return -EINVAL;
>> diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
>> index 247bc105bdd2..e07c64e3159c 100644
>> --- a/drivers/net/ethernet/mscc/ocelot_net.c
>> +++ b/drivers/net/ethernet/mscc/ocelot_net.c
>> @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>
>> static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>> struct net_device *dev,
>> - const unsigned char *addr, u16 vid)
>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>> {
>> struct ocelot_port_private *priv = netdev_priv(dev);
>> struct ocelot_port *ocelot_port = &priv->port;
>> struct ocelot *ocelot = ocelot_port->ocelot;
>> int port = priv->chip_port;
>>
>> - return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
>> + return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge, extack);
>> }
>>
>> static int ocelot_port_fdb_dump(struct sk_buff *skb,
>> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>> index d320567b2cca..51fa23418f6a 100644
>> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>> @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device *netdev, void *p)
>>
>> static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>> struct net_device *netdev,
>> - const unsigned char *addr, u16 vid)
>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>> {
>> struct qlcnic_adapter *adapter = netdev_priv(netdev);
>> int err = -EOPNOTSUPP;
>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>> index 069e8824c264..ffd34d9f7049 100644
>> --- a/drivers/net/macvlan.c
>> +++ b/drivers/net/macvlan.c
>> @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>
>> static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>> struct net_device *dev,
>> - const unsigned char *addr, u16 vid)
>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>> {
>> struct macvlan_dev *vlan = netdev_priv(dev);
>> int err = -EINVAL;
>> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
>> index de97ff98d36e..cf2f60037340 100644
>> --- a/drivers/net/vxlan/vxlan_core.c
>> +++ b/drivers/net/vxlan/vxlan_core.c
>> @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
>> /* Delete entry (via netlink) */
>> static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>> struct net_device *dev,
>> - const unsigned char *addr, u16 vid)
>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>> {
>> struct vxlan_dev *vxlan = netdev_priv(dev);
>> union vxlan_addr ip;
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 28ea4f8269d4..d0d2a8f33c73 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1509,7 +1509,7 @@ struct net_device_ops {
>> struct nlattr *tb[],
>> struct net_device *dev,
>> const unsigned char *addr,
>> - u16 vid);
>> + u16 vid, struct netlink_ext_ack *extack);
>> int (*ndo_fdb_dump)(struct sk_buff *skb,
>> struct netlink_callback *cb,
>> struct net_device *dev,
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 6ccda68bd473..5bfce2e9a553 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge *br,
>> /* Remove neighbor entry with RTM_DELNEIGH */
>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>> struct net_device *dev,
>> - const unsigned char *addr, u16 vid)
>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>> {
>> struct net_bridge_vlan_group *vg;
>> struct net_bridge_port *p = NULL;
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 18ccc3d5d296..95348c1c9ce5 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>> const unsigned char *addr, u16 vid, unsigned long flags);
>>
>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>> - struct net_device *dev, const unsigned char *addr, u16 vid);
>> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
> This is way too long (111 chars) and checkpatch should've complained about it.
> WARNING: line length of 111 exceeds 100 columns
> #234: FILE: net/bridge/br_private.h:782:
> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
I will fix it.
>
>> int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
>> const unsigned char *addr, u16 vid, u16 nlh_flags,
>> struct netlink_ext_ack *extack);
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 4041b3e2e8ec..99b30ae58a47 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>> const struct net_device_ops *ops = br_dev->netdev_ops;
>>
>> if (ops->ndo_fdb_del)
>> - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
>> + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
>>
>> if (err)
>> goto out;
>> @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>> if (ndm->ndm_flags & NTF_SELF) {
>> if (dev->netdev_ops->ndo_fdb_del)
>> err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
>> - vid);
>> + vid, extack);
>> else
>> err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
>>
>> --
>> 2.36.0
>>
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
From: Nikolay Aleksandrov @ 2022-04-24 19:55 UTC (permalink / raw)
To: Alaa Mohamed, netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
In-Reply-To: <d89eefc2-664f-8537-d10e-6fdfbb6823ed@gmail.com>
On 24/04/2022 22:49, Alaa Mohamed wrote:
>
> On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
>> On 24/04/2022 15:09, Alaa Mohamed wrote:
>>> Add extack support to .ndo_fdb_del in netdevice.h and
>>> all related methods.
>>>
>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>> ---
>>> changes in V3:
>>> fix errors reported by checkpatch.pl
>>> ---
>>> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
>>> drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
>>> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
>>> drivers/net/macvlan.c | 2 +-
>>> drivers/net/vxlan/vxlan_core.c | 2 +-
>>> include/linux/netdevice.h | 2 +-
>>> net/bridge/br_fdb.c | 2 +-
>>> net/bridge/br_private.h | 2 +-
>>> net/core/rtnetlink.c | 4 ++--
>>> 9 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
>>> index d768925785ca..7b55d8d94803 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],
>>> static int
>>> ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
>>> struct net_device *dev, const unsigned char *addr,
>>> - __always_unused u16 vid)
>>> + __always_unused u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> int err;
>>> -
>>> +
>> What's changed here?
>
> In the previous version, I removed the blank line after "int err;" and you said I shouldn't so I added blank line.
>
Yeah, my question is are you fixing a dos ending or something else?
The blank line is already there, what's wrong with it?
The point is it's not nice to mix style fixes and other changes, more so
if nothing is mentioned in the commit message.
>>
>>> if (ndm->ndm_state & NUD_PERMANENT) {
>>> netdev_err(dev, "FDB only supports static addresses\n");
>>> return -EINVAL;
>>> diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
>>> index 247bc105bdd2..e07c64e3159c 100644
>>> --- a/drivers/net/ethernet/mscc/ocelot_net.c
>>> +++ b/drivers/net/ethernet/mscc/ocelot_net.c
>>> @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>>
>>> static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>> struct net_device *dev,
>>> - const unsigned char *addr, u16 vid)
>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> struct ocelot_port_private *priv = netdev_priv(dev);
>>> struct ocelot_port *ocelot_port = &priv->port;
>>> struct ocelot *ocelot = ocelot_port->ocelot;
>>> int port = priv->chip_port;
>>>
>>> - return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
>>> + return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge, extack);
>>> }
>>>
>>> static int ocelot_port_fdb_dump(struct sk_buff *skb,
>>> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>> index d320567b2cca..51fa23418f6a 100644
>>> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>> @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device *netdev, void *p)
>>>
>>> static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>> struct net_device *netdev,
>>> - const unsigned char *addr, u16 vid)
>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> struct qlcnic_adapter *adapter = netdev_priv(netdev);
>>> int err = -EOPNOTSUPP;
>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>>> index 069e8824c264..ffd34d9f7049 100644
>>> --- a/drivers/net/macvlan.c
>>> +++ b/drivers/net/macvlan.c
>>> @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>>
>>> static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>> struct net_device *dev,
>>> - const unsigned char *addr, u16 vid)
>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> struct macvlan_dev *vlan = netdev_priv(dev);
>>> int err = -EINVAL;
>>> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
>>> index de97ff98d36e..cf2f60037340 100644
>>> --- a/drivers/net/vxlan/vxlan_core.c
>>> +++ b/drivers/net/vxlan/vxlan_core.c
>>> @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
>>> /* Delete entry (via netlink) */
>>> static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>> struct net_device *dev,
>>> - const unsigned char *addr, u16 vid)
>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> struct vxlan_dev *vxlan = netdev_priv(dev);
>>> union vxlan_addr ip;
>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>> index 28ea4f8269d4..d0d2a8f33c73 100644
>>> --- a/include/linux/netdevice.h
>>> +++ b/include/linux/netdevice.h
>>> @@ -1509,7 +1509,7 @@ struct net_device_ops {
>>> struct nlattr *tb[],
>>> struct net_device *dev,
>>> const unsigned char *addr,
>>> - u16 vid);
>>> + u16 vid, struct netlink_ext_ack *extack);
>>> int (*ndo_fdb_dump)(struct sk_buff *skb,
>>> struct netlink_callback *cb,
>>> struct net_device *dev,
>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>> index 6ccda68bd473..5bfce2e9a553 100644
>>> --- a/net/bridge/br_fdb.c
>>> +++ b/net/bridge/br_fdb.c
>>> @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge *br,
>>> /* Remove neighbor entry with RTM_DELNEIGH */
>>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>> struct net_device *dev,
>>> - const unsigned char *addr, u16 vid)
>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>> {
>>> struct net_bridge_vlan_group *vg;
>>> struct net_bridge_port *p = NULL;
>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>> index 18ccc3d5d296..95348c1c9ce5 100644
>>> --- a/net/bridge/br_private.h
>>> +++ b/net/bridge/br_private.h
>>> @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>> const unsigned char *addr, u16 vid, unsigned long flags);
>>>
>>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>> - struct net_device *dev, const unsigned char *addr, u16 vid);
>>> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
>> This is way too long (111 chars) and checkpatch should've complained about it.
>> WARNING: line length of 111 exceeds 100 columns
>> #234: FILE: net/bridge/br_private.h:782:
>> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
>
> I will fix it.
>
>>
>>> int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
>>> const unsigned char *addr, u16 vid, u16 nlh_flags,
>>> struct netlink_ext_ack *extack);
>>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>>> index 4041b3e2e8ec..99b30ae58a47 100644
>>> --- a/net/core/rtnetlink.c
>>> +++ b/net/core/rtnetlink.c
>>> @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>>> const struct net_device_ops *ops = br_dev->netdev_ops;
>>>
>>> if (ops->ndo_fdb_del)
>>> - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
>>> + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
>>>
>>> if (err)
>>> goto out;
>>> @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>>> if (ndm->ndm_flags & NTF_SELF) {
>>> if (dev->netdev_ops->ndo_fdb_del)
>>> err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
>>> - vid);
>>> + vid, extack);
>>> else
>>> err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
>>>
>>> --
>>> 2.36.0
>>>
^ permalink raw reply
* [PATCH net] tcp: make sure treq->af_specific is initialized
From: Eric Dumazet @ 2022-04-24 20:35 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Eric Dumazet, Eric Dumazet, Francesco Ruggeri
From: Eric Dumazet <edumazet@google.com>
syzbot complained about a recent change in TCP stack,
hitting a NULL pointer [1]
tcp request sockets have an af_specific pointer, which
was used before the blamed change only for SYNACK generation
in non SYNCOOKIE mode.
tcp requests sockets momentarily created when third packet
coming from client in SYNCOOKIE mode were not using
treq->af_specific.
Make sure this field is populated, in the same way normal
TCP requests sockets do in tcp_conn_request().
[1]
TCP: request_sock_TCPv6: Possible SYN flooding on port 20002. Sending cookies. Check SNMP counters.
general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
CPU: 1 PID: 3695 Comm: syz-executor864 Not tainted 5.18.0-rc3-syzkaller-00224-g5fd1fe4807f9 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:tcp_create_openreq_child+0xe16/0x16b0 net/ipv4/tcp_minisocks.c:534
Code: 48 c1 ea 03 80 3c 02 00 0f 85 e5 07 00 00 4c 8b b3 28 01 00 00 48 b8 00 00 00 00 00 fc ff df 49 8d 7e 08 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 c9 07 00 00 48 8b 3c 24 48 89 de 41 ff 56 08 48
RSP: 0018:ffffc90000de0588 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff888076490330 RCX: 0000000000000100
RDX: 0000000000000001 RSI: ffffffff87d67ff0 RDI: 0000000000000008
RBP: ffff88806ee1c7f8 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff87d67f00 R11: 0000000000000000 R12: ffff88806ee1bfc0
R13: ffff88801b0e0368 R14: 0000000000000000 R15: 0000000000000000
FS: 00007f517fe58700(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ffcead76960 CR3: 000000006f97b000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<IRQ>
tcp_v6_syn_recv_sock+0x199/0x23b0 net/ipv6/tcp_ipv6.c:1267
tcp_get_cookie_sock+0xc9/0x850 net/ipv4/syncookies.c:207
cookie_v6_check+0x15c3/0x2340 net/ipv6/syncookies.c:258
tcp_v6_cookie_check net/ipv6/tcp_ipv6.c:1131 [inline]
tcp_v6_do_rcv+0x1148/0x13b0 net/ipv6/tcp_ipv6.c:1486
tcp_v6_rcv+0x3305/0x3840 net/ipv6/tcp_ipv6.c:1725
ip6_protocol_deliver_rcu+0x2e9/0x1900 net/ipv6/ip6_input.c:422
ip6_input_finish+0x14c/0x2c0 net/ipv6/ip6_input.c:464
NF_HOOK include/linux/netfilter.h:307 [inline]
NF_HOOK include/linux/netfilter.h:301 [inline]
ip6_input+0x9c/0xd0 net/ipv6/ip6_input.c:473
dst_input include/net/dst.h:461 [inline]
ip6_rcv_finish net/ipv6/ip6_input.c:76 [inline]
NF_HOOK include/linux/netfilter.h:307 [inline]
NF_HOOK include/linux/netfilter.h:301 [inline]
ipv6_rcv+0x27f/0x3b0 net/ipv6/ip6_input.c:297
__netif_receive_skb_one_core+0x114/0x180 net/core/dev.c:5405
__netif_receive_skb+0x24/0x1b0 net/core/dev.c:5519
process_backlog+0x3a0/0x7c0 net/core/dev.c:5847
__napi_poll+0xb3/0x6e0 net/core/dev.c:6413
napi_poll net/core/dev.c:6480 [inline]
net_rx_action+0x8ec/0xc60 net/core/dev.c:6567
__do_softirq+0x29b/0x9c2 kernel/softirq.c:558
invoke_softirq kernel/softirq.c:432 [inline]
__irq_exit_rcu+0x123/0x180 kernel/softirq.c:637
irq_exit_rcu+0x5/0x20 kernel/softirq.c:649
sysvec_apic_timer_interrupt+0x93/0xc0 arch/x86/kernel/apic/apic.c:1097
Fixes: 5b0b9e4c2c89 ("tcp: md5: incorrect tcp_header_len for incoming connections")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Francesco Ruggeri <fruggeri@arista.com>
---
include/net/tcp.h | 1 +
net/ipv4/syncookies.c | 8 +++++++-
net/ipv6/syncookies.c | 3 ++-
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index be712fb9ddd71b2b320356677f3aa1c6759e2698..9987b3fba9f202632916cc439af9d17f1e68bcd3 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -480,6 +480,7 @@ int __cookie_v4_check(const struct iphdr *iph, const struct tcphdr *th,
u32 cookie);
struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb);
struct request_sock *cookie_tcp_reqsk_alloc(const struct request_sock_ops *ops,
+ const struct tcp_request_sock_ops *af_ops,
struct sock *sk, struct sk_buff *skb);
#ifdef CONFIG_SYN_COOKIES
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 2cb3b852d14861231ac47f0b3e4daeb57682ffd2..f33c31dd7366c06a642bdc5954856efa9d8da0ac 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -281,6 +281,7 @@ bool cookie_ecn_ok(const struct tcp_options_received *tcp_opt,
EXPORT_SYMBOL(cookie_ecn_ok);
struct request_sock *cookie_tcp_reqsk_alloc(const struct request_sock_ops *ops,
+ const struct tcp_request_sock_ops *af_ops,
struct sock *sk,
struct sk_buff *skb)
{
@@ -297,6 +298,10 @@ struct request_sock *cookie_tcp_reqsk_alloc(const struct request_sock_ops *ops,
return NULL;
treq = tcp_rsk(req);
+
+ /* treq->af_specific might be used to perform TCP_MD5 lookup */
+ treq->af_specific = af_ops;
+
treq->syn_tos = TCP_SKB_CB(skb)->ip_dsfield;
#if IS_ENABLED(CONFIG_MPTCP)
treq->is_mptcp = sk_is_mptcp(sk);
@@ -364,7 +369,8 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
goto out;
ret = NULL;
- req = cookie_tcp_reqsk_alloc(&tcp_request_sock_ops, sk, skb);
+ req = cookie_tcp_reqsk_alloc(&tcp_request_sock_ops,
+ &tcp_request_sock_ipv4_ops, sk, skb);
if (!req)
goto out;
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index d1b61d00368e1f58725e9997f74a0b144901277e..9cc123f000fbcfbeff7728bfee5339d6dd6470f9 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -170,7 +170,8 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
goto out;
ret = NULL;
- req = cookie_tcp_reqsk_alloc(&tcp6_request_sock_ops, sk, skb);
+ req = cookie_tcp_reqsk_alloc(&tcp6_request_sock_ops,
+ &tcp_request_sock_ipv6_ops, sk, skb);
if (!req)
goto out;
--
2.36.0.rc2.479.g8af0fa9b8e-goog
^ permalink raw reply related
* Re: Zero-Day bug in VLAN offloading + cooked AF_PACKET
From: alexandre.ferrieux @ 2022-04-24 20:57 UTC (permalink / raw)
To: netdev
In-Reply-To: <4756cc37-340b-f2f6-e004-0d77573f33df@orange.com>
On 4/23/22 22:02, alexandre.ferrieux@orange.com wrote:
>
> TL;DR: outgoing VLAN-tagged traffic to non-offloaded interfaces is captured as
> corrupted in cooked mode, and has been so since at least 3.4...
>
> [...]
>
> However, there's a catch: for outgoing packets, *if* the interface has no
> hardware VLAN offloading, the ethertype gets overwritten by ... the TPID
> (0x8100). As a result, a consumer of the L3 frame has absolutely no way to
> recover its type.
Digging a bit shows that the key is a discrepancy between where "layer 2.5" sits
wrt the link/network boundary.
Indeed:
- to L3 abstractions, the VLAN belongs to "link layer" and should be mostly
ignored: skb->protocol and skb->network_header rule the world.
- to (software) VLAN insertion code "__vlan_hwaccel_push_inside", when a frame
is prepared for transmission by a non-vlan-offload-capable device, the link
layer stops right after the MAC adresses, the TPID is provided as an ethertype
and written into skb->protocol, and the original ethertype is explicitly stacked
after the TCI. In other words, 802.1Q is a kind of layer 3 (though not
completely: skb->network_header is *not* set to the TPID position - it remains
at the true L3).
This discrepancy is mostly harmless, except in the presence of a tap: there
(e.g. in packet_rcv), on the tx path in cooked (SOCK_DGRAM) mode, the packet is
stripped of everything before its skb->network_header:
if (sk->sk_type != SOCK_DGRAM)
skb_push(skb, skb->data - skb_mac_header(skb));
else if (skb->pkt_type == PACKET_OUTGOING) {
/* Special case: outgoing packets have ll header at head */
skb_pull(skb, skb_network_offset(skb));
}
As a result, the original ethertype is lost, with the effect we know.
Now I can see two ways out:
(A) "Fix" _vlan_hwaccel_push_inside to update skb->network_header
(decrementing it by 4 bytes), to preserve the invariant "skb->protocol describes
what starts at skb->network_header".
(B) Refine the stripping in packet_rcv (and brothers) to special-case VLANs,
effectively re-parsing what we've just inserted.
To me, (A) is cleanest as it enforces a broken invariant; however, it possibly
modifies the tx path in ways I cannot fathom. Conversely, (B) looks like an ugly
hack whose sole value is to be confined to tap-cloned skbs, bounding the harm it
may cause, but with a bit of CPU waste...
Please advise :)
-Alex
_________________________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
From: Alaa Mohamed @ 2022-04-24 21:09 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
In-Reply-To: <4bf69eef-7444-1238-0f4a-fb0fccda080c@blackwall.org>
On ٢٤/٤/٢٠٢٢ ٢١:٥٥, Nikolay Aleksandrov wrote:
> On 24/04/2022 22:49, Alaa Mohamed wrote:
>> On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
>>> On 24/04/2022 15:09, Alaa Mohamed wrote:
>>>> Add extack support to .ndo_fdb_del in netdevice.h and
>>>> all related methods.
>>>>
>>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>>> ---
>>>> changes in V3:
>>>> fix errors reported by checkpatch.pl
>>>> ---
>>>> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
>>>> drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
>>>> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
>>>> drivers/net/macvlan.c | 2 +-
>>>> drivers/net/vxlan/vxlan_core.c | 2 +-
>>>> include/linux/netdevice.h | 2 +-
>>>> net/bridge/br_fdb.c | 2 +-
>>>> net/bridge/br_private.h | 2 +-
>>>> net/core/rtnetlink.c | 4 ++--
>>>> 9 files changed, 12 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
>>>> index d768925785ca..7b55d8d94803 100644
>>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>>> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],
>>>> static int
>>>> ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
>>>> struct net_device *dev, const unsigned char *addr,
>>>> - __always_unused u16 vid)
>>>> + __always_unused u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> int err;
>>>> -
>>>> +
>>> What's changed here?
>> In the previous version, I removed the blank line after "int err;" and you said I shouldn't so I added blank line.
>>
> Yeah, my question is are you fixing a dos ending or something else?
> The blank line is already there, what's wrong with it?
No, I didn't.
>
> The point is it's not nice to mix style fixes and other changes, more so
> if nothing is mentioned in the commit message.
Got it, So, what should I do to fix it?
>>>> if (ndm->ndm_state & NUD_PERMANENT) {
>>>> netdev_err(dev, "FDB only supports static addresses\n");
>>>> return -EINVAL;
>>>> diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
>>>> index 247bc105bdd2..e07c64e3159c 100644
>>>> --- a/drivers/net/ethernet/mscc/ocelot_net.c
>>>> +++ b/drivers/net/ethernet/mscc/ocelot_net.c
>>>> @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>>>
>>>> static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>>> struct net_device *dev,
>>>> - const unsigned char *addr, u16 vid)
>>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> struct ocelot_port_private *priv = netdev_priv(dev);
>>>> struct ocelot_port *ocelot_port = &priv->port;
>>>> struct ocelot *ocelot = ocelot_port->ocelot;
>>>> int port = priv->chip_port;
>>>>
>>>> - return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
>>>> + return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge, extack);
>>>> }
>>>>
>>>> static int ocelot_port_fdb_dump(struct sk_buff *skb,
>>>> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>>> index d320567b2cca..51fa23418f6a 100644
>>>> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>>> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
>>>> @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device *netdev, void *p)
>>>>
>>>> static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>>> struct net_device *netdev,
>>>> - const unsigned char *addr, u16 vid)
>>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> struct qlcnic_adapter *adapter = netdev_priv(netdev);
>>>> int err = -EOPNOTSUPP;
>>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>>>> index 069e8824c264..ffd34d9f7049 100644
>>>> --- a/drivers/net/macvlan.c
>>>> +++ b/drivers/net/macvlan.c
>>>> @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>>>
>>>> static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>>>> struct net_device *dev,
>>>> - const unsigned char *addr, u16 vid)
>>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> struct macvlan_dev *vlan = netdev_priv(dev);
>>>> int err = -EINVAL;
>>>> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
>>>> index de97ff98d36e..cf2f60037340 100644
>>>> --- a/drivers/net/vxlan/vxlan_core.c
>>>> +++ b/drivers/net/vxlan/vxlan_core.c
>>>> @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
>>>> /* Delete entry (via netlink) */
>>>> static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>>> struct net_device *dev,
>>>> - const unsigned char *addr, u16 vid)
>>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> struct vxlan_dev *vxlan = netdev_priv(dev);
>>>> union vxlan_addr ip;
>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>>> index 28ea4f8269d4..d0d2a8f33c73 100644
>>>> --- a/include/linux/netdevice.h
>>>> +++ b/include/linux/netdevice.h
>>>> @@ -1509,7 +1509,7 @@ struct net_device_ops {
>>>> struct nlattr *tb[],
>>>> struct net_device *dev,
>>>> const unsigned char *addr,
>>>> - u16 vid);
>>>> + u16 vid, struct netlink_ext_ack *extack);
>>>> int (*ndo_fdb_dump)(struct sk_buff *skb,
>>>> struct netlink_callback *cb,
>>>> struct net_device *dev,
>>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>>> index 6ccda68bd473..5bfce2e9a553 100644
>>>> --- a/net/bridge/br_fdb.c
>>>> +++ b/net/bridge/br_fdb.c
>>>> @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge *br,
>>>> /* Remove neighbor entry with RTM_DELNEIGH */
>>>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>>> struct net_device *dev,
>>>> - const unsigned char *addr, u16 vid)
>>>> + const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>>>> {
>>>> struct net_bridge_vlan_group *vg;
>>>> struct net_bridge_port *p = NULL;
>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>> index 18ccc3d5d296..95348c1c9ce5 100644
>>>> --- a/net/bridge/br_private.h
>>>> +++ b/net/bridge/br_private.h
>>>> @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>>> const unsigned char *addr, u16 vid, unsigned long flags);
>>>>
>>>> int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>>> - struct net_device *dev, const unsigned char *addr, u16 vid);
>>>> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
>>> This is way too long (111 chars) and checkpatch should've complained about it.
>>> WARNING: line length of 111 exceeds 100 columns
>>> #234: FILE: net/bridge/br_private.h:782:
>>> + struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);
>> I will fix it.
>>
>>>> int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
>>>> const unsigned char *addr, u16 vid, u16 nlh_flags,
>>>> struct netlink_ext_ack *extack);
>>>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>>>> index 4041b3e2e8ec..99b30ae58a47 100644
>>>> --- a/net/core/rtnetlink.c
>>>> +++ b/net/core/rtnetlink.c
>>>> @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>>>> const struct net_device_ops *ops = br_dev->netdev_ops;
>>>>
>>>> if (ops->ndo_fdb_del)
>>>> - err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
>>>> + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
>>>>
>>>> if (err)
>>>> goto out;
>>>> @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>>>> if (ndm->ndm_flags & NTF_SELF) {
>>>> if (dev->netdev_ops->ndo_fdb_del)
>>>> err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
>>>> - vid);
>>>> + vid, extack);
>>>> else
>>>> err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
>>>>
>>>> --
>>>> 2.36.0
>>>>
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
From: Nikolay Aleksandrov @ 2022-04-24 21:52 UTC (permalink / raw)
To: Alaa Mohamed, netdev
Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
claudiu.manoil, alexandre.belloni, shshaikh, manishc,
intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
bridge
In-Reply-To: <3bcb2d3d-8b8b-8a8f-1285-7277394b4e6b@gmail.com>
On 4/25/22 00:09, Alaa Mohamed wrote:
>
> On ٢٤/٤/٢٠٢٢ ٢١:٥٥, Nikolay Aleksandrov wrote:
>> On 24/04/2022 22:49, Alaa Mohamed wrote:
>>> On ٢٤/٤/٢٠٢٢ ٢١:٠٢, Nikolay Aleksandrov wrote:
>>>> On 24/04/2022 15:09, Alaa Mohamed wrote:
>>>>> Add extack support to .ndo_fdb_del in netdevice.h and
>>>>> all related methods.
>>>>>
>>>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>>>> ---
>>>>> changes in V3:
>>>>> fix errors reported by checkpatch.pl
>>>>> ---
>>>>> drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
>>>>> drivers/net/ethernet/mscc/ocelot_net.c | 4 ++--
>>>>> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
>>>>> drivers/net/macvlan.c | 2 +-
>>>>> drivers/net/vxlan/vxlan_core.c | 2 +-
>>>>> include/linux/netdevice.h | 2 +-
>>>>> net/bridge/br_fdb.c | 2 +-
>>>>> net/bridge/br_private.h | 2 +-
>>>>> net/core/rtnetlink.c | 4 ++--
>>>>> 9 files changed, 12 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> b/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> index d768925785ca..7b55d8d94803 100644
>>>>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>>>>> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct
>>>>> nlattr __always_unused *tb[],
>>>>> static int
>>>>> ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
>>>>> struct net_device *dev, const unsigned char *addr,
>>>>> - __always_unused u16 vid)
>>>>> + __always_unused u16 vid, struct netlink_ext_ack *extack)
>>>>> {
>>>>> int err;
>>>>> -
>>>>> +
>>>> What's changed here?
>>> In the previous version, I removed the blank line after "int err;"
>>> and you said I shouldn't so I added blank line.
>>>
>> Yeah, my question is are you fixing a dos ending or something else?
>> The blank line is already there, what's wrong with it?
> No, I didn't.
>>
>> The point is it's not nice to mix style fixes and other changes, more so
>> if nothing is mentioned in the commit message.
> Got it, So, what should I do to fix it?
Don't change that line? I mean I'm even surprised this made it in the
patch. As I mentioned above, there is already a new line there so I'm
not sure how you're removing it and adding it again. :)
Cheers,
Nik
^ permalink raw reply
* Re: net: stmmac: dwmac-imx: half duplex crash
From: Andrew Lunn @ 2022-04-24 22:01 UTC (permalink / raw)
To: Marcel Ziswiler
Cc: netdev@vger.kernel.org, alexandre.torgue@foss.st.com,
davem@davemloft.net, kernel@pengutronix.de, linux-imx@nxp.com,
linux-stm32@st-md-mailman.stormreply.com, festevam@gmail.com,
linux-kernel@vger.kernel.org, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org, s.hauer@pengutronix.de,
mcoquelin.stm32@gmail.com, pabeni@redhat.com,
peppe.cavallaro@st.com, joabreu@synopsys.com, kuba@kernel.org
In-Reply-To: <36ba455aad3e57c0c1f75cce4ee0f3da69e139a1.camel@toradex.com>
On Sat, Apr 23, 2022 at 10:58:07PM +0000, Marcel Ziswiler wrote:
> Hi there
>
> We lately tried operating the IMX8MPEVK ENET_QOS imx-dwmac driver in half-duplex modes which crashes as
> follows:
How many transmit queues do you have in operation:
/* Half-Duplex can only work with single queue */
if (priv->plat->tx_queues_to_use > 1)
priv->phylink_config.mac_capabilities &=
~(MAC_10HD | MAC_100HD | MAC_1000HD);
What does ethtool show before you force it? Does it actually list the
HALF modes?
Andrew
^ permalink raw reply
* Re: [PATCH v3] brcmfmac: of: introduce new property to allow disable PNO
From: Andrew Lunn @ 2022-04-24 22:04 UTC (permalink / raw)
To: Hermes Zhang
Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
David S. Miller, Jakub Kicinski, Paolo Abeni, kernel,
Hermes Zhang, linux-wireless, brcm80211-dev-list.pdl,
SHA-cyfmac-dev-list, netdev, linux-kernel
In-Reply-To: <20220424022224.3609950-1-chenhui.zhang@axis.com>
On Sun, Apr 24, 2022 at 10:22:24AM +0800, Hermes Zhang wrote:
> From: Hermes Zhang <chenhuiz@axis.com>
>
> Some versions of the Broadcom firmware for this chip seem to hang
> if the PNO feature is enabled when connecting to a dummy or
> non-existent AP.
> Add a new property to allow the disabling of PNO for devices with
> this specific firmware.
If you know the specific version of the firmware which is broken, why
do you need a DT property? Why not just check the firmware version and
disable it automatically?
It does not seem like you are describing hardware here, which is what
DT is for.
Andrew
^ permalink raw reply
* Re: [PATCH] FDDI: defxx: simplify if-if to if-else
From: Andrew Lunn @ 2022-04-24 22:17 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Wan Jiabing, David S. Miller, Jakub Kicinski, Paolo Abeni, netdev,
linux-kernel, kael_w
In-Reply-To: <alpine.DEB.2.21.2204241137440.9383@angie.orcam.me.uk>
On Sun, Apr 24, 2022 at 11:39:50AM +0100, Maciej W. Rozycki wrote:
> On Sun, 24 Apr 2022, Wan Jiabing wrote:
>
> > diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c
> > index b584ffe38ad6..3edb2e96f763 100644
> > --- a/drivers/net/fddi/defxx.c
> > +++ b/drivers/net/fddi/defxx.c
> > @@ -585,10 +585,10 @@ static int dfx_register(struct device *bdev)
> > bp->mmio = false;
> > dfx_get_bars(bp, bar_start, bar_len);
> > }
> > - }
> > - if (!dfx_use_mmio)
> > + } else {
> > region = request_region(bar_start[0], bar_len[0],
> > bdev->driver->name);
> > + }
>
> NAK. The first conditional optionally sets `bp->mmio = false', which
> changes the value of `dfx_use_mmio' in some configurations:
>
> #if defined(CONFIG_EISA) || defined(CONFIG_PCI)
> #define dfx_use_mmio bp->mmio
> #else
> #define dfx_use_mmio true
> #endif
Which is just asking for trouble like this.
Could i suggest dfx_use_mmio is changed to DFX_USE_MMIO to give a hint
something horrible is going on.
It probably won't stop the robots finding this if (x) if (!x), but
there is a chance the robot drivers will wonder why it is upper case.
Andrew
^ permalink raw reply
* Re: [PATCH v2] net: dsa: mv88e6xxx: Fix port_hidden_wait to account for port_base_addr
From: Andrew Lunn @ 2022-04-24 22:33 UTC (permalink / raw)
To: Marek Behún
Cc: Nathan Rossi, netdev, linux-kernel, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S. Miller,
Jakub Kicinski, Paolo Abeni
In-Reply-To: <20220424213359.246cd5ab@thinkpad>
On Sun, Apr 24, 2022 at 09:33:59PM +0200, Marek Behún wrote:
> On Sun, 24 Apr 2022 21:26:58 +0200
> Andrew Lunn <andrew@lunn.ch> wrote:
>
> > On Sun, Apr 24, 2022 at 03:31:43PM +0000, Nathan Rossi wrote:
> > > The other port_hidden functions rely on the port_read/port_write
> > > functions to access the hidden control port. These functions apply the
> > > offset for port_base_addr where applicable. Update port_hidden_wait to
> > > use the port_wait_bit so that port_base_addr offsets are accounted for
> > > when waiting for the busy bit to change.
> > >
> > > Without the offset the port_hidden_wait function would timeout on
> > > devices that have a non-zero port_base_addr (e.g. MV88E6141), however
> > > devices that have a zero port_base_addr would operate correctly (e.g.
> > > MV88E6390).
> > >
> > > Fixes: ea89098ef9a5 ("net: dsa: mv88x6xxx: mv88e6390 errata")
> >
> > That is further back than needed. And due to the code moving around
> > and getting renamed, you are added extra burden on those doing the
> > back port for no actual gain.
> >
> > Please verify what i suggested, 609070133aff1 is better and then
> > repost.
>
> The bug was introduced by ea89098ef9a5.
I have to disagree with that. ea89098ef9a5 adds:
mv88e6390_hidden_wait()
The mv88e6390_ means it should be used with the mv88e6390 family. And
all members of that family have port offset 0. There is no bug here.
609070133aff1 renames it to mv88e6xxx_port_hidden_wait(). It now has
the generic mv88e6xxx_ prefix, so we can expect it to work with any
device. But it does not. This is where the bug has introduced.
But what i think is more important, is i doubt git cherry-pick is
clever enough to be able to follow 609070133aff1 and know where to
make the change in revisions before then. So it is going to need a
human to figure out the backport. And that effort is a waist of time,
because there is no bug before then.
Andrew
^ permalink raw reply
* Re: [Patch net-next] net: dsa: ksz: added the generic port_stp_state_set function
From: Florian Fainelli @ 2022-04-24 23:01 UTC (permalink / raw)
To: Arun Ramadoss, linux-kernel, netdev
Cc: paolo Abeni, Jakub Kicinski, David S. Miller, Vladimir Oltean,
Vivien Didelot, Andrew Lunn, UNGLinuxDriver, Woojung Huh
In-Reply-To: <20220424112831.11504-1-arun.ramadoss@microchip.com>
On 4/24/2022 4:28 AM, Arun Ramadoss wrote:
> The ksz8795 and ksz9477 uses the same algorithm for the
> port_stp_state_set function except the register address is different. So
> moved the algorithm to the ksz_common.c and used the dev_ops for
> register read and write. This function can also used for the lan937x
> part. Hence making it generic for all the parts.
>
> Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v2] net: dsa: mv88e6xxx: Fix port_hidden_wait to account for port_base_addr
From: Marek Behún @ 2022-04-24 23:16 UTC (permalink / raw)
To: Andrew Lunn
Cc: Nathan Rossi, netdev, linux-kernel, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S. Miller,
Jakub Kicinski, Paolo Abeni
In-Reply-To: <YmXQK7Wzb1GDxwRP@lunn.ch>
On Mon, 25 Apr 2022 00:33:15 +0200
Andrew Lunn <andrew@lunn.ch> wrote:
> On Sun, Apr 24, 2022 at 09:33:59PM +0200, Marek Behún wrote:
> > On Sun, 24 Apr 2022 21:26:58 +0200
> > Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > On Sun, Apr 24, 2022 at 03:31:43PM +0000, Nathan Rossi wrote:
> > > > The other port_hidden functions rely on the port_read/port_write
> > > > functions to access the hidden control port. These functions apply the
> > > > offset for port_base_addr where applicable. Update port_hidden_wait to
> > > > use the port_wait_bit so that port_base_addr offsets are accounted for
> > > > when waiting for the busy bit to change.
> > > >
> > > > Without the offset the port_hidden_wait function would timeout on
> > > > devices that have a non-zero port_base_addr (e.g. MV88E6141), however
> > > > devices that have a zero port_base_addr would operate correctly (e.g.
> > > > MV88E6390).
> > > >
> > > > Fixes: ea89098ef9a5 ("net: dsa: mv88x6xxx: mv88e6390 errata")
> > >
> > > That is further back than needed. And due to the code moving around
> > > and getting renamed, you are added extra burden on those doing the
> > > back port for no actual gain.
> > >
> > > Please verify what i suggested, 609070133aff1 is better and then
> > > repost.
> >
> > The bug was introduced by ea89098ef9a5.
>
> I have to disagree with that. ea89098ef9a5 adds:
>
> mv88e6390_hidden_wait()
>
> The mv88e6390_ means it should be used with the mv88e6390 family. And
> all members of that family have port offset 0. There is no bug here.
>
> 609070133aff1 renames it to mv88e6xxx_port_hidden_wait(). It now has
> the generic mv88e6xxx_ prefix, so we can expect it to work with any
> device. But it does not. This is where the bug has introduced.
You are right. My bad, sorry.
Marek
^ permalink raw reply
* Re: [PATCH] FDDI: defxx: simplify if-if to if-else
From: Maciej W. Rozycki @ 2022-04-24 23:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: Wan Jiabing, David S. Miller, Jakub Kicinski, Paolo Abeni, netdev,
linux-kernel, kael_w
In-Reply-To: <YmXMcUAhUg/p1X3R@lunn.ch>
On Mon, 25 Apr 2022, Andrew Lunn wrote:
> > NAK. The first conditional optionally sets `bp->mmio = false', which
> > changes the value of `dfx_use_mmio' in some configurations:
> >
> > #if defined(CONFIG_EISA) || defined(CONFIG_PCI)
> > #define dfx_use_mmio bp->mmio
> > #else
> > #define dfx_use_mmio true
> > #endif
>
> Which is just asking for trouble like this.
>
> Could i suggest dfx_use_mmio is changed to DFX_USE_MMIO to give a hint
> something horrible is going on.
There's legacy behind it, `dfx_use_mmio' used to be a proper variable and
references were retained not to obfuscate the changes that ultimately led
to the current arrangement. I guess at this stage it could be changed to
a function-like macro or a static inline function taking `bp' as the
argument.
> It probably won't stop the robots finding this if (x) if (!x), but
> there is a chance the robot drivers will wonder why it is upper case.
Well, blindly relying on automation is bound to cause trouble. There has
to be a piece of intelligence signing the results off at the end.
And there's nothing wrong with if (x) if (!x) in the first place; any
sane compiler will produce reasonable output from it. Don't fix what
ain't broke! And watch out for volatiles!
Maciej
^ permalink raw reply
* Re: [PATCH net-next v1 1/4] net: phy: broadcom: Add PTP support for some Broadcom PHYs.
From: Florian Fainelli @ 2022-04-24 23:27 UTC (permalink / raw)
To: Jonathan Lemon, bcm-kernel-feedback-list, andrew, hkallweit1,
linux, richardcochran
Cc: netdev, kernel-team
In-Reply-To: <20220424022356.587949-2-jonathan.lemon@gmail.com>
On 4/23/2022 7:23 PM, Jonathan Lemon wrote:
> This adds PTP support for BCM54210E Broadcom PHYs, in particular,
> the BCM54213PE, as used in the Rasperry PI CM4. It has only been
> tested on that hardware.
>
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> ---
[snip]
Mostly checking register names/offsets/usage because I am not familiar
enough with how PTP is supposed to work.
> +#define MODE_SEL_SHIFT_PORT 0
> +#define MODE_SEL_SHIFT_CPU 8
> +
> +#define rx_mode(sel, evt, act) \
> + (((MODE_RX_##act) << (MODE_EVT_SHIFT_##evt)) << (MODE_SEL_SHIFT_##sel))
> +
> +#define tx_mode(sel, evt, act) \
> + (((MODE_TX_##act) << (MODE_EVT_SHIFT_##evt)) << (MODE_SEL_SHIFT_##sel))
I would capitalize these two macros to make it clear that they are
exactly that, macros.
> +
> +/* needs global TS capture first */
> +#define TX_TS_CAPTURE 0x0821
> +#define TX_TS_CAP_EN BIT(0)
> +#define RX_TS_CAPTURE 0x0822
> +#define RX_TS_CAP_EN BIT(0)
> +
> +#define TIME_CODE_0 0x0854
Maybe add a macro here as well:
#define TIME_CODE(x) (TIME_CODE0 + (x))
> +#define TIME_CODE_1 0x0855
> +#define TIME_CODE_2 0x0856
> +#define TIME_CODE_3 0x0857
> +#define TIME_CODE_4 0x0858
> +
> +#define DPLL_SELECT 0x085b
> +#define DPLL_HB_MODE2 BIT(6)
Seems like you have better documentation than I do :)
> +#define SHADOW_CTRL 0x085c
Unused
> +#define SHADOW_LOAD 0x085d
> +#define TIME_CODE_LOAD BIT(10)
> +#define SYNC_OUT_LOAD BIT(9)
> +#define NCO_TIME_LOAD BIT(7)
NCO Divider load is bit 8 and Local time Load bit is 7, can you check
which one you need?
> +#define FREQ_LOAD BIT(6)
> +#define INTR_MASK 0x085e
> +#define INTR_STATUS 0x085f
> +#define INTC_FSYNC BIT(0)
> +#define INTC_SOP BIT(1)
> +
> +#define FREQ_REG_LSB 0x0873
> +#define FREQ_REG_MSB 0x0874
Those two hold the NCO frequency, can you rename accordingly?
> +
> +#define NCO_TIME_0 0x0875
> +#define NCO_TIME_1 0x0876
> +#define NCO_TIME_2_CTRL 0x0877
> +#define FREQ_MDIO_SEL BIT(14)
> +
> +#define SYNC_OUT_0 0x0878
> +#define SYNC_OUT_1 0x0879
> +#define SYNC_OUT_2 0x087a
Likewise a macro could be useful here.
> +
> +#define TS_READ_CTRL 0x0885
> +#define TS_READ_START BIT(0)
> +#define TS_READ_END BIT(1)
> +
> +#define TIMECODE_CTRL 0x08c3
> +#define TX_TIMECODE_SEL GENMASK(7, 0)
> +#define RX_TIMECODE_SEL GENMASK(15, 8)
This one looks out of order compared to the other registers
> +
> +#define TS_REG_0 0x0889 > +#define TS_REG_1 0x088a
> +#define TS_REG_2 0x088b
> +#define TS_REG_3 0x08c4
> +#define TS_INFO_0 0x088c
> +#define TS_INFO_1 0x088d
> +
> +#define HB_REG_0 0x0886
> +#define HB_REG_1 0x0887
> +#define HB_REG_2 0x0888
> +#define HB_REG_3 0x08ec > +#define HB_REG_4 0x08ed
> +#define HB_STAT_CTRL 0x088e
> +#define HB_READ_START BIT(10)
> +#define HB_READ_END BIT(11)
> +#define HB_READ_MASK GENMASK(11, 10)
> +
> +#define NSE_CTRL 0x087f
This one is also out of order.
[snip]
> +struct bcm_ptp_private *bcm_ptp_probe(struct phy_device *phydev)
> +{
> + struct bcm_ptp_private *priv;
> + struct ptp_clock *clock;
> +
> + switch (BRCM_PHY_MODEL(phydev)) {
> + case PHY_ID_BCM54210E:
> +#ifdef PHY_ID_BCM54213PE
> + case PHY_ID_BCM54213PE:
> +#endif
This does not exist upstream.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v1 3/4] net: phy: broadcom: Hook up the PTP PHY functions
From: Florian Fainelli @ 2022-04-24 23:29 UTC (permalink / raw)
To: Jonathan Lemon, bcm-kernel-feedback-list, andrew, hkallweit1,
linux, richardcochran
Cc: netdev, kernel-team
In-Reply-To: <20220424022356.587949-4-jonathan.lemon@gmail.com>
On 4/23/2022 7:23 PM, Jonathan Lemon wrote:
> Add 'struct bcm_ptp_private' to bcm54xx_phy_priv which points to
> an optional PTP structure attached to the PHY. This is allocated
> on probe, if PHY PTP support is configured, and if the PHY has a
> PTP supported by the driver.
>
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v1 4/4] net: phy: Kconfig: Add broadcom PTP PHY library
From: Florian Fainelli @ 2022-04-24 23:29 UTC (permalink / raw)
To: Jonathan Lemon, bcm-kernel-feedback-list, andrew, hkallweit1,
linux, richardcochran
Cc: netdev, kernel-team
In-Reply-To: <20220424022356.587949-5-jonathan.lemon@gmail.com>
On 4/23/2022 7:23 PM, Jonathan Lemon wrote:
> Add a Broadcom PTP PHY library, initially supporting the BCM54213PE
> found in the RPi CM4.
>
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
This should be squashed into the first patch.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v1 2/4] net: phy: broadcom: Add Broadcom PTP hooks to bcm-phy-lib
From: Florian Fainelli @ 2022-04-24 23:35 UTC (permalink / raw)
To: Jonathan Lemon, bcm-kernel-feedback-list, andrew, hkallweit1,
linux, richardcochran
Cc: netdev, kernel-team
In-Reply-To: <20220424022356.587949-3-jonathan.lemon@gmail.com>
On 4/23/2022 7:23 PM, Jonathan Lemon wrote:
> Add the public bcm_ptp_probe() and bcm_ptp_config_init() functions
> to the bcm-phy library. The PTP functions are contained in a separate
> file for clarity, and also to simplify the PTP clock dependencies.
>
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> ---
> drivers/net/phy/bcm-phy-lib.c | 13 +++++++++++++
> drivers/net/phy/bcm-phy-lib.h | 3 +++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/net/phy/bcm-phy-lib.c b/drivers/net/phy/bcm-phy-lib.c
> index 287cccf8f7f4..b9d2d1d48402 100644
> --- a/drivers/net/phy/bcm-phy-lib.c
> +++ b/drivers/net/phy/bcm-phy-lib.c
> @@ -816,6 +816,19 @@ int bcm_phy_cable_test_get_status_rdb(struct phy_device *phydev,
> }
> EXPORT_SYMBOL_GPL(bcm_phy_cable_test_get_status_rdb);
>
> +#if !IS_ENABLED(CONFIG_BCM_NET_PHYPTP)
> +struct bcm_ptp_private *bcm_ptp_probe(struct phy_device *phydev)
> +{
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(bcm_ptp_probe);
> +
> +void bcm_ptp_config_init(struct phy_device *phydev)
> +{
> +}
> +EXPORT_SYMBOL_GPL(bcm_ptp_config_init);
> +#endif
I would place those in bcm-phy-lib.h instead of in bcm-phy-lib.c and
have them be static inline stubs.
--
Florian
^ permalink raw reply
* Re: [PATCH] rtlwifi: btcoex: fix if == else warning
From: Pkshih @ 2022-04-25 0:04 UTC (permalink / raw)
To: davem@davemloft.net, kvalo@kernel.org, guozhengkui@vivo.com,
linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
pabeni@redhat.com, netdev@vger.kernel.org, kuba@kernel.org
Cc: zhengkui_guo@outlook.com
In-Reply-To: <20220424075548.1544-1-guozhengkui@vivo.com>
On Sun, 2022-04-24 at 15:55 +0800, Guo Zhengkui wrote:
> Fix the following coccicheck warning:
>
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c:1604:2-4:
> WARNING: possible condition with no effect (if == else).
>
> Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
> ---
> .../realtek/rtlwifi/btcoexist/halbtc8821a1ant.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c
> b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c
> index a18dffc8753a..2f4c6a37a2e8 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c
> @@ -1601,17 +1601,10 @@ static void btc8821a1ant_act_wifi_con_bt_acl_busy(struct btc_coexist
> *btcoexist,
> }
> } else if (bt_link_info->hid_exist && bt_link_info->a2dp_exist) {
> /* HID+A2DP */
> - if ((bt_rssi_state == BTC_RSSI_STATE_HIGH) ||
> - (bt_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
> - btc8821a1ant_ps_tdma(btcoexist, NORMAL_EXEC,
> - true, 14);
> - coex_dm->auto_tdma_adjust = false;
> - } else {
> - /*for low BT RSSI*/
> - btc8821a1ant_ps_tdma(btcoexist, NORMAL_EXEC,
> - true, 14);
> - coex_dm->auto_tdma_adjust = false;
> - }
> + /* for low BT RSSI */
The comment shuold be removed, or "No need to consider BT RSSI".
> + btc8821a1ant_ps_tdma(btcoexist, NORMAL_EXEC,
> + true, 14);
> + coex_dm->auto_tdma_adjust = false;
>
> btc8821a1ant_coex_table_with_type(btcoexist, NORMAL_EXEC, 1);
> } else if ((bt_link_info->pan_only) ||
>
The code is to preserve a room to fine tune BT coexistence to get
better user experience for certain cases. Since it works well,
I think they can be removed now.
--
Ping-Ke
^ permalink raw reply
* [PATCH net] tcp: fix potential xmit stalls caused by TCP_NOTSENT_LOWAT
From: Eric Dumazet @ 2022-04-25 0:34 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, Eric Dumazet, Eric Dumazet, Doug Porter,
Soheil Hassas Yeganeh, Neal Cardwell
From: Eric Dumazet <edumazet@google.com>
I had this bug sitting for too long in my pile, it is time to fix it.
Thanks to Doug Porter for reminding me of it!
We had various attempts in the past, including commit
0cbe6a8f089e ("tcp: remove SOCK_QUEUE_SHRUNK"),
but the issue is that TCP stack currently only generates
EPOLLOUT from input path, when tp->snd_una has advanced
and skb(s) cleaned from rtx queue.
If a flow has a big RTT, and/or receives SACKs, it is possible
that the notsent part (tp->write_seq - tp->snd_nxt) reaches 0
and no more data can be sent until tp->snd_una finally advances.
What is needed is to also check if POLLOUT needs to be generated
whenever tp->snd_nxt is advanced, from output path.
This bug triggers more often after an idle period, as
we do not receive ACK for at least one RTT. tcp_notsent_lowat
could be a fraction of what CWND and pacing rate would allow to
send during this RTT.
In a followup patch, I will remove the bogus call
to tcp_chrono_stop(sk, TCP_CHRONO_SNDBUF_LIMITED)
from tcp_check_space(). Fact that we have decided to generate
an EPOLLOUT does not mean the application has immediately
refilled the transmit queue. This optimistic call
might have been the reason the bug seemed not too serious.
Tested:
200 ms rtt, 1% packet loss, 32 MB tcp_rmem[2] and tcp_wmem[2]
$ echo 500000 >/proc/sys/net/ipv4/tcp_notsent_lowat
$ cat bench_rr.sh
SUM=0
for i in {1..10}
do
V=`netperf -H remote_host -l30 -t TCP_RR -- -r 10000000,10000 -o LOCAL_BYTES_SENT | egrep -v "MIGRATED|Bytes"`
echo $V
SUM=$(($SUM + $V))
done
echo SUM=$SUM
Before patch:
$ bench_rr.sh
130000000
80000000
140000000
140000000
140000000
140000000
130000000
40000000
90000000
110000000
SUM=1140000000
After patch:
$ bench_rr.sh
430000000
590000000
530000000
450000000
450000000
350000000
450000000
490000000
480000000
460000000
SUM=4680000000 # This is 410 % of the value before patch.
Fixes: c9bee3b7fdec ("tcp: TCP_NOTSENT_LOWAT socket option")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Doug Porter <dsp@fb.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
---
include/net/tcp.h | 1 +
net/ipv4/tcp_input.c | 12 +++++++++++-
net/ipv4/tcp_output.c | 1 +
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 9987b3fba9f202632916cc439af9d17f1e68bcd3..cc1295037533a7741e454f7c040f77a21deae02b 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -621,6 +621,7 @@ void tcp_synack_rtt_meas(struct sock *sk, struct request_sock *req);
void tcp_reset(struct sock *sk, struct sk_buff *skb);
void tcp_skb_mark_lost_uncond_verify(struct tcp_sock *tp, struct sk_buff *skb);
void tcp_fin(struct sock *sk);
+void tcp_check_space(struct sock *sk);
/* tcp_timer.c */
void tcp_init_xmit_timers(struct sock *);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2088f93fa37b5fb9110e7933242a27bd4009990e..48f6075228600896daa6507c4cd06acfc851a0fa 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5454,7 +5454,17 @@ static void tcp_new_space(struct sock *sk)
INDIRECT_CALL_1(sk->sk_write_space, sk_stream_write_space, sk);
}
-static void tcp_check_space(struct sock *sk)
+/* Caller made space either from:
+ * 1) Freeing skbs in rtx queues (after tp->snd_una has advanced)
+ * 2) Sent skbs from output queue (and thus advancing tp->snd_nxt)
+ *
+ * We might be able to generate EPOLLOUT to the application if:
+ * 1) Space consumed in output/rtx queues is below sk->sk_sndbuf/2
+ * 2) notsent amount (tp->write_seq - tp->snd_nxt) became
+ * small enough that tcp_stream_memory_free() decides it
+ * is time to generate EPOLLOUT.
+ */
+void tcp_check_space(struct sock *sk)
{
/* pairs with tcp_poll() */
smp_mb();
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 9ede847f4199844c5884e3f62ea450562072a0a7..1ca2f28c9981018e6cfaee3435d711467af6048d 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -82,6 +82,7 @@ static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)
NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPORIGDATASENT,
tcp_skb_pcount(skb));
+ tcp_check_space(sk);
}
/* SND.NXT, if window was not shrunk or the amount of shrunk was less than one
--
2.36.0.rc2.479.g8af0fa9b8e-goog
^ permalink raw reply related
* Re: [PATCH net-next v1 0/4] Broadcom PTP PHY support
From: Richard Cochran @ 2022-04-25 1:01 UTC (permalink / raw)
To: Jonathan Lemon
Cc: f.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1, linux,
netdev, kernel-team
In-Reply-To: <20220424022356.587949-1-jonathan.lemon@gmail.com>
On Sat, Apr 23, 2022 at 07:23:52PM -0700, Jonathan Lemon wrote:
> There are other Broadcom chips which may benefit from using the
> same framework here, although with different register sets.
Based on two examples, the present 542xx and the 541xx (which I am
familiar with), it appears that the registers sets are the same in
gen1, just the base offset is different.
So it would be great to store the base in bcm_ptp_private, adding the
base into each read/write operation.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next v1 1/4] net: phy: broadcom: Add PTP support for some Broadcom PHYs.
From: Richard Cochran @ 2022-04-25 1:19 UTC (permalink / raw)
To: Jonathan Lemon
Cc: f.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1, linux,
netdev, kernel-team
In-Reply-To: <20220424022356.587949-2-jonathan.lemon@gmail.com>
On Sat, Apr 23, 2022 at 07:23:53PM -0700, Jonathan Lemon wrote:
> +static bool bcm_ptp_rxtstamp(struct mii_timestamper *mii_ts,
> + struct sk_buff *skb, int type)
> +{
> + struct bcm_ptp_private *priv = mii2priv(mii_ts);
> + struct skb_shared_hwtstamps *hwts;
> + struct ptp_header *header;
> + u32 sec, nsec;
> + u8 *data;
> +
> + if (!priv->hwts_rx)
> + return false;
> +
> + header = ptp_parse_header(skb, type);
> + if (!header)
> + return false;
> +
> + data = (u8 *)(header + 1);
No need to pointer math, as ptp_header already has reserved1 and reserved2.
> + sec = get_unaligned_be32(data);
Something is missing here. The seconds field is only four bits, so
the code needs to read the 80 bit counter once in a while and augment
the time stamp with the upper bits.
> + nsec = get_unaligned_be32(data + 4);
> +
> + hwts = skb_hwtstamps(skb);
> + hwts->hwtstamp = ktime_set(sec, nsec);
> +
> + return false;
> +}
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next v1 1/4] net: phy: broadcom: Add PTP support for some Broadcom PHYs.
From: Richard Cochran @ 2022-04-25 1:38 UTC (permalink / raw)
To: Jonathan Lemon
Cc: f.fainelli, bcm-kernel-feedback-list, andrew, hkallweit1, linux,
netdev, kernel-team
In-Reply-To: <20220424022356.587949-2-jonathan.lemon@gmail.com>
On Sat, Apr 23, 2022 at 07:23:53PM -0700, Jonathan Lemon wrote:
> +static bool bcm_ptp_get_tstamp(struct bcm_ptp_private *priv,
> + struct bcm_ptp_capture *capts)
> +{
> + struct phy_device *phydev = priv->phydev;
> + u16 ts[4], reg;
> + u32 sec, nsec;
> +
> + mutex_lock(&priv->mutex);
> +
> + reg = bcm_phy_read_exp(phydev, INTR_STATUS);
> + if ((reg & INTC_SOP) == 0) {
> + mutex_unlock(&priv->mutex);
> + return false;
> + }
> +
> + bcm_phy_write_exp(phydev, TS_READ_CTRL, TS_READ_START);
> +
> + ts[0] = bcm_phy_read_exp(phydev, TS_REG_0);
> + ts[1] = bcm_phy_read_exp(phydev, TS_REG_1);
> + ts[2] = bcm_phy_read_exp(phydev, TS_REG_2);
> + ts[3] = bcm_phy_read_exp(phydev, TS_REG_3);
> +
> + /* not in be32 format for some reason */
> + capts->seq_id = bcm_phy_read_exp(priv->phydev, TS_INFO_0);
> +
> + reg = bcm_phy_read_exp(phydev, TS_INFO_1);
> + capts->msgtype = reg >> 12;
> + capts->tx_dir = !!(reg & BIT(11));
Okay, so now I am sad. The 541xx has:
TIMESTAMP_INFO_1 0xA8C bit 0 DIR, bits 1-2 msg_type, etc
TIMESTAMP_INFO_2 0xA8D sequence ID
It is the same info, but randomly shuffled among the two registers in
a different way.
So much for supporting multiple devices with a common code base. :(
> + bcm_phy_write_exp(phydev, TS_READ_CTRL, TS_READ_END);
> + bcm_phy_write_exp(phydev, TS_READ_CTRL, 0);
> +
> + mutex_unlock(&priv->mutex);
> +
> + sec = (ts[3] << 16) | ts[2];
> + nsec = (ts[1] << 16) | ts[0];
> + capts->hwtstamp = ktime_set(sec, nsec);
> +
> + return true;
> +}
Thanks,
Richard
^ 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