Hi Neil,
Neil Horman wrote:
> Update e1000 driver to not allow dma beyond the end of the allocated skb
>
> Signed-off-by: Neil Horman <[email protected]>
>
>
> e1000_main.c | 34 +++++++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
>
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index 7e855f9..7600deb 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -1667,6 +1667,19 @@ int e1000_setup_all_rx_resources(struct e1000_adapter
> *adapter)
> return err;
> }
>
> +static inline u32 normalize_rx_len(u32 len)
> +{
> + u32 match, last_match;
> +
>
Skip newline and get rid of last_match. Also, there is a whitespace error...
> +
> + for (match = 0x100; match <= 0x4000; match *= 2) {
> + if (len <= match)
> + return match;
> + }
> +
>
> + return 0;
> +}
>
You should return (match >> 1), which is the largest size possible. (Which
is exactly what you need here).
> +
> /**
> * e1000_setup_rctl - configure the receive control registers
> * @adapter: Board private structure
> @@ -1675,6 +1688,7 @@ static void e1000_setup_rctl(struct e1000_adapter
> *adapter)
> {
> struct e1000_hw *hw = &adapter->hw;
> u32 rctl;
> + u32 normed_rx_len;
>
> rctl = er32(RCTL);
>
> @@ -1697,7 +1711,25 @@ static void e1000_setup_rctl(struct e1000_adapter
> *adapter)
> /* Setup buffer sizes */
> rctl &= ~E1000_RCTL_SZ_4096;
> rctl |= E1000_RCTL_BSEX;
> - switch (adapter->rx_buffer_len) {
> +
> + /*
> + * We need to normalize the rx_buffer_len here
> + * since the hardware only knows about 7 discrete
> + * frame lengths here. To accomodate that we need
> + * to set the rx length in the hardware to the next highest
> + * size over the rx_buffer_len, then increase rx_buffer_len
> + * to match it, so that we can get a full mtu sized frame
> + */
> + normed_rx_len = normalize_rx_len(adapter->rx_buffer_len);
> +
> + if (!normed_rx_len) {
> + printk(KERN_ERR "No valid rx len found, assume 2048\n");
> + normed_rx_len = 0x800;
> + }
> +
> + adapter->rx_buffer_len = normed_rx_len;
>
If you modify rx_buffer_len anyway, then get rid of normed_rx_len and
do a quick
adapter->rx_buffer_len = normalize_rx_len(adapter->rx_buffer_len);
instead. With the modification above, it never fails, so no need to check
for !normed_rx_len.
But I don't really know the context of this change. Is it okay to shorten
rx_buffer_len here? Why was it not set appropriately as the driver
expects?
Oh, BTW, the default case in the switch statement is stupid and should
be removed.
> +
> + switch (normed_rx_len) {
> case E1000_RXBUFFER_256:
> rctl |= E1000_RCTL_SZ_256;
> rctl &= ~E1000_RCTL_BSEX;
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Franco
------------------------------------------------------------------------------
Join us December 9, 2009 for the Red Hat Virtual Experience,
a free event focused on virtualization and cloud computing.
Attend in-depth sessions from your desk. Your couch. Anywhere.
http://p.sf.net/sfu/redhat-sfdev2dev
_______________________________________________
E1000-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/e1000-devel