Re: [PATCH] [v7] net: emac: emac gigabit ethernet controller driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Florian Fainelli wrote:

nr_frags can't be bigger than MAX_SKB_FRAGS, hence these checks all
other drivers do against 1 + MAX_SKB_FRAGS.

Doh, I just realized something. emac_mac_tx_buf_send() just needs to make sure that there's enough room for ONE skb. For some reason I thought it had to make sure there's enough room for multiple SKBs.

Now it makes a lot more sense.  Thank you.

So it looks like a given SKB can occupy 3 + nr_frags descriptors. So I need to change that line to:

	if (emac_tpd_num_free_descs(tx_q) < (MAX_SKB_FRAGS + 3))
		netif_stop_queue(adpt->netdev);

Question, some  drivers do <= instead of just <, like this:

	if (ring->free_bds <= (MAX_SKB_FRAGS + 1))
		netif_tx_stop_queue(txq);

Is it necessary to stop the queue if there exactly enough descriptors to hold an SKB? Shouldn't the above be this instead:

	if (ring->free_bds < (MAX_SKB_FRAGS + 1))
		netif_tx_stop_queue(txq);


However, I'm confused about one thing.  Almost every other driver just
sets "netdev->mtu = new_mtu" and does nothing else.  I can't find any
other driver that actually stops the RX path, reprograms the hardware,
and then restarts the RX path.  I know this is a stupid question, but
why is my driver doing that?

Most drivers allocate RX buffer sizes that are usually bigger than the
MTU, but would probably silently fail or expose transient behavior once
the MTU changes to greater than the size pre-defined.

So it looks like the real problem is a race condition between

	adpt->rxbuf_size = new_mtu > EMAC_DEF_RX_BUF_SIZE ?
		ALIGN(max_frame, 8) : EMAC_DEF_RX_BUF_SIZE;

and

	if (netif_running(netdev))
		return emac_reinit_locked(adpt);


That is, if the interface is running, I set rxbuf_size. If suddenly I receive some packets, then the driver will use the wrong buffer size.

Is there an easy way for me to stop the RX path before I set rxbuf_size? Some netif_xxx function I can call?


--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux