* [PATCH net 0/3] qed*: Fix series.
@ 2018-06-19 4:57 Sudarsana Reddy Kalluru
2018-06-19 4:58 ` [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling Sudarsana Reddy Kalluru
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Sudarsana Reddy Kalluru @ 2018-06-19 4:57 UTC (permalink / raw)
To: davem; +Cc: netdev, Ariel.Elior, Michal.Kalderon, Sudarsana Reddy Kalluru
From: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
The patch series fixes few issues in the qed/qede drivers.
Please consider applying this series to "net".
Sudarsana Reddy Kalluru (3):
qed: Fix possible memory leak in Rx error path handling.
qed: Add sanity check for SIMD fastpath handler.
qed: Do not advertise DCBX_LLD_MANAGED capability.
drivers/net/ethernet/qlogic/qed/qed_dcbx.c | 11 ++++-------
drivers/net/ethernet/qlogic/qed/qed_ll2.c | 11 +++++++++--
drivers/net/ethernet/qlogic/qed/qed_main.c | 12 ++++++++++--
3 files changed, 23 insertions(+), 11 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling. 2018-06-19 4:57 [PATCH net 0/3] qed*: Fix series Sudarsana Reddy Kalluru @ 2018-06-19 4:58 ` Sudarsana Reddy Kalluru 2018-06-19 6:01 ` Yunsheng Lin 2018-06-19 4:58 ` [PATCH net 2/3] qed: Add sanity check for SIMD fastpath handler Sudarsana Reddy Kalluru ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Sudarsana Reddy Kalluru @ 2018-06-19 4:58 UTC (permalink / raw) To: davem; +Cc: netdev, Ariel.Elior, Michal.Kalderon Memory for packet buffers need to be freed in the error paths as there is no consumer (e.g., upper layer) for such packets and that memory will never get freed. The issue was uncovered when port was attacked with flood of isatap packets, these are multicast packets hence were directed at all the PFs. For foce PF, this meant they were routed to the ll2 module which in turn drops such packets. Fixes: 0a7fb11c ("qed: Add Light L2 support") Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com> Signed-off-by: Ariel Elior <ariel.elior@cavium.com> Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com> --- drivers/net/ethernet/qlogic/qed/qed_ll2.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c index c97ebd6..012973d 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c @@ -201,8 +201,9 @@ void qed_ll2b_complete_rx_packet(void *cxt, struct qed_ll2_comp_rx_data *data) skb = build_skb(buffer->data, 0); if (!skb) { - rc = -ENOMEM; - goto out_post; + DP_INFO(cdev, "Failed to build SKB\n"); + kfree(buffer->data); + goto out_post1; } data->u.placement_offset += NET_SKB_PAD; @@ -224,8 +225,14 @@ void qed_ll2b_complete_rx_packet(void *cxt, struct qed_ll2_comp_rx_data *data) cdev->ll2->cbs->rx_cb(cdev->ll2->cb_cookie, skb, data->opaque_data_0, data->opaque_data_1); + } else { + DP_VERBOSE(p_hwfn, (NETIF_MSG_RX_STATUS | NETIF_MSG_PKTDATA | + QED_MSG_LL2 | QED_MSG_STORAGE), + "Dropping the packet\n"); + kfree(buffer->data); } +out_post1: /* Update Buffer information and update FW producer */ buffer->data = new_data; buffer->phys_addr = new_phys_addr; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling. 2018-06-19 4:58 ` [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling Sudarsana Reddy Kalluru @ 2018-06-19 6:01 ` Yunsheng Lin 2018-06-19 6:42 ` Kalluru, Sudarsana 0 siblings, 1 reply; 8+ messages in thread From: Yunsheng Lin @ 2018-06-19 6:01 UTC (permalink / raw) To: Sudarsana Reddy Kalluru, davem; +Cc: netdev, Ariel.Elior, Michal.Kalderon On 2018/6/19 12:58, Sudarsana Reddy Kalluru wrote: > Memory for packet buffers need to be freed in the error paths as there is > no consumer (e.g., upper layer) for such packets and that memory will never > get freed. > The issue was uncovered when port was attacked with flood of isatap > packets, these are multicast packets hence were directed at all the PFs. > For foce PF, this meant they were routed to the ll2 module which in turn > drops such packets. > > Fixes: 0a7fb11c ("qed: Add Light L2 support") > Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com> > Signed-off-by: Ariel Elior <ariel.elior@cavium.com> > Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com> > --- > drivers/net/ethernet/qlogic/qed/qed_ll2.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c > index c97ebd6..012973d 100644 > --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c > +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c > @@ -201,8 +201,9 @@ void qed_ll2b_complete_rx_packet(void *cxt, struct qed_ll2_comp_rx_data *data) > > skb = build_skb(buffer->data, 0); > if (!skb) { > - rc = -ENOMEM; > - goto out_post; > + DP_INFO(cdev, "Failed to build SKB\n"); > + kfree(buffer->data); > + goto out_post1; > } > > data->u.placement_offset += NET_SKB_PAD; > @@ -224,8 +225,14 @@ void qed_ll2b_complete_rx_packet(void *cxt, struct qed_ll2_comp_rx_data *data) > cdev->ll2->cbs->rx_cb(cdev->ll2->cb_cookie, skb, > data->opaque_data_0, > data->opaque_data_1); > + } else { > + DP_VERBOSE(p_hwfn, (NETIF_MSG_RX_STATUS | NETIF_MSG_PKTDATA | > + QED_MSG_LL2 | QED_MSG_STORAGE), > + "Dropping the packet\n"); > + kfree(buffer->data); What about the memory used by skb itself? Does skb need to be freed by kfree_skb or something like that? > } > > +out_post1: > /* Update Buffer information and update FW producer */ > buffer->data = new_data; > buffer->phys_addr = new_phys_addr; > ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling. 2018-06-19 6:01 ` Yunsheng Lin @ 2018-06-19 6:42 ` Kalluru, Sudarsana 2018-06-19 7:24 ` Yunsheng Lin 0 siblings, 1 reply; 8+ messages in thread From: Kalluru, Sudarsana @ 2018-06-19 6:42 UTC (permalink / raw) To: Yunsheng Lin, davem; +Cc: netdev, Elior, Ariel, Kalderon, Michal -----Original Message----- From: Yunsheng Lin [mailto:linyunsheng@huawei.com] Sent: 19 June 2018 11:32 To: Kalluru, Sudarsana <Sudarsana.Kalluru@cavium.com>; davem@davemloft.net Cc: netdev@vger.kernel.org; Elior, Ariel <Ariel.Elior@cavium.com>; Kalderon, Michal <Michal.Kalderon@cavium.com> Subject: Re: [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling. External Email On 2018/6/19 12:58, Sudarsana Reddy Kalluru wrote: > Memory for packet buffers need to be freed in the error paths as there > is no consumer (e.g., upper layer) for such packets and that memory > will never get freed. > The issue was uncovered when port was attacked with flood of isatap > packets, these are multicast packets hence were directed at all the PFs. > For foce PF, this meant they were routed to the ll2 module which in > turn drops such packets. > > Fixes: 0a7fb11c ("qed: Add Light L2 support") > Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com> > Signed-off-by: Ariel Elior <ariel.elior@cavium.com> > Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com> > --- > drivers/net/ethernet/qlogic/qed/qed_ll2.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c > b/drivers/net/ethernet/qlogic/qed/qed_ll2.c > index c97ebd6..012973d 100644 > --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c > +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c > @@ -201,8 +201,9 @@ void qed_ll2b_complete_rx_packet(void *cxt, struct > qed_ll2_comp_rx_data *data) > > skb = build_skb(buffer->data, 0); > if (!skb) { > - rc = -ENOMEM; > - goto out_post; > + DP_INFO(cdev, "Failed to build SKB\n"); > + kfree(buffer->data); > + goto out_post1; > } > > data->u.placement_offset += NET_SKB_PAD; @@ -224,8 +225,14 @@ > void qed_ll2b_complete_rx_packet(void *cxt, struct qed_ll2_comp_rx_data *data) > cdev->ll2->cbs->rx_cb(cdev->ll2->cb_cookie, skb, > data->opaque_data_0, > data->opaque_data_1); > + } else { > + DP_VERBOSE(p_hwfn, (NETIF_MSG_RX_STATUS | NETIF_MSG_PKTDATA | > + QED_MSG_LL2 | QED_MSG_STORAGE), > + "Dropping the packet\n"); > + kfree(buffer->data); What about the memory used by skb itself? Does skb need to be freed by kfree_skb or something like that? [Sudarsana] Thanks for reviewing the changes. qed_ll2_alloc_buffer() allocates this memory. The allocated buffer (i.e., buffer->data) holds complete memory for 'skb + data' as required by build_skb() implementation. Hence freeing of (buffer->data) would suffice here. > } > > +out_post1: > /* Update Buffer information and update FW producer */ > buffer->data = new_data; > buffer->phys_addr = new_phys_addr; > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling. 2018-06-19 6:42 ` Kalluru, Sudarsana @ 2018-06-19 7:24 ` Yunsheng Lin 0 siblings, 0 replies; 8+ messages in thread From: Yunsheng Lin @ 2018-06-19 7:24 UTC (permalink / raw) To: Kalluru, Sudarsana, davem; +Cc: netdev, Elior, Ariel, Kalderon, Michal On 2018/6/19 14:42, Kalluru, Sudarsana wrote: > > > -----Original Message----- > From: Yunsheng Lin [mailto:linyunsheng@huawei.com] > Sent: 19 June 2018 11:32 > To: Kalluru, Sudarsana <Sudarsana.Kalluru@cavium.com>; davem@davemloft.net > Cc: netdev@vger.kernel.org; Elior, Ariel <Ariel.Elior@cavium.com>; Kalderon, Michal <Michal.Kalderon@cavium.com> > Subject: Re: [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling. > > External Email > > On 2018/6/19 12:58, Sudarsana Reddy Kalluru wrote: >> Memory for packet buffers need to be freed in the error paths as there >> is no consumer (e.g., upper layer) for such packets and that memory >> will never get freed. >> The issue was uncovered when port was attacked with flood of isatap >> packets, these are multicast packets hence were directed at all the PFs. >> For foce PF, this meant they were routed to the ll2 module which in >> turn drops such packets. >> >> Fixes: 0a7fb11c ("qed: Add Light L2 support") >> Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com> >> Signed-off-by: Ariel Elior <ariel.elior@cavium.com> >> Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com> >> --- >> drivers/net/ethernet/qlogic/qed/qed_ll2.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c >> b/drivers/net/ethernet/qlogic/qed/qed_ll2.c >> index c97ebd6..012973d 100644 >> --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c >> +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c >> @@ -201,8 +201,9 @@ void qed_ll2b_complete_rx_packet(void *cxt, struct >> qed_ll2_comp_rx_data *data) >> >> skb = build_skb(buffer->data, 0); >> if (!skb) { >> - rc = -ENOMEM; >> - goto out_post; >> + DP_INFO(cdev, "Failed to build SKB\n"); >> + kfree(buffer->data); >> + goto out_post1; >> } >> >> data->u.placement_offset += NET_SKB_PAD; @@ -224,8 +225,14 @@ >> void qed_ll2b_complete_rx_packet(void *cxt, struct qed_ll2_comp_rx_data *data) >> cdev->ll2->cbs->rx_cb(cdev->ll2->cb_cookie, skb, >> data->opaque_data_0, >> data->opaque_data_1); >> + } else { >> + DP_VERBOSE(p_hwfn, (NETIF_MSG_RX_STATUS | NETIF_MSG_PKTDATA | >> + QED_MSG_LL2 | QED_MSG_STORAGE), >> + "Dropping the packet\n"); >> + kfree(buffer->data); > > What about the memory used by skb itself? > Does skb need to be freed by kfree_skb or something like that? > > [Sudarsana] Thanks for reviewing the changes. qed_ll2_alloc_buffer() allocates this memory. The allocated buffer (i.e., buffer->data) holds complete memory for 'skb + data' as required by build_skb() implementation. Hence freeing of (buffer->data) would suffice here. As I read through the code, the skb itself is allocated by kmem_cache_alloc, see below: build_skb -> __build_skb -> kmem_cache_alloc Hope I am not missing something here. > >> } >> >> +out_post1: >> /* Update Buffer information and update FW producer */ >> buffer->data = new_data; >> buffer->phys_addr = new_phys_addr; >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net 2/3] qed: Add sanity check for SIMD fastpath handler. 2018-06-19 4:57 [PATCH net 0/3] qed*: Fix series Sudarsana Reddy Kalluru 2018-06-19 4:58 ` [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling Sudarsana Reddy Kalluru @ 2018-06-19 4:58 ` Sudarsana Reddy Kalluru 2018-06-19 4:58 ` [PATCH net 3/3] qed: Do not advertise DCBX_LLD_MANAGED capability Sudarsana Reddy Kalluru 2018-06-19 22:18 ` [PATCH net 0/3] qed*: Fix series David Miller 3 siblings, 0 replies; 8+ messages in thread From: Sudarsana Reddy Kalluru @ 2018-06-19 4:58 UTC (permalink / raw) To: davem; +Cc: netdev, Ariel.Elior, Michal.Kalderon Avoid calling a SIMD fastpath handler if it is NULL. The check is needed to handle an unlikely scenario where unsolicited interrupt is destined to a PF in INTa mode. Fixes: fe56b9e6a ("qed: Add module with basic common support") Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com> Signed-off-by: Ariel Elior <ariel.elior@cavium.com> Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com> --- drivers/net/ethernet/qlogic/qed/qed_main.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c index b04d57c..5c10fd7 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_main.c +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c @@ -567,8 +567,16 @@ static irqreturn_t qed_single_int(int irq, void *dev_instance) /* Fastpath interrupts */ for (j = 0; j < 64; j++) { if ((0x2ULL << j) & status) { - hwfn->simd_proto_handler[j].func( - hwfn->simd_proto_handler[j].token); + struct qed_simd_fp_handler *p_handler = + &hwfn->simd_proto_handler[j]; + + if (p_handler->func) + p_handler->func(p_handler->token); + else + DP_NOTICE(hwfn, + "Not calling fastpath handler as it is NULL [handler #%d, status 0x%llx]\n", + j, status); + status &= ~(0x2ULL << j); rc = IRQ_HANDLED; } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net 3/3] qed: Do not advertise DCBX_LLD_MANAGED capability. 2018-06-19 4:57 [PATCH net 0/3] qed*: Fix series Sudarsana Reddy Kalluru 2018-06-19 4:58 ` [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling Sudarsana Reddy Kalluru 2018-06-19 4:58 ` [PATCH net 2/3] qed: Add sanity check for SIMD fastpath handler Sudarsana Reddy Kalluru @ 2018-06-19 4:58 ` Sudarsana Reddy Kalluru 2018-06-19 22:18 ` [PATCH net 0/3] qed*: Fix series David Miller 3 siblings, 0 replies; 8+ messages in thread From: Sudarsana Reddy Kalluru @ 2018-06-19 4:58 UTC (permalink / raw) To: davem; +Cc: netdev, Ariel.Elior, Michal.Kalderon Do not advertise DCBX_LLD_MANAGED capability i.e., do not allow external agent to manage the dcbx/lldp negotiation. MFW acts as lldp agent for qed* devices, and no other lldp agent is allowed to coexist with mfw. Also updated a debug print, to not to display the redundant info. Fixes: a1d8d8a51 ("qed: Add dcbnl support.") Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com> Signed-off-by: Ariel Elior <ariel.elior@cavium.com> Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com> --- drivers/net/ethernet/qlogic/qed/qed_dcbx.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c index 8f31406..f0b0138 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c +++ b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c @@ -255,9 +255,8 @@ static bool qed_dcbx_roce_v2_tlv(u32 app_info_bitmap, u16 proto_id, bool ieee) *type = DCBX_PROTOCOL_ROCE_V2; } else { *type = DCBX_MAX_PROTOCOL_TYPE; - DP_ERR(p_hwfn, - "No action required, App TLV id = 0x%x app_prio_bitmap = 0x%x\n", - id, app_prio_bitmap); + DP_ERR(p_hwfn, "No action required, App TLV entry = 0x%x\n", + app_prio_bitmap); return false; } @@ -1479,8 +1478,8 @@ static u8 qed_dcbnl_getcap(struct qed_dev *cdev, int capid, u8 *cap) *cap = 0x80; break; case DCB_CAP_ATTR_DCBX: - *cap = (DCB_CAP_DCBX_LLD_MANAGED | DCB_CAP_DCBX_VER_CEE | - DCB_CAP_DCBX_VER_IEEE | DCB_CAP_DCBX_STATIC); + *cap = (DCB_CAP_DCBX_VER_CEE | DCB_CAP_DCBX_VER_IEEE | + DCB_CAP_DCBX_STATIC); break; default: *cap = false; @@ -1548,8 +1547,6 @@ static u8 qed_dcbnl_getdcbx(struct qed_dev *cdev) if (!dcbx_info) return 0; - if (dcbx_info->operational.enabled) - mode |= DCB_CAP_DCBX_LLD_MANAGED; if (dcbx_info->operational.ieee) mode |= DCB_CAP_DCBX_VER_IEEE; if (dcbx_info->operational.cee) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net 0/3] qed*: Fix series. 2018-06-19 4:57 [PATCH net 0/3] qed*: Fix series Sudarsana Reddy Kalluru ` (2 preceding siblings ...) 2018-06-19 4:58 ` [PATCH net 3/3] qed: Do not advertise DCBX_LLD_MANAGED capability Sudarsana Reddy Kalluru @ 2018-06-19 22:18 ` David Miller 3 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2018-06-19 22:18 UTC (permalink / raw) To: sudarsana.kalluru; +Cc: netdev, Ariel.Elior, Michal.Kalderon From: Sudarsana Reddy Kalluru <sudarsana.kalluru@cavium.com> Date: Mon, 18 Jun 2018 21:57:59 -0700 > From: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com> > > The patch series fixes few issues in the qed/qede drivers. > Please consider applying this series to "net". Series applied. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-06-19 22:18 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-19 4:57 [PATCH net 0/3] qed*: Fix series Sudarsana Reddy Kalluru 2018-06-19 4:58 ` [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling Sudarsana Reddy Kalluru 2018-06-19 6:01 ` Yunsheng Lin 2018-06-19 6:42 ` Kalluru, Sudarsana 2018-06-19 7:24 ` Yunsheng Lin 2018-06-19 4:58 ` [PATCH net 2/3] qed: Add sanity check for SIMD fastpath handler Sudarsana Reddy Kalluru 2018-06-19 4:58 ` [PATCH net 3/3] qed: Do not advertise DCBX_LLD_MANAGED capability Sudarsana Reddy Kalluru 2018-06-19 22:18 ` [PATCH net 0/3] qed*: Fix series David Miller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.