* Re: [PATCH bpf-next v4 0/2] libbpf: adding AF_XDP support
From: Daniel Borkmann @ 2019-02-18 9:38 UTC (permalink / raw)
To: Magnus Karlsson
Cc: Jesper Dangaard Brouer, Jonathan Lemon, Magnus Karlsson,
Björn Töpel, ast, Network Development, Jakub Kicinski,
Björn Töpel, Zhang, Qi Z, xiaolong.ye,
xdp-newbies@vger.kernel.org
In-Reply-To: <CAJ8uoz3kRrXJCthhm4u=0Z-7=uNryB_r8Yj3p7inYEPjtLp2Kw@mail.gmail.com>
On 02/18/2019 09:20 AM, Magnus Karlsson wrote:
> On Fri, Feb 15, 2019 at 5:48 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> On 02/13/2019 12:55 PM, Jesper Dangaard Brouer wrote:
>>> On Wed, 13 Feb 2019 12:32:47 +0100
>>> Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
>>>> On Mon, Feb 11, 2019 at 9:44 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
>>>>> On 8 Feb 2019, at 5:05, Magnus Karlsson wrote:
>>>>>
>>>>>> This patch proposes to add AF_XDP support to libbpf. The main reason
>>>>>> for this is to facilitate writing applications that use AF_XDP by
>>>>>> offering higher-level APIs that hide many of the details of the AF_XDP
>>>>>> uapi. This is in the same vein as libbpf facilitates XDP adoption by
>>>>>> offering easy-to-use higher level interfaces of XDP
>>>>>> functionality. Hopefully this will facilitate adoption of AF_XDP, make
>>>>>> applications using it simpler and smaller, and finally also make it
>>>>>> possible for applications to benefit from optimizations in the AF_XDP
>>>>>> user space access code. Previously, people just copied and pasted the
>>>>>> code from the sample application into their application, which is not
>>>>>> desirable.
>>>>>
>>>>> I like the idea of encapsulating the boilerplate logic in a library.
>>>>>
>>>>> I do think there is an important missing piece though - there should be
>>>>> some code which queries the netdev for how many queues are attached, and
>>>>> create the appropriate number of umem/AF_XDP sockets.
>>>>>
>>>>> I ran into this issue when testing the current AF_XDP code - on my test
>>>>> boxes, the mlx5 card has 55 channels (aka queues), so when the test program
>>>>> binds only to channel 0, nothing works as expected, since not all traffic
>>>>> is being intercepted. While obvious in hindsight, this took a while to
>>>>> track down.
>>>>
>>>> Yes, agreed. You are not the first one to stumble upon this problem
>>>> :-). Let me think a little bit on how to solve this in a good way. We
>>>> need this to be simple and intuitive, as you say.
>>>
>>> I see people hitting this with AF_XDP all the time... I had some
>>> backup-slides[2] in our FOSDEM presentation[1] that describe the issue,
>>> give the performance reason why and propose a workaround.
>>
>> Magnus, I presume you're going to address this for the initial libbpf merge
>> since the plan is to make it easier to consume for users?
>
> I think the first thing we need is education and documentation. Have a
> FAQ or "common mistakes" section in the Documentation. And of course,
> sending Jesper around the world reminding people about this ;-).
>
> To address this on a libbpf interface level, I think the best way is
> to reprogram the NIC to send all traffic to the queue that you
> provided in the xsk_socket__create call. This "set up NIC routing"
> behavior can then be disable with a flag, just as the XDP program
> loading can be disabled. The standard config of xsk_socket__create
> will then set up as many things for the user as possible just to get
> up and running quickly. More advanced users can then disable parts of
> it to gain more flexibility. Does this sound OK? Do not want to go the
> route of polling multiple sockets and aggregating the traffic as this
> will have significant negative performance implications.
I think that is fine, I would probably make this one a dedicated API call
in order to have some more flexibility than just simple flag. E.g. once
nfp AF_XDP support lands at some point, I could imagine that this call
resp. a drop-in replacement API call for more advanced steering could
also take an offloaded BPF prog fd, for example, which then would program
the steering on the NIC [0]. Seems at least there's enough complexity on
its own to have a dedicated API for it. Thoughts?
Thanks,
Daniel
[0] https://patchwork.ozlabs.org/cover/910614/
^ permalink raw reply
* Re: [net PATCH] net: vrf: remove MTU limits for vrf device
From: Hangbin Liu @ 2019-02-18 9:51 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, David S . Miller, Davide Caratti
In-Reply-To: <20190218091425.11067-1-liuhangbin@gmail.com>
On Mon, Feb 18, 2019 at 05:14:25PM +0800, Hangbin Liu wrote:
> Similiar to commit e94cd8113ce63 ("net: remove MTU limits for dummy and
Similar...
Thanks Davide help point it out.
Regards
Hangbin
^ permalink raw reply
* [PATCH v2 1/8] net: thunderx: correct typo in macro name
From: Vadim Lomovtsev @ 2019-02-18 9:52 UTC (permalink / raw)
To: sgoutham@cavium.com, rric@kernel.org, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190218095159.3847-1-vlomovtsev@marvell.com>
Correct STREERING to STEERING at macro name for BGX steering register.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 2 +-
drivers/net/ethernet/cavium/thunder/thunder_bgx.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index e337da6ba2a4..673c57b8023f 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -1217,7 +1217,7 @@ static void bgx_init_hw(struct bgx *bgx)
/* Disable MAC steering (NCSI traffic) */
for (i = 0; i < RX_TRAFFIC_STEER_RULE_COUNT; i++)
- bgx_reg_write(bgx, 0, BGX_CMR_RX_STREERING + (i * 8), 0x00);
+ bgx_reg_write(bgx, 0, BGX_CMR_RX_STEERING + (i * 8), 0x00);
}
static u8 bgx_get_lane2sds_cfg(struct bgx *bgx, struct lmac *lmac)
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
index cbdd20b9ee6f..5cbc54e9eb19 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
@@ -60,7 +60,7 @@
#define RX_DMACX_CAM_EN BIT_ULL(48)
#define RX_DMACX_CAM_LMACID(x) (((u64)x) << 49)
#define RX_DMAC_COUNT 32
-#define BGX_CMR_RX_STREERING 0x300
+#define BGX_CMR_RX_STEERING 0x300
#define RX_TRAFFIC_STEER_RULE_COUNT 8
#define BGX_CMR_CHAN_MSK_AND 0x450
#define BGX_CMR_BIST_STATUS 0x460
--
2.17.2
^ permalink raw reply related
* [PATCH v2 7/8] net: thunderx: add LINK_CHANGE message handler at nicpf
From: Vadim Lomovtsev @ 2019-02-18 9:52 UTC (permalink / raw)
To: sgoutham@cavium.com, rric@kernel.org, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190218095159.3847-1-vlomovtsev@marvell.com>
Move the link change polling task to VF side in order to
prevent races between VF and PF while sending link change
message(s). This commit is to implement link change request
to be initiated by VF.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 2 +-
.../net/ethernet/cavium/thunder/nic_main.c | 39 ++++++++++++--
.../net/ethernet/cavium/thunder/nicvf_main.c | 54 +++++++++++++------
3 files changed, 76 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 86cda3f4b37b..62636c1ed141 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -331,7 +331,7 @@ struct nicvf {
struct workqueue_struct *nicvf_rx_mode_wq;
/* mutex to protect VF's mailbox contents from concurrent access */
struct mutex rx_mode_mtx;
-
+ struct delayed_work link_change_work;
/* PTP timestamp */
struct cavium_ptp *ptp_clock;
/* Inbound timestamping is on */
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 620dbe082ca0..8ab71dae3988 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -929,6 +929,35 @@ static void nic_config_timestamp(struct nicpf *nic, int vf, struct set_ptp *ptp)
nic_reg_write(nic, NIC_PF_PKIND_0_15_CFG | (pkind_idx << 3), pkind_val);
}
+static void nic_link_status_get(struct nicpf *nic, u8 vf)
+{
+ union nic_mbx mbx = {};
+ struct bgx_link_status link;
+ u8 bgx, lmac;
+
+ mbx.link_status.msg = NIC_MBOX_MSG_BGX_LINK_CHANGE;
+
+ /* Get BGX, LMAC indices for the VF */
+ bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+ lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+
+ /* Get interface link status */
+ bgx_get_lmac_link_state(nic->node, bgx, lmac, &link);
+
+ nic->link[vf] = link.link_up;
+ nic->duplex[vf] = link.duplex;
+ nic->speed[vf] = link.speed;
+
+ /* Send a mbox message to VF with current link status */
+ mbx.link_status.link_up = link.link_up;
+ mbx.link_status.duplex = link.duplex;
+ mbx.link_status.speed = link.speed;
+ mbx.link_status.mac_type = link.mac_type;
+
+ /* reply with link status */
+ nic_send_msg_to_vf(nic, vf, &mbx);
+}
+
/* Interrupt handler to handle mailbox messages from VFs */
static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
{
@@ -1108,6 +1137,13 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
bgx_set_xcast_mode(nic->node, bgx, lmac, mbx.xcast.mode);
break;
+ case NIC_MBOX_MSG_BGX_LINK_CHANGE:
+ if (vf >= nic->num_vf_en) {
+ ret = -1; /* NACK */
+ break;
+ }
+ nic_link_status_get(nic, vf);
+ goto unlock;
default:
dev_err(&nic->pdev->dev,
"Invalid msg from VF%d, msg 0x%x\n", vf, mbx.msg.msg);
@@ -1419,9 +1455,6 @@ static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_disable_sriov;
}
- INIT_DELAYED_WORK(&nic->dwork, nic_poll_for_link);
- queue_delayed_work(nic->check_link, &nic->dwork, 0);
-
return 0;
err_disable_sriov:
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index a05e2989ec76..2ecacd0e1b51 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -245,21 +245,24 @@ static void nicvf_handle_mbx_intr(struct nicvf *nic)
break;
case NIC_MBOX_MSG_BGX_LINK_CHANGE:
nic->pf_acked = true;
- nic->link_up = mbx.link_status.link_up;
- nic->duplex = mbx.link_status.duplex;
- nic->speed = mbx.link_status.speed;
- nic->mac_type = mbx.link_status.mac_type;
- if (nic->link_up) {
- netdev_info(nic->netdev, "Link is Up %d Mbps %s duplex\n",
- nic->speed,
- nic->duplex == DUPLEX_FULL ?
- "Full" : "Half");
- netif_carrier_on(nic->netdev);
- netif_tx_start_all_queues(nic->netdev);
- } else {
- netdev_info(nic->netdev, "Link is Down\n");
- netif_carrier_off(nic->netdev);
- netif_tx_stop_all_queues(nic->netdev);
+ if (nic->link_up != mbx.link_status.link_up) {
+ nic->link_up = mbx.link_status.link_up;
+ nic->duplex = mbx.link_status.duplex;
+ nic->speed = mbx.link_status.speed;
+ nic->mac_type = mbx.link_status.mac_type;
+ if (nic->link_up) {
+ netdev_info(nic->netdev,
+ "Link is Up %d Mbps %s duplex\n",
+ nic->speed,
+ nic->duplex == DUPLEX_FULL ?
+ "Full" : "Half");
+ netif_carrier_on(nic->netdev);
+ netif_tx_start_all_queues(nic->netdev);
+ } else {
+ netdev_info(nic->netdev, "Link is Down\n");
+ netif_carrier_off(nic->netdev);
+ netif_tx_stop_all_queues(nic->netdev);
+ }
}
break;
case NIC_MBOX_MSG_ALLOC_SQS:
@@ -1328,6 +1331,8 @@ int nicvf_stop(struct net_device *netdev)
struct nicvf_cq_poll *cq_poll = NULL;
union nic_mbx mbx = {};
+ cancel_delayed_work_sync(&nic->link_change_work);
+
/* wait till all queued set_rx_mode tasks completes */
drain_workqueue(nic->nicvf_rx_mode_wq);
@@ -1430,6 +1435,20 @@ static int nicvf_update_hw_max_frs(struct nicvf *nic, int mtu)
return nicvf_send_msg_to_pf(nic, &mbx);
}
+static void nicvf_link_status_check_task(struct work_struct *work_arg)
+{
+ struct nicvf *nic = container_of(work_arg,
+ struct nicvf,
+ link_change_work.work);
+ struct net_device *netdev = nic->netdev;
+ union nic_mbx mbx = {};
+
+ mbx.msg.msg = NIC_MBOX_MSG_BGX_LINK_CHANGE;
+ nicvf_send_msg_to_pf(nic, &mbx);
+ queue_delayed_work(nic->nicvf_rx_mode_wq,
+ &nic->link_change_work, 2 * HZ);
+}
+
int nicvf_open(struct net_device *netdev)
{
int cpu, err, qidx;
@@ -1536,6 +1555,11 @@ int nicvf_open(struct net_device *netdev)
/* Send VF config done msg to PF */
nicvf_send_cfg_done(nic);
+ INIT_DELAYED_WORK(&nic->link_change_work,
+ nicvf_link_status_check_task);
+ queue_delayed_work(nic->nicvf_rx_mode_wq,
+ &nic->link_change_work, 0);
+
return 0;
cleanup:
nicvf_disable_intr(nic, NICVF_INTR_MBOX, 0);
--
2.17.2
^ permalink raw reply related
* [PATCH v2 3/8] net: thunderx: make CFG_DONE message to run through generic send-ack sequence
From: Vadim Lomovtsev @ 2019-02-18 9:52 UTC (permalink / raw)
To: sgoutham@cavium.com, rric@kernel.org, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190218095159.3847-1-vlomovtsev@marvell.com>
At the end of NIC VF initialization VF sends CFG_DONE message to PF without
using nicvf_msg_send_to_pf routine. This potentially could re-write data in
mailbox. This commit is to implement common way of sending CFG_DONE message
by the same way with other configuration messages by using
nicvf_send_msg_to_pf() routine.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nic_main.c | 2 +-
.../net/ethernet/cavium/thunder/nicvf_main.c | 18 +++++++++++++++---
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 6c8dcb65ff03..90497a27df18 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -1039,7 +1039,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
case NIC_MBOX_MSG_CFG_DONE:
/* Last message of VF config msg sequence */
nic_enable_vf(nic, vf, true);
- goto unlock;
+ break;
case NIC_MBOX_MSG_SHUTDOWN:
/* First msg in VF teardown sequence */
if (vf >= nic->num_vf_en)
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index abf24e7dff2d..b0e8a04e0f1e 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -169,6 +169,20 @@ static int nicvf_check_pf_ready(struct nicvf *nic)
return 1;
}
+static int nicvf_send_cfg_done(struct nicvf *nic)
+{
+ union nic_mbx mbx = {};
+
+ mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE;
+ if (nicvf_send_msg_to_pf(nic, &mbx)) {
+ netdev_err(nic->netdev,
+ "PF didn't respond to CFG DONE msg\n");
+ return 0;
+ }
+
+ return 1;
+}
+
static void nicvf_read_bgx_stats(struct nicvf *nic, struct bgx_stats_msg *bgx)
{
if (bgx->rx)
@@ -1416,7 +1430,6 @@ int nicvf_open(struct net_device *netdev)
struct nicvf *nic = netdev_priv(netdev);
struct queue_set *qs = nic->qs;
struct nicvf_cq_poll *cq_poll = NULL;
- union nic_mbx mbx = {};
/* wait till all queued set_rx_mode tasks completes if any */
drain_workqueue(nic->nicvf_rx_mode_wq);
@@ -1515,8 +1528,7 @@ int nicvf_open(struct net_device *netdev)
nicvf_enable_intr(nic, NICVF_INTR_RBDR, qidx);
/* Send VF config done msg to PF */
- mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE;
- nicvf_write_to_mbx(nic, &mbx);
+ nicvf_send_cfg_done(nic);
return 0;
cleanup:
--
2.17.2
^ permalink raw reply related
* [PATCH v2 5/8] net: thunderx: rework xcast message structure to make it fit into 64 bit
From: Vadim Lomovtsev @ 2019-02-18 9:52 UTC (permalink / raw)
To: sgoutham@cavium.com, rric@kernel.org, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190218095159.3847-1-vlomovtsev@marvell.com>
To communicate to PF each of ThunderX NIC VF uses mailbox which is
pair of 64 bit registers available to both VFn and PF.
This commit is to change the xcast message structure in order to
fit it into 64 bit.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 6 ++----
drivers/net/ethernet/cavium/thunder/nic_main.c | 4 ++--
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 6 +++---
3 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 376a96bce33f..227343625e83 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -577,10 +577,8 @@ struct set_ptp {
struct xcast {
u8 msg;
- union {
- u8 mode;
- u64 mac;
- } data;
+ u8 mode;
+ u64 mac:48;
};
/* 128 bit shared memory between PF and each VF */
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 90497a27df18..620dbe082ca0 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -1094,7 +1094,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
bgx_set_dmac_cam_filter(nic->node, bgx, lmac,
- mbx.xcast.data.mac,
+ mbx.xcast.mac,
vf < NIC_VF_PER_MBX_REG ? vf :
vf - NIC_VF_PER_MBX_REG);
break;
@@ -1106,7 +1106,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
}
bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
- bgx_set_xcast_mode(nic->node, bgx, lmac, mbx.xcast.data.mode);
+ bgx_set_xcast_mode(nic->node, bgx, lmac, mbx.xcast.mode);
break;
default:
dev_err(&nic->pdev->dev,
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index dbd8862d60d6..30c7f54b4f17 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1964,7 +1964,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
* its' own LMAC to the filter to accept packets for it.
*/
mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
- mbx.xcast.data.mac = 0;
+ mbx.xcast.mac = 0;
if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
goto free_mc;
}
@@ -1974,7 +1974,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
/* now go through kernel list of MACs and add them one by one */
for (idx = 0; idx < mc_addrs->count; idx++) {
mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
- mbx.xcast.data.mac = mc_addrs->mc[idx];
+ mbx.xcast.mac = mc_addrs->mc[idx];
if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
goto free_mc;
}
@@ -1982,7 +1982,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
/* and finally set rx mode for PF accordingly */
mbx.xcast.msg = NIC_MBOX_MSG_SET_XCAST;
- mbx.xcast.data.mode = mode;
+ mbx.xcast.mode = mode;
nicvf_send_msg_to_pf(nic, &mbx);
free_mc:
--
2.17.2
^ permalink raw reply related
* [PATCH v2 4/8] net: thunderx: add nicvf_send_msg_to_pf result check for set_rx_mode_task
From: Vadim Lomovtsev @ 2019-02-18 9:52 UTC (permalink / raw)
To: sgoutham@cavium.com, rric@kernel.org, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190218095159.3847-1-vlomovtsev@marvell.com>
The rx_set_mode invokes number of messages to be send to PF for receive
mode configuration. In case if there any issues we need to stop sending
messages and release allocated memory.
This commit is to implement check of nicvf_msg_send_to_pf() result.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index b0e8a04e0f1e..dbd8862d60d6 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1956,7 +1956,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
/* flush DMAC filters and reset RX mode */
mbx.xcast.msg = NIC_MBOX_MSG_RESET_XCAST;
- nicvf_send_msg_to_pf(nic, &mbx);
+ if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
+ goto free_mc;
if (mode & BGX_XCAST_MCAST_FILTER) {
/* once enabling filtering, we need to signal to PF to add
@@ -1964,7 +1965,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
*/
mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
mbx.xcast.data.mac = 0;
- nicvf_send_msg_to_pf(nic, &mbx);
+ if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
+ goto free_mc;
}
/* check if we have any specific MACs to be added to PF DMAC filter */
@@ -1973,9 +1975,9 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
for (idx = 0; idx < mc_addrs->count; idx++) {
mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
mbx.xcast.data.mac = mc_addrs->mc[idx];
- nicvf_send_msg_to_pf(nic, &mbx);
+ if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
+ goto free_mc;
}
- kfree(mc_addrs);
}
/* and finally set rx mode for PF accordingly */
@@ -1983,6 +1985,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
mbx.xcast.data.mode = mode;
nicvf_send_msg_to_pf(nic, &mbx);
+free_mc:
+ kfree(mc_addrs);
}
static void nicvf_set_rx_mode_task(struct work_struct *work_arg)
--
2.17.2
^ permalink raw reply related
* [PATCH v2 8/8] net: thunderx: remove link change polling code and info from nicpf
From: Vadim Lomovtsev @ 2019-02-18 9:52 UTC (permalink / raw)
To: sgoutham@cavium.com, rric@kernel.org, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190218095159.3847-1-vlomovtsev@marvell.com>
Since link change polling routine was moved to nicvf side,
we don't need anymore polling function at nicpf side along
with link status info for all enabled Vfs as at VF side
this info is already tracked.
This commit is to remove unnecessary code & fields from
nicpf structure.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
.../net/ethernet/cavium/thunder/nic_main.c | 114 ++----------------
1 file changed, 12 insertions(+), 102 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 8ab71dae3988..c90252829ed3 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -57,14 +57,8 @@ struct nicpf {
#define NIC_GET_BGX_FROM_VF_LMAC_MAP(map) ((map >> 4) & 0xF)
#define NIC_GET_LMAC_FROM_VF_LMAC_MAP(map) (map & 0xF)
u8 *vf_lmac_map;
- struct delayed_work dwork;
- struct workqueue_struct *check_link;
- u8 *link;
- u8 *duplex;
- u32 *speed;
u16 cpi_base[MAX_NUM_VFS_SUPPORTED];
u16 rssi_base[MAX_NUM_VFS_SUPPORTED];
- bool mbx_lock[MAX_NUM_VFS_SUPPORTED];
/* MSI-X */
u8 num_vec;
@@ -929,6 +923,10 @@ static void nic_config_timestamp(struct nicpf *nic, int vf, struct set_ptp *ptp)
nic_reg_write(nic, NIC_PF_PKIND_0_15_CFG | (pkind_idx << 3), pkind_val);
}
+/* Get BGX LMAC link status and update corresponding VF
+ * if there is a change, valid only if internal L2 switch
+ * is not present otherwise VF link is always treated as up
+ */
static void nic_link_status_get(struct nicpf *nic, u8 vf)
{
union nic_mbx mbx = {};
@@ -944,10 +942,6 @@ static void nic_link_status_get(struct nicpf *nic, u8 vf)
/* Get interface link status */
bgx_get_lmac_link_state(nic->node, bgx, lmac, &link);
- nic->link[vf] = link.link_up;
- nic->duplex[vf] = link.duplex;
- nic->speed[vf] = link.speed;
-
/* Send a mbox message to VF with current link status */
mbx.link_status.link_up = link.link_up;
mbx.link_status.duplex = link.duplex;
@@ -970,8 +964,6 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
int i;
int ret = 0;
- nic->mbx_lock[vf] = true;
-
mbx_addr = nic_get_mbx_addr(vf);
mbx_data = (u64 *)&mbx;
@@ -986,12 +978,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
switch (mbx.msg.msg) {
case NIC_MBOX_MSG_READY:
nic_mbx_send_ready(nic, vf);
- if (vf < nic->num_vf_en) {
- nic->link[vf] = 0;
- nic->duplex[vf] = 0;
- nic->speed[vf] = 0;
- }
- goto unlock;
+ return;
case NIC_MBOX_MSG_QS_CFG:
reg_addr = NIC_PF_QSET_0_127_CFG |
(mbx.qs.num << NIC_QS_ID_SHIFT);
@@ -1060,7 +1047,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
break;
case NIC_MBOX_MSG_RSS_SIZE:
nic_send_rss_size(nic, vf);
- goto unlock;
+ return;
case NIC_MBOX_MSG_RSS_CFG:
case NIC_MBOX_MSG_RSS_CFG_CONT:
nic_config_rss(nic, &mbx.rss_cfg);
@@ -1078,19 +1065,19 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
break;
case NIC_MBOX_MSG_ALLOC_SQS:
nic_alloc_sqs(nic, &mbx.sqs_alloc);
- goto unlock;
+ return;
case NIC_MBOX_MSG_NICVF_PTR:
nic->nicvf[vf] = mbx.nicvf.nicvf;
break;
case NIC_MBOX_MSG_PNICVF_PTR:
nic_send_pnicvf(nic, vf);
- goto unlock;
+ return;
case NIC_MBOX_MSG_SNICVF_PTR:
nic_send_snicvf(nic, &mbx.nicvf);
- goto unlock;
+ return;
case NIC_MBOX_MSG_BGX_STATS:
nic_get_bgx_stats(nic, &mbx.bgx_stats);
- goto unlock;
+ return;
case NIC_MBOX_MSG_LOOPBACK:
ret = nic_config_loopback(nic, &mbx.lbk);
break;
@@ -1099,7 +1086,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
break;
case NIC_MBOX_MSG_PFC:
nic_pause_frame(nic, vf, &mbx.pfc);
- goto unlock;
+ return;
case NIC_MBOX_MSG_PTP_CFG:
nic_config_timestamp(nic, vf, &mbx.ptp);
break;
@@ -1143,7 +1130,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
break;
}
nic_link_status_get(nic, vf);
- goto unlock;
+ return;
default:
dev_err(&nic->pdev->dev,
"Invalid msg from VF%d, msg 0x%x\n", vf, mbx.msg.msg);
@@ -1157,8 +1144,6 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
mbx.msg.msg, vf);
nic_mbx_send_nack(nic, vf);
}
-unlock:
- nic->mbx_lock[vf] = false;
}
static irqreturn_t nic_mbx_intr_handler(int irq, void *nic_irq)
@@ -1306,52 +1291,6 @@ static int nic_sriov_init(struct pci_dev *pdev, struct nicpf *nic)
return 0;
}
-/* Poll for BGX LMAC link status and update corresponding VF
- * if there is a change, valid only if internal L2 switch
- * is not present otherwise VF link is always treated as up
- */
-static void nic_poll_for_link(struct work_struct *work)
-{
- union nic_mbx mbx = {};
- struct nicpf *nic;
- struct bgx_link_status link;
- u8 vf, bgx, lmac;
-
- nic = container_of(work, struct nicpf, dwork.work);
-
- mbx.link_status.msg = NIC_MBOX_MSG_BGX_LINK_CHANGE;
-
- for (vf = 0; vf < nic->num_vf_en; vf++) {
- /* Poll only if VF is UP */
- if (!nic->vf_enabled[vf])
- continue;
-
- /* Get BGX, LMAC indices for the VF */
- bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
- lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
- /* Get interface link status */
- bgx_get_lmac_link_state(nic->node, bgx, lmac, &link);
-
- /* Inform VF only if link status changed */
- if (nic->link[vf] == link.link_up)
- continue;
-
- if (!nic->mbx_lock[vf]) {
- nic->link[vf] = link.link_up;
- nic->duplex[vf] = link.duplex;
- nic->speed[vf] = link.speed;
-
- /* Send a mbox message to VF with current link status */
- mbx.link_status.link_up = link.link_up;
- mbx.link_status.duplex = link.duplex;
- mbx.link_status.speed = link.speed;
- mbx.link_status.mac_type = link.mac_type;
- nic_send_msg_to_vf(nic, vf, &mbx);
- }
- }
- queue_delayed_work(nic->check_link, &nic->dwork, HZ * 2);
-}
-
static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
struct device *dev = &pdev->dev;
@@ -1420,18 +1359,6 @@ static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (!nic->vf_lmac_map)
goto err_release_regions;
- nic->link = devm_kmalloc_array(dev, max_lmac, sizeof(u8), GFP_KERNEL);
- if (!nic->link)
- goto err_release_regions;
-
- nic->duplex = devm_kmalloc_array(dev, max_lmac, sizeof(u8), GFP_KERNEL);
- if (!nic->duplex)
- goto err_release_regions;
-
- nic->speed = devm_kmalloc_array(dev, max_lmac, sizeof(u32), GFP_KERNEL);
- if (!nic->speed)
- goto err_release_regions;
-
/* Initialize hardware */
nic_init_hw(nic);
@@ -1447,19 +1374,8 @@ static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (err)
goto err_unregister_interrupts;
- /* Register a physical link status poll fn() */
- nic->check_link = alloc_workqueue("check_link_status",
- WQ_UNBOUND | WQ_MEM_RECLAIM, 1);
- if (!nic->check_link) {
- err = -ENOMEM;
- goto err_disable_sriov;
- }
-
return 0;
-err_disable_sriov:
- if (nic->flags & NIC_SRIOV_ENABLED)
- pci_disable_sriov(pdev);
err_unregister_interrupts:
nic_unregister_interrupts(nic);
err_release_regions:
@@ -1480,12 +1396,6 @@ static void nic_remove(struct pci_dev *pdev)
if (nic->flags & NIC_SRIOV_ENABLED)
pci_disable_sriov(pdev);
- if (nic->check_link) {
- /* Destroy work Queue */
- cancel_delayed_work_sync(&nic->dwork);
- destroy_workqueue(nic->check_link);
- }
-
nic_unregister_interrupts(nic);
pci_release_regions(pdev);
--
2.17.2
^ permalink raw reply related
* [PATCH v2 2/8] net: thunderx: replace global nicvf_rx_mode_wq work queue for all VFs to private for each of them.
From: Vadim Lomovtsev @ 2019-02-18 9:52 UTC (permalink / raw)
To: sgoutham@cavium.com, rric@kernel.org, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190218095159.3847-1-vlomovtsev@marvell.com>
Having one work queue for receive mode configuration ndo_set_rx_mode()
call for all VFs results in making each of them wait till the
set_rx_mode() call completes for another VF if any of close, set
receive mode and change flags calls being already invoked. Potentially
this could cause device state change before appropriate call of receive
mode configuration completes, so the call itself became meaningless,
corrupt data or break configuration sequence.
We don't need any delays in NIC VF configuration sequence so having delayed
work call with 0 delay has no sense.
This commit is to implement one work queue for each NIC VF for set_rx_mode
task and to let them work independently and replacing delayed_work
with work_struct.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 4 ++-
.../net/ethernet/cavium/thunder/nicvf_main.c | 30 ++++++++++---------
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index f4d81765221e..376a96bce33f 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -271,7 +271,7 @@ struct xcast_addr_list {
};
struct nicvf_work {
- struct delayed_work work;
+ struct work_struct work;
u8 mode;
struct xcast_addr_list *mc;
};
@@ -327,6 +327,8 @@ struct nicvf {
struct nicvf_work rx_mode_work;
/* spinlock to protect workqueue arguments from concurrent access */
spinlock_t rx_mode_wq_lock;
+ /* workqueue for handling kernel ndo_set_rx_mode() calls */
+ struct workqueue_struct *nicvf_rx_mode_wq;
/* PTP timestamp */
struct cavium_ptp *ptp_clock;
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 88f8a8fa93cd..abf24e7dff2d 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -68,9 +68,6 @@ module_param(cpi_alg, int, 0444);
MODULE_PARM_DESC(cpi_alg,
"PFC algorithm (0=none, 1=VLAN, 2=VLAN16, 3=IP Diffserv)");
-/* workqueue for handling kernel ndo_set_rx_mode() calls */
-static struct workqueue_struct *nicvf_rx_mode_wq;
-
static inline u8 nicvf_netdev_qidx(struct nicvf *nic, u8 qidx)
{
if (nic->sqs_mode)
@@ -1311,6 +1308,9 @@ int nicvf_stop(struct net_device *netdev)
struct nicvf_cq_poll *cq_poll = NULL;
union nic_mbx mbx = {};
+ /* wait till all queued set_rx_mode tasks completes */
+ drain_workqueue(nic->nicvf_rx_mode_wq);
+
mbx.msg.msg = NIC_MBOX_MSG_SHUTDOWN;
nicvf_send_msg_to_pf(nic, &mbx);
@@ -1418,6 +1418,9 @@ int nicvf_open(struct net_device *netdev)
struct nicvf_cq_poll *cq_poll = NULL;
union nic_mbx mbx = {};
+ /* wait till all queued set_rx_mode tasks completes if any */
+ drain_workqueue(nic->nicvf_rx_mode_wq);
+
netif_carrier_off(netdev);
err = nicvf_register_misc_interrupt(nic);
@@ -1973,7 +1976,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
static void nicvf_set_rx_mode_task(struct work_struct *work_arg)
{
struct nicvf_work *vf_work = container_of(work_arg, struct nicvf_work,
- work.work);
+ work);
struct nicvf *nic = container_of(vf_work, struct nicvf, rx_mode_work);
u8 mode;
struct xcast_addr_list *mc;
@@ -2030,7 +2033,7 @@ static void nicvf_set_rx_mode(struct net_device *netdev)
kfree(nic->rx_mode_work.mc);
nic->rx_mode_work.mc = mc_list;
nic->rx_mode_work.mode = mode;
- queue_delayed_work(nicvf_rx_mode_wq, &nic->rx_mode_work.work, 0);
+ queue_work(nic->nicvf_rx_mode_wq, &nic->rx_mode_work.work);
spin_unlock(&nic->rx_mode_wq_lock);
}
@@ -2187,7 +2190,10 @@ static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
INIT_WORK(&nic->reset_task, nicvf_reset_task);
- INIT_DELAYED_WORK(&nic->rx_mode_work.work, nicvf_set_rx_mode_task);
+ nic->nicvf_rx_mode_wq = alloc_ordered_workqueue("nicvf_rx_mode_wq_VF%d",
+ WQ_MEM_RECLAIM,
+ nic->vf_id);
+ INIT_WORK(&nic->rx_mode_work.work, nicvf_set_rx_mode_task);
spin_lock_init(&nic->rx_mode_wq_lock);
err = register_netdev(netdev);
@@ -2228,13 +2234,15 @@ static void nicvf_remove(struct pci_dev *pdev)
nic = netdev_priv(netdev);
pnetdev = nic->pnicvf->netdev;
- cancel_delayed_work_sync(&nic->rx_mode_work.work);
-
/* Check if this Qset is assigned to different VF.
* If yes, clean primary and all secondary Qsets.
*/
if (pnetdev && (pnetdev->reg_state == NETREG_REGISTERED))
unregister_netdev(pnetdev);
+ if (nic->nicvf_rx_mode_wq) {
+ destroy_workqueue(nic->nicvf_rx_mode_wq);
+ nic->nicvf_rx_mode_wq = NULL;
+ }
nicvf_unregister_interrupts(nic);
pci_set_drvdata(pdev, NULL);
if (nic->drv_stats)
@@ -2261,17 +2269,11 @@ static struct pci_driver nicvf_driver = {
static int __init nicvf_init_module(void)
{
pr_info("%s, ver %s\n", DRV_NAME, DRV_VERSION);
- nicvf_rx_mode_wq = alloc_ordered_workqueue("nicvf_generic",
- WQ_MEM_RECLAIM);
return pci_register_driver(&nicvf_driver);
}
static void __exit nicvf_cleanup_module(void)
{
- if (nicvf_rx_mode_wq) {
- destroy_workqueue(nicvf_rx_mode_wq);
- nicvf_rx_mode_wq = NULL;
- }
pci_unregister_driver(&nicvf_driver);
}
--
2.17.2
^ permalink raw reply related
* [PATCH v2 0/8] nic: thunderx: fix communication races between VF & PF
From: Vadim Lomovtsev @ 2019-02-18 9:52 UTC (permalink / raw)
To: sgoutham@cavium.com, rric@kernel.org, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: dnelson@redhat.com, Vadim Lomovtsev
From: Vadim Lomovtsev <vadim.lomovtsev@auriga.com>
The ThunderX CN88XX NIC Virtual Function driver uses mailbox interface
to communicate to physical function driver. Each of VF has it's own pair
of mailbox registers to read from and write to. The mailbox registers
has no protection from possible races, so it has to be implemented
at software side.
After long term testing by loop of 'ip link set <ifname> up/down'
command it was found that there are two possible scenarios when
race condition appears:
1. VF receives link change message from PF and VF send RX mode
configuration message to PF in the same time from separate thread.
2. PF receives RX mode configuration from VF and in the same time,
in separate thread PF detects link status change and sends appropriate
message to particular VF.
Both cases leads to mailbox data to be rewritten, NIC VF messaging control
data to be updated incorrectly and communication sequence gets broken.
This patch series is to address race condition with VF & PF communication.
Vadim Lomovtsev (8):
net: thunderx: correct typo in macro name
net: thunderx: replace global nicvf_rx_mode_wq work queue for all VFs
to private for each of them.
net: thunderx: make CFG_DONE message to run through generic send-ack
sequence
net: thunderx: add nicvf_send_msg_to_pf result check for
set_rx_mode_task
net: thunderx: rework xcast message structure to make it fit into 64
bit
net: thunderx: add mutex to protect mailbox from concurrent calls for
same VF
net: thunderx: add LINK_CHANGE message handler at nicpf
net: thunderx: remove link change polling code and info from nicpf
drivers/net/ethernet/cavium/thunder/nic.h | 14 +-
.../net/ethernet/cavium/thunder/nic_main.c | 149 ++++++------------
.../net/ethernet/cavium/thunder/nicvf_main.c | 133 +++++++++++-----
.../net/ethernet/cavium/thunder/thunder_bgx.c | 2 +-
.../net/ethernet/cavium/thunder/thunder_bgx.h | 2 +-
5 files changed, 147 insertions(+), 153 deletions(-)
--
2.17.2
^ permalink raw reply
* [PATCH v2 6/8] net: thunderx: add mutex to protect mailbox from concurrent calls for same VF
From: Vadim Lomovtsev @ 2019-02-18 9:52 UTC (permalink / raw)
To: sgoutham@cavium.com, rric@kernel.org, davem@davemloft.net,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190218095159.3847-1-vlomovtsev@marvell.com>
In some cases it could happen that nicvf_send_msg_to_pf() could be called
concurrently for the same NIC VF, and thus re-writing mailbox contents and
breaking messaging sequence with PF by re-writing NICVF data.
This commit is to implement mutex for NICVF to protect mailbox registers
and NICVF messaging control data from concurrent access.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 2 ++
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 13 ++++++++++---
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 227343625e83..86cda3f4b37b 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -329,6 +329,8 @@ struct nicvf {
spinlock_t rx_mode_wq_lock;
/* workqueue for handling kernel ndo_set_rx_mode() calls */
struct workqueue_struct *nicvf_rx_mode_wq;
+ /* mutex to protect VF's mailbox contents from concurrent access */
+ struct mutex rx_mode_mtx;
/* PTP timestamp */
struct cavium_ptp *ptp_clock;
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 30c7f54b4f17..a05e2989ec76 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -124,6 +124,9 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx)
{
int timeout = NIC_MBOX_MSG_TIMEOUT;
int sleep = 10;
+ int ret = 0;
+
+ mutex_lock(&nic->rx_mode_mtx);
nic->pf_acked = false;
nic->pf_nacked = false;
@@ -136,7 +139,8 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx)
netdev_err(nic->netdev,
"PF NACK to mbox msg 0x%02x from VF%d\n",
(mbx->msg.msg & 0xFF), nic->vf_id);
- return -EINVAL;
+ ret = -EINVAL;
+ break;
}
msleep(sleep);
if (nic->pf_acked)
@@ -146,10 +150,12 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx)
netdev_err(nic->netdev,
"PF didn't ACK to mbox msg 0x%02x from VF%d\n",
(mbx->msg.msg & 0xFF), nic->vf_id);
- return -EBUSY;
+ ret = -EBUSY;
+ break;
}
}
- return 0;
+ mutex_unlock(&nic->rx_mode_mtx);
+ return ret;
}
/* Checks if VF is able to comminicate with PF
@@ -2211,6 +2217,7 @@ static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
nic->vf_id);
INIT_WORK(&nic->rx_mode_work.work, nicvf_set_rx_mode_task);
spin_lock_init(&nic->rx_mode_wq_lock);
+ mutex_init(&nic->rx_mode_mtx);
err = register_netdev(netdev);
if (err) {
--
2.17.2
^ permalink raw reply related
* [PATCH ipsec] xfrm: Fix inbound traffic via XFRM interfaces across network namespaces
From: Tobias Brunner @ 2019-02-18 9:49 UTC (permalink / raw)
To: Steffen Klassert; +Cc: netdev
After moving an XFRM interface to another namespace it stays associated
with the original namespace (net in `struct xfrm_if` and the list keyed
with `xfrmi_net_id`), allowing processes in the new namespace to use
SAs/policies that were created in the original namespace. For instance,
this allows a keying daemon in one namespace to establish IPsec SAs for
other namespaces without processes there having access to the keys or IKE
credentials.
This worked fine for outbound traffic, however, for inbound traffic the
lookup for the interfaces and the policies used the incorrect namespace
(the one the XFRM interface was moved to).
Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
Signed-off-by: Tobias Brunner <tobias@strongswan.org>
---
net/xfrm/xfrm_interface.c | 4 ++--
net/xfrm/xfrm_policy.c | 4 +++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index 6be8c7df15bb..dbb3c1945b5c 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -76,10 +76,10 @@ static struct xfrm_if *xfrmi_decode_session(struct sk_buff *skb)
int ifindex;
struct xfrm_if *xi;
- if (!skb->dev)
+ if (!secpath_exists(skb) || !skb->dev)
return NULL;
- xfrmn = net_generic(dev_net(skb->dev), xfrmi_net_id);
+ xfrmn = net_generic(xs_net(xfrm_input_state(skb)), xfrmi_net_id);
ifindex = skb->dev->ifindex;
for_each_xfrmi_rcu(xfrmn->xfrmi[0], xi) {
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index ba0a4048c846..8d1a898d0ba5 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -3314,8 +3314,10 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
if (ifcb) {
xi = ifcb->decode_session(skb);
- if (xi)
+ if (xi) {
if_id = xi->p.if_id;
+ net = xi->net;
+ }
}
rcu_read_unlock();
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next v4 05/17] net: sched: traverse chains in block with tcf_get_next_chain()
From: Vlad Buslov @ 2019-02-18 10:07 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <CAM_iQpUnmKPh+RCi-JyouKz7PrAiWPp30Ro2qZFrR=MjVHogHA@mail.gmail.com>
Hi Cong,
Thanks for reviewing!
On Fri 15 Feb 2019 at 22:21, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> (Sorry for joining this late.)
>
> On Mon, Feb 11, 2019 at 12:56 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>> @@ -2432,7 +2474,11 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
>> index_start = cb->args[0];
>> index = 0;
>>
>> - list_for_each_entry(chain, &block->chain_list, list) {
>> + for (chain = __tcf_get_next_chain(block, NULL);
>> + chain;
>> + chain_prev = chain,
>> + chain = __tcf_get_next_chain(block, chain),
>> + tcf_chain_put(chain_prev)) {
>
> Why do you want to take the block->lock in each iteration
> of the loop rather than taking once for the whole loop?
This loop calls classifier ops callback in tc_chain_fill_node(). I don't
call any classifier ops callbacks while holding block or chain lock in
this change because the goal is to achieve fine-grained locking for data
structures used by filter update path. Locking per-block or per-chain is
much coarser than taking reference counters to parent structures and
allowing classifiers to implement their own locking.
In this case call to ops->tmplt_dump() is probably quite fast and its
execution time doesn't depend on number of filters on the classifier, so
releasing block->lock on each iteration doesn't provide much benefit, if
at all. However, it is easier for me to reason about locking correctness
in this refactoring by following a simple rule that no locks (besides
rtnl mutex) can be held when calling classifier ops callbacks.
^ permalink raw reply
* Re: [PATCH bpf-next v4 0/2] libbpf: adding AF_XDP support
From: Magnus Karlsson @ 2019-02-18 10:09 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Jesper Dangaard Brouer, Jonathan Lemon, Magnus Karlsson,
Björn Töpel, ast, Network Development, Jakub Kicinski,
Björn Töpel, Zhang, Qi Z, xiaolong.ye,
xdp-newbies@vger.kernel.org
In-Reply-To: <5ed22245-fe6b-14a9-9c93-f039828a02b6@iogearbox.net>
On Mon, Feb 18, 2019 at 10:38 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 02/18/2019 09:20 AM, Magnus Karlsson wrote:
> > On Fri, Feb 15, 2019 at 5:48 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >>
> >> On 02/13/2019 12:55 PM, Jesper Dangaard Brouer wrote:
> >>> On Wed, 13 Feb 2019 12:32:47 +0100
> >>> Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> >>>> On Mon, Feb 11, 2019 at 9:44 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
> >>>>> On 8 Feb 2019, at 5:05, Magnus Karlsson wrote:
> >>>>>
> >>>>>> This patch proposes to add AF_XDP support to libbpf. The main reason
> >>>>>> for this is to facilitate writing applications that use AF_XDP by
> >>>>>> offering higher-level APIs that hide many of the details of the AF_XDP
> >>>>>> uapi. This is in the same vein as libbpf facilitates XDP adoption by
> >>>>>> offering easy-to-use higher level interfaces of XDP
> >>>>>> functionality. Hopefully this will facilitate adoption of AF_XDP, make
> >>>>>> applications using it simpler and smaller, and finally also make it
> >>>>>> possible for applications to benefit from optimizations in the AF_XDP
> >>>>>> user space access code. Previously, people just copied and pasted the
> >>>>>> code from the sample application into their application, which is not
> >>>>>> desirable.
> >>>>>
> >>>>> I like the idea of encapsulating the boilerplate logic in a library.
> >>>>>
> >>>>> I do think there is an important missing piece though - there should be
> >>>>> some code which queries the netdev for how many queues are attached, and
> >>>>> create the appropriate number of umem/AF_XDP sockets.
> >>>>>
> >>>>> I ran into this issue when testing the current AF_XDP code - on my test
> >>>>> boxes, the mlx5 card has 55 channels (aka queues), so when the test program
> >>>>> binds only to channel 0, nothing works as expected, since not all traffic
> >>>>> is being intercepted. While obvious in hindsight, this took a while to
> >>>>> track down.
> >>>>
> >>>> Yes, agreed. You are not the first one to stumble upon this problem
> >>>> :-). Let me think a little bit on how to solve this in a good way. We
> >>>> need this to be simple and intuitive, as you say.
> >>>
> >>> I see people hitting this with AF_XDP all the time... I had some
> >>> backup-slides[2] in our FOSDEM presentation[1] that describe the issue,
> >>> give the performance reason why and propose a workaround.
> >>
> >> Magnus, I presume you're going to address this for the initial libbpf merge
> >> since the plan is to make it easier to consume for users?
> >
> > I think the first thing we need is education and documentation. Have a
> > FAQ or "common mistakes" section in the Documentation. And of course,
> > sending Jesper around the world reminding people about this ;-).
> >
> > To address this on a libbpf interface level, I think the best way is
> > to reprogram the NIC to send all traffic to the queue that you
> > provided in the xsk_socket__create call. This "set up NIC routing"
> > behavior can then be disable with a flag, just as the XDP program
> > loading can be disabled. The standard config of xsk_socket__create
> > will then set up as many things for the user as possible just to get
> > up and running quickly. More advanced users can then disable parts of
> > it to gain more flexibility. Does this sound OK? Do not want to go the
> > route of polling multiple sockets and aggregating the traffic as this
> > will have significant negative performance implications.
>
> I think that is fine, I would probably make this one a dedicated API call
> in order to have some more flexibility than just simple flag. E.g. once
> nfp AF_XDP support lands at some point, I could imagine that this call
> resp. a drop-in replacement API call for more advanced steering could
> also take an offloaded BPF prog fd, for example, which then would program
> the steering on the NIC [0]. Seems at least there's enough complexity on
> its own to have a dedicated API for it. Thoughts?
I agree that there is probably enough complexity to warrant adding a
higher level API to deal with this problem (flow steering). But there
are likely a number of cases we have not thought that would complicate
it even further. This is why I suggest that this functionality should
be in its own patch set that I can devote some time and thought to.
IMO, the current patch set and functionality does already lower the
bar of entry significantly and has a value even without hiding or
controlling the steering of traffic. What I would like to do in this
patch set is to add a FAQ section in
Documentation/networking/af_xdp.rst explaining this problem. Something
like: "Q: Why am I not seeing any traffic? A: Check these four
things.....". Could add some text in the libbpf README referring to
this document also. Opinions?
Thanks: Magnus
> Thanks,
> Daniel
>
> [0] https://patchwork.ozlabs.org/cover/910614/
^ permalink raw reply
* [PATCH v2 1/2] net: phy: at803x: dont inline helpers
From: Vinod Koul @ 2019-02-18 10:18 UTC (permalink / raw)
To: David S Miller
Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, netdev, Niklas Cassel,
Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
Marc Gonzalez
Some helpers were inlined, but makes more sense to allow compiler
to do the right optiomazations instead, so remove inline for
at803x_disable_rx_delay() and at803x_disable_tx_delay()
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
drivers/net/phy/at803x.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 8ff12938ab47..c6e7d800fd7a 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -110,13 +110,13 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
return phy_write(phydev, AT803X_DEBUG_DATA, val);
}
-static inline int at803x_disable_rx_delay(struct phy_device *phydev)
+static int at803x_disable_rx_delay(struct phy_device *phydev)
{
return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
AT803X_DEBUG_RX_CLK_DLY_EN, 0);
}
-static inline int at803x_disable_tx_delay(struct phy_device *phydev)
+static int at803x_disable_tx_delay(struct phy_device *phydev)
{
return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5,
AT803X_DEBUG_TX_CLK_DLY_EN, 0);
--
2.20.1
^ permalink raw reply related
* [PATCH v2 2/2] net: phy: at803x: disable delay only for RGMII mode
From: Vinod Koul @ 2019-02-18 10:18 UTC (permalink / raw)
To: David S Miller
Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, netdev, Niklas Cassel,
Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
Marc Gonzalez
In-Reply-To: <20190218101853.4290-1-vkoul@kernel.org>
Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode
should not have delay in PHY wheras RGMII_ID and RGMII_RXID/RGMII_TXID
can have delay in phy.
So disable the delay only for RGMII mode and enable for other modes.
Also treat the default case as disabled delays.
Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode")
Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
drivers/net/phy/at803x.c | 47 ++++++++++++++++++++++++++++++----------
1 file changed, 36 insertions(+), 11 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c6e7d800fd7a..dc1b13f7fc12 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -110,6 +110,18 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
return phy_write(phydev, AT803X_DEBUG_DATA, val);
}
+static int at803x_enable_rx_delay(struct phy_device *phydev)
+{
+ return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
+ AT803X_DEBUG_RX_CLK_DLY_EN);
+}
+
+static int at803x_enable_tx_delay(struct phy_device *phydev)
+{
+ return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
+ AT803X_DEBUG_TX_CLK_DLY_EN);
+}
+
static int at803x_disable_rx_delay(struct phy_device *phydev)
{
return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
@@ -255,23 +267,36 @@ static int at803x_config_init(struct phy_device *phydev)
if (ret < 0)
return ret;
- if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
- phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
- phydev->interface == PHY_INTERFACE_MODE_RGMII) {
- ret = at803x_disable_rx_delay(phydev);
+ /* The hardware register default is RX and TX delay enabled, so lets
+ * first disable the RX and TX delays in phy and enable them based
+ * on the mode selected
+ */
+ ret = at803x_disable_rx_delay(phydev);
+ if (ret < 0)
+ return ret;
+ ret = at803x_disable_tx_delay(phydev);
+ if (ret < 0)
+ return ret;
+
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
+ /* If RGMII_ID or RGMII_RXID are specified enable RX delay,
+ * otherwise keep it disabled
+ */
+ ret = at803x_enable_rx_delay(phydev);
if (ret < 0)
return ret;
}
- if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
- phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
- phydev->interface == PHY_INTERFACE_MODE_RGMII) {
- ret = at803x_disable_tx_delay(phydev);
- if (ret < 0)
- return ret;
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+ /* If RGMII_ID or RGMII_TXID are specified enable TX delay,
+ * otherwise keep it disabled
+ */
+ ret = at803x_enable_tx_delay(phydev);
}
- return 0;
+ return ret;
}
static int at803x_ack_interrupt(struct phy_device *phydev)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] lib/test_rhashtable: fix spelling mistake "existant" -> "existent"
From: Herbert Xu @ 2019-02-18 10:23 UTC (permalink / raw)
To: Colin King; +Cc: Thomas Graf, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20190217225209.17536-1-colin.king@canonical.com>
On Sun, Feb 17, 2019 at 10:52:09PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There are spelling mistakes in warning macro messages. Fix them.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [v3 PATCH 0/4] mac80211: Fix incorrect usage of rhashtable walk API
From: Herbert Xu @ 2019-02-18 10:25 UTC (permalink / raw)
To: Johannes Berg
Cc: David Miller, linux-wireless, netdev, j, tgraf, Julia Lawall
In-Reply-To: <52bdb66f7e9e26ce9225ae11e37e22766d73694d.camel@sipsolutions.net>
On Fri, Feb 15, 2019 at 01:21:55PM +0100, Johannes Berg wrote:
>
> Applied those now, I'll send a pull request to Dave with them. Once that
> trickles back into net-next I'll apply the third patch (it doesn't apply
> without the others), and then Dave you can take the rhashtable one.
Thanks Johannes.
> Let me know if you'd prefer I take the rhashtable one through my tree,
> which really would be only so you don't have to track the dependency.
I don't mind.
> NB: it'd be easier in patchwork if you tagged all the patches with v3 in
> their own PATCH tag, or put the "v3" tag into the actual subject (not
> the "[PATCH 0/4]" tag because evidently patchwork drops the tags and
> doesn't track them for the *series* just each *patch* ... so with what
> you did nothing is visible in patchwork, even just appending "(v3)" to
> the subject of the cover letter would've fixed that...
Noted.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] net/mlx4_en: fix spelling mistake: "quiting" -> "quitting"
From: Dan Carpenter @ 2019-02-18 10:25 UTC (permalink / raw)
To: Tariq Toukan
Cc: Colin King, David S . Miller, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, kernel-janitors@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <2a1a6636-7795-addb-d7b7-711a23dbafa5@mellanox.com>
On Mon, Feb 18, 2019 at 09:37:22AM +0000, Tariq Toukan wrote:
>
>
> On 2/18/2019 1:03 AM, Colin King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > There is a spelling mistake in a en_err error message. Fix it.
> >
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > ---
> > drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> > index 6b88881b8e35..c1438ae52a11 100644
> > --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> > +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> > @@ -3360,7 +3360,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
> > dev->addr_len = ETH_ALEN;
> > mlx4_en_u64_to_mac(dev->dev_addr, mdev->dev->caps.def_mac[priv->port]);
> > if (!is_valid_ether_addr(dev->dev_addr)) {
> > - en_err(priv, "Port: %d, invalid mac burned: %pM, quiting\n",
> > + en_err(priv, "Port: %d, invalid mac burned: %pM, quitting\n",
> > priv->port, dev->dev_addr);
> > err = -EINVAL;
> > goto out;
> >
>
> Hi Colin, thanks for your patch.
>
> Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
>
> I would suggest adding a Fixes line, but looking into the history of the
> typo, it went through many patches that modified this line but preserved
> the typo.
> Actually, it dates back to the very first commit that introduces mlx4
> driver:
>
> Patches history:
> 2b3ddf27f48c net/mlx4_core: Replace VF zero mac with random mac in mlx4_core
> ef96f7d46ad8 net/mlx4_en: Handle unassigned VF MAC address correctly
> 6bbb6d99f3d2 net/mlx4_en: Optimize Rx fast path filter checks
> 453a60827735 mlx4_en: Giving interface name in debug messages
> c27a02cd94d6 mlx4_en: Add driver for Mellanox ConnectX 10GbE NIC
>
> I'm not sure what the "Fixes:" policy is in these cases.
I wouldn't necessarily put a Fixes tag on this, because does fixing the
spelling really count as a bugfix? It's borderline whether it's a fix
or a cleanup.
regards,
daan carpenter
^ permalink raw reply
* Re: [PATCH net-next 10/13] net: mvpp2: reset the XPCS while reconfiguring the serdes lanes
From: Antoine Tenart @ 2019-02-18 10:26 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Antoine Tenart, davem, netdev, linux-kernel, thomas.petazzoni,
maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
stefanc, ymarkman, mw
In-Reply-To: <20190215171224.sjfrid5csseywuks@shell.armlinux.org.uk>
Hi Russell,
On Fri, Feb 15, 2019 at 05:12:24PM +0000, Russell King - ARM Linux admin wrote:
> On Fri, Feb 15, 2019 at 04:32:38PM +0100, Antoine Tenart wrote:
> > The documentation advises to set the XPCS in reset while reconfiguring
> > the serdes lanes. This seems to be a good thing to do, but the PPv2
> > driver wasn't doing it. This patch fixes it.
>
> Hmm. That statment seems to have some ambiguity in it - we do two
> "reconfigurations" - one may be upon initialisation, where the lane
> is already configured for 10Gbase-KR, and we're re-initialising it
> for the same mode. The other case is when we're switching between
> 10Gbase-KR and SGMII, or as will be the case with 2.5G support for
> the Alaska PHYs, 2500base-X.
The configuration at the lane at boot time is dependent to the
firmware or bootloader configuration. On the mcbin, the lane may be
configured in 10Gbase-KR, but it could be configured in SGMII as well.
The configuration upon initialization and the re-configuration are quite
similar then, as we might change mode as well at boot time.
You're right in that we might be re-configuring the lane for the same
exact mode at boot time, if it was already configured in the same mode.
> Does this apply to reconfiguration of the serdes lane between
> 10Gbase-KR and slower modes?
This applies only when configuring a line in a 10G mode,
mvpp22_gop_init_10gkr isn't called otherwise.
When switching to an non-10G mode we might want to put the XPCS in reset
though, which is not done today with this patch.
Thanks,
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v2 1/2] net: phy: at803x: dont inline helpers
From: Niklas Cassel @ 2019-02-18 10:28 UTC (permalink / raw)
To: Vinod Koul
Cc: David S Miller, linux-arm-msm, Bjorn Andersson, netdev,
Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
Marc Gonzalez
In-Reply-To: <20190218101853.4290-1-vkoul@kernel.org>
On Mon, Feb 18, 2019 at 03:48:52PM +0530, Vinod Koul wrote:
> Some helpers were inlined, but makes more sense to allow compiler
> to do the right optiomazations instead, so remove inline for
> at803x_disable_rx_delay() and at803x_disable_tx_delay()
>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> drivers/net/phy/at803x.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 8ff12938ab47..c6e7d800fd7a 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -110,13 +110,13 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
> return phy_write(phydev, AT803X_DEBUG_DATA, val);
> }
>
> -static inline int at803x_disable_rx_delay(struct phy_device *phydev)
> +static int at803x_disable_rx_delay(struct phy_device *phydev)
> {
> return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> AT803X_DEBUG_RX_CLK_DLY_EN, 0);
> }
>
> -static inline int at803x_disable_tx_delay(struct phy_device *phydev)
> +static int at803x_disable_tx_delay(struct phy_device *phydev)
> {
> return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5,
> AT803X_DEBUG_TX_CLK_DLY_EN, 0);
> --
> 2.20.1
>
Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org>
^ permalink raw reply
* Re: [PATCH v2 2/2] net: phy: at803x: disable delay only for RGMII mode
From: Niklas Cassel @ 2019-02-18 10:28 UTC (permalink / raw)
To: Vinod Koul
Cc: David S Miller, linux-arm-msm, Bjorn Andersson, netdev,
Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
Marc Gonzalez
In-Reply-To: <20190218101853.4290-2-vkoul@kernel.org>
On Mon, Feb 18, 2019 at 03:48:53PM +0530, Vinod Koul wrote:
> Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode
> should not have delay in PHY wheras RGMII_ID and RGMII_RXID/RGMII_TXID
> can have delay in phy.
>
> So disable the delay only for RGMII mode and enable for other modes.
> Also treat the default case as disabled delays.
>
> Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode")
> Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> drivers/net/phy/at803x.c | 47 ++++++++++++++++++++++++++++++----------
> 1 file changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index c6e7d800fd7a..dc1b13f7fc12 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -110,6 +110,18 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
> return phy_write(phydev, AT803X_DEBUG_DATA, val);
> }
>
> +static int at803x_enable_rx_delay(struct phy_device *phydev)
> +{
> + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
> + AT803X_DEBUG_RX_CLK_DLY_EN);
> +}
> +
> +static int at803x_enable_tx_delay(struct phy_device *phydev)
> +{
> + return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
> + AT803X_DEBUG_TX_CLK_DLY_EN);
> +}
> +
> static int at803x_disable_rx_delay(struct phy_device *phydev)
> {
> return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> @@ -255,23 +267,36 @@ static int at803x_config_init(struct phy_device *phydev)
> if (ret < 0)
> return ret;
>
> - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> - ret = at803x_disable_rx_delay(phydev);
> + /* The hardware register default is RX and TX delay enabled, so lets
> + * first disable the RX and TX delays in phy and enable them based
> + * on the mode selected
> + */
> + ret = at803x_disable_rx_delay(phydev);
> + if (ret < 0)
> + return ret;
> + ret = at803x_disable_tx_delay(phydev);
> + if (ret < 0)
> + return ret;
> +
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> + /* If RGMII_ID or RGMII_RXID are specified enable RX delay,
> + * otherwise keep it disabled
> + */
> + ret = at803x_enable_rx_delay(phydev);
> if (ret < 0)
> return ret;
> }
>
> - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> - ret = at803x_disable_tx_delay(phydev);
> - if (ret < 0)
> - return ret;
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> + /* If RGMII_ID or RGMII_TXID are specified enable TX delay,
> + * otherwise keep it disabled
> + */
> + ret = at803x_enable_tx_delay(phydev);
> }
>
> - return 0;
> + return ret;
> }
>
> static int at803x_ack_interrupt(struct phy_device *phydev)
> --
> 2.20.1
>
Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org>
^ permalink raw reply
* [PATCH net] mac80211: mesh: fix missing unlock on error in table_path_del()
From: Johannes Berg @ 2019-02-18 10:29 UTC (permalink / raw)
To: linux-wireless, netdev; +Cc: Herbert Xu, Wei Yongjun
From: Wei Yongjun <weiyongjun1@huawei.com>
spin_lock_bh() is used in table_path_del() but rcu_read_unlock()
is used for unlocking. Fix it by using spin_unlock_bh() instead
of rcu_read_unlock() in the error handling case.
Fixes: b4c3fbe63601 ("mac80211: Use linked list instead of rhashtable walk for mesh tables")
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
Dave, can you take this directly? I see little value in sending a
pull request for this one patch (but if you prefer can do so.)
---
net/mac80211/mesh_pathtbl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c
index c3a7396fb955..88a6d5e18ccc 100644
--- a/net/mac80211/mesh_pathtbl.c
+++ b/net/mac80211/mesh_pathtbl.c
@@ -627,7 +627,7 @@ static int table_path_del(struct mesh_table *tbl,
spin_lock_bh(&tbl->walk_lock);
mpath = rhashtable_lookup_fast(&tbl->rhead, addr, mesh_rht_params);
if (!mpath) {
- rcu_read_unlock();
+ spin_unlock_bh(&tbl->walk_lock);
return -ENXIO;
}
--
2.17.2
^ permalink raw reply related
* Re: [PATCH net-next 01/13] net: mvpp2: do not call phylink_mac_change if there is no event
From: Antoine Tenart @ 2019-02-18 10:40 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Antoine Tenart, davem, netdev, linux-kernel, thomas.petazzoni,
maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
stefanc, ymarkman, mw
In-Reply-To: <20190215170553.ur725djxoudtnjne@shell.armlinux.org.uk>
Hi Russell,
On Fri, Feb 15, 2019 at 05:05:53PM +0000, Russell King - ARM Linux admin wrote:
> On Fri, Feb 15, 2019 at 04:32:29PM +0100, Antoine Tenart wrote:
> > This patch makes the link interrupt handler to avoid calling
> > phylink_mac_change when there are no event.
>
> The reasoning being?
The interrupt can fire for a number of reasons, some of which may not be
related to changes in the MAC state (it can fire upon errors or counter
wraparound).
This interrupt handler currently checks if an event occurred bu checking
if the cause of the interrupt is a link change. I wanted to extend this
logic to phylink to avoid having two link change handlers (phylink and
!phylink) being called in different ways.
Anyway, the only cause unmask so far is the link one, so it won't make
any difference so far. We can probably drop this one for now.
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH net-next 10/13] net: mvpp2: reset the XPCS while reconfiguring the serdes lanes
From: Russell King - ARM Linux admin @ 2019-02-18 10:43 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, netdev, linux-kernel, thomas.petazzoni, maxime.chevallier,
gregory.clement, miquel.raynal, nadavh, stefanc, ymarkman, mw
In-Reply-To: <20190218102630.GA3784@kwain>
On Mon, Feb 18, 2019 at 11:26:30AM +0100, Antoine Tenart wrote:
> Hi Russell,
>
> On Fri, Feb 15, 2019 at 05:12:24PM +0000, Russell King - ARM Linux admin wrote:
> > On Fri, Feb 15, 2019 at 04:32:38PM +0100, Antoine Tenart wrote:
> > > The documentation advises to set the XPCS in reset while reconfiguring
> > > the serdes lanes. This seems to be a good thing to do, but the PPv2
> > > driver wasn't doing it. This patch fixes it.
> >
> > Hmm. That statment seems to have some ambiguity in it - we do two
> > "reconfigurations" - one may be upon initialisation, where the lane
> > is already configured for 10Gbase-KR, and we're re-initialising it
> > for the same mode. The other case is when we're switching between
> > 10Gbase-KR and SGMII, or as will be the case with 2.5G support for
> > the Alaska PHYs, 2500base-X.
>
> The configuration at the lane at boot time is dependent to the
> firmware or bootloader configuration. On the mcbin, the lane may be
> configured in 10Gbase-KR, but it could be configured in SGMII as well.
> The configuration upon initialization and the re-configuration are quite
> similar then, as we might change mode as well at boot time.
>
> You're right in that we might be re-configuring the lane for the same
> exact mode at boot time, if it was already configured in the same mode.
>
> > Does this apply to reconfiguration of the serdes lane between
> > 10Gbase-KR and slower modes?
>
> This applies only when configuring a line in a 10G mode,
> mvpp22_gop_init_10gkr isn't called otherwise.
>
> When switching to an non-10G mode we might want to put the XPCS in reset
> though, which is not done today with this patch.
I'm merely pointing out the discrepency between the commit message and
what is actually being done. I'm not particularly concerned about what
happens at boot.
We have four different transitions a port can go through, all of which
reconfigure the serdes lanes:
1. 10gkr -> 10gkr
2. 10gkr -> non-10gkr
3. non-10gkr -> non-10gkr
4. non-10gkr -> 10gkr
With this patch, XPCS is only placed into reset during the
reconfiguration for cases 1 and 4. Case 3 doesn't matter (the XPCS
should already be in reset right?) Case 2 isn't covered, and this
leaves a rather big hole.
It seems to me that if the documentation states that the XPCS needs to
be placed in reset while the serdes is reconfigured, then what should
be happening is:
- at boot, place the XPCS into reset.
- when we configure for 10gkr, release the reset once we've finished
configuring the serdes.
- when we configure away from 10gkr, place the XPCS back into reset
before configuring the serdes.
Merely placing the XPCS into reset while we configure the serdes for
10gkr doesn't seem to be "fixing" the driver to conform to your commit
message opening statement.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ 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