netdev - Re: [PATCH v6 1/9] bgmac: simplify tx ring index handling

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACna6ryBVXS0-bC_L7KnSr_BBRLMekYJ9-p+zF_a5-hHKewbKQ@mail.gmail.com>
Date:	Tue, 14 Apr 2015 07:38:47 +0200
From:	Rafał Miłecki <zajec5@...il.com>
To:	Felix Fietkau <nbd@...nwrt.org>
Cc:	Network Development <netdev@...r.kernel.org>,
	Hauke Mehrtens <hauke@...ke-m.de>
Subject: Re: [PATCH v6 1/9] bgmac: simplify tx ring index handling

On 14 April 2015 at 01:42, Felix Fietkau <nbd@...nwrt.org> wrote:
> @@ -158,13 +157,7 @@ static netdev_tx_t bgmac_dma_tx_add(struct bgmac *bgmac,
>                 skb_checksum_help(skb);
>
>         nr_frags = skb_shinfo(skb)->nr_frags;
> -
> -       if (ring->start <= ring->end)
> -               free_slots = ring->start - ring->end + BGMAC_TX_RING_SLOTS;
> -       else
> -               free_slots = ring->start - ring->end;
> -
> -       if (free_slots <= nr_frags + 1) {
> +       if (ring->end - ring->start + nr_frags + 1 >= BGMAC_TX_RING_SLOTS) {
>                 bgmac_err(bgmac, "TX ring is full, queue should be stopped!\n");
>                 netif_stop_queue(net_dev);
>                 return NETDEV_TX_BUSY;

Regarding my previous report if following problem:
ring->end:0x00 ring->start:0xfd (ring->end - ring->start + nr_frags + 1):-252
ring->end:0x00 ring->start:0xff (ring->end - ring->start + nr_frags + 1):-254
ring->end:0x00 ring->start:0xff (ring->end - ring->start + nr_frags + 1):-254
ring->end:0x00 ring->start:0xff (ring->end - ring->start + nr_frags + 1):-254
ring->end:0x00 ring->start:0xfe (ring->end - ring->start + nr_frags + 1):-253
ring->end:0x00 ring->start:0xff (ring->end - ring->start + nr_frags + 1):-254
ring->end:0x00 ring->start:0xfe (ring->end - ring->start + nr_frags + 1):-253
ring->end:0x00 ring->start:0xfd (ring->end - ring->start + nr_frags + 1):-252
ring->end:0x00 ring->start:0xfd (ring->end - ring->start + nr_frags + 1):-252
(with hacked *_RING_SLOTS and u8 start, end;)

My hacked code didn't work, because using u8 killed your trick with
using the way of treating numbers (32b limit) in condition.

When using u32 your trick seems to work:
bgmac: [bgmac_dma_tx_add] ring->end:0x00000000 ring->start:0xfffffff9
bgmac: [bgmac_dma_tx_add] (ring->end - ring->start + nr_frags + 1):8

bgmac: [bgmac_dma_tx_add] ring->end:0x00000001 ring->start:0xfffffff9
bgmac: [bgmac_dma_tx_add] (ring->end - ring->start + nr_frags + 1):9

bgmac: [bgmac_dma_tx_add] ring->end:0x00000002 ring->start:0xfffffff9
bgmac: [bgmac_dma_tx_add] (ring->end - ring->start + nr_frags + 1):10

If you'll be sending V7, what about adding something like
/* Ring start and end are u32, subtraction gives number of free slots */
Would that be OK? To point this way the code is OK and help understand why?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html