* Re: [PATCH net 0/5] Implement ndo_gso_check() for vxlan nics
From: Or Gerlitz @ 2014-11-06 8:23 UTC (permalink / raw)
To: Tom Herbert
Cc: David Miller, Joe Stringer, Linux Netdev List, Sathya Perla,
Jeff Kirsher, linux.nics, Amir Vadai, shahed.shaikh,
dept-gelinuxnicdev, LKML
In-Reply-To: <CA+mtBx-Js62X3BpfjQkcsh=w=-cgvSyM5ueCpufGVv0uu1nphw@mail.gmail.com>
On Thu, Nov 6, 2014 at 4:44 AM, Tom Herbert <therbert@google.com> wrote:
> On Wed, Nov 5, 2014 at 6:15 PM, David Miller <davem@davemloft.net> wrote:
>> From: Joe Stringer <joestringer@nicira.com>
>> Date: Wed, 5 Nov 2014 17:06:46 -0800
>>
>>> My impression was that the changes are more likely to be
>>> hardware-specific (like the i40e changes) rather than software-specific,
>>> like changes that might be integrated into the helper.
>>
>> I think there is more commonality amongst hardware capabilities,
>> and this is why I want the helper to play itself out.
>>
>>> That said, I can rework for one helper. The way I see it would be the
>>> same code as these patches, as "vxlan_gso_check(struct sk_buff *)" in
>>> drivers/net/vxlan.c which would be called from each driver. Is that what
>>> you had in mind?
>>
>> Yes.
>
> Note that this code is not VXLAN specific, it will also accept NVGRE
> and GRE/UDP with keyid and TEB. I imagine all these cases should be
> indistinguishable to the hardware so they probably just work (which
> would be cool!). It might be better to name and locate the helper
> function to reflect that.
Unlike the VXLAN case, currently there's no indication from the
network stack towards the driver that an NVGRE tunnel was set, so in
our case we're not arming the HW offloads for NVGRE. I'll look into
that, maybe we can make them work always. Also re the math there to be
the same for VXLAN/NVGRE -- skb_inner_mac_header(skb) -
skb_transport_header(skb) is exactly 8 (sizeof struct gre_base_hdr),
isn't that?
^ permalink raw reply
* Re: M_CAN message RAM initialization AppNote - was: Re: [PATCH V3 3/3] can: m_can: workaround for transmit data less than 4 bytes
From: Dong Aisheng @ 2014-11-06 8:09 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Marc Kleine-Budde, linux-can, wg, varkabhadram, netdev,
linux-arm-kernel
In-Reply-To: <545B1D71.4000408@hartkopp.net>
On Thu, Nov 06, 2014 at 08:04:17AM +0100, Oliver Hartkopp wrote:
> On 06.11.2014 02:57, Dong Aisheng wrote:
> >On Wed, Nov 05, 2014 at 07:15:10PM +0100, Oliver Hartkopp wrote:
>
> >>The Message RAM is usually equipped with a parity or ECC functionality.
> >>But RAM cells suffer a hardware reset and can therefore hold
> >>arbitrary content at startup - including parity and/or ECC bits.
> >>
> >>So when you write only the CAN ID and the first four bytes the last
> >>four bytes remain untouched. Then the M_CAN starts to read in 32bit
> >>words from the start of the Tx Message element. So it is very likely
> >>to trigger the message RAM error when reading the uninitialized
> >>32bit word from the last four bytes.
> >>
> >>Finally it turns out that an initial writing (with any kind of data)
> >>to the entire message RAM is mandatory to create valid parity/ECC
> >>checksums.
> >>
> >>That's it.
> >>
> >
> >Thanks for sharing this information.
> >Does it mean this issue is related to the nature of Message RAM and is
> >supposed to exist on all M_CAN IP versions?
>
> From what I know from the 3.1.x revision there's no change regarding
> IR.BRU and IR.BEC - so I would assume this to stay on all M_CAN IP
> revisions.
>
> But after some sleep I wonder if this patch [3/3] would need an update too.
>
> Writing to the TX message RAM is obviously no workaround but a valid
> and needed initialization process.
>
> I would tend to make this patch:
>
> ---
>
> can: m_can: add missing TX message RAM initialization
>
> The M_CAN message RAM is usually equipped with a parity or ECC functionality.
> But RAM cells suffer a hardware reset and can therefore hold
> arbitrary content at startup - including parity and/or ECC bits.
>
> To prevent the M_CAN controller detecting checksum errors when
> reading potentially uninitialized TX message RAM content to transmit
> CAN frames the TX message RAM has to be written with (any kind of)
> initial data.
>
The key point of the issue is that why M_CAN will read potentially uninitialized
TX message RAM content which should not happen?
e.g. for our case of the issue, if we sending a no data frame or a less
than 4 bytes frame, why m_can will read extra 4 bytes uninitialized/unset
data which seems not reasonable?
>From IP code logic, it will read full 8 bytes of data no matter how many data
actually to be transfered which is strange.
For sending data over 4 bytes, since the Message RAM content will be filled
with the real data to be transfered so there's no such issue.
> ---
>
> Then the code should memset() the entire TX FIFO element - and not
> only the 8 data bytes we are addressing now.
>
Our IC guy told me the issue only happened on transferring a data size
of less than 4 bytes and my test also proved that.
So i'm not sure memset() the entire TX FIFO element is neccesary...
Do you think we could keep the current solution firstly and updated later
if needed?
> Maybe it makes sense to send the entire updated patch set (3) again ...
>
> [1/3] can: add can_is_canfd_skb() API
> [2/3] can: m_can: update to support CAN FD features
> [3/3] can: m_can: add missing message RAM initialization
>
> Are you ok with that?
>
> Regards,
> Oliver
>
Regards
Dong Aisheng
^ permalink raw reply
* RE: [PATCH] bridge: missing null bridge device check causing null pointer dereference (bugfix)
From: 박수현 @ 2014-11-06 7:58 UTC (permalink / raw)
To: Toshiaki Makita, Stephen Hemminger, David S. Miller
Cc: bridge@lists.linux-foundation.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <545B1E27.3080302@lab.ntt.co.jp>
>-----Original Message-----
>From: Toshiaki Makita [mailto:makita.toshiaki@lab.ntt.co.jp]
>Sent: Thursday, November 06, 2014 4:07 PM
>To: 박수현; Stephen Hemminger; David S. Miller
>Cc: bridge@lists.linux-foundation.org; netdev@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [PATCH] bridge: missing null bridge device check causing null
>pointer dereference (bugfix)
>
>On 2014/11/06 15:26, Su-Hyun Park wrote:
>> the bridge device can be null if the bridge is being deleted while
>> processing the packet, which causes the null pointer dereference in
>switch statement.
>
>How can this happen??
>It is guarded by rcu.
>netdev_rx_handler_unregister() ensures rx_handler_data is non NULL.
>
The RCU protect rx_handler_data, not the bridge member port. It can be NULL according to below code.
static inline struct net_bridge_port *br_port_get_rcu(const struct net_device *dev) {
struct net_bridge_port *port = rcu_dereference(dev->rx_handler_data);
return br_port_exists(dev) ? port : NULL;
}
The crash happens at the below switch statement in br_handle_frame, where p is NULL.
switch (p->state)
>>
>> crash dump snippet:
>>
>> <1>BUG: unable to handle kernel NULL pointer dereference at
>> 0000000000000021
>> <1>IP: [<ffffffff814179f6>] br_handle_frame+0xe6/0x270
>>
>> <0>Code: 4c 0f 44 f0 89 f8 66 33 15 32 52 24 00 66 33 05 29 52 24 00
>> 09 c2 89
>> f0 66 33 05 22 52 24 00 80 e4 f0 66 09 c2 0f 84 eb 00 00 00 <41> 0f b6
>> 46 21 3c 02 74 61 3c 03 74 1d 48 89 df e8 d5 bc f0 ff
>> ---
>> net/bridge/br_input.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index
>> 6fd5522..7e899ca 100644
>> --- a/net/bridge/br_input.c
>> +++ b/net/bridge/br_input.c
>> @@ -176,6 +176,8 @@ rx_handler_result_t br_handle_frame(struct sk_buff
>**pskb)
>> return RX_HANDLER_CONSUMED;
>>
>> p = br_port_get_rcu(skb->dev);
>> + if (!p)
>> + goto drop;
>>
>> if (unlikely(is_link_local_ether_addr(dest))) {
>> u16 fwd_mask = p->br->group_fwd_mask_required;
>>
>
^ permalink raw reply
* Re: getting rid of ->splice_write?
From: Christoph Hellwig @ 2014-11-06 7:55 UTC (permalink / raw)
To: Al Viro; +Cc: Miklos Szeredi, linux-fsdevel, netdev
In-Reply-To: <20141105184945.GS7996@ZenIV.linux.org.uk>
On Wed, Nov 05, 2014 at 06:49:45PM +0000, Al Viro wrote:
> Not really. A minor nitpick is that you've missed port_fops_splice_write(),
> but the real bitch isn't that and not even the sockets (see the fun with
> iov_iter sendmsg/recvmsg work getting resurrected). It's that NULL
> ->splice_write() means default_file_splice_write. IOW, you'd need either
> ->write_iter() for _everything_ (with support of bvec-backed ones included,
> since that's what iter_file_splice_write() will feed to ->write_iter()),
> or you need to have do_splice_from() check if ->write_iter is NULL and
> go for default_file_splice_write() instead of iter_file_splice_write().
>
> The latter might be doable, but the former is really over the top - for
> that we'd need to touch every driver instance of ->write() out there.
> You want to do that, it's your funeral...
The latter is what I thought off. And yes, the socket work looks good,
especially if we can get rid of ->sendpage as well. That'll require
passing new flags somewhere, the ones in the iocb added for
preadv2/pwritev2 might be usable.
> > Similarly it seems to be like we could kill ->splice_read by
> > implementing an equivalent iteration over ->read_iter.
>
> Hard to do. I agree that we want to, but it'll take quite a bit of work
> on iov_iter primitives, I'm afraid. At the very least, we want a variant
> of iov_iter that could steal pages. Don't forget that a large part of
> the rationale behind splice_read was the ability to play zero-copy games.
>
> I'm not sure if it will happen this cycle; there's more than enough fun
> on the net/* side. _If_ that ends up done faster than I expect it to be,
> ->splice_read() is the obvious next target.
And zero copy games would become a lot less nasty if they could go
straight through ->read_iter instead of the current abuses of splice
infrastructure.
Same for sendfile, btw.
^ permalink raw reply
* Re: mlx4+vxlan offload breaks gre tunnels
From: Or Gerlitz @ 2014-11-06 7:53 UTC (permalink / raw)
To: Sathya Perla
Cc: Or Gerlitz, Florian Westphal, netdev@vger.kernel.org, Tom Herbert,
Jesse Gross, amirv@mellanox.com
In-Reply-To: <637ce8e1-89e5-4254-a928-4ab2a4eb8c29@CMEXHTCAS1.ad.emulex.com>
On Thu, Nov 6, 2014 at 9:21 AM, Sathya Perla <Sathya.Perla@emulex.com> wrote:
>> I think the best effort we can do now is
>>
>> 1. come up with something such as the below patch for 3.18 which is
>> back-ward portable for -stable kernels, it will only arm the hw offloads
>> if the OS tells us there's VXLAN in action
> Or, wouldn't the patch below not work (i.e., the same issue would persist)
> when there is both VXLAN and some other (say GRE) tunnel in the system
> and the NIC HW is capable of supporting checksum offload only on VxLAN.
Indeed, this would be the case. But the patch below will make things
to work when
only GRE is used (or when only VXLAN is used). So we're making
progress vs. the current
situation where GRE breaks over a HW which is capable to do VXLAN offloads even
if there's no VXLAN tunnel around.
Or.
>> 2. come up with proper kernel APIs to let NICs advertize which encap
>> schemes they can actually offload the inner checksum, Tom... your work
>> which now runs over netdev.
>> tested to work with the following which is a bit different, tell me if
>> it works for you
>>
>> # node A - with mlx4_en address192.168.31.18
>> ip tunnel add gre1 mode gre local 192.168.31.18 remote 192.168.31.17 ttl 255
>> ifconfig gre1 10.10.10.18/24 up
>> ifconfig gre1 mtu 1450
>>
>> # node B - with mlx4_en address192.168.31.17
>> ip tunnel add gre1 mode gre local 192.168.31.17 remote 192.168.31.18 ttl 255
>> ifconfig gre1 10.10.10.17/24 up
>> ifconfig gre1 mtu 1450
^ permalink raw reply
* RE: mlx4+vxlan offload breaks gre tunnels
From: Sathya Perla @ 2014-11-06 7:21 UTC (permalink / raw)
To: Or Gerlitz, Florian Westphal, netdev@vger.kernel.org, Tom Herbert,
Jesse Gross
Cc: amirv@mellanox.com, Sathya Perla
In-Reply-To: <545A4DB7.5010603@mellanox.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
>
> On 11/5/2014 5:04 PM, Florian Westphal wrote:
> > tl,dr: all tcp packets sent via gre tunnel have broken tcp csum if vxlan
> offload
> > is enabled with mlx4 driver.
> >
> > Given following config on tx-side:
> > dev=enp3s0
> > ip addr add dev $dev 192.168.23.1/24
> > ip link set $dev up
> > ip link add mygre type gretap remote 192.168.23.2 local 192.168.23.1
> > ip addr add dev mygre 192.168.42.1/24
> > ip link set gre0 up
> > ip link set mygre up
> >
> > and
> >
> > options mlx4_core log_num_mgm_entry_size=-1 debug_level=1
> > port_type_array=2,2
> >
> > in
> > /etc/modprobe.d/mlx4.conf
> >
> > all tcp packets sent to destinations over the gre tunnel have bogus tcp
> > checksums (and are tossed on rx side when stack validates tcp checksum).
> >
> > net-next head is commit 30349bdbc4da5ecf0efa25556e3caff9c9b8c5f7 .
> >
> > What makes things work for me:
> > either
> >
> > options mlx4_core 1 debug_level=1 port_type_array=2,2
> >
> > (ie. no MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
> >
> > or not setting NETIF_F_IP_CSUM in enc_features:
> >
> > --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> > +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> > @@ -2579,10 +2579,12 @@ int mlx4_en_init_netdev(struct mlx4_en_dev
> *mdev, int port,
> > dev->priv_flags |= IFF_UNICAST_FLT;
> >
> > if (mdev->dev->caps.tunnel_offload_mode ==
> MLX4_TUNNEL_OFFLOAD_MODE_VXLAN) {
> > - dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM
> |
> > + dev->hw_enc_features |= NETIF_F_RXCSUM |
> > NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL;
> >
> > I am not sure if its right fix, but to my eyes this basically looks like
> > mlx4 is telling stack that it can handle tcp checksum offload within
> > tunnels, and that doesn't seem to be the case for all types (e.g. gre).
> >
> > Could someone who understand the enc_features specifics better confirm
> that
> > above patch is correct (or provide a better/proper fix)?
>
> Yep, I can see now the problem. It comes into play with ConnectX3-pro
> NICs that support VXLAN offloads (but not with ConnectX3 NIC which
> don't) when you enable the offloads support on the CX3-pro.
>
> The problem originates from the fact that we can't advertize something
> like "the HW can offload the inner checksum of UDP/VXLAN encapsulated
> (but not for GRE)", e.g in a similar manner that exists in the GSO
> space, where you have NETIF_F_GSO _YYY for each yyy in {UDP, SIT, GRE,
> etc} tunneling scheme.
>
> I think the best effort we can do now is
>
> 1. come up with something such as the below patch for 3.18 which is
> back-ward portable for -stable kernels, it will only arm the hw offloads
> if the OS tells us there's VXLAN in action
Or, wouldn't the patch below not work (i.e., the same issue would persist)
when there is both VXLAN and some other (say GRE) tunnel in the system
and the NIC HW is capable of supporting checksum offload only on VxLAN.
Do you expect a user who uses VxLAN to not use other kinds of tunnels?
>
> 2. come up with proper kernel APIs to let NICs advertize which encap
> schemes they can actually offload the inner checksum, Tom... your work
> which now runs over netdev.
>
> Tom/Jesse- thoughts? are you +1-ing the below approach?
>
> Or.
>
> tested to work with the following which is a bit different, tell me if
> it works for you
>
> # node A - with mlx4_en address192.168.31.18
> ip tunnel add gre1 mode gre local 192.168.31.18 remote 192.168.31.17 ttl 255
> ifconfig gre1 10.10.10.18/24 up
> ifconfig gre1 mtu 1450
>
> # node B - with mlx4_en address192.168.31.17
> ip tunnel add gre1 mode gre local 192.168.31.17 remote 192.168.31.18 ttl 255
> ifconfig gre1 10.10.10.17/24 up
> ifconfig gre1 mtu 1450
>
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 0efbae9..7753833 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -2292,6 +2292,12 @@ static void mlx4_en_add_vxlan_offloads(struct
> work_struct *work)
> out:
> if (ret)
> en_err(priv, "failed setting L2 tunnel configuration
> ret %d\n", ret);
> +
> + /* set offloads */
> + priv->dev->hw_enc_features |= NETIF_F_IP_CSUM |
> NETIF_F_RXCSUM |
> + NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL;
> + priv->dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
> + priv->dev->features |= NETIF_F_GSO_UDP_TUNNEL;
> }
>
> static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
> @@ -2299,6 +2305,10 @@ static void mlx4_en_del_vxlan_offloads(struct
> work_struct *work)
> int ret;
> struct mlx4_en_priv *priv = container_of(work, struct mlx4_en_priv,
> vxlan_del_task);
> + /* unset offloads */
> + priv->dev->hw_enc_features = 0;
> + priv->dev->hw_features &= ~NETIF_F_GSO_UDP_TUNNEL;
> + priv->dev->features &= ~NETIF_F_GSO_UDP_TUNNEL;
>
> ret = mlx4_SET_PORT_VXLAN(priv->mdev->dev, priv->port,
> VXLAN_STEER_BY_OUTER_MAC, 0);
> @@ -2578,13 +2588,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev
> *mdev,
> int port,
> if (mdev->dev->caps.steering_mode != MLX4_STEERING_MODE_A0)
> dev->priv_flags |= IFF_UNICAST_FLT;
>
> - if (mdev->dev->caps.tunnel_offload_mode ==
> MLX4_TUNNEL_OFFLOAD_MODE_VXLAN) {
> - dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> - NETIF_F_TSO |
> NETIF_F_GSO_UDP_TUNNEL;
> - dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
> - dev->features |= NETIF_F_GSO_UDP_TUNNEL;
> - }
> -
> mdev->pndev[port] = dev;
>
> netif_carrier_off(dev);
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: ipv4: Use standard iovec primitive in raw_probe_proto_opt
From: Al Viro @ 2014-11-06 7:11 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, netdev, linux-kernel, bcrl, Masahide Nakamura,
Hideaki YOSHIFUJI
In-Reply-To: <20141106064629.GA29321@gondor.apana.org.au>
On Thu, Nov 06, 2014 at 02:46:29PM +0800, Herbert Xu wrote:
> On Thu, Nov 06, 2014 at 06:43:18AM +0000, Al Viro wrote:
> > On Thu, Nov 06, 2014 at 01:50:23PM +0800, Herbert Xu wrote:
> > > + /* We only need the first two bytes. */
> > > + err = memcpy_fromiovecend((void *)&icmph, msg->msg_iov, 0, 2);
> > > + if (err)
> > > + return err;
> > > +
> > > + fl4->fl4_icmp_type = icmph.type;
> > > + fl4->fl4_icmp_code = icmph.code;
> >
> > That's more readable, but that exposes another problem in there - we read
> > the same piece of userland data twice, with no promise whatsoever that we'll
> > get the same value both times...
>
> Sure, but you have to be root anyway to write to raw sockets.
Point, but that might very well be a pattern to watch for - there's at least
one more instance in TIPC (also not exploitable, according to TIPC folks)
and such bugs are easily repeated...
BTW, I've picked the tun and macvtap related bits from another part of old
queue; see vfs.git#untested-macvtap - it's on top of #iov_iter-net and it's
really completely untested. Back then I was mostly interested in killing
as many ->aio_write() instances as I could, so it's only the "send" side of
things.
^ permalink raw reply
* Re: [PATCH] bridge: missing null bridge device check causing null pointer dereference (bugfix)
From: Toshiaki Makita @ 2014-11-06 7:07 UTC (permalink / raw)
To: Su-Hyun Park, Stephen Hemminger, David S. Miller
Cc: netdev, bridge, linux-kernel
In-Reply-To: <1415255192-13584-1-git-send-email-suhyun.park@ahnlab.com>
On 2014/11/06 15:26, Su-Hyun Park wrote:
> the bridge device can be null if the bridge is being deleted while processing
> the packet, which causes the null pointer dereference in switch statement.
How can this happen??
It is guarded by rcu.
netdev_rx_handler_unregister() ensures rx_handler_data is non NULL.
Thanks,
Toshiaki Makita
>
> crash dump snippet:
>
> <1>BUG: unable to handle kernel NULL pointer dereference at 0000000000000021
> <1>IP: [<ffffffff814179f6>] br_handle_frame+0xe6/0x270
>
> <0>Code: 4c 0f 44 f0 89 f8 66 33 15 32 52 24 00 66 33 05 29 52 24 00 09 c2 89
> f0 66 33 05 22 52 24 00 80 e4 f0 66 09 c2 0f 84 eb 00 00 00 <41> 0f b6 46 21
> 3c 02 74 61 3c 03 74 1d 48 89 df e8 d5 bc f0 ff
> ---
> net/bridge/br_input.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 6fd5522..7e899ca 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -176,6 +176,8 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
> return RX_HANDLER_CONSUMED;
>
> p = br_port_get_rcu(skb->dev);
> + if (!p)
> + goto drop;
>
> if (unlikely(is_link_local_ether_addr(dest))) {
> u16 fwd_mask = p->br->group_fwd_mask_required;
>
^ permalink raw reply
* Re: M_CAN message RAM initialization AppNote - was: Re: [PATCH V3 3/3] can: m_can: workaround for transmit data less than 4 bytes
From: Oliver Hartkopp @ 2014-11-06 7:04 UTC (permalink / raw)
To: Dong Aisheng
Cc: Marc Kleine-Budde, linux-can, wg, varkabhadram, netdev,
linux-arm-kernel
In-Reply-To: <20141106015716.GB7642@shlinux1.ap.freescale.net>
On 06.11.2014 02:57, Dong Aisheng wrote:
> On Wed, Nov 05, 2014 at 07:15:10PM +0100, Oliver Hartkopp wrote:
>> The Message RAM is usually equipped with a parity or ECC functionality.
>> But RAM cells suffer a hardware reset and can therefore hold
>> arbitrary content at startup - including parity and/or ECC bits.
>>
>> So when you write only the CAN ID and the first four bytes the last
>> four bytes remain untouched. Then the M_CAN starts to read in 32bit
>> words from the start of the Tx Message element. So it is very likely
>> to trigger the message RAM error when reading the uninitialized
>> 32bit word from the last four bytes.
>>
>> Finally it turns out that an initial writing (with any kind of data)
>> to the entire message RAM is mandatory to create valid parity/ECC
>> checksums.
>>
>> That's it.
>>
>
> Thanks for sharing this information.
> Does it mean this issue is related to the nature of Message RAM and is
> supposed to exist on all M_CAN IP versions?
From what I know from the 3.1.x revision there's no change regarding IR.BRU
and IR.BEC - so I would assume this to stay on all M_CAN IP revisions.
But after some sleep I wonder if this patch [3/3] would need an update too.
Writing to the TX message RAM is obviously no workaround but a valid and
needed initialization process.
I would tend to make this patch:
---
can: m_can: add missing TX message RAM initialization
The M_CAN message RAM is usually equipped with a parity or ECC functionality.
But RAM cells suffer a hardware reset and can therefore hold arbitrary content
at startup - including parity and/or ECC bits.
To prevent the M_CAN controller detecting checksum errors when reading
potentially uninitialized TX message RAM content to transmit CAN frames the TX
message RAM has to be written with (any kind of) initial data.
---
Then the code should memset() the entire TX FIFO element - and not only the 8
data bytes we are addressing now.
Maybe it makes sense to send the entire updated patch set (3) again ...
[1/3] can: add can_is_canfd_skb() API
[2/3] can: m_can: update to support CAN FD features
[3/3] can: m_can: add missing message RAM initialization
Are you ok with that?
Regards,
Oliver
^ permalink raw reply
* Re: mlx4+vxlan offload breaks gre tunnels
From: Or Gerlitz @ 2014-11-06 7:03 UTC (permalink / raw)
To: Tom Herbert; +Cc: Florian Westphal, Linux Netdev List, Jesse Gross, Amir Vadai
In-Reply-To: <CA+mtBx_oxJxteeBnbyboGTSKn3JdBRx2Nb3a9EuCZzG+_U-8Zw@mail.gmail.com>
On Wed, Nov 5, 2014 at 11:59 PM, Tom Herbert <therbert@google.com> wrote:
> On Wed, Nov 5, 2014 at 8:17 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
>> On 11/5/2014 5:04 PM, Florian Westphal wrote:
>>>
>>> tl,dr: all tcp packets sent via gre tunnel have broken tcp csum if vxlan
>>> offload
>>> is enabled with mlx4 driver.
>>>
>>> Given following config on tx-side:
>>> dev=enp3s0
>>> ip addr add dev $dev 192.168.23.1/24
>>> ip link set $dev up
>>> ip link add mygre type gretap remote 192.168.23.2 local 192.168.23.1
>>> ip addr add dev mygre 192.168.42.1/24
>>> ip link set gre0 up
>>> ip link set mygre up
>>>
>>> and
>>>
>>> options mlx4_core log_num_mgm_entry_size=-1 debug_level=1
>>> port_type_array=2,2
>>>
>>> in
>>> /etc/modprobe.d/mlx4.conf
>>>
>>> all tcp packets sent to destinations over the gre tunnel have bogus tcp
>>> checksums (and are tossed on rx side when stack validates tcp checksum).
>>>
>>> net-next head is commit 30349bdbc4da5ecf0efa25556e3caff9c9b8c5f7 .
>>>
>>> What makes things work for me:
>>> either
>>>
>>> options mlx4_core 1 debug_level=1 port_type_array=2,2
>>>
>>> (ie. no MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
>>>
>>> or not setting NETIF_F_IP_CSUM in enc_features:
>>>
>>> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>>> @@ -2579,10 +2579,12 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev,
>>> int port,
>>> dev->priv_flags |= IFF_UNICAST_FLT;
>>> if (mdev->dev->caps.tunnel_offload_mode ==
>>> MLX4_TUNNEL_OFFLOAD_MODE_VXLAN) {
>>> - dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
>>> + dev->hw_enc_features |= NETIF_F_RXCSUM |
>>> NETIF_F_TSO |
>>> NETIF_F_GSO_UDP_TUNNEL;
>>>
>>> I am not sure if its right fix, but to my eyes this basically looks like
>>> mlx4 is telling stack that it can handle tcp checksum offload within
>>> tunnels, and that doesn't seem to be the case for all types (e.g. gre).
>>>
>>> Could someone who understand the enc_features specifics better confirm
>>> that
>>> above patch is correct (or provide a better/proper fix)?
>>
>>
>> Yep, I can see now the problem. It comes into play with ConnectX3-pro NICs
>> that support VXLAN offloads (but not with ConnectX3 NIC which don't) when
>> you enable the offloads support on the CX3-pro.
>>
>> The problem originates from the fact that we can't advertize something like
>> "the HW can offload the inner checksum of UDP/VXLAN encapsulated (but not
>> for GRE)", e.g in a similar manner that exists in the GSO space, where you
>> have NETIF_F_GSO _YYY for each yyy in {UDP, SIT, GRE, etc} tunneling scheme.
>>
>> I think the best effort we can do now is
>>
>> 1. come up with something such as the below patch for 3.18 which is
>> back-ward portable for -stable kernels, it will only arm the hw offloads if
>> the OS tells us there's VXLAN in action
>>
>> 2. come up with proper kernel APIs to let NICs advertize which encap
>> schemes they can actually offload the inner checksum, Tom... your work which
>> now runs over netdev.
> Possibly #3: add ndo_gso_check to detect nested tunneling. In this
> case it would see that gso_type has both SKB_GSO_GRE and
> SKB_GSO_UDP_TUNNEL set.
I am not with you, where do we have nested tunneling here? the below
setting is simple GRE tunnel
to host (say) TCP traffic
>> tested to work with the following which is a bit different, tell me if it
>> works for you
>>
>> # node A - with mlx4_en address192.168.31.18
>> ip tunnel add gre1 mode gre local 192.168.31.18 remote 192.168.31.17 ttl 255
>> ifconfig gre1 10.10.10.18/24 up
>> ifconfig gre1 mtu 1450
>>
>> # node B - with mlx4_en address192.168.31.17
>> ip tunnel add gre1 mode gre local 192.168.31.17 remote 192.168.31.18 ttl 255
>> ifconfig gre1 10.10.10.17/24 up
>> ifconfig gre1 mtu 1450
^ permalink raw reply
* Re: ipv4: Use standard iovec primitive in raw_probe_proto_opt
From: Herbert Xu @ 2014-11-06 6:46 UTC (permalink / raw)
To: Al Viro
Cc: David Miller, netdev, linux-kernel, bcrl, Masahide Nakamura,
Hideaki YOSHIFUJI
In-Reply-To: <20141106064318.GW7996@ZenIV.linux.org.uk>
On Thu, Nov 06, 2014 at 06:43:18AM +0000, Al Viro wrote:
> On Thu, Nov 06, 2014 at 01:50:23PM +0800, Herbert Xu wrote:
> > + /* We only need the first two bytes. */
> > + err = memcpy_fromiovecend((void *)&icmph, msg->msg_iov, 0, 2);
> > + if (err)
> > + return err;
> > +
> > + fl4->fl4_icmp_type = icmph.type;
> > + fl4->fl4_icmp_code = icmph.code;
>
> That's more readable, but that exposes another problem in there - we read
> the same piece of userland data twice, with no promise whatsoever that we'll
> get the same value both times...
Sure, but you have to be root anyway to write to raw sockets.
Patches are welcome :)
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: ipv4: Use standard iovec primitive in raw_probe_proto_opt
From: Al Viro @ 2014-11-06 6:43 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, netdev, linux-kernel, bcrl, Masahide Nakamura,
Hideaki YOSHIFUJI
In-Reply-To: <20141106055023.GA28865@gondor.apana.org.au>
On Thu, Nov 06, 2014 at 01:50:23PM +0800, Herbert Xu wrote:
> + /* We only need the first two bytes. */
> + err = memcpy_fromiovecend((void *)&icmph, msg->msg_iov, 0, 2);
> + if (err)
> + return err;
> +
> + fl4->fl4_icmp_type = icmph.type;
> + fl4->fl4_icmp_code = icmph.code;
That's more readable, but that exposes another problem in there - we read
the same piece of userland data twice, with no promise whatsoever that we'll
get the same value both times...
^ permalink raw reply
* [PATCH] bridge: missing null bridge device check causing null pointer dereference (bugfix)
From: Su-Hyun Park @ 2014-11-06 6:26 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller
Cc: bridge, netdev, linux-kernel, Su-Hyun Park
the bridge device can be null if the bridge is being deleted while processing
the packet, which causes the null pointer dereference in switch statement.
crash dump snippet:
<1>BUG: unable to handle kernel NULL pointer dereference at 0000000000000021
<1>IP: [<ffffffff814179f6>] br_handle_frame+0xe6/0x270
<0>Code: 4c 0f 44 f0 89 f8 66 33 15 32 52 24 00 66 33 05 29 52 24 00 09 c2 89
f0 66 33 05 22 52 24 00 80 e4 f0 66 09 c2 0f 84 eb 00 00 00 <41> 0f b6 46 21
3c 02 74 61 3c 03 74 1d 48 89 df e8 d5 bc f0 ff
---
net/bridge/br_input.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 6fd5522..7e899ca 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -176,6 +176,8 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
return RX_HANDLER_CONSUMED;
p = br_port_get_rcu(skb->dev);
+ if (!p)
+ goto drop;
if (unlikely(is_link_local_ether_addr(dest))) {
u16 fwd_mask = p->br->group_fwd_mask_required;
--
1.8.1.4
^ permalink raw reply related
* [PATCH net-next] PPC: bpf_jit_comp: add SKF_AD_HATYPE instruction
From: Denis Kirjanov @ 2014-11-06 6:02 UTC (permalink / raw)
To: netdev
Cc: linuxppc-dev, Denis Kirjanov, Alexei Starovoitov, Daniel Borkmann,
Philippe Bergheaud
Add BPF extension SKF_AD_HATYPE to ppc JIT to check
the hw type of the interface
JIT off:
[ 69.106783] test_bpf: #20 LD_HATYPE 48 48 PASS
JIT on:
[ 64.721757] test_bpf: #20 LD_HATYPE 7 6 PASS
CC: Alexei Starovoitov<alexei.starovoitov@gmail.com>
CC: Daniel Borkmann<dborkman@redhat.com>
CC: Philippe Bergheaud<felix@linux.vnet.ibm.com>
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
arch/powerpc/net/bpf_jit_comp.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index d110e28..8bf4fc2 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -412,6 +412,22 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
PPC_ANDI(r_A, r_A, PKT_TYPE_MAX);
PPC_SRWI(r_A, r_A, 5);
break;
+ case BPF_ANC | SKF_AD_HATYPE:
+ BUILD_BUG_ON(FIELD_SIZEOF(struct net_device, type) != 2);
+ PPC_LD_OFFS(r_scratch1, r_skb, offsetof(struct sk_buff,
+ dev));
+ PPC_CMPDI(r_scratch1, 0);
+ if (ctx->pc_ret0 != -1) {
+ PPC_BCC(COND_EQ, addrs[ctx->pc_ret0]);
+ } else {
+ /* Exit, returning 0; first pass hits here. */
+ PPC_BCC_SHORT(COND_NE, (ctx->idx*4)+12);
+ PPC_LI(r_ret, 0);
+ PPC_JMP(exit_addr);
+ }
+ PPC_LHZ_OFFS(r_A, r_scratch1,
+ offsetof(struct net_device, type));
+ break;
case BPF_ANC | SKF_AD_CPU:
#ifdef CONFIG_SMP
/*
--
2.1.0
^ permalink raw reply related
* ipv4: Use standard iovec primitive in raw_probe_proto_opt
From: Herbert Xu @ 2014-11-06 5:50 UTC (permalink / raw)
To: Al Viro
Cc: David Miller, netdev, linux-kernel, bcrl, Masahide Nakamura,
Hideaki YOSHIFUJI
In-Reply-To: <20141106032533.GU7996@ZenIV.linux.org.uk>
On Thu, Nov 06, 2014 at 03:25:34AM +0000, Al Viro wrote:
>
> * there's some really weird stuff in there. Just what is this
> static int raw_probe_proto_opt(struct flowi4 *fl4, struct msghdr *msg)
> {
It looks like newbie coding that's all. There's nothing tricky
here as far as I can tell. We're just trying to fetch the ICMP
header to seed the IPsec lookup.
So how about this rewrite? I'm assuming that you're not going
to get rid of memcpy_fromiovecend/memcpy_toiovecend, if you
are, let me know and I'll redo this with iterators.
ipv4: Use standard iovec primitive in raw_probe_proto_opt
The function raw_probe_proto_opt tries to extract the first two
bytes from the user input in order to seed the IPsec lookup for
ICMP packets. In doing so it's processing iovec by hand and
overcomplicating things.
This patch replaces the manual iovec processing with a call to
memcpy_fromiovecend.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 739db31..04f67e1 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -422,48 +422,20 @@ error:
static int raw_probe_proto_opt(struct flowi4 *fl4, struct msghdr *msg)
{
- struct iovec *iov;
- u8 __user *type = NULL;
- u8 __user *code = NULL;
- int probed = 0;
- unsigned int i;
+ struct icmphdr icmph;
+ int err;
- if (!msg->msg_iov)
+ if (fl4->flowi4_proto != IPPROTO_ICMP)
return 0;
- for (i = 0; i < msg->msg_iovlen; i++) {
- iov = &msg->msg_iov[i];
- if (!iov)
- continue;
-
- switch (fl4->flowi4_proto) {
- case IPPROTO_ICMP:
- /* check if one-byte field is readable or not. */
- if (iov->iov_base && iov->iov_len < 1)
- break;
-
- if (!type) {
- type = iov->iov_base;
- /* check if code field is readable or not. */
- if (iov->iov_len > 1)
- code = type + 1;
- } else if (!code)
- code = iov->iov_base;
-
- if (type && code) {
- if (get_user(fl4->fl4_icmp_type, type) ||
- get_user(fl4->fl4_icmp_code, code))
- return -EFAULT;
- probed = 1;
- }
- break;
- default:
- probed = 1;
- break;
- }
- if (probed)
- break;
- }
+ /* We only need the first two bytes. */
+ err = memcpy_fromiovecend((void *)&icmph, msg->msg_iov, 0, 2);
+ if (err)
+ return err;
+
+ fl4->fl4_icmp_type = icmph.type;
+ fl4->fl4_icmp_code = icmph.code;
+
return 0;
}
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* [PATCH net-next 1/3] r8152: move r8152b_get_version
From: Hayes Wang @ 2014-11-06 4:47 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-84-Taiwan-albertk@realtek.com>
Move r8152b_get_version() to the location before rtl_ops_init().
Then, the rtl_ops_init() could use tp->version.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index fd41675..4b6db8a 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3833,6 +3833,7 @@ static int rtl8152_probe(struct usb_interface *intf,
tp->netdev = netdev;
tp->intf = intf;
+ r8152b_get_version(tp);
ret = rtl_ops_init(tp, id);
if (ret)
goto out;
@@ -3866,11 +3867,9 @@ static int rtl8152_probe(struct usb_interface *intf,
tp->mii.phy_id_mask = 0x3f;
tp->mii.reg_num_mask = 0x1f;
tp->mii.phy_id = R8152_PHY_ID;
- tp->mii.supports_gmii = 0;
intf->needs_remote_wakeup = 1;
- r8152b_get_version(tp);
tp->rtl_ops.init(tp);
set_ethernet_addr(tp);
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 3/3] r8152: remove the definitions of the PID
From: Hayes Wang @ 2014-11-06 4:47 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-84-Taiwan-albertk@realtek.com>
The PIDs are only used in the id table, so the definitions are
unnacessary. Remove them wouldn't have confusion.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index cf1b8a7..66b139a 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -461,11 +461,7 @@ enum rtl8152_flags {
/* Define these values to match your device */
#define VENDOR_ID_REALTEK 0x0bda
-#define PRODUCT_ID_RTL8152 0x8152
-#define PRODUCT_ID_RTL8153 0x8153
-
#define VENDOR_ID_SAMSUNG 0x04e8
-#define PRODUCT_ID_SAMSUNG 0xa101
#define MCU_TYPE_PLA 0x0100
#define MCU_TYPE_USB 0x0000
@@ -3898,9 +3894,9 @@ static void rtl8152_disconnect(struct usb_interface *intf)
/* table of devices that work with this driver */
static struct usb_device_id rtl8152_table[] = {
- {USB_DEVICE(VENDOR_ID_REALTEK, PRODUCT_ID_RTL8152)},
- {USB_DEVICE(VENDOR_ID_REALTEK, PRODUCT_ID_RTL8153)},
- {USB_DEVICE(VENDOR_ID_SAMSUNG, PRODUCT_ID_SAMSUNG)},
+ {USB_DEVICE(VENDOR_ID_REALTEK, 0x8152)},
+ {USB_DEVICE(VENDOR_ID_REALTEK, 0x8153)},
+ {USB_DEVICE(VENDOR_ID_SAMSUNG, 0xa101)},
{}
};
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 2/3] r8152: modify rtl_ops_init
From: Hayes Wang @ 2014-11-06 4:47 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-84-Taiwan-albertk@realtek.com>
Replace using VID/PID with using tp->version to initialize the ops.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 79 ++++++++++++++++++-------------------------------
1 file changed, 28 insertions(+), 51 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 4b6db8a..cf1b8a7 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3742,66 +3742,43 @@ static void rtl8153_unload(struct r8152 *tp)
r8153_power_cut_en(tp, false);
}
-static int rtl_ops_init(struct r8152 *tp, const struct usb_device_id *id)
+static int rtl_ops_init(struct r8152 *tp)
{
struct rtl_ops *ops = &tp->rtl_ops;
- int ret = -ENODEV;
-
- switch (id->idVendor) {
- case VENDOR_ID_REALTEK:
- switch (id->idProduct) {
- case PRODUCT_ID_RTL8152:
- ops->init = r8152b_init;
- ops->enable = rtl8152_enable;
- ops->disable = rtl8152_disable;
- ops->up = rtl8152_up;
- ops->down = rtl8152_down;
- ops->unload = rtl8152_unload;
- ops->eee_get = r8152_get_eee;
- ops->eee_set = r8152_set_eee;
- ret = 0;
- break;
- case PRODUCT_ID_RTL8153:
- ops->init = r8153_init;
- ops->enable = rtl8153_enable;
- ops->disable = rtl8153_disable;
- ops->up = rtl8153_up;
- ops->down = rtl8153_down;
- ops->unload = rtl8153_unload;
- ops->eee_get = r8153_get_eee;
- ops->eee_set = r8153_set_eee;
- ret = 0;
- break;
- default:
- break;
- }
+ int ret = 0;
+
+ switch (tp->version) {
+ case RTL_VER_01:
+ case RTL_VER_02:
+ ops->init = r8152b_init;
+ ops->enable = rtl8152_enable;
+ ops->disable = rtl8152_disable;
+ ops->up = rtl8152_up;
+ ops->down = rtl8152_down;
+ ops->unload = rtl8152_unload;
+ ops->eee_get = r8152_get_eee;
+ ops->eee_set = r8152_set_eee;
break;
- case VENDOR_ID_SAMSUNG:
- switch (id->idProduct) {
- case PRODUCT_ID_SAMSUNG:
- ops->init = r8153_init;
- ops->enable = rtl8153_enable;
- ops->disable = rtl8153_disable;
- ops->up = rtl8153_up;
- ops->down = rtl8153_down;
- ops->unload = rtl8153_unload;
- ops->eee_get = r8153_get_eee;
- ops->eee_set = r8153_set_eee;
- ret = 0;
- break;
- default:
- break;
- }
+ case RTL_VER_03:
+ case RTL_VER_04:
+ case RTL_VER_05:
+ ops->init = r8153_init;
+ ops->enable = rtl8153_enable;
+ ops->disable = rtl8153_disable;
+ ops->up = rtl8153_up;
+ ops->down = rtl8153_down;
+ ops->unload = rtl8153_unload;
+ ops->eee_get = r8153_get_eee;
+ ops->eee_set = r8153_set_eee;
break;
default:
+ ret = -ENODEV;
+ netif_err(tp, probe, tp->netdev, "Unknown Device\n");
break;
}
- if (ret)
- netif_err(tp, probe, tp->netdev, "Unknown Device\n");
-
return ret;
}
@@ -3834,7 +3811,7 @@ static int rtl8152_probe(struct usb_interface *intf,
tp->intf = intf;
r8152b_get_version(tp);
- ret = rtl_ops_init(tp, id);
+ ret = rtl_ops_init(tp);
if (ret)
goto out;
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 0/3] r8152: rtl_ops_init modify
From: Hayes Wang @ 2014-11-06 4:47 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Hayes Wang
Initialize the ops through tp->version. This could skip checking
each VID/PID.
Hayes Wang (3):
r8152: move r8152b_get_version
r8152: modify rtl_ops_init
r8152: remove the definitions of the PID
drivers/net/usb/r8152.c | 92 +++++++++++++++++--------------------------------
1 file changed, 32 insertions(+), 60 deletions(-)
--
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/4] inet: Add skb_copy_datagram_iter
From: Al Viro @ 2014-11-06 3:25 UTC (permalink / raw)
To: David Miller; +Cc: herbert, netdev, linux-kernel, bcrl
In-Reply-To: <20141105.165719.835728206041332333.davem@davemloft.net>
On Wed, Nov 05, 2014 at 04:57:19PM -0500, David Miller wrote:
> From: Al Viro <viro@ZenIV.linux.org.uk>
> Date: Wed, 5 Nov 2014 21:07:45 +0000
>
> > Ping me when you put it there, OK? I'll rebase the rest of old stuff on
> > top of it (similar helpers, mostly).
>
> I just pushed it into net-next, thanks Al.
OK, I've taken the beginning of the old queue on top of net-next; it's
in git://git.kernel.org//pub/scm/linux/kernel/git/viro/vfs.git iov_iter-net.
>From the quick look at the remaining ->msg_iov users:
* I'll need to add several iov_iter primitives - counterparts of
checksum.h stuff (copy_and_csum_{from,to}_iter(), maybe some more). Not
a big deal, I'll do that tomorrow. That will give us a clean iov_iter-based
counterpart of skb_copy_and_csum_datagram_iovec().
* a new helper: zerocopy_sg_from_iter(). I have it, actually,
but I'd rather not step on Herbert's toes - it's too close to the areas
his series will touch, so that's probably for when his series goes in.
It will be needed for complete macvtap conversion...
* why doesn't verify_iovec() use rw_copy_check_uvector()? The only
real differences I see is that (a) you do allocation in callers (same as
rw_copy_check_uvector() would've done), (b) you return EMSGSIZE in case of
too long vector, while rw_copy_check_uvector() returns EINVAL in that case
and (c) you don't do access_ok(). The last one is described as optimization,
but for iov_iter primitives it's a serious PITA - for iovec-backed instances
they are using __copy_from_user()/__copy_to_user(), etc.
It certainly would be nice to have the same code doing all copying
of iovecs from userland - readv/writev/aio/sendmsg/recvmsg/etc. Am I
missing something subtle semantical difference in there? EMSGSIZE vs EINVAL
is trivial (we can lift that check into the callers, if nothing else), but
I could miss something more interesting...
* various getfrag will need to grow iov_iter-based counterparts,
but ip_append_output() needs no changes, AFAICS.
* crypto stuff will be easy to convert - iov_iter_get_pages()
would suffice for a primitive
* there's some really weird stuff in there. Just what is this
static int raw_probe_proto_opt(struct flowi4 *fl4, struct msghdr *msg)
{
struct iovec *iov;
u8 __user *type = NULL;
u8 __user *code = NULL;
int probed = 0;
unsigned int i;
if (!msg->msg_iov)
return 0;
for (i = 0; i < msg->msg_iovlen; i++) {
iov = &msg->msg_iov[i];
if (!iov)
continue;
trying to do? "If non-NULL pointer + i somehow happened to be NULL, skip it
and try to use the same pointer + i + 1"? Huh? Had been that way since
the function first went in back in 2004 ("[IPV4] XFRM: probe icmp type/code
when sending packets via raw socket.", according to historical tree)...
* rds, bluetooth and vsock are doing something odd; need to RTFS some
more.
* not sure I understand what TIPC is doing - does it prohibit too
short first segment of ->msg_iov? net/tipc/socket.c:dest_name_check() looks
odd _and_ potentially racy - we read the same data twice and hope our checks
still apply. I asked TIPC folks about that race back in April, but it
looks like that fell through the cracks...
Overall, so far it looks more or less feasible - other than the missing csum
primitives, current mm/iov_iter.c should suffice. I have _not_ seriously
looked into sendpage yet; that might very well require some more.
^ permalink raw reply
* Re: [PATCH net-next] fou: Fix typo in returning flags in netlink
From: David Miller @ 2014-11-06 3:18 UTC (permalink / raw)
To: therbert; +Cc: netdev
In-Reply-To: <1415234978-31931-1-git-send-email-therbert@google.com>
From: Tom Herbert <therbert@google.com>
Date: Wed, 5 Nov 2014 16:49:38 -0800
> When filling netlink info, dport is being returned as flags. Fix
> instances to return correct value.
>
> Signed-off-by: Tom Herbert <therbert@google.com>
Applied, thanks Tom.
^ permalink raw reply
* Re: [PATCH net-next] r8152: disable the tasklet by default
From: David Miller @ 2014-11-06 3:17 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-83-Taiwan-albertk@realtek.com>
From: Hayes Wang <hayeswang@realtek.com>
Date: Wed, 5 Nov 2014 10:17:02 +0800
> Let the tasklet only be enabled after open(), and be disabled for
> the other situation. The tasklet is only necessary after open() for
> tx/rx, so it could be disabled by default.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net v4] ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs
From: David Miller @ 2014-11-06 3:14 UTC (permalink / raw)
To: dborkman; +Cc: lw1a2.jing, netdev, edumazet, hannes, david.stevens
In-Reply-To: <1415215658-10054-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Wed, 5 Nov 2014 20:27:38 +0100
> It has been reported that generating an MLD listener report on
> devices with large MTUs (e.g. 9000) and a high number of IPv6
> addresses can trigger a skb_over_panic():
...
> mld_newpack() skb allocations are usually requested with dev->mtu
> in size, since commit 72e09ad107e7 ("ipv6: avoid high order allocations")
> we have changed the limit in order to be less likely to fail.
>
> However, in MLD/IGMP code, we have some rather ugly AVAILABLE(skb)
> macros, which determine if we may end up doing an skb_put() for
> adding another record. To avoid possible fragmentation, we check
> the skb's tailroom as skb->dev->mtu - skb->len, which is a wrong
> assumption as the actual max allocation size can be much smaller.
>
> The IGMP case doesn't have this issue as commit 57e1ab6eaddc
> ("igmp: refine skb allocations") stores the allocation size in
> the cb[].
>
> Set a reserved_tailroom to make it fit into the MTU and use
> skb_availroom() helper instead. This also allows to get rid of
> igmp_skb_size().
>
> Reported-by: Wei Liu <lw1a2.jing@gmail.com>
> Fixes: 72e09ad107e7 ("ipv6: avoid high order allocations")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
This has always been a tricky area, applied and queued up for
-stable, thanks everyone.
^ permalink raw reply
* Re: [PATCH v2 net-next] udp: Increment UDP_MIB_IGNOREDMULTI for arriving unmatched multicasts
From: David Miller @ 2014-11-06 3:11 UTC (permalink / raw)
To: raj; +Cc: netdev
In-Reply-To: <20141104234710.7FC7C290039D@tardy>
From: raj@tardy.usa.hp.com (Rick Jones)
Date: Tue, 4 Nov 2014 15:47:10 -0800 (PST)
> @@ -1656,6 +1657,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
> int dif = skb->dev->ifindex;
> unsigned int count = 0, offset = offsetof(typeof(*sk), sk_nulls_node);
> unsigned int hash2 = 0, hash2_any = 0, use_hash2 = (hslot->count > 10);
> + unsigned int inner_flushed = 0;
>
> if (use_hash2) {
> hash2_any = udp4_portaddr_hash(net, htonl(INADDR_ANY), hnum) &
...
> @@ -781,6 +781,7 @@ static int __udp6_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
> int dif = inet6_iif(skb);
> unsigned int count = 0, offset = offsetof(typeof(*sk), sk_nulls_node);
> unsigned int hash2 = 0, hash2_any = 0, use_hash2 = (hslot->count > 10);
> + int inner_flushed = 0;
Please use bool/true/false for inner_flushed in these two functions.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: Convert SEQ_START_TOKEN/seq_printf to seq_puts
From: David Miller @ 2014-11-06 3:05 UTC (permalink / raw)
To: joe; +Cc: netdev
In-Reply-To: <1415144223.1508.1.camel@perches.com>
From: Joe Perches <joe@perches.com>
Date: Tue, 04 Nov 2014 15:37:03 -0800
> Using a single fixed string is smaller code size than using
> a format and many string arguments.
>
> Reduces overall code size a little.
>
> $ size net/ipv4/igmp.o* net/ipv6/mcast.o* net/ipv6/ip6_flowlabel.o*
> text data bss dec hex filename
> 34269 7012 14824 56105 db29 net/ipv4/igmp.o.new
> 34315 7012 14824 56151 db57 net/ipv4/igmp.o.old
> 30078 7869 13200 51147 c7cb net/ipv6/mcast.o.new
> 30105 7869 13200 51174 c7e6 net/ipv6/mcast.o.old
> 11434 3748 8580 23762 5cd2 net/ipv6/ip6_flowlabel.o.new
> 11491 3748 8580 23819 5d0b net/ipv6/ip6_flowlabel.o.old
>
> Signed-off-by: Joe Perches <joe@perches.com>
Ok, I'm fine with this, applied.
Thanks Joe.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox