netdev - Re: [PATCH 2/2] net, can, ifi: loopback Tx message in IFI block

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 6 Feb 2018 13:50:21 +0100
From:   Marek Vasut <marex@...x.de>
To:     Heiko Schocher <hs@...x.de>, linux-can@...r.kernel.org
Cc:     Markus Marb <markus@...b.org>, netdev@...r.kernel.org,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        linux-kernel@...r.kernel.org,
        Wolfgang Grandegger <wg@...ndegger.com>
Subject: Re: [PATCH 2/2] net, can, ifi: loopback Tx message in IFI block

On 02/06/2018 01:02 PM, Heiko Schocher wrote:
> Current ifi driver reads first Rx messages, than loopback
> the Tx message, if the IFI_CANFD_INTERRUPT_TXFIFO_REMOVE
> bit is set. This can lead into the case, that Rx messages
> overhelm Tx messages!
> 
> Fixed this in the following way:
> 
> Set in the IFI_CANFD_TXFIFO_DLC register the FN value to
> 1, so the IFI block loopsback itself the Tx message when
> sended correctly on the canfd bus. Only the IFI block can
> insert the Tx message in the correct place.
> 
> The linux driver now needs only to free the skb, when
> the Tx message was sended correctly.
> 
> Signed-off-by: Heiko Schocher <hs@...x.de>
> ---
> 
>  drivers/net/can/ifi_canfd/ifi_canfd.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c
> index 05feb8177936..0d36cb8659ae 100644
> --- a/drivers/net/can/ifi_canfd/ifi_canfd.c
> +++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
> @@ -211,6 +211,7 @@ struct ifi_canfd_priv {
>  	struct napi_struct	napi;
>  	struct net_device	*ndev;
>  	void __iomem		*base;
> +	unsigned int		tx_len;
>  };
>  
>  static void ifi_canfd_irq_enable(struct net_device *ndev, bool enable)
> @@ -617,8 +618,10 @@ static irqreturn_t ifi_canfd_isr(int irq, void *dev_id)
>  
>  	/* TX IRQ */
>  	if (isr & IFI_CANFD_INTERRUPT_TXFIFO_REMOVE) {
> -		stats->tx_bytes += can_get_echo_skb(ndev, 0);
> +		can_free_echo_skb(ndev, 0);
> +		stats->tx_bytes += priv->tx_len;
>  		stats->tx_packets++;
> +		priv->tx_len = 0;
>  		can_led_event(ndev, CAN_LED_EVENT_TX);
>  	}
>  
> @@ -889,6 +892,7 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
>  	}
>  
>  	txdlc = can_len2dlc(cf->len);
> +	priv->tx_len = txdlc;
>  	if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb)) {
>  		txdlc |= IFI_CANFD_TXFIFO_DLC_EDL;
>  		if (cf->flags & CANFD_BRS)
> @@ -898,6 +902,12 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
>  	if (cf->can_id & CAN_RTR_FLAG)
>  		txdlc |= IFI_CANFD_TXFIFO_DLC_RTR;
>  
> +	/*
> +	 * set FNR to 1, so we get our Tx Message looped back
> +	 * into RxFIFO
> +	 */

Nit, you can make this into a single-line comment ;-)

Otherwise,

Reviewed-by: Marek Vasut <marex@...x.de>

> +	txdlc += (1 << IFI_CANFD_TXFIFO_DLC_FNR_OFFSET);
> +
>  	/* message ram configuration */
>  	writel(txid, priv->base + IFI_CANFD_TXFIFO_ID);
>  	writel(txdlc, priv->base + IFI_CANFD_TXFIFO_DLC);
> 


-- 
Best regards,
Marek Vasut