* RE: [PATCH net 2/2] qed: Fix non TCP packets should be dropped on iWARP ll2 connection
From: Kalderon, Michal @ 2018-03-14 13:29 UTC (permalink / raw)
To: Joe Perches, davem@davemloft.net
Cc: netdev@vger.kernel.org, dledford@redhat.com, jgg@mellanox.com,
linux-rdma@vger.kernel.org, Elior, Ariel
In-Reply-To: <1521032565.2049.68.camel@perches.com>
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Wednesday, March 14, 2018 3:03 PM
> To: Kalderon, Michal <Michal.Kalderon@cavium.com>;
> davem@davemloft.net
> Cc: netdev@vger.kernel.org; dledford@redhat.com; jgg@mellanox.com;
> linux-rdma@vger.kernel.org; Elior, Ariel <Ariel.Elior@cavium.com>
> Subject: Re: [PATCH net 2/2] qed: Fix non TCP packets should be dropped on
> iWARP ll2 connection
>
> [This sender failed our fraud detection checks and may not be who they
> appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
>
> On Wed, 2018-03-14 at 14:49 +0200, Michal Kalderon wrote:
> > FW workaround. The iWARP LL2 connection did not expect TCP packets to
> > arrive on it's connection. The fix drops any non-tcp packets
> []
> > diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> > b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> []
> > @@ -1703,6 +1703,13 @@ qed_iwarp_parse_rx_pkt(struct qed_hwfn
> *p_hwfn,
> > iph = (struct iphdr *)((u8 *)(ethh) + eth_hlen);
> >
> > if (eth_type == ETH_P_IP) {
> > + if (iph->protocol != IPPROTO_TCP) {
> > + DP_NOTICE(p_hwfn,
> > + "Unexpected ip protocol on ll2 %x\n",
> > + iph->protocol);
> > + return -EINVAL;
> > + }
>
> Perhaps this should be ratelimited.
The rate of the packets that could arrive here is very low. It has to do with a corner case
Of RoCEv2 packets being sent to a device that was enabled with iWARP.
>
> > +
> > cm_info->local_ip[0] = ntohl(iph->daddr);
> > cm_info->remote_ip[0] = ntohl(iph->saddr);
> > cm_info->ip_version = TCP_IPV4; @@ -1711,6 +1718,14 @@
> > qed_iwarp_parse_rx_pkt(struct qed_hwfn *p_hwfn,
> > *payload_len = ntohs(iph->tot_len) - ip_hlen;
> > } else if (eth_type == ETH_P_IPV6) {
> > ip6h = (struct ipv6hdr *)iph;
> > +
> > + if (ip6h->nexthdr != IPPROTO_TCP) {
> > + DP_NOTICE(p_hwfn,
> > + "Unexpected ip protocol on ll2 %x\n",
> > + iph->protocol);
> > + return -EINVAL;
>
> here too
>
> > + }
> > +
> > for (i = 0; i < 4; i++) {
> > cm_info->local_ip[i] =
> > ntohl(ip6h->daddr.in6_u.u6_addr32[i]);
^ permalink raw reply
* Re: pull-request: can 2018-03-14
From: Andri Yngvason @ 2018-03-14 13:07 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: netdev, wg, davem, kernel, linux-can
In-Reply-To: <7b820ad7-580b-8cd2-cd1d-cea364e30b12@pengutronix.de>
On Wed, Mar 14, 2018 at 01:52:46PM +0100, Marc Kleine-Budde wrote:
> On 03/14/2018 01:24 PM, Andri Yngvason wrote:
> > On Wed, Mar 14, 2018 at 01:05:21PM +0100, Marc Kleine-Budde wrote:
> >> Hello David,
> >>
> >> this is a pull request of two patches for net/master.
> >>
> >> Both patches are by Andri Yngvason and fix problems in the cc770 driver,
> >> that show up quite fast on RT systems, but also on non RT setups.
> >>
> >> regards,
> >> Marc
> >>
> >> ---
> >>
> >> The following changes since commit f89782c2d131e6eae0d1ea2569ba76bc4c5875fe:
> >>
> >> qed: Use after free in qed_rdma_free() (2018-03-13 10:54:17 -0400)
> >>
> >> are available in the Git repository at:
> >>
> >> ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.16-20180314
> >>
> >> for you to fetch changes up to 746201235b3f876792099079f4c6fea941d76183:
> >>
> >> can: cc770: Fix queue stall & dropped RTR reply (2018-03-14 13:01:22 +0100)
> >>
> >> ----------------------------------------------------------------
> >> linux-can-fixes-for-4.16-20180314
> >>
> >> ----------------------------------------------------------------
> >> Andri Yngvason (2):
> >> can: cc770: Fix stalls on rt-linux, remove redundant IRQ ack
> >> can: cc770: Fix queue stall & dropped RTR reply
> >>
> >> drivers/net/can/cc770/cc770.c | 103 ++++++++++++++++++++++++++----------------
> >> drivers/net/can/cc770/cc770.h | 2 +
> >> 2 files changed, 65 insertions(+), 40 deletions(-)
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > Wait, do we not want Wolfgang's signed-off-by or reviewed-by?
>
> In the review Wolfgang was involved in "[PATCH v2 2/3] can: cc770: Stop
> queue on NETDEV_TX_BUSY" and we all agreed to drop the patch.
>
> The other patches were successfully tested by Richard, AFAIK there are
> no open points anymore and there was no discussion on these patches for
> more than 10 days. So assumed the patches are "ready" for upstreaming now.
>
All right!
Thanks,
Andri
^ permalink raw reply
* Re: [PATCH net 2/2] qed: Fix non TCP packets should be dropped on iWARP ll2 connection
From: Joe Perches @ 2018-03-14 13:02 UTC (permalink / raw)
To: Michal Kalderon, davem; +Cc: netdev, dledford, jgg, linux-rdma, Ariel Elior
In-Reply-To: <1521031768-19131-3-git-send-email-Michal.Kalderon@cavium.com>
On Wed, 2018-03-14 at 14:49 +0200, Michal Kalderon wrote:
> FW workaround. The iWARP LL2 connection did not expect TCP packets
> to arrive on it's connection. The fix drops any non-tcp packets
[]
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
[]
> @@ -1703,6 +1703,13 @@ qed_iwarp_parse_rx_pkt(struct qed_hwfn *p_hwfn,
> iph = (struct iphdr *)((u8 *)(ethh) + eth_hlen);
>
> if (eth_type == ETH_P_IP) {
> + if (iph->protocol != IPPROTO_TCP) {
> + DP_NOTICE(p_hwfn,
> + "Unexpected ip protocol on ll2 %x\n",
> + iph->protocol);
> + return -EINVAL;
> + }
Perhaps this should be ratelimited.
> +
> cm_info->local_ip[0] = ntohl(iph->daddr);
> cm_info->remote_ip[0] = ntohl(iph->saddr);
> cm_info->ip_version = TCP_IPV4;
> @@ -1711,6 +1718,14 @@ qed_iwarp_parse_rx_pkt(struct qed_hwfn *p_hwfn,
> *payload_len = ntohs(iph->tot_len) - ip_hlen;
> } else if (eth_type == ETH_P_IPV6) {
> ip6h = (struct ipv6hdr *)iph;
> +
> + if (ip6h->nexthdr != IPPROTO_TCP) {
> + DP_NOTICE(p_hwfn,
> + "Unexpected ip protocol on ll2 %x\n",
> + iph->protocol);
> + return -EINVAL;
here too
> + }
> +
> for (i = 0; i < 4; i++) {
> cm_info->local_ip[i] =
> ntohl(ip6h->daddr.in6_u.u6_addr32[i]);
^ permalink raw reply
* Re: [PATCH] brcmfmac: drop Inter-Access Point Protocol packets by default
From: Arend van Spriel @ 2018-03-14 12:58 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo
Cc: Franky Lin, Hante Meuleman, Chi-Hsien Lin, Wright Feng,
Pieter-Paul Giesberts, James Hughes,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
brcm80211-dev-list.pdl-dY08KVG/lbpWk0Htik3J/w,
brcm80211-dev-list-+wT8y+m8/X5BDgjK7y7TUQ,
netdev-u79uwXL29TY76Z2rM5mHXA, Linus Lüssing, Felix Fietkau,
bridge-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Rafał Miłecki
In-Reply-To: <20180314110119.13631-1-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 3/14/2018 12:01 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>
> Testing brcmfmac with more recent firmwares resulted in AP interfaces
> not working in some specific setups. Debugging resulted in discovering
> support for IAPP in Broadcom's firmwares. This is an obsoleted standard
> and its implementation is something that:
> 1) Most people don't need / want to use
> 2) Can allow local DoS attacks
> 3) Breaks AP interfaces in some specific bridge setups
>
> To solve issues it can cause this commit modifies brcmfmac to drop IAPP
> packets. If affects:
> 1) Rx path: driver won't be sending these unwanted packets up.
> 2) Tx path: driver will reject packets that would trigger STA
> disassociation perfromed by a firmware (possible local DoS attack).
>
> It appears there are some Broadcom's clients/users who care about this
> feature despite the drawbacks. They can switch it on by a newly added
> Kconfig option.
Thanks for taking this approach. Looks fine except for .... (see below)
Reviewed-by: Arend van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
> drivers/net/wireless/broadcom/brcm80211/Kconfig | 20 +++++++++++
> .../wireless/broadcom/brcm80211/brcmfmac/core.c | 39 ++++++++++++++++++++++
> 2 files changed, 59 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/Kconfig b/drivers/net/wireless/broadcom/brcm80211/Kconfig
> index 9d99eb42d917..876787ef991a 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/Kconfig
> +++ b/drivers/net/wireless/broadcom/brcm80211/Kconfig
> @@ -68,6 +68,26 @@ config BRCMFMAC_PCIE
> IEEE802.11ac embedded FullMAC WLAN driver. Say Y if you want to
> use the driver for an PCIE wireless card.
>
> +config BRCMFMAC_IAPP
> + bool "Partial support for obsoleted Inter-Access Point Protocol"
> + depends on BRCMFMAC
> + ---help---
> + Most of Broadcom's firmwares can send 802.11f ADD frame every
> + time new STA connects to the AP interface. Some recent ones
> + can also disassociate STA when they receive such a frame.
I do not see any evidence that this would occur only for recent
firmware. That stuff is old and not touched recently.
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> index 19048526b4af..db6987015fb1 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
[...]
> static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
> struct net_device *ndev)
> {
> @@ -250,6 +278,12 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
> goto done;
> }
>
> + if (!IS_ENABLED(CONFIG_BRCMFMAC_IAPP) && brcmf_skb_is_iapp(skb)) {
> + dev_kfree_skb(skb);
> + ret = -EINVAL;
> + goto done;
> + }
This is not right. The function must return netdev_tx_t type. Here is
kerneldoc of .start_xmit():
* netdev_tx_t (*ndo_start_xmit)(struct sk_buff *skb,
* struct net_device *dev);
* Called when a packet needs to be transmitted.
* Returns NETDEV_TX_OK. Can return NETDEV_TX_BUSY, but you should stop
* the queue before that can happen; it's for obsolete devices and weird
* corner cases, but the stack really does a non-trivial amount
* of useless work if you return NETDEV_TX_BUSY.
* Required; cannot be NULL.
You may want to increase dropped netstat or add driver internal
statistic counter so there is visibility of IAPP packets being dropped.
Regards,
Arend
^ permalink raw reply
* [PATCH net] qede: Fix qedr link update
From: Michal Kalderon @ 2018-03-14 12:56 UTC (permalink / raw)
To: michal.kalderon, davem; +Cc: netdev, linux-rdma, Michal Kalderon, Ariel Elior
Link updates were not reported to qedr correctly.
Leading to cases where a link could be down, but qedr
would see it as up.
In addition, once qede was loaded, link state would be up,
regardless of the actual link state.
Signed-off-by: Michal Kalderon <michal.kalderon@cavium.com>
Signed-off-by: Ariel Elior <ariel.elior@cavium.com>
---
drivers/net/ethernet/qlogic/qede/qede_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 2db70ea..5c28209 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -2067,8 +2067,6 @@ static int qede_load(struct qede_dev *edev, enum qede_load_mode mode,
link_params.link_up = true;
edev->ops->common->set_link(edev->cdev, &link_params);
- qede_rdma_dev_event_open(edev);
-
edev->state = QEDE_STATE_OPEN;
DP_INFO(edev, "Ending successfully qede load\n");
@@ -2169,12 +2167,14 @@ static void qede_link_update(void *dev, struct qed_link_output *link)
DP_NOTICE(edev, "Link is up\n");
netif_tx_start_all_queues(edev->ndev);
netif_carrier_on(edev->ndev);
+ qede_rdma_dev_event_open(edev);
}
} else {
if (netif_carrier_ok(edev->ndev)) {
DP_NOTICE(edev, "Link is down\n");
netif_tx_disable(edev->ndev);
netif_carrier_off(edev->ndev);
+ qede_rdma_dev_event_close(edev);
}
}
}
--
2.9.5
^ permalink raw reply related
* Re: pull-request: can 2018-03-14
From: Marc Kleine-Budde @ 2018-03-14 12:52 UTC (permalink / raw)
To: Andri Yngvason; +Cc: netdev, wg, davem, kernel, linux-can
In-Reply-To: <20180314122452.GA12518@maxwell>
[-- Attachment #1.1: Type: text/plain, Size: 2336 bytes --]
On 03/14/2018 01:24 PM, Andri Yngvason wrote:
> On Wed, Mar 14, 2018 at 01:05:21PM +0100, Marc Kleine-Budde wrote:
>> Hello David,
>>
>> this is a pull request of two patches for net/master.
>>
>> Both patches are by Andri Yngvason and fix problems in the cc770 driver,
>> that show up quite fast on RT systems, but also on non RT setups.
>>
>> regards,
>> Marc
>>
>> ---
>>
>> The following changes since commit f89782c2d131e6eae0d1ea2569ba76bc4c5875fe:
>>
>> qed: Use after free in qed_rdma_free() (2018-03-13 10:54:17 -0400)
>>
>> are available in the Git repository at:
>>
>> ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.16-20180314
>>
>> for you to fetch changes up to 746201235b3f876792099079f4c6fea941d76183:
>>
>> can: cc770: Fix queue stall & dropped RTR reply (2018-03-14 13:01:22 +0100)
>>
>> ----------------------------------------------------------------
>> linux-can-fixes-for-4.16-20180314
>>
>> ----------------------------------------------------------------
>> Andri Yngvason (2):
>> can: cc770: Fix stalls on rt-linux, remove redundant IRQ ack
>> can: cc770: Fix queue stall & dropped RTR reply
>>
>> drivers/net/can/cc770/cc770.c | 103 ++++++++++++++++++++++++++----------------
>> drivers/net/can/cc770/cc770.h | 2 +
>> 2 files changed, 65 insertions(+), 40 deletions(-)
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-can" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Wait, do we not want Wolfgang's signed-off-by or reviewed-by?
In the review Wolfgang was involved in "[PATCH v2 2/3] can: cc770: Stop
queue on NETDEV_TX_BUSY" and we all agreed to drop the patch.
The other patches were successfully tested by Richard, AFAIK there are
no open points anymore and there was no discussion on these patches for
more than 10 days. So assumed the patches are "ready" for upstreaming now.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH net 2/2] qed: Fix non TCP packets should be dropped on iWARP ll2 connection
From: Michal Kalderon @ 2018-03-14 12:49 UTC (permalink / raw)
To: michal.kalderon, davem
Cc: netdev, dledford, jgg, linux-rdma, Michal Kalderon, Ariel Elior
In-Reply-To: <1521031768-19131-1-git-send-email-Michal.Kalderon@cavium.com>
FW workaround. The iWARP LL2 connection did not expect TCP packets
to arrive on it's connection. The fix drops any non-tcp packets
Fixes b5c29ca ("qed: iWARP CM - setup a ll2 connection for handling
SYN packets")
Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com>
Signed-off-by: Ariel Elior <Ariel.Elior@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed_iwarp.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
index fefe527..d5d02be 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
@@ -1703,6 +1703,13 @@ qed_iwarp_parse_rx_pkt(struct qed_hwfn *p_hwfn,
iph = (struct iphdr *)((u8 *)(ethh) + eth_hlen);
if (eth_type == ETH_P_IP) {
+ if (iph->protocol != IPPROTO_TCP) {
+ DP_NOTICE(p_hwfn,
+ "Unexpected ip protocol on ll2 %x\n",
+ iph->protocol);
+ return -EINVAL;
+ }
+
cm_info->local_ip[0] = ntohl(iph->daddr);
cm_info->remote_ip[0] = ntohl(iph->saddr);
cm_info->ip_version = TCP_IPV4;
@@ -1711,6 +1718,14 @@ qed_iwarp_parse_rx_pkt(struct qed_hwfn *p_hwfn,
*payload_len = ntohs(iph->tot_len) - ip_hlen;
} else if (eth_type == ETH_P_IPV6) {
ip6h = (struct ipv6hdr *)iph;
+
+ if (ip6h->nexthdr != IPPROTO_TCP) {
+ DP_NOTICE(p_hwfn,
+ "Unexpected ip protocol on ll2 %x\n",
+ iph->protocol);
+ return -EINVAL;
+ }
+
for (i = 0; i < 4; i++) {
cm_info->local_ip[i] =
ntohl(ip6h->daddr.in6_u.u6_addr32[i]);
--
2.9.5
^ permalink raw reply related
* [PATCH net 1/2] qed: Fix MPA unalign flow in case header is split across two packets.
From: Michal Kalderon @ 2018-03-14 12:49 UTC (permalink / raw)
To: michal.kalderon, davem
Cc: netdev, dledford, jgg, linux-rdma, Michal Kalderon, Ariel Elior
In-Reply-To: <1521031768-19131-1-git-send-email-Michal.Kalderon@cavium.com>
There is a corner case in the MPA unalign flow where a FPDU header is
split over two tcp segments. The length of the first fragment in this
case was not initialized properly and should be '1'
Fixes: c7d1d839 ("qed: Add support for MPA header being split over two tcp packets")
Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com>
Signed-off-by: Ariel Elior <Ariel.Elior@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed_iwarp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
index ca4a81d..fefe527 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
@@ -1928,8 +1928,8 @@ qed_iwarp_update_fpdu_length(struct qed_hwfn *p_hwfn,
/* Missing lower byte is now available */
mpa_len = fpdu->fpdu_length | *mpa_data;
fpdu->fpdu_length = QED_IWARP_FPDU_LEN_WITH_PAD(mpa_len);
- fpdu->mpa_frag_len = fpdu->fpdu_length;
/* one byte of hdr */
+ fpdu->mpa_frag_len = 1;
fpdu->incomplete_bytes = fpdu->fpdu_length - 1;
DP_VERBOSE(p_hwfn,
QED_MSG_RDMA,
--
2.9.5
^ permalink raw reply related
* [PATCH net 0/2] qed: iWARP related fixes
From: Michal Kalderon @ 2018-03-14 12:49 UTC (permalink / raw)
To: michal.kalderon, davem
Cc: netdev, dledford, jgg, linux-rdma, Michal Kalderon, Ariel Elior
This series contains two fixes related to iWARP flow.
Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com>
Signed-off-by: Ariel Elior <Ariel.Elior@cavium.com>
Michal Kalderon (2):
qed: Fix MPA unalign flow in case header is split across two packets.
qed: Fix non TCP packets should be dropped on iWARP ll2 connection
drivers/net/ethernet/qlogic/qed/qed_iwarp.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
--
2.9.5
^ permalink raw reply
* Re: [PATCH] net: dsa: drop some VLAs in switch.c
From: Salvatore Mesoraca @ 2018-03-14 12:48 UTC (permalink / raw)
To: David Laight
Cc: Vivien Didelot, linux-kernel@vger.kernel.org, Kernel Hardening,
netdev@vger.kernel.org, David S. Miller, Andrew Lunn,
Florian Fainelli, Kees Cook
In-Reply-To: <9843d23677af411aa75a5fb24df3c97b@AcuMS.aculab.com>
2018-03-14 12:24 GMT+01:00 David Laight <David.Laight@aculab.com>:
> Isn't using DECLARE_BITMAP() completely OTT when the maximum size is less
> than the number of bits in a word?
It allocates ceiling(size/8) "unsigned long"s, so yes.
^ permalink raw reply
* Re: [PATCH] can: enable multi-queue for SocketCAN devices
From: Marc Kleine-Budde @ 2018-03-14 12:44 UTC (permalink / raw)
To: Jonas Mark (BT-FIR/ENG1), Wolfgang Grandegger
Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, hs@denx.de,
ZHU Yi (BT-FIR/ENG1-Zhu)
In-Reply-To: <9ce65a3a1535456dbad7120c5775a8f9@de.bosch.com>
[-- Attachment #1.1: Type: text/plain, Size: 1243 bytes --]
On 03/14/2018 01:26 PM, Jonas Mark (BT-FIR/ENG1) wrote:
>> Do you have a driver or a patch to make a driver mq aware?
>
> Yes, we have CAN hardware with multiple queues and we also have a
> SocketCAN driver for it.
>
> IMHO the driver will be of very little use for the Linux community
> because the HW is proprietary.
That doesn't matter. It would be the first driver that makes use of the
feature, so we can learn from it. And you might get a free review of
your driver.
> Our motivation to separate this patch from the proprietary SocketCAN
> driver
Looking at your email address I assume your employer sells devices with
this hardware and the driver. So someone has to provide the sources for
it anyways to fulfil the GPL license requirements. :)
> is that we felt it was odd that the multi-queue infrastructure is in
> place and only very few lines of code are missing to offer it to the
> CAN networking subsystem as well.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] can: enable multi-queue for SocketCAN devices
From: Jonas Mark (BT-FIR/ENG1) @ 2018-03-14 12:26 UTC (permalink / raw)
To: Marc Kleine-Budde, Wolfgang Grandegger
Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, hs@denx.de,
ZHU Yi (BT-FIR/ENG1-Zhu), Jonas Mark (BT-FIR/ENG1)
Hello Marc,
> > The existing SocketCAN implementation provides alloc_candev() to
> > allocate a CAN device using a single Tx and Rx queue. This can lead to
> > priority inversion in case the single Tx queue is already full with low
> > priority messages and a high priority message needs to be sent while the
> > bus is fully loaded with medium priority messages.
> >
> > This problem can be solved by using the existing multi-queue support of
> > the network subsystem. The commit makes it possible to use multi-queue in
> > the CAN subsystem in the same way it is used in the Ethernet subsystem
> > by adding an alloc_candev_mqs() call and accompanying macros. With this
> > support a CAN device can use multi-queue qdisc (e.g. mqprio) to avoid
> > the aforementioned priority inversion.
> >
> > The existing functionality of alloc_candev() is the same as before.
> >
> > CAN devices need to have prioritized multiple hardware queues or are
> > able to abort waiting for arbitration to make sensible use of
> > multi-queues.
> >
> > Signed-off-by: Zhu Yi <yi.zhu5@cn.bosch.com>
> > Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
> > Reviewed-by: Heiko Schocher <hs@denx.de>
>
> Do you have a driver or a patch to make a driver mq aware?
Yes, we have CAN hardware with multiple queues and we also have a SocketCAN driver for it.
IMHO the driver will be of very little use for the Linux community because the HW is proprietary.
Our motivation to separate this patch from the proprietary SocketCAN driver is that we felt it was odd that the multi-queue infrastructure is in place and only very few lines of code are missing to offer it to the CAN networking subsystem as well.
Greetings,
Mark
^ permalink raw reply
* Re: pull-request: can 2018-03-14
From: Andri Yngvason @ 2018-03-14 12:24 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: netdev, davem, linux-can, kernel, wg
In-Reply-To: <20180314120523.2295-1-mkl@pengutronix.de>
On Wed, Mar 14, 2018 at 01:05:21PM +0100, Marc Kleine-Budde wrote:
> Hello David,
>
> this is a pull request of two patches for net/master.
>
> Both patches are by Andri Yngvason and fix problems in the cc770 driver,
> that show up quite fast on RT systems, but also on non RT setups.
>
> regards,
> Marc
>
> ---
>
> The following changes since commit f89782c2d131e6eae0d1ea2569ba76bc4c5875fe:
>
> qed: Use after free in qed_rdma_free() (2018-03-13 10:54:17 -0400)
>
> are available in the Git repository at:
>
> ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.16-20180314
>
> for you to fetch changes up to 746201235b3f876792099079f4c6fea941d76183:
>
> can: cc770: Fix queue stall & dropped RTR reply (2018-03-14 13:01:22 +0100)
>
> ----------------------------------------------------------------
> linux-can-fixes-for-4.16-20180314
>
> ----------------------------------------------------------------
> Andri Yngvason (2):
> can: cc770: Fix stalls on rt-linux, remove redundant IRQ ack
> can: cc770: Fix queue stall & dropped RTR reply
>
> drivers/net/can/cc770/cc770.c | 103 ++++++++++++++++++++++++++----------------
> drivers/net/can/cc770/cc770.h | 2 +
> 2 files changed, 65 insertions(+), 40 deletions(-)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Wait, do we not want Wolfgang's signed-off-by or reviewed-by?
Thanks,
Andri
^ permalink raw reply
* Re: [PATCH v4 net-next 3/6] net/ipv6: Add l3mdev check to ipv6_chk_addr_and_flags
From: Ido Schimmel @ 2018-03-14 12:25 UTC (permalink / raw)
To: David Ahern; +Cc: netdev
In-Reply-To: <20180313152941.31218-4-dsahern@gmail.com>
On Tue, Mar 13, 2018 at 08:29:38AM -0700, David Ahern wrote:
> Lookup the L3 master device for the passed in device. Only consider
> addresses on netdev's with the same master device. If the device is
> not enslaved or is NULL, then the l3mdev is NULL which means only
> devices not enslaved (ie, in the default domain) are considered.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Thanks for working on this. I also encountered this problem when working
on the selftests, but didn't have an immediate use case which required
me to use a non-linklocal address as a gateway.
^ permalink raw reply
* Re: [PATCH v4 net-next 2/6] net/ipv6: Change address check to always take a device argument
From: Ido Schimmel @ 2018-03-14 12:22 UTC (permalink / raw)
To: David Ahern; +Cc: netdev
In-Reply-To: <20180313152941.31218-3-dsahern@gmail.com>
On Tue, Mar 13, 2018 at 08:29:37AM -0700, David Ahern wrote:
> ipv6_chk_addr_and_flags determines if an address is a local address and
> optionally if it is an address on a specific device. For example, it is
> called by ip6_route_info_create to determine if a given gateway address
> is a local address. The address check currently does not consider L3
> domains and as a result does not allow a route to be added in one VRF
> if the nexthop points to an address in a second VRF. e.g.,
>
> $ ip route add 2001:db8:1::/64 vrf r2 via 2001:db8:102::23
> Error: Invalid gateway address.
>
> where 2001:db8:102::23 is an address on an interface in vrf r1.
>
> ipv6_chk_addr_and_flags needs to allow callers to always pass in a device
> with a separate argument to not limit the address to the specific device.
> The device is used used to determine the L3 domain of interest.
>
> To that end add an argument to skip the device check and update callers
> to always pass a device where possible and use the new argument to mean
> any address in the domain.
>
> Update a handful of users of ipv6_chk_addr with a NULL dev argument. This
> patch handles the change to these callers without adding the domain check.
>
> ip6_validate_gw needs to handle 2 cases - one where the device is given
> as part of the nexthop spec and the other where the device is resolved.
> There is at least 1 VRF case where deferring the check to only after
> the route lookup has resolved the device fails with an unintuitive error
> "RTNETLINK answers: No route to host" as opposed to the preferred
> "Error: Gateway can not be a local address." The 'no route to host'
> error is because of the fallback to a full lookup. The check is done
> twice to avoid this error.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Thanks for the detailed commit message.
^ permalink raw reply
* Re: [PATCH v4 net-next 1/6] net/ipv6: Refactor gateway validation on route add
From: Ido Schimmel @ 2018-03-14 12:21 UTC (permalink / raw)
To: David Ahern; +Cc: netdev
In-Reply-To: <20180313152941.31218-2-dsahern@gmail.com>
On Tue, Mar 13, 2018 at 08:29:36AM -0700, David Ahern wrote:
> Move gateway validation code from ip6_route_info_create into
> ip6_validate_gw. Code move plus adjustments to handle the potential
> reset of dev and idev and to make checkpatch happy.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
^ permalink raw reply
* Re: [PATCH v5 1/6] staging: fsl-dpaa2/ethsw: Add APIs for DPSW object
From: Greg KH @ 2018-03-14 12:17 UTC (permalink / raw)
To: Razvan Stefanescu
Cc: devel, arnd, netdev, alexandru.marginean, linux-kernel, agraf,
stuyoder, ioana.ciornei, laurentiu.tudor
In-Reply-To: <20180313135156.3322-2-razvan.stefanescu@nxp.com>
On Tue, Mar 13, 2018 at 08:51:51AM -0500, Razvan Stefanescu wrote:
> Add the command build/parse APIs for operating on DPSW objects through
> the DPAA2 Management Complex.
>
> Signed-off-by: Razvan Stefanescu <razvan.stefanescu@nxp.com>
> ---
> Changelog:
> v2:
> - use u8 for en parameter of dpsw_if_set_flooding/broadcast()
> v3:
> - no changes
> v4:
> - adjust to moving MC-bus out of staging
> - fix sparse warnings
> v5:
> - no changes
>
> drivers/staging/fsl-dpaa2/Kconfig | 8 +
> drivers/staging/fsl-dpaa2/Makefile | 1 +
> drivers/staging/fsl-dpaa2/ethsw/Makefile | 7 +
> drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h | 358 +++++++++
> drivers/staging/fsl-dpaa2/ethsw/dpsw.c | 1116 ++++++++++++++++++++++++++++
> drivers/staging/fsl-dpaa2/ethsw/dpsw.h | 579 +++++++++++++++
> 6 files changed, 2069 insertions(+)
> create mode 100644 drivers/staging/fsl-dpaa2/ethsw/Makefile
> create mode 100644 drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
> create mode 100644 drivers/staging/fsl-dpaa2/ethsw/dpsw.c
> create mode 100644 drivers/staging/fsl-dpaa2/ethsw/dpsw.h
>
> diff --git a/drivers/staging/fsl-dpaa2/Kconfig b/drivers/staging/fsl-dpaa2/Kconfig
> index dfff675..8a508ef 100644
> --- a/drivers/staging/fsl-dpaa2/Kconfig
> +++ b/drivers/staging/fsl-dpaa2/Kconfig
> @@ -16,3 +16,11 @@ config FSL_DPAA2_ETH
> ---help---
> Ethernet driver for Freescale DPAA2 SoCs, using the
> Freescale MC bus driver
> +
> +config FSL_DPAA2_ETHSW
> + tristate "Freescale DPAA2 Ethernet Switch"
> + depends on FSL_DPAA2
> + depends on NET_SWITCHDEV
> + ---help---
> + Driver for Freescale DPAA2 Ethernet Switch. Select
> + BRIDGE to have support for bridge tools.
> diff --git a/drivers/staging/fsl-dpaa2/Makefile b/drivers/staging/fsl-dpaa2/Makefile
> index 0836ba8..6cfd76b 100644
> --- a/drivers/staging/fsl-dpaa2/Makefile
> +++ b/drivers/staging/fsl-dpaa2/Makefile
> @@ -3,3 +3,4 @@
> #
>
> obj-$(CONFIG_FSL_DPAA2_ETH) += ethernet/
> +obj-$(CONFIG_FSL_DPAA2_ETHSW) += ethsw/
> diff --git a/drivers/staging/fsl-dpaa2/ethsw/Makefile b/drivers/staging/fsl-dpaa2/ethsw/Makefile
> new file mode 100644
> index 0000000..db137f7
> --- /dev/null
> +++ b/drivers/staging/fsl-dpaa2/ethsw/Makefile
> @@ -0,0 +1,7 @@
> +#
> +# Makefile for the Freescale DPAA2 Ethernet Switch
> +#
> +
> +obj-$(CONFIG_FSL_DPAA2_ETHSW) += dpaa2-ethsw.o
> +
> +dpaa2-ethsw-objs := dpsw.o
> diff --git a/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h b/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
> new file mode 100644
> index 0000000..36edef6
> --- /dev/null
> +++ b/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
> @@ -0,0 +1,358 @@
> +/* Copyright 2013-2016 Freescale Semiconductor Inc.
> + * Copyright 2017-2018 NXP
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * * Neither the name of the above-listed copyright holders nor the
> + * names of any contributors may be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
Can you resend this series and just use the correct SPDX identifiers for
all of the new files, instead of all of this horrid boiler-plate code?
That will save me time when I have to go delete all of this in the near
future :)
Also, why dual license it? Are you _SURE_ you want to do that, and are
totally aware of all of the crazy issues surrounding it? Hint, it
almost never means what you might think it does, and I have yet to know
of anyone doing anything "real" with any dual-licensed Linux kernel
code.
Remember, BSD is not the license you want/need for GPL header files that
are exposed to userspace...
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 7/7] ixgbevf: eliminate duplicate barriers on weakly-ordered archs
From: okaya @ 2018-03-14 12:13 UTC (permalink / raw)
To: Timur Tabi
Cc: netdev, sulrich, linux-arm-msm, linux-arm-kernel, Jeff Kirsher,
intel-wired-lan, linux-kernel
In-Reply-To: <12150aa0-77ba-878e-31f4-d4f8d6a28ccb@codeaurora.org>
On 2018-03-14 01:08, Timur Tabi wrote:
> On 3/13/18 10:20 PM, Sinan Kaya wrote:
>> +/* Assumes caller has executed a write barrier to order memory and
>> device
>> + * requests.
>> + */
>> static inline void ixgbevf_write_tail(struct ixgbevf_ring *ring, u32
>> value)
>> {
>> - writel(value, ring->tail);
>> + writel_relaxed(value, ring->tail);
>> }
>
> Why not put the wmb() in this function, or just get rid of the wmb()
> in the rest of the file and keep this as writel? That way, you can
> avoid the comment and the risk that comes with it.
Sure, both solutions will work. I want to see what the maintainer
prefers. I can repost accordingly.
^ permalink raw reply
* Re: [PATCH 3/7] RDMA/qedr: eliminate duplicate barriers on weakly-ordered archs
From: okaya @ 2018-03-14 12:06 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
Michal Kalderon, Ariel Elior, Doug Ledford, linux-rdma,
linux-kernel
In-Reply-To: <20180314041249.GA18314@ziepe.ca>
On 2018-03-14 00:12, Jason Gunthorpe wrote:
> On Tue, Mar 13, 2018 at 11:20:24PM -0400, Sinan Kaya wrote:
>> Code includes wmb() followed by writel() in multiple places. writel()
>> already has a barrier on some architectures like arm64.
>>
>> This ends up CPU observing two barriers back to back before executing
>> the
>> register write.
>>
>> Since code already has an explicit barrier call, changing writel() to
>> writel_relaxed().
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> drivers/infiniband/hw/qedr/verbs.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Sure matches my understanding of writel_relaxed
>
> This is part of a series, should we take just this patch through the
> rdma tree? If not:
>
> Acked-by: Jason Gunthorpe <jgg@mellanox.com>
Feel free to take pieces.
>
> Thanks,
> Jason
^ permalink raw reply
* pull-request: can 2018-03-14
From: Marc Kleine-Budde @ 2018-03-14 12:05 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel
Hello David,
this is a pull request of two patches for net/master.
Both patches are by Andri Yngvason and fix problems in the cc770 driver,
that show up quite fast on RT systems, but also on non RT setups.
regards,
Marc
---
The following changes since commit f89782c2d131e6eae0d1ea2569ba76bc4c5875fe:
qed: Use after free in qed_rdma_free() (2018-03-13 10:54:17 -0400)
are available in the Git repository at:
ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.16-20180314
for you to fetch changes up to 746201235b3f876792099079f4c6fea941d76183:
can: cc770: Fix queue stall & dropped RTR reply (2018-03-14 13:01:22 +0100)
----------------------------------------------------------------
linux-can-fixes-for-4.16-20180314
----------------------------------------------------------------
Andri Yngvason (2):
can: cc770: Fix stalls on rt-linux, remove redundant IRQ ack
can: cc770: Fix queue stall & dropped RTR reply
drivers/net/can/cc770/cc770.c | 103 ++++++++++++++++++++++++++----------------
drivers/net/can/cc770/cc770.h | 2 +
2 files changed, 65 insertions(+), 40 deletions(-)
^ permalink raw reply
* [PATCH 2/2] can: cc770: Fix queue stall & dropped RTR reply
From: Marc Kleine-Budde @ 2018-03-14 12:05 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Andri Yngvason, linux-stable,
Marc Kleine-Budde
In-Reply-To: <20180314120523.2295-1-mkl@pengutronix.de>
From: Andri Yngvason <andri.yngvason@marel.com>
While waiting for the TX object to send an RTR, an external message with a
matching id can overwrite the TX data. In this case we must call the rx
routine and then try transmitting the message that was overwritten again.
The queue was being stalled because the RX event did not generate an
interrupt to wake up the queue again and the TX event did not happen
because the TXRQST flag is reset by the chip when new data is received.
According to the CC770 datasheet the id of a message object should not be
changed while the MSGVAL bit is set. This has been fixed by resetting the
MSGVAL bit before modifying the object in the transmit function and setting
it after. It is not enough to set & reset CPUUPD.
It is important to keep the MSGVAL bit reset while the message object is
being modified. Otherwise, during RTR transmission, a frame with matching
id could trigger an rx-interrupt, which would cause a race condition
between the interrupt routine and the transmit function.
Signed-off-by: Andri Yngvason <andri.yngvason@marel.com>
Tested-by: Richard Weinberger <richard@nod.at>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/cc770/cc770.c | 94 ++++++++++++++++++++++++++++++-------------
drivers/net/can/cc770/cc770.h | 2 +
2 files changed, 68 insertions(+), 28 deletions(-)
diff --git a/drivers/net/can/cc770/cc770.c b/drivers/net/can/cc770/cc770.c
index 9fed163262e0..2743d82d4424 100644
--- a/drivers/net/can/cc770/cc770.c
+++ b/drivers/net/can/cc770/cc770.c
@@ -390,37 +390,23 @@ static int cc770_get_berr_counter(const struct net_device *dev,
return 0;
}
-static netdev_tx_t cc770_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static void cc770_tx(struct net_device *dev, int mo)
{
struct cc770_priv *priv = netdev_priv(dev);
- struct net_device_stats *stats = &dev->stats;
- struct can_frame *cf = (struct can_frame *)skb->data;
- unsigned int mo = obj2msgobj(CC770_OBJ_TX);
+ struct can_frame *cf = (struct can_frame *)priv->tx_skb->data;
u8 dlc, rtr;
u32 id;
int i;
- if (can_dropped_invalid_skb(dev, skb))
- return NETDEV_TX_OK;
-
- if ((cc770_read_reg(priv,
- msgobj[mo].ctrl1) & TXRQST_UNC) == TXRQST_SET) {
- netdev_err(dev, "TX register is still occupied!\n");
- return NETDEV_TX_BUSY;
- }
-
- netif_stop_queue(dev);
-
dlc = cf->can_dlc;
id = cf->can_id;
- if (cf->can_id & CAN_RTR_FLAG)
- rtr = 0;
- else
- rtr = MSGCFG_DIR;
+ rtr = cf->can_id & CAN_RTR_FLAG ? 0 : MSGCFG_DIR;
+
+ cc770_write_reg(priv, msgobj[mo].ctrl0,
+ MSGVAL_RES | TXIE_RES | RXIE_RES | INTPND_RES);
cc770_write_reg(priv, msgobj[mo].ctrl1,
RMTPND_RES | TXRQST_RES | CPUUPD_SET | NEWDAT_RES);
- cc770_write_reg(priv, msgobj[mo].ctrl0,
- MSGVAL_SET | TXIE_SET | RXIE_RES | INTPND_RES);
+
if (id & CAN_EFF_FLAG) {
id &= CAN_EFF_MASK;
cc770_write_reg(priv, msgobj[mo].config,
@@ -439,13 +425,30 @@ static netdev_tx_t cc770_start_xmit(struct sk_buff *skb, struct net_device *dev)
for (i = 0; i < dlc; i++)
cc770_write_reg(priv, msgobj[mo].data[i], cf->data[i]);
- /* Store echo skb before starting the transfer */
- can_put_echo_skb(skb, dev, 0);
-
cc770_write_reg(priv, msgobj[mo].ctrl1,
- RMTPND_RES | TXRQST_SET | CPUUPD_RES | NEWDAT_UNC);
+ RMTPND_UNC | TXRQST_SET | CPUUPD_RES | NEWDAT_UNC);
+ cc770_write_reg(priv, msgobj[mo].ctrl0,
+ MSGVAL_SET | TXIE_SET | RXIE_SET | INTPND_UNC);
+}
+
+static netdev_tx_t cc770_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ struct cc770_priv *priv = netdev_priv(dev);
+ unsigned int mo = obj2msgobj(CC770_OBJ_TX);
+
+ if (can_dropped_invalid_skb(dev, skb))
+ return NETDEV_TX_OK;
- stats->tx_bytes += dlc;
+ netif_stop_queue(dev);
+
+ if ((cc770_read_reg(priv,
+ msgobj[mo].ctrl1) & TXRQST_UNC) == TXRQST_SET) {
+ netdev_err(dev, "TX register is still occupied!\n");
+ return NETDEV_TX_BUSY;
+ }
+
+ priv->tx_skb = skb;
+ cc770_tx(dev, mo);
return NETDEV_TX_OK;
}
@@ -671,13 +674,47 @@ static void cc770_tx_interrupt(struct net_device *dev, unsigned int o)
struct cc770_priv *priv = netdev_priv(dev);
struct net_device_stats *stats = &dev->stats;
unsigned int mo = obj2msgobj(o);
+ struct can_frame *cf;
+ u8 ctrl1;
+
+ ctrl1 = cc770_read_reg(priv, msgobj[mo].ctrl1);
- /* Nothing more to send, switch off interrupts */
cc770_write_reg(priv, msgobj[mo].ctrl0,
MSGVAL_RES | TXIE_RES | RXIE_RES | INTPND_RES);
+ cc770_write_reg(priv, msgobj[mo].ctrl1,
+ RMTPND_RES | TXRQST_RES | MSGLST_RES | NEWDAT_RES);
- stats->tx_packets++;
+ if (unlikely(!priv->tx_skb)) {
+ netdev_err(dev, "missing tx skb in tx interrupt\n");
+ return;
+ }
+
+ if (unlikely(ctrl1 & MSGLST_SET)) {
+ stats->rx_over_errors++;
+ stats->rx_errors++;
+ }
+
+ /* When the CC770 is sending an RTR message and it receives a regular
+ * message that matches the id of the RTR message, it will overwrite the
+ * outgoing message in the TX register. When this happens we must
+ * process the received message and try to transmit the outgoing skb
+ * again.
+ */
+ if (unlikely(ctrl1 & NEWDAT_SET)) {
+ cc770_rx(dev, mo, ctrl1);
+ cc770_tx(dev, mo);
+ return;
+ }
+
+ can_put_echo_skb(priv->tx_skb, dev, 0);
can_get_echo_skb(dev, 0);
+
+ cf = (struct can_frame *)priv->tx_skb->data;
+ stats->tx_bytes += cf->can_dlc;
+ stats->tx_packets++;
+
+ priv->tx_skb = NULL;
+
netif_wake_queue(dev);
}
@@ -789,6 +826,7 @@ struct net_device *alloc_cc770dev(int sizeof_priv)
priv->can.do_set_bittiming = cc770_set_bittiming;
priv->can.do_set_mode = cc770_set_mode;
priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
+ priv->tx_skb = NULL;
memcpy(priv->obj_flags, cc770_obj_flags, sizeof(cc770_obj_flags));
diff --git a/drivers/net/can/cc770/cc770.h b/drivers/net/can/cc770/cc770.h
index a1739db98d91..95752e1d1283 100644
--- a/drivers/net/can/cc770/cc770.h
+++ b/drivers/net/can/cc770/cc770.h
@@ -193,6 +193,8 @@ struct cc770_priv {
u8 cpu_interface; /* CPU interface register */
u8 clkout; /* Clock out register */
u8 bus_config; /* Bus conffiguration register */
+
+ struct sk_buff *tx_skb;
};
struct net_device *alloc_cc770dev(int sizeof_priv);
--
2.16.1
^ permalink raw reply related
* [PATCH 1/2] can: cc770: Fix stalls on rt-linux, remove redundant IRQ ack
From: Marc Kleine-Budde @ 2018-03-14 12:05 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Andri Yngvason, linux-stable,
Marc Kleine-Budde
In-Reply-To: <20180314120523.2295-1-mkl@pengutronix.de>
From: Andri Yngvason <andri.yngvason@marel.com>
This has been reported to cause stalls on rt-linux.
Suggested-by: Richard Weinberger <richard@nod.at>
Tested-by: Richard Weinberger <richard@nod.at>
Signed-off-by: Andri Yngvason <andri.yngvason@marel.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/cc770/cc770.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/drivers/net/can/cc770/cc770.c b/drivers/net/can/cc770/cc770.c
index 1e37313054f3..9fed163262e0 100644
--- a/drivers/net/can/cc770/cc770.c
+++ b/drivers/net/can/cc770/cc770.c
@@ -447,15 +447,6 @@ static netdev_tx_t cc770_start_xmit(struct sk_buff *skb, struct net_device *dev)
stats->tx_bytes += dlc;
-
- /*
- * HM: We had some cases of repeated IRQs so make sure the
- * INT is acknowledged I know it's already further up, but
- * doing again fixed the issue
- */
- cc770_write_reg(priv, msgobj[mo].ctrl0,
- MSGVAL_UNC | TXIE_UNC | RXIE_UNC | INTPND_RES);
-
return NETDEV_TX_OK;
}
@@ -684,12 +675,6 @@ static void cc770_tx_interrupt(struct net_device *dev, unsigned int o)
/* Nothing more to send, switch off interrupts */
cc770_write_reg(priv, msgobj[mo].ctrl0,
MSGVAL_RES | TXIE_RES | RXIE_RES | INTPND_RES);
- /*
- * We had some cases of repeated IRQ so make sure the
- * INT is acknowledged
- */
- cc770_write_reg(priv, msgobj[mo].ctrl0,
- MSGVAL_UNC | TXIE_UNC | RXIE_UNC | INTPND_RES);
stats->tx_packets++;
can_get_echo_skb(dev, 0);
--
2.16.1
^ permalink raw reply related
* Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns
From: Yuval Shaia @ 2018-03-14 12:03 UTC (permalink / raw)
To: Liran Alon; +Cc: davem, netdev, linux-kernel, idan.brown, sharon.s.liu
In-Reply-To: <20180313161345.GC4023@yuvallap>
On Tue, Mar 13, 2018 at 06:13:45PM +0200, Yuval Shaia wrote:
> On Tue, Mar 13, 2018 at 05:07:22PM +0200, Liran Alon wrote:
> > Before this commit, dev_forward_skb() always cleared packet's
> > per-network-namespace info. Even if the packet doesn't cross
> > network namespaces.
> >
> > The comment above dev_forward_skb() describes that this is done
> > because the receiving device may be in another network namespace.
> > However, this case can easily be tested for and therefore we can
> > scrub packet's per-network-namespace info only when receiving device
> > is indeed in another network namespace.
> >
> > Therefore, this commit changes ____dev_forward_skb() to tell
> > skb_scrub_packet() that skb has crossed network-namespace only in case
> > transmitting device (skb->dev) network namespace is different then
> > receiving device (dev) network namespace.
> >
> > An example of a netdev that use skb_forward_skb() is veth.
> > Thus, before this commit a packet transmitted from one veth peer to
> > another when both veth peers are on same network namespace will lose
> > it's skb->mark. The bug could easily be demonstrated by the following:
> >
> > ip netns add test
> > ip netns exec test bash
> > ip link add veth-a type veth peer name veth-b
> > ip link set veth-a up
> > ip link set veth-b up
> > ip addr add dev veth-a 12.0.0.1/24
> > tc qdisc add dev veth-a root handle 1 prio
> > tc qdisc add dev veth-b ingress
> > tc filter add dev veth-a parent 1: u32 match u32 0 0 action skbedit mark 1337
> > tc filter add dev veth-b parent ffff: basic match 'meta(nf_mark eq 1337)' action simple "skb->mark 1337!"
> > dmesg -C
> > ping 12.0.0.2
> > dmesg
> >
> > Before this change, the above will print nothing to dmesg.
> > After this change, "skb->mark 1337!" will be printed as necessary.
>
> Hi Liran,
>
> >
> > Signed-off-by: Liran Alon <liran.alon@oracle.com>
> > Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
> > Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
>
> I did not earned the credits for SOB, only r-b.
Had an offlist conversation with Liran,
Turns out that this SOB is ok.
Yuval
>
> Yuval
>
> > ---
> > include/linux/netdevice.h | 2 +-
> > net/core/dev.c | 6 +++---
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 5eef6c8e2741..5908f1e31ee2 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -3371,7 +3371,7 @@ static __always_inline int ____dev_forward_skb(struct net_device *dev,
> > return NET_RX_DROP;
> > }
> >
> > - skb_scrub_packet(skb, true);
> > + skb_scrub_packet(skb, !net_eq(dev_net(dev), dev_net(skb->dev)));
> > skb->priority = 0;
> > return 0;
> > }
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 2cedf520cb28..087787dd0a50 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -1877,9 +1877,9 @@ int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
> > * start_xmit function of one device into the receive queue
> > * of another device.
> > *
> > - * The receiving device may be in another namespace, so
> > - * we have to clear all information in the skb that could
> > - * impact namespace isolation.
> > + * The receiving device may be in another namespace.
> > + * In that case, we have to clear all information in the
> > + * skb that could impact namespace isolation.
> > */
> > int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
> > {
> > --
> > 1.9.1
> >
^ permalink raw reply
* RE: [PATCH v3] kernel.h: Skip single-eval logic on literals in min()/max()
From: David Laight @ 2018-03-14 11:35 UTC (permalink / raw)
To: 'Kees Cook', Andrew Morton
Cc: Linus Torvalds, Linux Kernel Mailing List, Josh Poimboeuf,
Rasmus Villemoes, Gustavo A. R. Silva, Tobin C. Harding,
Steven Rostedt, Jonathan Corbet, Chris Mason, Josef Bacik,
David Sterba, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Ingo Molnar, Peter Zijlstra, Thomas Gleixner,
Masahiro Yamada, Borislav Petkov, Randy
In-Reply-To: <CAGXu5j+JUfvYCej17Gx__3Vhu4JmimddrSgV89c2tkfwshH_Mw@mail.gmail.com>
From: Kees Cook
> Sent: 13 March 2018 22:15
...
> I'll send a "const_max()" which will refuse to work on
> non-constant-values (so it doesn't get accidentally used on variables
> that could be exposed to double-evaluation), and will work for stack
> array declarations (to avoid the overly-sensitive -Wvla checks).
ISTR the definitions were of the form:
char foo[max(sizeof (struct bah), sizeof (struct baz))];
This doesn't generate a 'foo' with the required alignment.
It would be much better to use a union.
David
^ permalink raw reply
* RE: [PATCH] net: dsa: drop some VLAs in switch.c
From: David Laight @ 2018-03-14 11:24 UTC (permalink / raw)
To: 'Salvatore Mesoraca', Vivien Didelot
Cc: linux-kernel@vger.kernel.org, Kernel Hardening,
netdev@vger.kernel.org, David S. Miller, Andrew Lunn,
Florian Fainelli, Kees Cook
In-Reply-To: <CAJHCu1JCCkkoL92tL+Zpx6te1A=-B1dNbAFu4sFEHiHa4vB6BQ@mail.gmail.com>
From: Salvatore Mesoraca
> Sent: 13 March 2018 22:01
> 2018-03-13 20:58 GMT+01:00 Vivien Didelot <vivien.didelot@savoirfairelinux.com>:
> > Hi Salvatore,
>
> Hi Vivien,
>
> > Salvatore Mesoraca <s.mesoraca16@gmail.com> writes:
> >
> >> dsa_switch's num_ports is currently fixed to DSA_MAX_PORTS. So we avoid
> >> 2 VLAs[1] by using DSA_MAX_PORTS instead of ds->num_ports.
> >>
> >> [1] https://lkml.org/lkml/2018/3/7/621
> >>
> >> Signed-off-by: Salvatore Mesoraca <s.mesoraca16@gmail.com>
> >
> > NAK.
> >
> > We are in the process to remove hardcoded limits such as DSA_MAX_PORTS
> > and DSA_MAX_SWITCHES, so we have to stick with ds->num_ports.
>
> I can rewrite the patch using kmalloc.
> Although, if ds->num_ports will always be less than or equal to 12, it
> should be better to
> just use DSA_MAX_PORTS.
Isn't using DECLARE_BITMAP() completely OTT when the maximum size is less
than the number of bits in a word?
David
^ 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