* [PATCH net v2] ice: Protect vf_state check by cfg_lock in ice_vc_process_vf_msg()
From: Ivan Vecera @ 2022-04-19 14:22 UTC (permalink / raw)
To: netdev
Cc: poros, mschmidt, Fei Liu, Jesse Brandeburg, Tony Nguyen,
David S. Miller, Jakub Kicinski, Paolo Abeni, Brett Creeley,
moderated list:INTEL ETHERNET DRIVERS, open list
Previous patch labelled "ice: Fix incorrect locking in
ice_vc_process_vf_msg()" fixed an issue with ignored messages
sent by VF driver but a small race window still left.
Recently caught trace during 'ip link set ... vf 0 vlan ...' operation:
[ 7332.995625] ice 0000:3b:00.0: Clearing port VLAN on VF 0
[ 7333.001023] iavf 0000:3b:01.0: Reset indication received from the PF
[ 7333.007391] iavf 0000:3b:01.0: Scheduling reset task
[ 7333.059575] iavf 0000:3b:01.0: PF returned error -5 (IAVF_ERR_PARAM) to our request 3
[ 7333.059626] ice 0000:3b:00.0: Invalid message from VF 0, opcode 3, len 4, error -1
Setting of VLAN for VF causes a reset of the affected VF using
ice_reset_vf() function that runs with cfg_lock taken:
1. ice_notify_vf_reset() informs IAVF driver that reset is needed and
IAVF schedules its own reset procedure
2. Bit ICE_VF_STATE_DIS is set in vf->vf_state
3. Misc initialization steps
4. ice_sriov_post_vsi_rebuild() -> ice_vf_set_initialized() and that
clears ICE_VF_STATE_DIS in vf->vf_state
Step 3 is mentioned race window because IAVF reset procedure runs in
parallel and one of its step is sending of VIRTCHNL_OP_GET_VF_RESOURCES
message (opcode==3). This message is handled in ice_vc_process_vf_msg()
and if it is received during the mentioned race window then it's
marked as invalid and error is returned to VF driver.
Protect vf_state check in ice_vc_process_vf_msg() by cfg_lock to avoid
this race condition.
Fixes: e6ba5273d4ed ("ice: Fix race conditions between virtchnl handling and VF ndo ops")
Tested-by: Fei Liu <feliu@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/net/ethernet/intel/ice/ice_virtchnl.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
index 5612c032f15a..b72606c9e6d0 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
@@ -3625,6 +3625,8 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event)
return;
}
+ mutex_lock(&vf->cfg_lock);
+
/* Check if VF is disabled. */
if (test_bit(ICE_VF_STATE_DIS, vf->vf_states)) {
err = -EPERM;
@@ -3648,19 +3650,14 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event)
NULL, 0);
dev_err(dev, "Invalid message from VF %d, opcode %d, len %d, error %d\n",
vf_id, v_opcode, msglen, err);
- ice_put_vf(vf);
- return;
+ goto finish;
}
- mutex_lock(&vf->cfg_lock);
-
if (!ice_vc_is_opcode_allowed(vf, v_opcode)) {
ice_vc_send_msg_to_vf(vf, v_opcode,
VIRTCHNL_STATUS_ERR_NOT_SUPPORTED, NULL,
0);
- mutex_unlock(&vf->cfg_lock);
- ice_put_vf(vf);
- return;
+ goto finish;
}
switch (v_opcode) {
@@ -3773,6 +3770,7 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event)
vf_id, v_opcode, err);
}
+finish:
mutex_unlock(&vf->cfg_lock);
ice_put_vf(vf);
}
--
2.35.1
^ permalink raw reply related
* Re: [Intel-wired-lan] [PATCH 2/2] Trigger proper interrupts in igc_xsk_wakeup
From: Paul Menzel @ 2022-04-19 14:19 UTC (permalink / raw)
To: Jeff Evanson
Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Jakub Kicinski,
intel-wired-lan, netdev, linux-kernel, jeff.evanson
In-Reply-To: <20220415210546.11294-1-jeff.evanson@qsc.com>
Dear Jeff,
Thank you for your patch.
Am 15.04.22 um 23:05 schrieb Jeff Evanson:
1. Add a From tag(?), so your company instead of gmail.com email is used?
2. Please add a prefix to the commit message summary. See `git log
--oneline drivers/net/ethernet/igc` for examples.
> in igc_xsk_wakeup, trigger the proper interrupt based on whether flags
> contains XDP_WAKEUP_RX and/or XDP_WAKEUP_TX
Nit. Please add a dot/period to the end of sentences.
Can you please add a paragraph on what system you experienced the
problem, and how to verify your fix?
> Signed-off-by: Jeff Evanson <jeff.evanson@qsc.com>
> ---
> drivers/net/ethernet/intel/igc/igc_main.c | 36 +++++++++++++++++------
> 1 file changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index a36a18c84aeb..d706de95dc06 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6073,7 +6073,7 @@ static void igc_trigger_rxtxq_interrupt(struct igc_adapter *adapter,
> int igc_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags)
> {
> struct igc_adapter *adapter = netdev_priv(dev);
> - struct igc_q_vector *q_vector;
> + struct igc_q_vector *txq_vector = 0, *rxq_vector = 0;
Should you use NULL instead of 0?
Kind regards,
Paul
> struct igc_ring *ring;
>
> if (test_bit(__IGC_DOWN, &adapter->state))
> @@ -6082,17 +6082,35 @@ int igc_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags)
> if (!igc_xdp_is_enabled(adapter))
> return -ENXIO;
>
> - if (queue_id >= adapter->num_rx_queues)
> - return -EINVAL;
> + if (flags & XDP_WAKEUP_RX) {
> + if (queue_id >= adapter->num_rx_queues)
> + return -EINVAL;
>
> - ring = adapter->rx_ring[queue_id];
> + ring = adapter->rx_ring[queue_id];
> + if (!ring->xsk_pool)
> + return -ENXIO;
>
> - if (!ring->xsk_pool)
> - return -ENXIO;
> + rxq_vector = ring->q_vector;
> + }
> +
> + if (flags & XDP_WAKEUP_TX) {
> + if (queue_id >= adapter->num_tx_queues)
> + return -EINVAL;
> +
> + ring = adapter->tx_ring[queue_id];
> + if (!ring->xsk_pool)
> + return -ENXIO;
> +
> + txq_vector = ring->q_vector;
> + }
> +
> + if (rxq_vector &&
> + !napi_if_scheduled_mark_missed(&rxq_vector->napi))
> + igc_trigger_rxtxq_interrupt(adapter, rxq_vector);
>
> - q_vector = adapter->q_vector[queue_id];
> - if (!napi_if_scheduled_mark_missed(&q_vector->napi))
> - igc_trigger_rxtxq_interrupt(adapter, q_vector);
> + if (txq_vector && txq_vector != rxq_vector &&
> + !napi_if_scheduled_mark_missed(&txq_vector->napi))
> + igc_trigger_rxtxq_interrupt(adapter, txq_vector);
>
> return 0;
> }
^ permalink raw reply
* [PATCH] USB2NET : SR9800 : change SR9800_BULKIN_SIZE from global to static
From: Tom Rix @ 2022-04-19 14:06 UTC (permalink / raw)
To: davem, kuba, pabeni; +Cc: linux-usb, netdev, linux-kernel, Tom Rix
Smatch reports this issue
sr9800.h:166:53: warning: symbol 'SR9800_BULKIN_SIZE' was not declared. Should it be static?
Global variables should not be defined in header files.
This only works because sr9800.h in only included by sr9800.c
Change the storage-class specifier to static.
And since it does not change add type qualifier const.
Signed-off-by: Tom Rix <trix@redhat.com>
---
drivers/net/usb/sr9800.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/sr9800.h b/drivers/net/usb/sr9800.h
index 18f670251275..952e6f7c0321 100644
--- a/drivers/net/usb/sr9800.h
+++ b/drivers/net/usb/sr9800.h
@@ -163,7 +163,7 @@
#define SR9800_MAX_BULKIN_24K 6
#define SR9800_MAX_BULKIN_32K 7
-struct {unsigned short size, byte_cnt, threshold; } SR9800_BULKIN_SIZE[] = {
+static const struct {unsigned short size, byte_cnt, threshold; } SR9800_BULKIN_SIZE[] = {
/* 2k */
{2048, 0x8000, 0x8001},
/* 4k */
--
2.27.0
^ permalink raw reply related
* Re: [PATCH net-next v2 1/2] tcp: ensure to use the most recently sent skb when filling the rate sample
From: Paolo Abeni @ 2022-04-19 13:59 UTC (permalink / raw)
To: Neal Cardwell, Pengcheng Yang
Cc: Eric Dumazet, Yuchung Cheng, netdev, David S. Miller,
Jakub Kicinski
In-Reply-To: <CADVnQymGad1=tvLocBCrGK5vtVDKv8m-dYP83VsZfmE-WFcL3w@mail.gmail.com>
On Sun, 2022-04-17 at 14:51 -0400, Neal Cardwell wrote:
> On Sat, Apr 16, 2022 at 5:20 AM Pengcheng Yang <yangpc@wangsu.com> wrote:
> >
> > If an ACK (s)acks multiple skbs, we favor the information
> > from the most recently sent skb by choosing the skb with
> > the highest prior_delivered count. But in the interval
> > between receiving ACKs, we send multiple skbs with the same
> > prior_delivered, because the tp->delivered only changes
> > when we receive an ACK.
> >
> > We used RACK's solution, copying tcp_rack_sent_after() as
> > tcp_skb_sent_after() helper to determine "which packet was
> > sent last?". Later, we will use tcp_skb_sent_after() instead
> > in RACK.
> >
> > Signed-off-by: Pengcheng Yang <yangpc@wangsu.com>
> > Cc: Neal Cardwell <ncardwell@google.com>
> > ---
> > include/net/tcp.h | 6 ++++++
> > net/ipv4/tcp_rate.c | 11 ++++++++---
> > 2 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index 6d50a66..fcd69fc 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -1042,6 +1042,7 @@ struct rate_sample {
> > int losses; /* number of packets marked lost upon ACK */
> > u32 acked_sacked; /* number of packets newly (S)ACKed upon ACK */
> > u32 prior_in_flight; /* in flight before this ACK */
> > + u32 last_end_seq; /* end_seq of most recently ACKed packet */
> > bool is_app_limited; /* is sample from packet with bubble in pipe? */
> > bool is_retrans; /* is sample from retransmission? */
> > bool is_ack_delayed; /* is this (likely) a delayed ACK? */
> > @@ -1158,6 +1159,11 @@ void tcp_rate_gen(struct sock *sk, u32 delivered, u32 lost,
> > bool is_sack_reneg, struct rate_sample *rs);
> > void tcp_rate_check_app_limited(struct sock *sk);
> >
> > +static inline bool tcp_skb_sent_after(u64 t1, u64 t2, u32 seq1, u32 seq2)
> > +{
> > + return t1 > t2 || (t1 == t2 && after(seq1, seq2));
> > +}
> > +
> > /* These functions determine how the current flow behaves in respect of SACK
> > * handling. SACK is negotiated with the peer, and therefore it can vary
> > * between different flows.
> > diff --git a/net/ipv4/tcp_rate.c b/net/ipv4/tcp_rate.c
> > index 617b818..a8f6d9d 100644
> > --- a/net/ipv4/tcp_rate.c
> > +++ b/net/ipv4/tcp_rate.c
> > @@ -74,27 +74,32 @@ void tcp_rate_skb_sent(struct sock *sk, struct sk_buff *skb)
> > *
> > * If an ACK (s)acks multiple skbs (e.g., stretched-acks), this function is
> > * called multiple times. We favor the information from the most recently
> > - * sent skb, i.e., the skb with the highest prior_delivered count.
> > + * sent skb, i.e., the skb with the most recently sent time and the highest
> > + * sequence.
> > */
> > void tcp_rate_skb_delivered(struct sock *sk, struct sk_buff *skb,
> > struct rate_sample *rs)
> > {
> > struct tcp_sock *tp = tcp_sk(sk);
> > struct tcp_skb_cb *scb = TCP_SKB_CB(skb);
> > + u64 tx_tstamp;
> >
> > if (!scb->tx.delivered_mstamp)
> > return;
> >
> > + tx_tstamp = tcp_skb_timestamp_us(skb);
> > if (!rs->prior_delivered ||
> > - after(scb->tx.delivered, rs->prior_delivered)) {
> > + tcp_skb_sent_after(tx_tstamp, tp->first_tx_mstamp,
> > + scb->end_seq, rs->last_end_seq)) {
> > rs->prior_delivered_ce = scb->tx.delivered_ce;
> > rs->prior_delivered = scb->tx.delivered;
> > rs->prior_mstamp = scb->tx.delivered_mstamp;
> > rs->is_app_limited = scb->tx.is_app_limited;
> > rs->is_retrans = scb->sacked & TCPCB_RETRANS;
> > + rs->last_end_seq = scb->end_seq;
> >
> > /* Record send time of most recently ACKed packet: */
> > - tp->first_tx_mstamp = tcp_skb_timestamp_us(skb);
> > + tp->first_tx_mstamp = tx_tstamp;
> > /* Find the duration of the "send phase" of this window: */
> > rs->interval_us = tcp_stamp_us_delta(tp->first_tx_mstamp,
> > scb->tx.first_tx_mstamp);
> > --
>
> Thanks for the patch! The change looks good to me, and it passes our
> team's packetdrill tests.
>
> One suggestion: currently this patch seems to be targeted to the
> net-next branch. However, since it's a bug fix my sense is that it
> would be best to target this to the net branch, so that it gets
> backported to stable releases.
>
> One complication is that the follow-on patch in this series ("tcp: use
> tcp_skb_sent_after() instead in RACK") is a pure re-factor/cleanup,
> which is more appropriate for net-next. So the plan I was trying to
> describe in the previous thread was that this series could be
> implemented as:
>
> (1) first, submit "tcp: ensure to use the most recently sent skb when
> filling the rate sample" to the net branch
> (2) wait for the fix in the net branch to be merged into the net-next branch
> (3) second, submit "tcp: use tcp_skb_sent_after() instead in RACK" to
> the net-next branch
>
> What do folks think?
+1 for the above.
@Pengcheng: please additionally provide a suitable 'fixes' tag for
patch 1/2.
Thanks!
Paolo
^ permalink raw reply
* Re: [PATCH net 1/2] net/af_packet: adjust network header position for VLAN tagged packets
From: Willem de Bruijn @ 2022-04-19 13:56 UTC (permalink / raw)
To: Hangbin Liu
Cc: Willem de Bruijn, Maxim Mikityanskiy, Mike Pattrick,
Michael S . Tsirkin, netdev, Eric Dumazet, virtualization,
Balazs Nemeth, Flavio Leitner, Jakub Kicinski, Paolo Abeni,
David S . Miller, Jason Wang
In-Reply-To: <Yl4mU0XLmPukG0WO@Laptop-X1>
On Mon, Apr 18, 2022 at 11:02 PM Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> On Mon, Apr 18, 2022 at 11:38:14AM -0400, Willem de Bruijn wrote:
> > Strictly speaking VLAN tagged GSO packets have never been supported.
>
> OK, I thought we just forgot to handle the VLAN header for RAW af socket.
> As in the later path skb_mac_gso_segment() deal with VLAN correctly.
>
> If you think this should be a new feature instead of fixes. I can remove the
> fixes tag and re-post it to net-next, as you said.
>
> > The only defined types are TCP and UDP over IPv4 and IPv6:
> >
> > define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */
> > define VIRTIO_NET_HDR_GSO_UDP 3 /* GSO frame, IPv4 UDP (UFO) */
> > define VIRTIO_NET_HDR_GSO_TCPV6 4 /* GSO frame, IPv6 TCP */
> >
> > I don't think this is a bug, more a stretching of the definition of those flags.
>
> I think VLAN is a L2 header, so I just reset the network header position.
>
> I'm not familiar with virtio coded. Do you mean to add a new flag like VIRTIO_NET_HDR_GSO_VLAN?
> > > @@ -3055,11 +3068,6 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
> > > virtio_net_hdr_set_proto(skb, &vnet_hdr);
> > > }
> > >
> > > - packet_parse_headers(skb, sock);
> > > -
> > > - if (unlikely(extra_len == 4))
> > > - skb->no_fcs = 1;
> > > -
> >
> > Moving packet_parse_headers before or after virtio_net_hdr_to_skb may
> > have additional subtle effects on protocol detection.
> >
> > I think it's probably okay, as tpacket_snd also calls in the inverse
> > order. But there have been many issues in this codepath.
>
> Yes
>
> >
> > We should also maintain feature consistency between packet_snd,
> > tpacket_snd and to the limitations of its feature set to
> > packet_sendmsg_spkt. The no_fcs is already lacking in tpacket_snd as
> > far as I can tell. But packet_sendmsg_spkt also sets it and calls
> > packet_parse_headers.
>
> Yes, I think we could fix the tpacket_snd() in another patch.
>
> There are also some duplicated codes in these *_snd functions.
> I think we can move them out to one single function.
Please don't refactor this code. It will complicate future backports
of stable fixes.
> > Because this patch touches many other packets besides the ones
> > intended, I am a bit concerned about unintended consequences. Perhaps
>
> Yes, makes sense.
>
> > stretching the definition of the flags to include VLAN is acceptable
> > (unlike outright tunnels), but even then I would suggest for net-next.
>
> As I asked, I'm not familiar with virtio code. Do you think if I should
> add a new VIRTIO_NET_HDR_GSO_VLAN flag? It's only a L2 flag without any L3
> info. If I add something like VIRTIO_NET_HDR_GSO_VLAN_TCPV4/TCPV6/UDP. That
> would add more combinations. Which doesn't like a good idea.
I would prefer a new flag to denote this type, so that we can be
strict and only change the datapath for packets that have this flag
set (and thus express the intent).
But the VIRTIO_NET_HDR types are defined in the virtio spec. The
maintainers should probably chime in.
^ permalink raw reply
* Re: [PATCH net 2/2] virtio_net: check L3 protocol for VLAN packets
From: Willem de Bruijn @ 2022-04-19 13:52 UTC (permalink / raw)
To: Hangbin Liu
Cc: Willem de Bruijn, Maxim Mikityanskiy, Mike Pattrick,
Michael S . Tsirkin, netdev, Eric Dumazet, virtualization,
Balazs Nemeth, Jakub Kicinski, Paolo Abeni, David S . Miller
In-Reply-To: <Yl4pG8MN7jxVybPB@Laptop-X1>
On Mon, Apr 18, 2022 at 11:14 PM Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> On Mon, Apr 18, 2022 at 11:40:44AM -0400, Willem de Bruijn wrote:
> > On Mon, Apr 18, 2022 at 12:44 AM Hangbin Liu <liuhangbin@gmail.com> wrote:
> > >
> > > For gso packets, virtio_net_hdr_to_skb() will check the protocol via
> > > virtio_net_hdr_match_proto(). But a packet may come from a raw socket
> > > with a VLAN tag. Checking the VLAN protocol for virtio net_hdr makes no
> > > sense. Let's check the L3 protocol if it's a VLAN packet.
> > >
> > > Make the virtio_net_hdr_match_proto() checking for all skbs instead of
> > > only skb without protocol setting.
> > >
> > > Also update the data, protocol parameter for
> > > skb_flow_dissect_flow_keys_basic() as the skb->protocol may not IP or IPv6.
> > >
> > > Fixes: 7e5cced9ca84 ("net: accept UFOv6 packages in virtio_net_hdr_to_skb")
> > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > > ---
> > > include/linux/virtio_net.h | 26 +++++++++++++++++++-------
> > > 1 file changed, 19 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > > index a960de68ac69..97b4f9680786 100644
> > > --- a/include/linux/virtio_net.h
> > > +++ b/include/linux/virtio_net.h
> > > @@ -3,6 +3,7 @@
> > > #define _LINUX_VIRTIO_NET_H
> > >
> > > #include <linux/if_vlan.h>
> > > +#include <uapi/linux/if_arp.h>
> > > #include <uapi/linux/tcp.h>
> > > #include <uapi/linux/udp.h>
> > > #include <uapi/linux/virtio_net.h>
> > > @@ -102,25 +103,36 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > */
> > > if (gso_type && skb->network_header) {
> >
> > This whole branch should not be taken by well formed packets. It is
> > inside the else clause of
> >
> > if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
> > ..
> > } else {
> >
> > GSO packets should always request checksum offload. The fact that we
> > try to patch up some incomplete packets should not have to be expanded
> > if we expand support to include VLAN.
>
> Hi Willem,
>
> I'm not sure if I understand correctly. Do you mean we don't need to check
> L3 protocols for VLAN packet without NEEDS_CSUM flag? Which like
>
> if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
> ...
> } else if (!eth_type_vlan(skb->protocol)) {
> ...
> }
Segmentation offload requires checksum offload. Packets that request
GSO but not NEEDS_CSUM are an aberration. We had to go out of our way
to handle them because the original implementation did not explicitly
flag and drop these. But we should not extend that to new types.
^ permalink raw reply
* [PATCH net 2/2] selftests: mlxsw: vxlan_flooding_ipv6: Prevent flooding of unwanted packets
From: Ido Schimmel @ 2022-04-19 13:51 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, pabeni, petrm, amcohen, mlxsw, Ido Schimmel
In-Reply-To: <20220419135155.2987141-1-idosch@nvidia.com>
The test verifies that packets are correctly flooded by the bridge and
the VXLAN device by matching on the encapsulated packets at the other
end. However, if packets other than those generated by the test also
ingress the bridge (e.g., MLD packets), they will be flooded as well and
interfere with the expected count.
Make the test more robust by making sure that only the packets generated
by the test can ingress the bridge. Drop all the rest using tc filters
on the egress of 'br0' and 'h1'.
In the software data path, the problem can be solved by matching on the
inner destination MAC or dropping unwanted packets at the egress of the
VXLAN device, but this is not currently supported by mlxsw.
Fixes: d01724dd2a66 ("selftests: mlxsw: spectrum-2: Add a test for VxLAN flooding with IPv6")
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Amit Cohen <amcohen@nvidia.com>
---
.../net/mlxsw/spectrum-2/vxlan_flooding_ipv6.sh | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/vxlan_flooding_ipv6.sh b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/vxlan_flooding_ipv6.sh
index 429f7ee735cf..fd23c80eba31 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/vxlan_flooding_ipv6.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/vxlan_flooding_ipv6.sh
@@ -159,6 +159,17 @@ flooding_remotes_add()
local lsb
local i
+ # Prevent unwanted packets from entering the bridge and interfering
+ # with the test.
+ tc qdisc add dev br0 clsact
+ tc filter add dev br0 egress protocol all pref 1 handle 1 \
+ matchall skip_hw action drop
+ tc qdisc add dev $h1 clsact
+ tc filter add dev $h1 egress protocol all pref 1 handle 1 \
+ flower skip_hw dst_mac de:ad:be:ef:13:37 action pass
+ tc filter add dev $h1 egress protocol all pref 2 handle 2 \
+ matchall skip_hw action drop
+
for i in $(eval echo {1..$num_remotes}); do
lsb=$((i + 1))
@@ -195,6 +206,12 @@ flooding_filters_del()
done
tc qdisc del dev $rp2 clsact
+
+ tc filter del dev $h1 egress protocol all pref 2 handle 2 matchall
+ tc filter del dev $h1 egress protocol all pref 1 handle 1 flower
+ tc qdisc del dev $h1 clsact
+ tc filter del dev br0 egress protocol all pref 1 handle 1 matchall
+ tc qdisc del dev br0 clsact
}
flooding_check_packets()
--
2.33.1
^ permalink raw reply related
* [PATCH net 1/2] selftests: mlxsw: vxlan_flooding: Prevent flooding of unwanted packets
From: Ido Schimmel @ 2022-04-19 13:51 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, pabeni, petrm, amcohen, mlxsw, Ido Schimmel
In-Reply-To: <20220419135155.2987141-1-idosch@nvidia.com>
The test verifies that packets are correctly flooded by the bridge and
the VXLAN device by matching on the encapsulated packets at the other
end. However, if packets other than those generated by the test also
ingress the bridge (e.g., MLD packets), they will be flooded as well and
interfere with the expected count.
Make the test more robust by making sure that only the packets generated
by the test can ingress the bridge. Drop all the rest using tc filters
on the egress of 'br0' and 'h1'.
In the software data path, the problem can be solved by matching on the
inner destination MAC or dropping unwanted packets at the egress of the
VXLAN device, but this is not currently supported by mlxsw.
Fixes: 94d302deae25 ("selftests: mlxsw: Add a test for VxLAN flooding")
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Amit Cohen <amcohen@nvidia.com>
---
.../drivers/net/mlxsw/vxlan_flooding.sh | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/vxlan_flooding.sh b/tools/testing/selftests/drivers/net/mlxsw/vxlan_flooding.sh
index fedcb7b35af9..af5ea50ed5c0 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/vxlan_flooding.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/vxlan_flooding.sh
@@ -172,6 +172,17 @@ flooding_filters_add()
local lsb
local i
+ # Prevent unwanted packets from entering the bridge and interfering
+ # with the test.
+ tc qdisc add dev br0 clsact
+ tc filter add dev br0 egress protocol all pref 1 handle 1 \
+ matchall skip_hw action drop
+ tc qdisc add dev $h1 clsact
+ tc filter add dev $h1 egress protocol all pref 1 handle 1 \
+ flower skip_hw dst_mac de:ad:be:ef:13:37 action pass
+ tc filter add dev $h1 egress protocol all pref 2 handle 2 \
+ matchall skip_hw action drop
+
tc qdisc add dev $rp2 clsact
for i in $(eval echo {1..$num_remotes}); do
@@ -194,6 +205,12 @@ flooding_filters_del()
done
tc qdisc del dev $rp2 clsact
+
+ tc filter del dev $h1 egress protocol all pref 2 handle 2 matchall
+ tc filter del dev $h1 egress protocol all pref 1 handle 1 flower
+ tc qdisc del dev $h1 clsact
+ tc filter del dev br0 egress protocol all pref 1 handle 1 matchall
+ tc qdisc del dev br0 clsact
}
flooding_check_packets()
--
2.33.1
^ permalink raw reply related
* [PATCH net 0/2] selftests: mlxsw: Make VXLAN flooding tests more robust
From: Ido Schimmel @ 2022-04-19 13:51 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, pabeni, petrm, amcohen, mlxsw, Ido Schimmel
Make the VXLAN flooding tests (with IPv4 and IPv6 underlay) more robust
by preventing flooding of unwanted packets. See detailed description of
the problem and solution in the commit messages.
Ido Schimmel (2):
selftests: mlxsw: vxlan_flooding: Prevent flooding of unwanted packets
selftests: mlxsw: vxlan_flooding_ipv6: Prevent flooding of unwanted
packets
.../net/mlxsw/spectrum-2/vxlan_flooding_ipv6.sh | 17 +++++++++++++++++
.../drivers/net/mlxsw/vxlan_flooding.sh | 17 +++++++++++++++++
2 files changed, 34 insertions(+)
--
2.33.1
^ permalink raw reply
* [PATCH nf v2 2/2] netfilter: Use l3mdev flow key when re-routing mangled packets
From: Martin Willi @ 2022-04-19 13:47 UTC (permalink / raw)
To: Pablo Neira Ayuso, Florian Westphal, David Ahern; +Cc: netfilter-devel, netdev
In-Reply-To: <20220419134701.153090-1-martin@strongswan.org>
Commit 40867d74c374 ("net: Add l3mdev index to flow struct and avoid oif
reset for port devices") introduces a flow key specific for layer 3
domains, such as a VRF master device. This allows for explicit VRF domain
selection instead of abusing the oif flow key.
Update ip[6]_route_me_harder() to make use of that new key when re-routing
mangled packets within VRFs instead of setting the flow oif, making it
consistent with other users.
Signed-off-by: Martin Willi <martin@strongswan.org>
---
net/ipv4/netfilter.c | 3 +--
net/ipv6/netfilter.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/netfilter.c b/net/ipv4/netfilter.c
index aff707988e23..bd135165482a 100644
--- a/net/ipv4/netfilter.c
+++ b/net/ipv4/netfilter.c
@@ -45,8 +45,7 @@ int ip_route_me_harder(struct net *net, struct sock *sk, struct sk_buff *skb, un
fl4.saddr = saddr;
fl4.flowi4_tos = RT_TOS(iph->tos);
fl4.flowi4_oif = sk ? sk->sk_bound_dev_if : 0;
- if (!fl4.flowi4_oif)
- fl4.flowi4_oif = l3mdev_master_ifindex(dev);
+ fl4.flowi4_l3mdev = l3mdev_master_ifindex(dev);
fl4.flowi4_mark = skb->mark;
fl4.flowi4_flags = flags;
fib4_rules_early_flow_dissect(net, skb, &fl4, &flkeys);
diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index 8ce60ab89015..857713d7a38a 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -31,6 +31,7 @@ int ip6_route_me_harder(struct net *net, struct sock *sk_partial, struct sk_buff
int strict = (ipv6_addr_type(&iph->daddr) &
(IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL));
struct flowi6 fl6 = {
+ .flowi6_l3mdev = l3mdev_master_ifindex(dev),
.flowi6_mark = skb->mark,
.flowi6_uid = sock_net_uid(net, sk),
.daddr = iph->daddr,
@@ -42,8 +43,6 @@ int ip6_route_me_harder(struct net *net, struct sock *sk_partial, struct sk_buff
fl6.flowi6_oif = sk->sk_bound_dev_if;
else if (strict)
fl6.flowi6_oif = dev->ifindex;
- else
- fl6.flowi6_oif = l3mdev_master_ifindex(dev);
fib6_rules_early_flow_dissect(net, skb, &fl6, &flkeys);
dst = ip6_route_output(net, sk, &fl6);
--
2.25.1
^ permalink raw reply related
* [PATCH nf v2 0/2] netfilter: Fix/update mangled packet re-routing within VRF domains
From: Martin Willi @ 2022-04-19 13:46 UTC (permalink / raw)
To: Pablo Neira Ayuso, Florian Westphal, David Ahern; +Cc: netfilter-devel, netdev
The first patch fixes re-routing of IPv6 packets mangled by Netfilter
rules to consider the layer 3 VRF domain. The second patch updates both
IPv4 and IPv6 re-routing to use the recently added l3mdev flow key instead
of abusing the oif flow key to select the L3 domain.
These patches have been explicitly split up to allow stable to pick up the
first patch as-is.
Changes in v2:
- Add a second patch to migrate IPv4/6 re-routing to l3mdev flow key
Martin Willi (2):
netfilter: Update ip6_route_me_harder to consider L3 domain
netfilter: Use l3mdev flow key when re-routing mangled packets
net/ipv4/netfilter.c | 3 +--
net/ipv6/netfilter.c | 9 +++++++--
2 files changed, 8 insertions(+), 4 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH nf v2 1/2] netfilter: Update ip6_route_me_harder to consider L3 domain
From: Martin Willi @ 2022-04-19 13:47 UTC (permalink / raw)
To: Pablo Neira Ayuso, Florian Westphal, David Ahern; +Cc: netfilter-devel, netdev
In-Reply-To: <20220419134701.153090-1-martin@strongswan.org>
The commit referenced below fixed packet re-routing if Netfilter mangles
a routing key property of a packet and the packet is routed in a VRF L3
domain. The fix, however, addressed IPv4 re-routing, only.
This commit applies the same behavior for IPv6. While at it, untangle
the nested ternary operator to make the code more readable.
Fixes: 6d8b49c3a3a3 ("netfilter: Update ip_route_me_harder to consider L3 domain")
Cc: stable@vger.kernel.org
Signed-off-by: Martin Willi <martin@strongswan.org>
---
net/ipv6/netfilter.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index 1da332450d98..8ce60ab89015 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -24,14 +24,13 @@ int ip6_route_me_harder(struct net *net, struct sock *sk_partial, struct sk_buff
{
const struct ipv6hdr *iph = ipv6_hdr(skb);
struct sock *sk = sk_to_full_sk(sk_partial);
+ struct net_device *dev = skb_dst(skb)->dev;
struct flow_keys flkeys;
unsigned int hh_len;
struct dst_entry *dst;
int strict = (ipv6_addr_type(&iph->daddr) &
(IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL));
struct flowi6 fl6 = {
- .flowi6_oif = sk && sk->sk_bound_dev_if ? sk->sk_bound_dev_if :
- strict ? skb_dst(skb)->dev->ifindex : 0,
.flowi6_mark = skb->mark,
.flowi6_uid = sock_net_uid(net, sk),
.daddr = iph->daddr,
@@ -39,6 +38,13 @@ int ip6_route_me_harder(struct net *net, struct sock *sk_partial, struct sk_buff
};
int err;
+ if (sk && sk->sk_bound_dev_if)
+ fl6.flowi6_oif = sk->sk_bound_dev_if;
+ else if (strict)
+ fl6.flowi6_oif = dev->ifindex;
+ else
+ fl6.flowi6_oif = l3mdev_master_ifindex(dev);
+
fib6_rules_early_flow_dissect(net, skb, &fl6, &flkeys);
dst = ip6_route_output(net, sk, &fl6);
err = dst->error;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH net-next 03/12] dt-bindings: net: pcs: add bindings for Renesas RZ/N1 MII converter
From: Rob Herring @ 2022-04-19 13:43 UTC (permalink / raw)
To: Clément Léger
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S . Miller, Jakub Kicinski, Paolo Abeni,
Krzysztof Kozlowski, Geert Uytterhoeven, Magnus Damm,
Heiner Kallweit, Russell King, Thomas Petazzoni, Herve Codina,
Miquèl Raynal, Milan Stevanovic, Jimmy Lalande, linux-kernel,
devicetree, linux-renesas-soc, netdev
In-Reply-To: <20220414122250.158113-4-clement.leger@bootlin.com>
On Thu, Apr 14, 2022 at 02:22:41PM +0200, Clément Léger wrote:
> This MII converter can be found on the RZ/N1 processor family. The MII
> converter ports are declared as subnodes which are then referenced by
> users of the PCS driver such as the switch.
>
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> ---
> .../bindings/net/pcs/renesas,rzn1-miic.yaml | 95 +++++++++++++++++++
> include/dt-bindings/net/pcs-rzn1-miic.h | 19 ++++
> 2 files changed, 114 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/pcs/renesas,rzn1-miic.yaml
> create mode 100644 include/dt-bindings/net/pcs-rzn1-miic.h
>
> diff --git a/Documentation/devicetree/bindings/net/pcs/renesas,rzn1-miic.yaml b/Documentation/devicetree/bindings/net/pcs/renesas,rzn1-miic.yaml
> new file mode 100644
> index 000000000000..ccb25ce6cbde
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pcs/renesas,rzn1-miic.yaml
> @@ -0,0 +1,95 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/pcs/renesas,rzn1-miic.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/N1 MII converter
> +
> +maintainers:
> + - Clément Léger <clement.leger@bootlin.com>
> +
> +description: |
> + This MII converter is present on the Renesas RZ/N1 SoC family. It is
> + responsible to do MII passthrough or convert it to RMII/RGMII.
> +
> +properties:
> + compatible:
> + const: renesas,rzn1-miic
Need SoC specific compatibles.
> +
> + reg:
> + maxItems: 1
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + clocks:
> + items:
> + - description: MII reference clock
> + - description: RGMII reference clock
> + - description: RMII reference clock
> + - description: AHB clock used for the MII converter register interface
> +
> + renesas,miic-cfg-mode:
> + description: MII mux configuration mode. This value should use one of the
> + value defined in dt-bindings/net/pcs-rzn1-miic.h.
Describe possible values here as constraints. At present, I don't see
the point of this property if there is only 1 possible value and it is
required.
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> +patternProperties:
> + "^mii-conv@[0-4]$":
> + type: object
additionalProperties: false
> + description: MII converter port
> +
> + properties:
> + reg:
> + maxItems: 1
Why do you need sub-nodes? They don't have any properties. A simple mask
property could tell you which ports are present/active/enabled if that's
what you are tracking. Or the SoC specific compatibles you need to add
can imply the ports if they are SoC specific.
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - renesas,miic-cfg-mode
> + - "#address-cells"
> + - "#size-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/net/pcs-rzn1-miic.h>
> + #include <dt-bindings/clock/r9a06g032-sysctrl.h>
> +
> + eth-miic@44030000 {
> + compatible = "renesas,rzn1-miic";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x44030000 0x10000>;
> + clocks = <&sysctrl R9A06G032_CLK_MII_REF>,
> + <&sysctrl R9A06G032_CLK_RGMII_REF>,
> + <&sysctrl R9A06G032_CLK_RMII_REF>,
> + <&sysctrl R9A06G032_HCLK_SWITCH_RG>;
> + renesas,miic-cfg-mode = <MIIC_MUX_MAC2_MAC1_SWD_SWC_SWB_SWA>;
> +
> + mii_conv0: mii-conv@0 {
> + reg = <0>;
> + };
> +
> + mii_conv1: mii-conv@1 {
> + reg = <1>;
> + };
> +
> + mii_conv2: mii-conv@2 {
> + reg = <2>;
> + };
> +
> + mii_conv3: mii-conv@3 {
> + reg = <3>;
> + };
> +
> + mii_conv4: mii-conv@4 {
> + reg = <4>;
> + };
> + };
> \ No newline at end of file
Fix this.
> diff --git a/include/dt-bindings/net/pcs-rzn1-miic.h b/include/dt-bindings/net/pcs-rzn1-miic.h
> new file mode 100644
> index 000000000000..c5a0f382967b
> --- /dev/null
> +++ b/include/dt-bindings/net/pcs-rzn1-miic.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
Dual license please.
> +/*
> + * Copyright (C) 2022 Schneider-Electric
> + *
> + * Clément Léger <clement.leger@bootlin.com>
> + */
> +
> +#ifndef _DT_BINDINGS_PCS_RZN1_MIIC
> +#define _DT_BINDINGS_PCS_RZN1_MIIC
> +
> +/*
> + * Reefer to the datasheet [1] section 8.2.1, Internal Connection of Ethernet
> + * Ports to check the meaning of these values.
> + *
> + * [1] REN_r01uh0750ej0140-rzn1-introduction_MAT_20210228.pdf
> + */
> +#define MIIC_MUX_MAC2_MAC1_SWD_SWC_SWB_SWA 0x13
> +
> +#endif
> --
> 2.34.1
>
>
^ permalink raw reply
* Re: [PATCH] net: ax88179: add proper error handling of usb read errors
From: Paolo Abeni @ 2022-04-19 13:41 UTC (permalink / raw)
To: David Kahurani, netdev
Cc: syzbot+d3dbdf31fbe9d8f5f311, davem, jgg, kuba, linux-kernel,
linux-usb, phil, syzkaller-bugs, arnd, dan.carpenter
In-Reply-To: <20220416074817.571160-1-k.kahurani@gmail.com>
On Sat, 2022-04-16 at 10:48 +0300, David Kahurani wrote:
> Reads that are lesser than the requested size lead to uninit-value bugs.
> In this particular case a variable which was supposed to be initialized
> after a read is left uninitialized after a partial read.
>
> Qualify such reads as errors and handle them correctly and while at it
> convert the reader functions to return zero on success for easier error
> handling.
>
> Fixes: e2ca90c276e1 ("ax88179_178a: ASIX AX88179_178A USB 3.0/2.0 to
> gigabit ethernet adapter driver")
In the next version, please additionally fix the above tag: it must use
a single line, even if that means exceeding the 72 chars length limit.
Thanks!
Paolo
^ permalink raw reply
* Re: [PATCH] wfx: use container_of() to get vif
From: Jérôme Pouiller @ 2022-04-19 13:39 UTC (permalink / raw)
To: Kalle Valo, David S. Miller, Jakub Kicinski, Paolo Abeni,
linux-wireless, netdev, linux-kernel, linux-staging, outreachy,
Jaehee Park, Stefano Brivio, Jaehee Park
In-Reply-To: <20220418035110.GA937332@jaehee-ThinkPad-X1-Extreme>
On Monday 18 April 2022 05:51:10 CEST Jaehee Park wrote:
>
> Currently, upon virtual interface creation, wfx_add_interface() stores
> a reference to the corresponding struct ieee80211_vif in private data,
> for later usage. This is not needed when using the container_of
> construct. This construct already has all the info it needs to retrieve
> the reference to the corresponding struct from the offset that is
> already available, inherent in container_of(), between its type and
> member inputs (struct ieee80211_vif and drv_priv, respectively).
> Remove vif (which was previously storing the reference to the struct
> ieee80211_vif) from the struct wfx_vif, define a function
> wvif_to_vif(wvif) for container_of(), and replace all wvif->vif with
> the newly defined container_of construct.
>
> Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> ---
>
> Changes from staging to wireless-next tree
> - changed macro into function and named it back to wvif_to_vif
> - fit all lines in patch to 80 columns
> - decared a reference to vif at the beginning of the functions
>
> NOTE: Jérôme is going to be testing this patch on his hardware
Don't forget to increment the version number of your submission (option
-v of git send-email).
> drivers/net/wireless/silabs/wfx/wfx.h | 6 +-
> drivers/net/wireless/silabs/wfx/data_rx.c | 5 +-
> drivers/net/wireless/silabs/wfx/data_tx.c | 3 +-
> drivers/net/wireless/silabs/wfx/key.c | 4 +-
> drivers/net/wireless/silabs/wfx/queue.c | 3 +-
> drivers/net/wireless/silabs/wfx/scan.c | 9 ++-
> drivers/net/wireless/silabs/wfx/sta.c | 69 ++++++++++++++---------
> 7 files changed, 63 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/wireless/silabs/wfx/wfx.h b/drivers/net/wireless/silabs/wfx/wfx.h
> index 6594cc647c2f..718693a4273d 100644
> --- a/drivers/net/wireless/silabs/wfx/wfx.h
> +++ b/drivers/net/wireless/silabs/wfx/wfx.h
> @@ -61,7 +61,6 @@ struct wfx_dev {
>
> struct wfx_vif {
> struct wfx_dev *wdev;
> - struct ieee80211_vif *vif;
> struct ieee80211_channel *channel;
> int id;
>
> @@ -91,6 +90,11 @@ struct wfx_vif {
> struct completion set_pm_mode_complete;
> };
>
> +static inline struct ieee80211_vif *wvif_to_vif(struct wfx_vif *wvif)
> +{
> + return container_of((void *)wvif, struct ieee80211_vif, drv_priv);
> +}
> +
> static inline struct wfx_vif *wdev_to_wvif(struct wfx_dev *wdev, int vif_id)
> {
> if (vif_id >= ARRAY_SIZE(wdev->vif)) {
> diff --git a/drivers/net/wireless/silabs/wfx/data_rx.c b/drivers/net/wireless/silabs/wfx/data_rx.c
> index a4b5ffe158e4..342b9cd0e74c 100644
> --- a/drivers/net/wireless/silabs/wfx/data_rx.c
> +++ b/drivers/net/wireless/silabs/wfx/data_rx.c
> @@ -16,6 +16,7 @@
> static void wfx_rx_handle_ba(struct wfx_vif *wvif, struct ieee80211_mgmt *mgmt)
> {
> int params, tid;
> + struct ieee80211_vif *vif = wvif_to_vif(wvif);
When you can, try to place the longest declaration first ("reverse
Christmas tree order").
[...]
> diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c
> index 3297d73c327a..97fcbad23c94 100644
> --- a/drivers/net/wireless/silabs/wfx/sta.c
> +++ b/drivers/net/wireless/silabs/wfx/sta.c
[...]
> @@ -152,19 +153,28 @@ static int wfx_get_ps_timeout(struct wfx_vif *wvif, bool *enable_ps)
> {
> struct ieee80211_channel *chan0 = NULL, *chan1 = NULL;
> struct ieee80211_conf *conf = &wvif->wdev->hw->conf;
> + struct ieee80211_vif *vif = wvif_to_vif(wvif);
>
> - WARN(!wvif->vif->bss_conf.assoc && enable_ps,
> + WARN(!vif->bss_conf.assoc && enable_ps,
> "enable_ps is reliable only if associated");
> - if (wdev_to_wvif(wvif->wdev, 0))
> - chan0 = wdev_to_wvif(wvif->wdev, 0)->vif->bss_conf.chandef.chan;
> - if (wdev_to_wvif(wvif->wdev, 1))
> - chan1 = wdev_to_wvif(wvif->wdev, 1)->vif->bss_conf.chandef.chan;
> - if (chan0 && chan1 && wvif->vif->type != NL80211_IFTYPE_AP) {
> + if (wdev_to_wvif(wvif->wdev, 0)) {
> + struct wfx_vif *wvif_ch0 = wdev_to_wvif(wvif->wdev, 0);
> + struct ieee80211_vif *vif_ch0 = wvif_to_vif(wvif_ch0);
> +
> + chan0 = vif_ch0->bss_conf.chandef.chan;
> + }
> + if (wdev_to_wvif(wvif->wdev, 1)) {
> + struct wfx_vif *wvif_ch1 = wdev_to_wvif(wvif->wdev, 1);
> + struct ieee80211_vif *vif_ch1 = wvif_to_vif(wvif_ch1);
> +
> + chan1 = vif_ch1->bss_conf.chandef.chan;
> + }
I think this code could be simplified into:
if (wvif->wdev->vif[1])
chan1 = wvif->wdev->vif[1]->bss_conf.chandef.chan;
(If you choose this way, I suggest to place this change in a separate
patch)
[...]
--
Jérôme Pouiller
^ permalink raw reply
* Re: [PATCH v3] intel: igb: igb_ethtool.c: Convert kmap() to kmap_local_page()
From: Alaa Mohamed @ 2022-04-19 13:37 UTC (permalink / raw)
To: Ira Weiny
Cc: Julia Lawall, outreachy, jesse.brandeburg, anthony.l.nguyen,
davem, kuba, pabeni, intel-wired-lan, netdev, linux-kernel
In-Reply-To: <Yl3j/bOvoX13WGSW@iweiny-desk3>
On ١٩/٤/٢٠٢٢ ٠٠:١٩, Ira Weiny wrote:
> On Sat, Apr 16, 2022 at 03:14:57PM +0200, Alaa Mohamed wrote:
>> On ١٦/٤/٢٠٢٢ ١٣:٣١, Julia Lawall wrote:
>>
>>
>> On Sat, 16 Apr 2022, Alaa Mohamed wrote:
>>
>>
>> Convert kmap() to kmap_local_page()
>>
>> With kmap_local_page(), the mapping is per thread, CPU local and not
>> globally visible.
>>
>> It's not clearer.
>>
>> I mean this " fix kunmap_local path value to take address of the mapped
>> page" be more clearer
>>
>> This is a general statement about the function. You
>> need to explain why it is appropriate to use it here. Unless it is the
>> case that all calls to kmap should be converted to call kmap_local_page.
>>
>> It's required to convert all calls kmap to kmap_local_page. So, I don't
>> what should the commit message be?
>>
>> Is this will be good :
>>
>> "kmap_local_page() was recently developed as a replacement for kmap().
>> The
>> kmap_local_page() creates a mapping which is restricted to local use by a
>> single thread of execution. "
>>
> I think I am missing some thread context here. I'm not sure who said what
> above. So I'm going to start over.
>
> Alaa,
>
> It is important to remember that a good commit message says 2 things.
>
> 1) What is the problem you are trying to solve
> 2) Overview of the solution
>
> First off I understand your frustration. In my opinion fixes and clean ups
> like this are very hard to write good commit messages for because so often the
> code diff seems so self explanatory. However, each code change comes at the
> identification of a problem. And remember that 'problem' does not always mean
> a bug fix.
>
> The deprecation of kmap() may not seem like a problem. I mean why can't we
> just leave kmap() as it is? It works right?
>
> But the problem is that the kmap (highmem) interface has become stale and its
> original purpose was targeted toward large memory systems with 32 bit kernels.
> There are very few systems being run like that any longer.
>
> So how do we clean up the kmap interface to be more useful to the kernel
> community now that 32 bit kernels with highmem are so rare?
>
> The community has identified that a first step of that is to move away from and
> eventually remove the kmap() call. This is due to the call being incorrectly
> used to create long term mappings. Most calls to kmap() are not used
> incorrectly but those call sites needed something in between kmap() and
> kmap_atmoic(). That call is kmap_local_page().
>
> Now that kmap_local_page() exists the kmap() calls can be audited and most (I
> hope most)[1] can be replaced with kmap_local_page().
>
> The change you have below is correct. But it lacks a good commit message. We
> need to cover the 2 points above.
>
> 1) Julia is asking why you needed to do this change. What is the
> problem or reason for this change? (Ira told you to is not a good
> reason. ;-)
>
> PS In fact me telling you to may actually be a very bad reason...
> j/k ;-)
>
> 2) Why is this solution ok as part of the deprecation and removal of
> kmap()?
>
> A final note; the 2 above points don't need a lot of text. Here I used
> 2 simple sentences.
>
> https://lore.kernel.org/lkml/20220124015409.807587-2-ira.weiny@intel.com/
>
> I hope this helps,
> Ira
>
> [1] But not all... some uses of kmap() have been identified as being pretty
> complex.
Thanks a lot for detailed explaining , yes you help me a lot.
Thanks again,
Alaa
>> julia
>>
>>
>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>> ---
>> changes in V2:
>> fix kunmap_local path value to take address of the mapped page.
>> ---
>> changes in V3:
>> edit commit message to be clearer
>> ---
>> drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
>> index 2a5782063f4c..c14fc871dd41 100644
>> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
>> @@ -1798,14 +1798,14 @@ static int igb_check_lbtest_frame(struct igb_rx_buffer *rx_buffer,
>>
>> frame_size >>= 1;
>>
>> - data = kmap(rx_buffer->page);
>> + data = kmap_local_page(rx_buffer->page);
>>
>> if (data[3] != 0xFF ||
>> data[frame_size + 10] != 0xBE ||
>> data[frame_size + 12] != 0xAF)
>> match = false;
>>
>> - kunmap(rx_buffer->page);
>> + kunmap_local(data);
>>
>> return match;
>> }
>> --
>> 2.35.2
^ permalink raw reply
* Re: [PATCH 2/2] Trigger proper interrupts in igc_xsk_wakeup
From: Paolo Abeni @ 2022-04-19 13:33 UTC (permalink / raw)
To: Jeff Evanson, Jesse Brandeburg, Tony Nguyen, David S. Miller,
Jakub Kicinski, intel-wired-lan, netdev, linux-kernel
Cc: jeff.evanson
In-Reply-To: <20220415210546.11294-1-jeff.evanson@qsc.com>
On Fri, 2022-04-15 at 15:05 -0600, Jeff Evanson wrote:
> in igc_xsk_wakeup, trigger the proper interrupt based on whether flags
> contains XDP_WAKEUP_RX and/or XDP_WAKEUP_TX
>
> Signed-off-by: Jeff Evanson <jeff.evanson@qsc.com>
> ---
> drivers/net/ethernet/intel/igc/igc_main.c | 36 +++++++++++++++++------
> 1 file changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index a36a18c84aeb..d706de95dc06 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6073,7 +6073,7 @@ static void igc_trigger_rxtxq_interrupt(struct igc_adapter *adapter,
> int igc_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags)
> {
> struct igc_adapter *adapter = netdev_priv(dev);
> - struct igc_q_vector *q_vector;
> + struct igc_q_vector *txq_vector = 0, *rxq_vector = 0;
Since a v2 is likely required - see even Vinicius's comments on patch
1/2 - please reorder the above to respect the reverse x-mas tree order
and fix the sparse warning introduced above (s/ = 0/ = NULL/)
Thanks!
Paolo
^ permalink raw reply
* Re: [PATCH net 1/1] net: stmmac: add fsleep() in HW Rx timestamp checking loop
From: Richard Cochran @ 2022-04-19 13:28 UTC (permalink / raw)
To: Tan Tee Min
Cc: Jakub Kicinski, Tan Tee Min, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, David S . Miller, Paolo Abeni, Maxime Coquelin,
Rayagond Kokatanur, netdev, linux-stm32, linux-arm-kernel,
linux-kernel, stable, Voon Wei Feng, Wong Vee Khee,
Song Yoong Siang, Alexandre Torgue
In-Reply-To: <20220419005220.GA17634@linux.intel.com>
On Tue, Apr 19, 2022 at 08:52:20AM +0800, Tan Tee Min wrote:
> I agree that the fsleep(1) (=1us) is a big hammer.
> Thus in order to improve this, I’ve figured out a smaller delay
> time that is enough for the context descriptor to be ready which
> is ndelay(500) (=500ns).
Why isn't the context descriptor ready?
I mean, the frame already belongs to the CPU, right?
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net] netlink: reset network and mac headers in netlink_dump()
From: patchwork-bot+netdevbpf @ 2022-04-19 13:20 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, kuba, pabeni, netdev, edumazet, syzkaller
In-Reply-To: <20220415181442.551228-1-eric.dumazet@gmail.com>
Hello:
This patch was applied to netdev/net.git (master)
by Paolo Abeni <pabeni@redhat.com>:
On Fri, 15 Apr 2022 11:14:42 -0700 you wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> netlink_dump() is allocating an skb, reserves space in it
> but forgets to reset network header.
>
> This allows a BPF program, invoked later from sk_filter()
> to access uninitialized kernel memory from the reserved
> space.
>
> [...]
Here is the summary with links:
- [net] netlink: reset network and mac headers in netlink_dump()
https://git.kernel.org/netdev/net/c/99c07327ae11
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH ethtool-next] ethtool: netlink: add support to get/set tx push by ethtool -G/g
From: wangjie (L) @ 2022-04-19 13:20 UTC (permalink / raw)
To: Michal Kubecek, Guangbin Huang; +Cc: davem, kuba, netdev, lipeng321
In-Reply-To: <20220419130702.xlnodeeeycn6jja6@lion.mk-sys.cz>
On 2022/4/19 21:07, Michal Kubecek wrote:
> On Tue, Apr 19, 2022 at 08:50:30PM +0800, Guangbin Huang wrote:
>> From: Jie Wang <wangjie125@huawei.com>
>>
>> Currently tx push is a standard feature for NICs such as Mellanox, HNS3.
>> But there is no command to set or get this feature.
>>
>> So this patch adds support for "ethtool -G <dev> tx-push on|off" and
>> "ethtool -g <dev>" to set/get tx push mode.
>>
>> Signed-off-by: Jie Wang <wangjie125@huawei.com>
>> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
>> ---
> [...]
>> diff --git a/uapi/linux/ethtool_netlink.h b/uapi/linux/ethtool_netlink.h
>> index d8b19cf98003..79fe0bf686f3 100644
>> --- a/uapi/linux/ethtool_netlink.h
>> +++ b/uapi/linux/ethtool_netlink.h
>> @@ -330,6 +330,7 @@ enum {
>> ETHTOOL_A_RINGS_RX_JUMBO, /* u32 */
>> ETHTOOL_A_RINGS_TX, /* u32 */
>> ETHTOOL_A_RINGS_RX_BUF_LEN, /* u32 */
>> + ETHTOOL_A_RINGS_TX_PUSH = 13, /* u8 */
>>
>> /* add new constants above here */
>> __ETHTOOL_A_RINGS_CNT,
>
> Please update the uapi headers from sanitized kernel headers as
> described here:
>
> https://www.kernel.org/pub/software/network/ethtool/devel.html
>
> (the paragraph starting "If you need new or updated definitions..." near
> the end of the page).
>
> Michal
Thank you for your guidance. I will use the method to update the uapi
headers in patch v2
>
^ permalink raw reply
* Re: [PATCH v2 net-next] dt-bindings: net: mediatek,net: convert to the json-schema
From: Rob Herring @ 2022-04-19 13:19 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: netdev, nbd, lorenzo.bianconi, devicetree, davem, kuba, pabeni,
john
In-Reply-To: <2e9baf65a82de427a59fbaeaf04dde7bc5598375.1650041917.git.lorenzo@kernel.org>
On Fri, Apr 15, 2022 at 07:06:09PM +0200, Lorenzo Bianconi wrote:
> This patch converts the existing mediatek-net.txt binding file
> in yaml format.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> Changes since v1:
> - set resets maxItems to 3
> - fix cci-control-port usage in example
>
> This patch is based on commits [0] and [1] available in net-next tree but not
> in Linus's one yet.
>
> [0] 1dafd0d60703 ("dt-bindings: net: mediatek: add optional properties for the SoC ethernet core")
> [1] 4263f77a5144 ("net: ethernet: mtk_eth_soc: use standard property for cci-control-port")
> ---
> .../devicetree/bindings/net/mediatek,net.yaml | 294 ++++++++++++++++++
> .../devicetree/bindings/net/mediatek-net.txt | 108 -------
> 2 files changed, 294 insertions(+), 108 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/mediatek,net.yaml
> delete mode 100644 Documentation/devicetree/bindings/net/mediatek-net.txt
>
> diff --git a/Documentation/devicetree/bindings/net/mediatek,net.yaml b/Documentation/devicetree/bindings/net/mediatek,net.yaml
> new file mode 100644
> index 000000000000..e744ab96ecac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mediatek,net.yaml
> @@ -0,0 +1,294 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/mediatek,net.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Frame Engine Ethernet controller
> +
> +maintainers:
> + - Lorenzo Bianconi <lorenzo@kernel.org>
> + - Felix Fietkau <nbd@nbd.name>
> +
> +description:
> + The frame engine ethernet controller can be found on MediaTek SoCs. These SoCs
> + have dual GMAC ports.
> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt2701-eth
> + - mediatek,mt7623-eth
> + - mediatek,mt7622-eth
> + - mediatek,mt7629-eth
> + - ralink,rt5350-eth
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 3
> + maxItems: 3
> +
> + power-domains:
> + maxItems: 1
> +
> + resets:
> + maxItems: 3
> +
> + reset-names:
> + items:
> + - const: fe
> + - const: gmac
> + - const: ppe
> +
> + mediatek,ethsys:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the syscon node that handles the port setup.
> +
> + cci-control-port: true
> +
> + mediatek,hifsys:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the mediatek hifsys controller used to provide various clocks
> + and reset to the system.
> +
> + dma-coherent: true
> +
> + mdio-bus:
> + $ref: mdio.yaml#
> + unevaluatedProperties: false
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> +allOf:
> + - $ref: "ethernet-controller.yaml#"
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - mediatek,mt2701-eth
> + - mediatek,mt7623-eth
> + then:
> + properties:
> + clocks:
> + minItems: 4
> + maxItems: 4
> +
> + clock-names:
> + additionalItems: false
You shouldn't need this.
> + items:
> + - const: ethif
> + - const: esw
> + - const: gp1
> + - const: gp2
> +
> + mediatek,pctl:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the syscon node that handles the ports slew rate and
> + driver current.
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mediatek,mt7622-eth
> + then:
> + properties:
> + clocks:
> + minItems: 11
> + maxItems: 11
> +
> + clock-names:
> + additionalItems: false
> + items:
> + - const: ethif
> + - const: esw
> + - const: gp0
> + - const: gp1
> + - const: gp2
> + - const: sgmii_tx250m
> + - const: sgmii_rx250m
> + - const: sgmii_cdr_ref
> + - const: sgmii_cdr_fb
> + - const: sgmii_ck
> + - const: eth2pll
> +
> + mediatek,sgmiisys:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + maxItems: 1
A list or 1 item? If only 1, then use 'phandle'. A 'phandle-array' is
really a matrix, so if you have multiple phandles, you need:
minItems: 1
maxItems: max # of phandles
items:
maxItems: 1
> + description:
> + A list of phandle to the syscon node that handles the SGMII setup which is required for
> + those SoCs equipped with SGMII.
> +
> + mediatek,wed:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + minItems: 2
> + maxItems: 2
minItems: 2
maxItems: 2
items:
maxItems: 1
> + description:
> + List of phandles to wireless ethernet dispatch nodes.
> +
> + mediatek,pcie-mirror:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the mediatek pcie-mirror controller.
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mediatek,mt7629-eth
> + then:
> + properties:
> + clocks:
> + minItems: 17
> + maxItems: 17
> +
> + clock-names:
> + additionalItems: false
> + items:
> + - const: ethif
> + - const: sgmiitop
> + - const: esw
> + - const: gp0
> + - const: gp1
> + - const: gp2
> + - const: fe
> + - const: sgmii_tx250m
> + - const: sgmii_rx250m
> + - const: sgmii_cdr_ref
> + - const: sgmii_cdr_fb
> + - const: sgmii2_tx250m
> + - const: sgmii2_rx250m
> + - const: sgmii2_cdr_ref
> + - const: sgmii2_cdr_fb
> + - const: sgmii_ck
> + - const: eth2pll
> +
> + mediatek,infracfg:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the syscon node that handles the path from GMAC to
> + PHY variants.
> +
> + mediatek,sgmiisys:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + maxItems: 2
> + description:
> + A list of phandle to the syscon node that handles the SGMII setup which is required for
> + those SoCs equipped with SGMII.
Don't define properties twice. Define it at the top level and then just
add any additional constraints in the if/then schemas.
> +
> +patternProperties:
> + "^mac@[0-1]$":
> + type: object
> + additionalProperties: false
> + allOf:
> + - $ref: ethernet-controller.yaml#
> + description:
> + Ethernet MAC node
> + properties:
> + compatible:
> + const: mediatek,eth-mac
> +
> + reg:
> + maxItems: 1
> +
> + phy-handle: true
> +
> + phy-mode: true
> +
> + required:
> + - reg
> + - compatible
> + - phy-handle
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - power-domains
> + - mediatek,ethsys
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/clock/mt7622-clk.h>
> + #include <dt-bindings/power/mt7622-power.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + ethernet: ethernet@1b100000 {
> + compatible = "mediatek,mt7622-eth";
> + reg = <0 0x1b100000 0 0x20000>;
> + interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 224 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 225 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&topckgen CLK_TOP_ETH_SEL>,
> + <ðsys CLK_ETH_ESW_EN>,
> + <ðsys CLK_ETH_GP0_EN>,
> + <ðsys CLK_ETH_GP1_EN>,
> + <ðsys CLK_ETH_GP2_EN>,
> + <&sgmiisys CLK_SGMII_TX250M_EN>,
> + <&sgmiisys CLK_SGMII_RX250M_EN>,
> + <&sgmiisys CLK_SGMII_CDR_REF>,
> + <&sgmiisys CLK_SGMII_CDR_FB>,
> + <&topckgen CLK_TOP_SGMIIPLL>,
> + <&apmixedsys CLK_APMIXED_ETH2PLL>;
> + clock-names = "ethif", "esw", "gp0", "gp1", "gp2",
> + "sgmii_tx250m", "sgmii_rx250m",
> + "sgmii_cdr_ref", "sgmii_cdr_fb", "sgmii_ck",
> + "eth2pll";
> + power-domains = <&scpsys MT7622_POWER_DOMAIN_ETHSYS>;
> + mediatek,ethsys = <ðsys>;
> + mediatek,sgmiisys = <&sgmiisys>;
> + cci-control-port = <&cci_control2>;
> + mediatek,pcie-mirror = <&pcie_mirror>;
> + mediatek,hifsys = <&hifsys>;
> + dma-coherent;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mdio0: mdio-bus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy0: ethernet-phy@0 {
> + reg = <0>;
> + };
> +
> + phy1: ethernet-phy@1 {
> + reg = <1>;
> + };
> + };
> +
> + gmac0: mac@0 {
> + compatible = "mediatek,eth-mac";
> + phy-mode = "rgmii";
> + phy-handle = <&phy0>;
> + reg = <0>;
> + };
> +
> + gmac1: mac@1 {
> + compatible = "mediatek,eth-mac";
> + phy-mode = "rgmii";
> + phy-handle = <&phy1>;
> + reg = <1>;
> + };
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt b/Documentation/devicetree/bindings/net/mediatek-net.txt
> deleted file mode 100644
> index f18d70189375..000000000000
> --- a/Documentation/devicetree/bindings/net/mediatek-net.txt
> +++ /dev/null
> @@ -1,108 +0,0 @@
> -MediaTek Frame Engine Ethernet controller
> -=========================================
> -
> -The frame engine ethernet controller can be found on MediaTek SoCs. These SoCs
> -have dual GMAC each represented by a child node..
> -
> -* Ethernet controller node
> -
> -Required properties:
> -- compatible: Should be
> - "mediatek,mt2701-eth": for MT2701 SoC
> - "mediatek,mt7623-eth", "mediatek,mt2701-eth": for MT7623 SoC
> - "mediatek,mt7622-eth": for MT7622 SoC
> - "mediatek,mt7629-eth": for MT7629 SoC
> - "ralink,rt5350-eth": for Ralink Rt5350F and MT7628/88 SoC
> -- reg: Address and length of the register set for the device
> -- interrupts: Should contain the three frame engines interrupts in numeric
> - order. These are fe_int0, fe_int1 and fe_int2.
> -- clocks: the clock used by the core
> -- clock-names: the names of the clock listed in the clocks property. These are
> - "ethif", "esw", "gp2", "gp1" : For MT2701 and MT7623 SoC
> - "ethif", "esw", "gp0", "gp1", "gp2", "sgmii_tx250m", "sgmii_rx250m",
> - "sgmii_cdr_ref", "sgmii_cdr_fb", "sgmii_ck", "eth2pll" : For MT7622 SoC
> - "ethif", "sgmiitop", "esw", "gp0", "gp1", "gp2", "fe", "sgmii_tx250m",
> - "sgmii_rx250m", "sgmii_cdr_ref", "sgmii_cdr_fb", "sgmii2_tx250m",
> - "sgmii2_rx250m", "sgmii2_cdr_ref", "sgmii2_cdr_fb", "sgmii_ck",
> - "eth2pll" : For MT7629 SoC.
> -- power-domains: phandle to the power domain that the ethernet is part of
> -- resets: Should contain phandles to the ethsys reset signals
> -- reset-names: Should contain the names of reset signal listed in the resets
> - property
> - These are "fe", "gmac" and "ppe"
> -- mediatek,ethsys: phandle to the syscon node that handles the port setup
> -- mediatek,infracfg: phandle to the syscon node that handles the path from
> - GMAC to PHY variants, which is required for MT7629 SoC.
> -- mediatek,sgmiisys: a list of phandles to the syscon node that handles the
> - SGMII setup which is required for those SoCs equipped with SGMII such
> - as MT7622 and MT7629 SoC. And MT7622 have only one set of SGMII shared
> - by GMAC1 and GMAC2; MT7629 have two independent sets of SGMII directed
> - to GMAC1 and GMAC2, respectively.
> -- mediatek,pctl: phandle to the syscon node that handles the ports slew rate
> - and driver current: only for MT2701 and MT7623 SoC
> -
> -Optional properties:
> -- dma-coherent: present if dma operations are coherent
> -- mediatek,cci-control: phandle to the cache coherent interconnect node
> -- mediatek,hifsys: phandle to the mediatek hifsys controller used to provide
> - various clocks and reset to the system.
> -- mediatek,wed: a list of phandles to wireless ethernet dispatch nodes for
> - MT7622 SoC.
> -- mediatek,pcie-mirror: phandle to the mediatek pcie-mirror controller for
> - MT7622 SoC.
> -
> -* Ethernet MAC node
> -
> -Required properties:
> -- compatible: Should be "mediatek,eth-mac"
> -- reg: The number of the MAC
> -- phy-handle: see ethernet.txt file in the same directory and
> - the phy-mode "trgmii" required being provided when reg
> - is equal to 0 and the MAC uses fixed-link to connect
> - with internal switch such as MT7530.
> -
> -Example:
> -
> -eth: ethernet@1b100000 {
> - compatible = "mediatek,mt7623-eth";
> - reg = <0 0x1b100000 0 0x20000>;
> - clocks = <&topckgen CLK_TOP_ETHIF_SEL>,
> - <ðsys CLK_ETHSYS_ESW>,
> - <ðsys CLK_ETHSYS_GP2>,
> - <ðsys CLK_ETHSYS_GP1>;
> - clock-names = "ethif", "esw", "gp2", "gp1";
> - interrupts = <GIC_SPI 200 IRQ_TYPE_LEVEL_LOW
> - GIC_SPI 199 IRQ_TYPE_LEVEL_LOW
> - GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
> - power-domains = <&scpsys MT2701_POWER_DOMAIN_ETH>;
> - resets = <ðsys MT2701_ETHSYS_ETH_RST>;
> - reset-names = "eth";
> - mediatek,ethsys = <ðsys>;
> - mediatek,pctl = <&syscfg_pctl_a>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - gmac1: mac@0 {
> - compatible = "mediatek,eth-mac";
> - reg = <0>;
> - phy-handle = <&phy0>;
> - };
> -
> - gmac2: mac@1 {
> - compatible = "mediatek,eth-mac";
> - reg = <1>;
> - phy-handle = <&phy1>;
> - };
> -
> - mdio-bus {
> - phy0: ethernet-phy@0 {
> - reg = <0>;
> - phy-mode = "rgmii";
> - };
> -
> - phy1: ethernet-phy@1 {
> - reg = <1>;
> - phy-mode = "rgmii";
> - };
> - };
> -};
> --
> 2.35.1
>
>
^ permalink raw reply
* Re: [PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings
From: Alvin Šipraga @ 2022-04-19 13:13 UTC (permalink / raw)
To: Luiz Angelo Daros de Luca
Cc: netdev@vger.kernel.org, linus.walleij@linaro.org, andrew@lunn.ch,
vivien.didelot@gmail.com, f.fainelli@gmail.com, olteanv@gmail.com,
davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com,
robh+dt@kernel.org, krzk+dt@kernel.org, arinc.unal@arinc9.com,
devicetree@vger.kernel.org
In-Reply-To: <20220418233558.13541-1-luizluca@gmail.com>
On Mon, Apr 18, 2022 at 08:35:57PM -0300, Luiz Angelo Daros de Luca wrote:
> Compatible strings are used to help the driver find the chip ID/version
> register for each chip family. After that, the driver can setup the
> switch accordingly. Keep only the first supported model for each family
> as a compatible string and reference other chip models in the
> description.
>
> The removed compatible strings have never been used in a released kernel.
>
> CC: devicetree@vger.kernel.org
> Link: https://lore.kernel.org/netdev/20220414014055.m4wbmr7tdz6hsa3m@bang-olufsen.dk/
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> ---
> .../devicetree/bindings/net/dsa/realtek.yaml | 35 ++++++++-----------
> 1 file changed, 14 insertions(+), 21 deletions(-)
I think this is OK now. I'll follow up with whatever reply I get from Realtek
regarding the revision ID register values for switches we do not own.
Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
^ permalink raw reply
* Re: [PATCH net v2 1/2] dt-bindings: net: dsa: realtek: cleanup compatible strings
From: Alvin Šipraga @ 2022-04-19 13:10 UTC (permalink / raw)
To: Arınç ÜNAL
Cc: Luiz Angelo Daros de Luca, netdev@vger.kernel.org,
linus.walleij@linaro.org, andrew@lunn.ch,
vivien.didelot@gmail.com, f.fainelli@gmail.com, olteanv@gmail.com,
davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com,
robh+dt@kernel.org, krzk+dt@kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <41b4c9c7-871a-83c4-5df0-24b85ce0cb24@arinc9.com>
On Tue, Apr 19, 2022 at 03:47:40AM +0300, Arınç ÜNAL wrote:
> On 19/04/2022 02:35, Luiz Angelo Daros de Luca wrote:
> > Compatible strings are used to help the driver find the chip ID/version
> > register for each chip family. After that, the driver can setup the
> > switch accordingly. Keep only the first supported model for each family
> > as a compatible string and reference other chip models in the
> > description.
> >
> > The removed compatible strings have never been used in a released kernel.
> >
> > CC: devicetree@vger.kernel.org
> > Link: https://lore.kernel.org/netdev/20220414014055.m4wbmr7tdz6hsa3m@bang-olufsen.dk/
> > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
>
> Do we know the chip ID and version of all of the switches this driver _can_
> support? So we have all the switches actually supported under a single
> compatible string.
>
> The chip ID seems to be the same across all the switches under this defacto
> rtl8367c family.
Some of them will have a different chip ID, but we haven't had anyone try those
switches yet. Currently they will be unsupported, and I wouldn't want to claim
otherwise though because nobody has actually tested. There are small differences
per switch but in general these differences disappear if they have the same chip
ID. To give a more precise answer will require a lot more detail which I don't
think is relevant, but if you are curious, you can check how the vendor driver
does the detection and what the different parameters then are:
https://github.com/openwrt/openwrt/blob/aae7af4219e56c2787f675109d9dd1a44a5dcba4/target/linux/mediatek/files-5.10/drivers/net/phy/rtk/rtl8367c/rtk_switch.c#L712
>
> Alvin, could your contacts at Realtek provide the chip ID and version for
> the switches we don’t know:
> RTL8363NB, RTL8363NB-VB, RTL8363SC, RTL8363SC-VB, RTL8364NB, RTL8364NB-VB,
> RTL8366SC, RTL8367SB, RTL8370MB, RTL8310SR
I'll ask my contact at Realtek if he can give me a full list of chip/version
values per switch, but given that the vendor driver is already doing similar
auto-detection, I think it is safe to assume that we don't require an additional
compatible string for the switches listed in Luiz' patch. The vendor driver
simply doesn't have a very granular check - presumably because the switches in
this family share many similarities - so it's not possible to infer the
chip/version ID per switch.
Kind regards,
Alvin
^ permalink raw reply
* Re: [PATCH ethtool-next] ethtool: netlink: add support to get/set tx push by ethtool -G/g
From: Michal Kubecek @ 2022-04-19 13:07 UTC (permalink / raw)
To: Guangbin Huang; +Cc: davem, kuba, netdev, lipeng321
In-Reply-To: <20220419125030.3230-1-huangguangbin2@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 1245 bytes --]
On Tue, Apr 19, 2022 at 08:50:30PM +0800, Guangbin Huang wrote:
> From: Jie Wang <wangjie125@huawei.com>
>
> Currently tx push is a standard feature for NICs such as Mellanox, HNS3.
> But there is no command to set or get this feature.
>
> So this patch adds support for "ethtool -G <dev> tx-push on|off" and
> "ethtool -g <dev>" to set/get tx push mode.
>
> Signed-off-by: Jie Wang <wangjie125@huawei.com>
> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
> ---
[...]
> diff --git a/uapi/linux/ethtool_netlink.h b/uapi/linux/ethtool_netlink.h
> index d8b19cf98003..79fe0bf686f3 100644
> --- a/uapi/linux/ethtool_netlink.h
> +++ b/uapi/linux/ethtool_netlink.h
> @@ -330,6 +330,7 @@ enum {
> ETHTOOL_A_RINGS_RX_JUMBO, /* u32 */
> ETHTOOL_A_RINGS_TX, /* u32 */
> ETHTOOL_A_RINGS_RX_BUF_LEN, /* u32 */
> + ETHTOOL_A_RINGS_TX_PUSH = 13, /* u8 */
>
> /* add new constants above here */
> __ETHTOOL_A_RINGS_CNT,
Please update the uapi headers from sanitized kernel headers as
described here:
https://www.kernel.org/pub/software/network/ethtool/devel.html
(the paragraph starting "If you need new or updated definitions..." near
the end of the page).
Michal
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v8 2/2] net: ethernet: Add driver for Sunplus SP7021
From: Andrew Lunn @ 2022-04-19 13:03 UTC (permalink / raw)
To: Wells Lu 呂芳騰
Cc: Jakub Kicinski, Wells Lu, davem@davemloft.net, robh+dt@kernel.org,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, p.zabel@pengutronix.de,
pabeni@redhat.com, krzk+dt@kernel.org, roopa@nvidia.com,
edumazet@google.com
In-Reply-To: <e784ab5356aa4b6e93765b54bdefea0a@sphcmbx02.sunplus.com.tw>
On Tue, Apr 19, 2022 at 10:07:55AM +0000, Wells Lu 呂芳騰 wrote:
> > > > + /* Get mac-address from nvmem. */
> > > > + ret = spl2sw_nvmem_get_mac_address(&pdev->dev, port_np, mac_addr);
> > > > + if (ret) {
> > > > + dev_info(&pdev->dev, "Generate a random mac address!\n");
> > > > +
> > > > + /* Generate a mac address using OUI of Sunplus Technology
> > > > + * and random controller number.
> > > > + */
> > > > + mac_addr[0] = 0xfc; /* OUI of Sunplus: fc:4b:bc */
> > > > + mac_addr[1] = 0x4b;
> > > > + mac_addr[2] = 0xbc;
> > > > + mac_addr[3] = get_random_int() % 256;
> > > > + mac_addr[4] = get_random_int() % 256;
> > > > + mac_addr[5] = get_random_int() % 256;
> > >
> > > I don't think you can do that. Either you use your OUI and assign the
> > > address at manufacture or you must use a locally administered address.
> > > And if locally administered address is used it better be completely
> > > random to lower the probability of collision to absolute minimum.
> >
> > I commented about that in an earlier version of these patches. We probably need a quote
> > from the 802.1 or 802.3 which says this is O.K.
> >
> > Andrew
>
> Hi Andrew,
>
> I plan to replace above statements with:
>
> eth_random_addr(mac_addr);
O.K, that is good.
> Do you mean I can keep use the mac address: "OUI + random number"?
If you can show us text in an IEEE 802.1, IEEE 802.3, or some other
IEEE document which says this is allowed.
> Only need to add comment for it.
Add a comment which points to a document which says you are allowed to
do this. This is very unusual, so questions will be asked, and if you
point people at the answer it will help.
Andrew
^ 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