Re: [PATCH v4] net: add Faraday FTMAC100 10/100 Ethernet driver

Dear Michał,

2011/1/25 Michał Mirosław [off-list ref]:

W dniu 24 stycznia 2011 09:26 użytkownik Po-Yu Chuang
[off-list ref] napisał:

quoted

2011/1/21 Michał Mirosław [off-list ref]:

quoted

Did you test this? This looks like it will result in double free after
packet RX, as you are giving the same page (referenced once) to two
distinct RX descriptors, that may be assigned different packets.

Yes, this is tested.

quoted

Since your not implementing any RX offloads, you might just allocate
fresh skb's with alloc_skb() and store skb pointer in rxdes3. Since

rxdes3 does not store virtual address of an skb.
It stores the address of the buffer allocated while open() and freed
only when stop().
The data in that buffer will be memcpy()ed to an skb allocated in
ftmac100_rx_packet().
No double free happens.

Ah, I blindly assumed that you're just appending the buffers to the
skb (using skb_fill_page_desc() and friends). Since you have to mark
descriptors for the device anyway, it might be faster to allocate new
skbs and map those as rx buffers (changing the descriptor's buffer
address after every RX) instead of keeping static buffer and copying
every time. (For small packets it wastes lot of memory, though - so
the right choice depends on the expected workload.)

The reason I chose to use memcpy rx buffer to skb is that I didn't know
how to deal with multi-segment packets if I preallocated skb for each
rx descriptor. This function seems to be what I need.
Let me rework this. Thanks.

best regards,
Po-Yu Chuang