Re: [PATCH] net: geneve: Fix a possible null-pointer dereference in geneve_link_config()
On Mon, 29 Jul 2019 12:30:55 +0200, Jiri Benc wrote: > Are you sure rt6_lookup can never return a non-NULL rt with rt->dst.dev > being NULL? You'd leak the reference in such case. In fact, you're introducing a bug, not fixing one. ip6_rt_put does accept NULL parameter. And it seems you already have been told that? Nacked-by: Jiri Benc Jiri
Re: [PATCH] net: geneve: Fix a possible null-pointer dereference in geneve_link_config()
On Mon, 29 Jul 2019 18:26:11 +0800, Jia-Ju Bai wrote: > --- a/drivers/net/geneve.c > +++ b/drivers/net/geneve.c > @@ -1521,9 +1521,10 @@ static void geneve_link_config(struct net_device *dev, > rt = rt6_lookup(geneve->net, &info->key.u.ipv6.dst, NULL, 0, > NULL, 0); > > - if (rt && rt->dst.dev) > + if (rt && rt->dst.dev) { > ldev_mtu = rt->dst.dev->mtu - GENEVE_IPV6_HLEN; > - ip6_rt_put(rt); > + ip6_rt_put(rt); > + } > break; > } > #endif Are you sure rt6_lookup can never return a non-NULL rt with rt->dst.dev being NULL? You'd leak the reference in such case. Jiri
Re: [PATCH AUTOSEL 5.2 226/249] selftests: bpf: fix inlines in test_lwt_seg6local
On Thu, 18 Jul 2019 11:55:34 -0700 (PDT), David Miller wrote: > It has a significant impact on automated testing which lots of > individuals and entities perform, therefore I think it very much is > -stable material. Okay. Thanks, Jiri
Re: [PATCH AUTOSEL 5.2 226/249] selftests: bpf: fix inlines in test_lwt_seg6local
On Wed, 17 Jul 2019 19:47:57 -0400, Sasha Levin wrote: > It fixes a bug, right? A bug in selftests. And quite likely, it probably happens only with some compiler versions. I don't think patches only touching tools/testing/selftests/ qualify for stable in general. They don't affect the end users. Jiri
Re: [PATCH AUTOSEL 5.2 226/249] selftests: bpf: fix inlines in test_lwt_seg6local
On Mon, 15 Jul 2019 09:46:31 -0400, Sasha Levin wrote: > From: Jiri Benc > > [ Upstream commit 11aca65ec4db09527d3e9b6b41a0615b7da4386b ] > > Selftests are reporting this failure in test_lwt_seg6local.sh: I don't think this is critical in any way and I don't think this is a stable material. How was this selected? Jiri
Re: [PATCH net 1/1 v4] rtnetlink: require unique netns identifier
On Wed, 7 Feb 2018 14:36:21 +0100, Christian Brauner wrote: > On Wed, Feb 07, 2018 at 04:20:01PM +0300, Kirill Tkhai wrote: > > Can't we write these 3 above branches more compact? Something like this: > > > > if (!!tb[IFLA_NET_NS_FD] + !!tb[IFLA_IF_NETNSID] + > > !!tb[IFLA_NET_NS_PID] <= 1) > > return 0; > > I always prefer for conditions to be separate and readable even if it > means additional code. But if others feel that there's value in avoiding > two additional conditions I'm happy to adapt the patch. FWIW, I don't like the n x n conditions much. But Kirill's proposal seems not to be much better. I was thinking about: int cnt = 0; if (tb[IFLA_NET_NS_FD]) cnt++; if (tb[IFLA_NET_NS_PID]) cnt++; if (tb[IFLA_NET_NETNSID]) cnt++; if (cnt > 1) { ...errorr... } but that's not better, either. As we're unlikely to add a fourth value, I guess I'm okay with the current approach in the patch. > Before I added support for netns ids for additional requests Jiri made > it so that all request that specified properties that they did not > support returned ENOTSUPP instead of EINVAL. This just keeps things > consistent. Users would now suddenly receive EINVAL. That's potentially > confusing. Yes, please, keep -EOPNOTSUPP. > As for the case of passing multiple netns identifying properties into > the same request EINVAL seems the perfect candidate and the error > message seems instructive to userspace programs. Agreed. Acked-by: Jiri Benc Thanks, Jiri
Re: [PATCH net 1/1 v3] rtnetlink: require unique netns identifier
On Tue, 6 Feb 2018 14:19:02 +0100, Christian Brauner wrote: > +/* Verify that rtnetlink requests supporting network namespace ids > + * do not pass additional properties potentially referring to different > + * network namespaces. > + */ > +static int rtnl_ensure_unique_netns(struct nlattr *tb[], > + struct netlink_ext_ack *extack) > +{ > + /* Requests without network namespace ids have been able to specify > + * multiple properties referring to different network namespaces so > + * don't regress them. > + */ > + if (!tb[IFLA_IF_NETNSID]) > + return 0; I agree with Eric that we should enforce this also for the existing pid/fd attributes. > + > + /* Caller operates on the current network namespace. */ > + if (!tb[IFLA_NET_NS_PID] && !tb[IFLA_NET_NS_FD]) > + return 0; > + > + NL_SET_ERR_MSG(extack, "multiple netns identifying attributes > specified"); > + return -EINVAL; But if we don't reach an agreement on that, this version is the next best one. No reason to compare the namespaces whether they're the same, a message with more than one such attribute is just invalid. > @@ -2649,6 +2675,10 @@ static int rtnl_dellink(struct sk_buff *skb, struct > nlmsghdr *nlh, > if (err < 0) > return err; > > + err = rtnl_ensure_unique_netns(tb, extack); > + if (err < 0) > + return err; > + > if (tb[IFLA_IFNAME]) > nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ); > > @@ -3045,6 +3079,10 @@ static int rtnl_getlink(struct sk_buff *skb, struct > nlmsghdr *nlh, > if (err < 0) > return err; > > + err = rtnl_ensure_unique_netns(tb, extack); > + if (err < 0) > + return err; > + > if (tb[IFLA_IF_NETNSID]) { > netnsid = nla_get_s32(tb[IFLA_IF_NETNSID]); > tgt_net = get_target_net(NETLINK_CB(skb).sk, netnsid); dellink and getlink support only netnsid, we should just reject a message with pid or fd set. Jiri
Re: [PATCH net 1/1 v2] rtnetlink: require unique netns identifier
On Tue, 06 Feb 2018 16:31:29 -0600, Eric W. Biederman wrote: > Frankly. If we are talking precedence it should be: > fds > netnsids > pids The current order is 1. pids, 2. fds, though. Not that it matters much, see below. > I do think it makes a lot of sense to error if someone passes in > duplicate arguments. AKA multiple attribute that could select for > the same thing. No one will do that deliberately. It doesn't make > sense. So it is just a nonsense case we have to handle gracefully, > and correctly. With correctness being the most important as otherwise > people might just send in nonsense to exploit bugs. Completely agreed. Let's just start returning error if more than one of the pid/fs/netnsid attributes is specified. I don't think this is going to break any user. And we'll not have to care about the order. > I agree refusing to combine multiple attributes for the same thing > sounds the most sensible course. Yes, please. Thanks! Jiri
Re: [PATCH net-next 0/3 V1] rtnetlink: enable IFLA_IF_NETNSID for RTM_{DEL,SET}LINK
On Fri, 26 Jan 2018 00:34:51 +0100, Nicolas Dichtel wrote: > Why meaningful? The user knows that the answer is like if if was done in > another > netns. It enables to have only one netlink socket instead of one per netns. > But > the code using it will be the same. Because you can't use it to query the linked interface. You can't even use it as an opaque value to track interfaces (netnsid+ifindex) because netnsids are not unique across net name spaces. You can easily have two interfaces that have all the ifindex, ifname, netnsid (and basically everything else) identical but being completely different interfaces. That's really not helpful. > I fear that with your approach, it will results to a lot of complexity in the > kernel. The complexity is (at least partly) already there. It's an inevitable result of the design decision to have relative identifiers. I agree that we should think about how to make this easy to implement. I like your idea of doing this somehow generically. Perhaps it's possible to do while keeping the netnsids valid in the caller's netns? > What is really missing for me, is a way to get a fd from an nsid. The user > should be able to call RTM_GETNSID with an fd and a nsid and the kernel > performs > the needed operations so that the fd points to the corresponding netns. That's what I was missing, too. I even looked into implementing it. But opening a fd on behalf of the process and returning it over netlink is a wrong thing to do. Netlink messages can get lost. Then you have a fd leak you can do nothing about. Given that we have netnsids used for so much stuff already (like NETLINK_LISTEN_ALL_NSID) you need to track them anyway. And if you need to track them, why bother with another identifier? It would be better if netnsid can be used universally for anything. Then there will be no need for the conversion. Jiri
Re: [PATCH net-next 0/3 V1] rtnetlink: enable IFLA_IF_NETNSID for RTM_{DEL,SET}LINK
On Thu, 25 Jan 2018 15:20:59 +0100, Nicolas Dichtel wrote: > Hmm, I don't agree. For me, it would be the correct answer. If user has a > socket > in ns_a and targets a RTM_GETLINK in ns_b, the answer he gets should be like > if > it was done in ns_b. But that information would be useless for the caller. Why return a value that has no meaning for the caller and can not be used? More so when the kernel is aware of what the correct meaningful value is? > This is already the case with messages received with NETLINK_LISTEN_ALL_NSID, > there is no reason to do something different. NETLINK_LISTEN_ALL_NSID is tough due to way it is implemented. But yes, it should translate the netnsids to be valid in the socket's netns. That's the only sane way for the listener. Jiri
Re: [PATCH net-next 0/3 V1] rtnetlink: enable IFLA_IF_NETNSID for RTM_{DEL,SET}LINK
On Wed, 24 Jan 2018 16:24:34 +0100, Nicolas Dichtel wrote: > I wonder if it would be possible to do something in the netlink framework, > like > NETLINK_LISTEN_ALL_NSID. > Having some ancillary data at the netlink socket level and a function like > nlsock_net() (instead of sock_net()) to get the corresponding netns. > With that, it would be possible, in a generci way, to support this feature for > all netlink family. I'm not sure it's worth the effort to do that in the framework. You'll need modifications all the way down to the code that generates the attributes anyway. It's not enough to just specify that the operation should be done on a different netns and hide that from the handlers. Take for example the existing RTM_GETLINK. Let's say it's executed from within ns_a and targeted to ns_b (thus IFLA_IF_NETNSID = netnsid of ns_b). Now, if there's a veth interface in ns_b whose other end is in ns_c, there will be IFLA_LINK_NETNSID attribute in the response. But the value has to be netnsid of ns_c as seen from *ns_a*. If you just pretended to switch to ns_b before invoking rtnl_getlink, it would be netnsid of ns_c as seen from ns_b which would be wrong. That's why 79e1ad148c844 added the tgt_net stuff. Jiri
Re: [PATCH net-next 1/1] rtnetlink: request RTM_GETLINK by pid or fd
On Wed, 24 Jan 2018 11:53:11 +0100, Nicolas Dichtel wrote: > Le 23/01/2018 à 18:08, Jiri Benc a écrit : > > It would be much better if the whole (ifindex, netnsid) pair was > > returned. I think it could be added. > Sure. Do you plan to send a patch? I can do that but it will take a while. I'll be happy to leave that to someone else but if there's no one, I'll eventually do it. > > I think we need that. > If you agree, I can send a patch to remove this limitation. That would be great. Thanks! Jiri
Re: [PATCH net-next 1/1] rtnetlink: request RTM_GETLINK by pid or fd
On Tue, 23 Jan 2018 17:37:11 +0100, Nicolas Dichtel wrote: > When a virtual interface moves to another netns, the netlink RTM_DELLINK > message > contains the attribute IFLA_NEW_NETNSID, which identifies where the interface > moves. The nsid may be allocated if needed. The problem is that ifindex may change and it's not announced. The only way is to track both ifindex and ifname, watch for the ifname to appear in the target netns and update the application's view of ifindex. It would be much better if the whole (ifindex, netnsid) pair was returned. I think it could be added. > I don't know if it's acceptable to also allocate an nsid in case of a physical > interface. I think we need that. Thanks, Jiri
Re: [PATCH net-next 1/1] rtnetlink: request RTM_GETLINK by pid or fd
On Tue, 23 Jan 2018 11:26:58 +0100, Wolfgang Bumiller wrote: > Even if you know the netnsid, do the mentioned watches work for > nested/child namespaces if eg. a container creates new namespace before > and/or after the watch was established and moves interfaces to these > child namespaces, would you just see them disappear, or can you keep > track of them later on as well? What do you mean by "nested namespaces"? There's no such thing for net name spaces. As for missing API to get netnsid of the netns the interface is moved to, see my previous emails in this thread. This needs to be added. > Even if that works, from what the documentation tells me netlink is an > unreliable protocol, so if my watcher's socket buffer is full, wouldn't > I be losing important tracking information? Sure. But that's fundamentally unfixable independently on netlink, the kernel needs to take an action if a program is not reading its messages. Either some messages get dropped or the program is killed or infinite amount of memory is consumed. This has nothing to do with uAPI design. > I think one possible solution to tracking interfaces would be to have a > unique identifier that never changes (even if it's just a simple > uint64_t incremented whenever an interface is created). But since > they're not local to the current namespace that may require a lot of > extra permission checks (but I'm just speculating here...). You'll get a hard NACK from CRIU folks if you try to propose this. > In any case, IFLA_NET_NS_FD/PID are already there and I had been > wondering previously why they couldn't be used with RTM_GETLINK, it > would just make sense. Those predate netnsids and we can't get rid of them now, since they're part of uAPI. But we can (and should) make sure we don't add more of those. Jiri
Re: [PATCH net-next 1/1] rtnetlink: request RTM_GETLINK by pid or fd
On Mon, 22 Jan 2018 23:25:41 +0100, Christian Brauner wrote: > This is not necessarily true in scenarios where I move a network device > via RTM_NEWLINK + IFLA_NET_NS_PID into a network namespace I haven't > created. Here is an example: > > nlmsghdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; > nlmsghdr->nlmsg_type = RTM_NEWLINK; > /* move to network namespace of pid */ > nla_put_u32(nlmsg, IFLA_NET_NS_PID, pid) > /* give interface new name */ > nla_put_string(nlmsg, IFLA_IFNAME, ifname) > > The only thing I have is the pid that identifies the network namespace. How do you know the interface did not get renamed in the new netns? This is racy and won't work reliably. You really need to know the netnsid before moving the interface to the netns to be able to do meaningful queries. You may argue that for your case, you're fine with the race. But I know about use cases where it matters a lot: those are tools that show network topology including changes in real time, such as Skydive. It's important to have the uAPI designed right. And we don't want two different APIs for the same thing. If you want to do any watching over the interfaces (as opposed to "move to the netns and forget"), you really have to work with netnsids. Let's focus on how to do that more easily. We don't return netnsid at all places where we should return it and we need to fix that. > There's no non-syscall way to learn the netnsid. And that is the primary problem. Jiri
Re: [PATCH net-next 1/1] rtnetlink: request RTM_GETLINK by pid or fd
On Mon, 22 Jan 2018 22:23:54 +0100, Christian Brauner wrote: > That is certainly a good idea and I'm happy to send a follow-up patch > for that! Note that I haven't looked into that and I don't know whether it is easily possible. I'll appreciate if you could try that. > But there's still value in being able to use > IFLA_NET_NS_{FD,PID} in scenarios where the network namespace has been > created by another process. In this case we don't know what its netnsid > is and not even if it had been assigned one at creation time or not. In > this case it would be useful to refer to the netns via a pid or fd. A > more concrete and frequent example is querying a network namespace of a > (sorry for the buzzword :)) container for all defined network > interfaces. That's what spurred my original comment. If you don't know the netnsid in such case, we're missing something in uAPI but at a different point than RTM_GETLINK. When you find yourself in a need to query an interface in another netns, you had to learn about that interface in the first place. Meaning you got its ifindex (or ifname, perhaps) somehow. My point is, you should have learned the netnsid at the same time. ifindex alone is not an unique identifier of an interface. The (ifindex, netnsid) pair is. (Also, note that ifindex can change when moving the interface to a different netns.) So you should never get ifindex alone when the interface is in another netns than the current one. If that happens, that is the uAPI that needs to be fixed. You have to always get the (ifindex, netnsid) pair. And that's also the way how it should operate in the other direction: (ifindex, netnsid) is the identifier to query interface in another netns. Note that many APIs in networking are based around netnsid - look at NETLINK_LISTEN_ALL_NSID. It allows you to keep track of interfaces as they are moved from name spaces to name spaces using a single socket: you get notifications about interfaces disappearing or appearing in "watched" name spaces. The name spaces are, of course, referenced by their netnsid. In order to add another "watched" name space, just assign it (or, more typically, let the kernel assign) a netnsid. Btw, we have one missing piece here: when an interface is moved to a name space that does not have netnsid attached, we want to find out where the interface was moved to. But there's currently no reliable way to do it. For veth, the other end can be used to get the netnsid (note that RTM_GETLINK will return the correct link netnsid even when the queried veth interface is in a different name space), for openvswitch, we can now use genetlink, etc., but using different ways for different interface types is not the best API ever and for standalone interfaces we have nothing. I'd like to see something added to uAPI to cover this in a generic way. But as for this patch, I don't think it's the correct way. We do have missing pieces in uAPI wrt. netns support but I think they're at different places. If you have a counter example, please speak up. Thanks, Jiri
Re: [PATCH net-next 1/1] rtnetlink: request RTM_GETLINK by pid or fd
On Thu, 18 Jan 2018 21:55:53 +0100, Christian Brauner wrote: > A more concrete scenario is creating a network namespace, moving a > device into it via RTM_SETLINK which also supports IFLA_NET_NS_{FD,PID} > and then wanting to query the device. This would be very easy to do if > one could reuse the IFLA_NET_NS_{FD,PID} without having to set a > netnsid. Wouldn't be a better solution to have a way to request the netnsid allocation (and return it) right away when creating the name space, then? Jiri
Re: [PATCH net-next 1/1] rtnetlink: request RTM_GETLINK by pid or fd
On Thu, 18 Jan 2018 21:21:24 +0100, Christian Brauner wrote: > In such scenarios setting a netns id property is > not really wanted Why? I think that's what you should do if you want to avoid setns. Just use netnsid. I don't see any problem with that and you didn't provide any explanation. Jiri
Re: [PATCH] rtnetlink: fix missing size for IFLA_IF_NETNSID
On Mon, 6 Nov 2017 15:04:54 +, Colin King wrote: > The size for IFLA_IF_NETNSID is missing from the size calculation > because the proceeding semicolon was not removed. Fix this by removing > the semicolon. Acked-by: Jiri Benc Thanks for spotting this! Looking at my initial code, I had that right, this was probably introduced during one of rebases, so I blame Flavio :-p (On a serious note, thank you, Flavio, for taking care of the rebases.) Hopefully, with the "+ 0" added, this won't happen again in this particular piece of code in the future. Jiri
Re: [PATCH net-next 2/2] vxlan: add back error messages to vxlan_config_validate() as extended netlink acks
On Tue, 27 Jun 2017 22:47:58 +0200, Matthias Schiffer wrote: > if ((conf->flags & ~VXLAN_F_ALLOWED_GPE) || > !(conf->flags & VXLAN_F_COLLECT_METADATA)) { > + NL_SET_ERR_MSG(extack, > +"unsupported combination of extensions"); Since we're redesigning this, let's be more helpful to the user. There's probably not going to be tremendous improvements here but let's try at least a bit. "VXLAN GPE does not support this combination of extensions" > if (local_type & IPV6_ADDR_LINKLOCAL) { > if (!(remote_type & IPV6_ADDR_LINKLOCAL) && > - (remote_type != IPV6_ADDR_ANY)) > + (remote_type != IPV6_ADDR_ANY)) { > + NL_SET_ERR_MSG(extack, > +"invalid combination of > address scopes"); "invalid combination of local and remote address scopes" > return -EINVAL; > + } > > conf->flags |= VXLAN_F_IPV6_LINKLOCAL; > } else { > if (remote_type == > - (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL)) > + (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL)) { > + NL_SET_ERR_MSG(extack, > +"invalid combination of > address scopes"); ditto The rest looks good to me. Thanks a lot for doing the work, Matthias! Jiri
Re: [PATCH net-next 1/2] vxlan: change vxlan_validate() to use netlink_ext_ack for error reporting
On Tue, 27 Jun 2017 22:47:57 +0200, Matthias Schiffer wrote: > if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS]))) { > - pr_debug("invalid all zero ethernet address\n"); > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_ADDRESS], > + "invalid ethernet address"); Could we be more specific here? This is better than nothing but still not as helpful to the user as it could be. What about something like "the provided ethernet address is not unicast"? > - if (mtu < ETH_MIN_MTU || mtu > ETH_MAX_MTU) > + if (mtu < ETH_MIN_MTU || mtu > ETH_MAX_MTU) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_MTU], > + "invalid MTU"); "MTU must be between 68 and 65535" > - if (id >= VXLAN_N_VID) > + if (id >= VXLAN_N_VID) { > + NL_SET_ERR_MSG_ATTR(extack, data[IFLA_VXLAN_ID], > + "invalid VXLAN ID"); "VXLAN ID must be lower than 16777216" > if (ntohs(p->high) < ntohs(p->low)) { > - pr_debug("port range %u .. %u not valid\n", > - ntohs(p->low), ntohs(p->high)); > + NL_SET_ERR_MSG_ATTR(extack, data[IFLA_VXLAN_PORT_RANGE], > + "port range not valid"); Since you're getting rid of the values output, I'd rather suggest more explicit "the first value of the port range must not be higher than the second value" or so. Shorter wording is welcome :-) Thanks, Jiri
Re: [PATCH net-next v3 1/6] vxlan: refactor verification and application of configuration
This patchset looks good overall (would send my Acked-by for most of this but I'm late). On Mon, 19 Jun 2017 10:03:55 +0200, Matthias Schiffer wrote: > Log messages in these > functions are removed, as it is generally unexpected to find error output > for netlink requests in the kernel log. Userspace should be able to handle > errors based on the error codes returned via netlink just fine. However, this is not really true. It's impossible to find out what went wrong when you use e.g. iproute2 to configure a vxlan link. We really need to convert the kernel log messages to the extended netlink errors. Since you removed them prematurely, could you please work on that? Thanks, Jiri
Re: [PATCH net v2] openvswitch: Fix ovs_flow_key_update()
On Thu, 30 Mar 2017 12:36:03 -0700, Yi-Hung Wei wrote: > ovs_flow_key_update() is called when the flow key is invalid, and it is > used to update and revalidate the flow key. Commit 329f45bc4f19 > ("openvswitch: add mac_proto field to the flow key") introduces mac_proto > field to flow key and use it to determine whether the flow key is valid. > However, the commit does not update the code path in ovs_flow_key_update() > to revalidate the flow key which may cause BUG_ON() on execute_recirc(). > This patch addresses the aforementioned issue. > > Fixes: 329f45bc4f19 ("openvswitch: add mac_proto field to the flow key") > Signed-off-by: Yi-Hung Wei Acked-by: Jiri Benc Thanks! Jiri
Re: [PATCH net] openvswitch: Fix ovs_flow_key_update()
On Thu, 30 Mar 2017 11:39:35 -0700, Yi-Hung Wei wrote: > If we invalidate a flow key of a L3 packet, the flow's mac_proto is like this > (MAC_PROTO_NONE | SW_FLOW_KEY_INVALID), then key_extract() will > process the link layer of this L3 packet since mac_proto !=MAC_PROTO_NONE? > > In this case, shall we update key_extract() like this > static int key_extract(struct sk_buff *skb, struct sw_flow_key *key) > > /* Link layer. */ > clear_vlan(key); > - if (key->mac_proto == MAC_PROTO_NONE) { > + if (key->mac_proto & MAC_PROTO_NONE) { Use ovs_key_mac_proto(key) == MAC_PROTO_NONE. Jiri
Re: [PATCH net] openvswitch: Fix ovs_flow_key_update()
On Wed, 29 Mar 2017 17:14:10 -0700, Yi-Hung Wei wrote: > ovs_flow_key_update() is called when the flow key is invalid, and it is > used to update and revalidate the flow key. Commit 329f45bc4f19 > ("openvswitch: add mac_proto field to the flow key") introduces mac_proto > field to flow key and use it to determine whether the flow key is valid. > However, the commit does not update the code path in ovs_flow_key_update() > to revalidate the flow key which may cause BUG_ON() on execute_recirc(). > This patch addresses the aforementioned issue. Good catch. > int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key) > { > + int res; > + > + res = key_extract_mac_proto(skb); > + if (res < 0) > + return res; > + key->mac_proto = res; > + > return key_extract(skb, key); > } But this should just reset the SW_FLOW_KEY_INVALID flag, there's no need to recompute mac_proto. Something like this: int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key) { - return key_extract(skb, key); + int res; + + res = key_extract(skb, key); + if (!res) + key->mac_proto &= ~SW_FLOW_KEY_INVALID; + return res; } Thanks, Jiri
Re: [PATCH net-next 3/3] vxlan: allow multiple VXLANs with same VNI for IPv6 link-local addresses
On Wed, 15 Mar 2017 15:29:29 +0100, Matthias Schiffer wrote: > While ensuring that the destination address is link-local iff the source > address is would also be an option, it didn't seem too useful as the > destination address will be a multicast address anyways in "normal" VXLAN > configurations. If we really want to check this, I guess the valid > combinations are: > > source link-local - destination link-local UC > source link-local - destination link-local MC > source global/... - destination global/... UC > source global/... - destination any MC > > Does this make sense? It does. Thanks! Jiri
Re: [PATCH net-next 3/3] vxlan: allow multiple VXLANs with same VNI for IPv6 link-local addresses
On Fri, 10 Mar 2017 23:39:44 +0100, Matthias Schiffer wrote: > @@ -233,17 +234,30 @@ static struct vxlan_dev *vxlan_vs_find_vni(struct > vxlan_sock *vs, __be32 vni) > vni = 0; > > hlist_for_each_entry_rcu(vxlan, vni_head(vs, vni), hlist) { > - if (vxlan->default_dst.remote_vni == vni) > - return vxlan; > + if (vxlan->default_dst.remote_vni != vni) > + continue; > + > + if (IS_ENABLED(CONFIG_IPV6)) { > + const struct vxlan_config *cfg = &vxlan->cfg; > + > + if (cfg->remote_ifindex != 0 && > + cfg->remote_ifindex != ifindex && > + cfg->saddr.sa.sa_family == AF_INET6 && > + (ipv6_addr_type(&cfg->saddr.sin6.sin6_addr) & > + IPV6_ADDR_LINKLOCAL)) Calculating this (especially ipv6_addr_type) on every received packet looks unnecessarily expensive. Just store the fact the the local address is link-local in a flag during config. And compare the flag first before considering remote_ifindex. This is especially important for lwtunnels which can have anything in the saddr and remote_ifindex, yet those fields are ignored and the vni 0 entry has to be returned. It also means that the link-local flag must not be set for lwtunnels. > +#if IS_ENABLED(CONFIG_IPV6) > + if (conf->remote_ifindex != tmp->cfg.remote_ifindex && > + conf->saddr.sa.sa_family == AF_INET6 && > + tmp->cfg.saddr.sa.sa_family == AF_INET6 && > + (ipv6_addr_type(&conf->saddr.sin6.sin6_addr) & > + IPV6_ADDR_LINKLOCAL) && > + (ipv6_addr_type(&tmp->cfg.saddr.sin6.sin6_addr) & > + IPV6_ADDR_LINKLOCAL)) > + continue; > +#endif In patch 1, you're checking for either source and destination address being link-local, while here you're consider only those that have both addresses link-local. Wouldn't it be better to plainly reject configuration that has one address link-local but not the other one? Jiri
Re: [PATCH net-next 2/3] vxlan: fix snooping for link-local IPv6 addresses
On Fri, 10 Mar 2017 23:39:43 +0100, Matthias Schiffer wrote: > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -941,16 +941,24 @@ static int vxlan_fdb_dump(struct sk_buff *skb, struct > netlink_callback *cb, > */ > static bool vxlan_snoop(struct net_device *dev, > union vxlan_addr *src_ip, const u8 *src_mac, > - __be32 vni) > + __u32 src_ifindex, __be32 vni) Just "u32", please. No double underscores for u32 inside the kernel-only code. (Also in other places in this patch.) Thanks, Jiri
Re: [PATCH net-next 1/3] vxlan: don't allow link-local IPv6 local/remote addresses without interface
On Fri, 10 Mar 2017 23:39:42 +0100, Matthias Schiffer wrote: > Signed-off-by: Matthias Schiffer Acked-by: Jiri Benc
Re: Misalignment, MIPS, and ip_hdr(skb)->version
On Wed, 07 Dec 2016 23:34:21 -0500, Daniel Kahn Gillmor wrote: > fwiw, i'm not convinced that "most protocols of the IETF follow this > mantra". we've had multiple discussions in different protocol groups > about shaving or bloating by a few bytes here or there in different > protocols, and i don't think anyone has brought up memory alignment as > an argument in any of the discussions i've followed. Which is sad. One would expect that this would be well understood for decades already. Jiri
Re: [PATCH v2] vxlan: fix a potential issue when create a new vxlan fdb entry.
On Tue, 29 Nov 2016 09:59:36 +0800, Haishuang Yan wrote: > vxlan_fdb_append may return error, so add the proper check, > otherwise it will cause memory leak. > > Signed-off-by: Haishuang Yan > > Changes in v2: > - Unnecessary to initialize rc to zero. Acked-by: Jiri Benc
Re: [PATCH] vxlan: fix a potential issue when create a new vxlan fdb entry.
On Mon, 28 Nov 2016 15:02:23 +0800, Haishuang Yan wrote: > vxlan_fdb_append may return error, so add the proper check, > otherwise it will cause memory leak. > > Signed-off-by: Haishuang Yan > --- > drivers/net/vxlan.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c > index 21e92be..3b7b237 100644 > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -611,6 +611,7 @@ static int vxlan_fdb_create(struct vxlan_dev *vxlan, > struct vxlan_rdst *rd = NULL; > struct vxlan_fdb *f; > int notify = 0; > + int rc = 0; The initialization to 0 should not be needed. Looks good otherwise. Thanks, Jiri
Re: [PATCH] vxlan: hide unused local variable
On Mon, 7 Nov 2016 22:09:07 +0100, Arnd Bergmann wrote: > A bugfix introduced a harmless warning in v4.9-rc4: > > drivers/net/vxlan.c: In function 'vxlan_group_used': > drivers/net/vxlan.c:947:21: error: unused variable 'sock6' > [-Werror=unused-variable] > > This hides the variable inside of the same #ifdef that is > around its user. The extraneous initialization is removed > at the same time, it was accidentally introduced in the > same commit. > > Fixes: c6fcc4fc5f8b ("vxlan: avoid using stale vxlan socket.") > Signed-off-by: Arnd Bergmann I think this should be applied instead of Pravin's patch. It fixes just the one problem, contains the proper Fixes: tag and addresses net.git. Acked-by: Jiri Benc
Re: [PATCH net-next 4/6] net: use core MTU range checking in core net infra
On Tue, 18 Oct 2016 22:33:31 -0400, Jarod Wilson wrote: > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -2367,43 +2367,31 @@ static void vxlan_set_multicast_list(struct > net_device *dev) > { > } > > -static int __vxlan_change_mtu(struct net_device *dev, > - struct net_device *lowerdev, > - struct vxlan_rdst *dst, int new_mtu, bool strict) > +static int vxlan_change_mtu(struct net_device *dev, int new_mtu) > { > - int max_mtu = IP_MAX_MTU; > - > - if (lowerdev) > - max_mtu = lowerdev->mtu; > + struct vxlan_dev *vxlan = netdev_priv(dev); > + struct vxlan_rdst *dst = &vxlan->default_dst; > + struct net_device *lowerdev = __dev_get_by_index(vxlan->net, > + dst->remote_ifindex); > + bool use_ipv6 = false; > > if (dst->remote_ip.sa.sa_family == AF_INET6) > - max_mtu -= VXLAN6_HEADROOM; > - else > - max_mtu -= VXLAN_HEADROOM; > - > - if (new_mtu < 68) > - return -EINVAL; > + use_ipv6 = true; > > - if (new_mtu > max_mtu) { > - if (strict) > + /* We re-check this, because users *could* alter the mtu of the > + * lower device after we've initialized dev->max_mtu. > + */ > + if (lowerdev) { > + dev->max_mtu = lowerdev->mtu - > +(use_ipv6 ? VXLAN6_HEADROOM : VXLAN_HEADROOM); > + if (new_mtu > dev->max_mtu) > return -EINVAL; > - > - new_mtu = max_mtu; > } > > dev->mtu = new_mtu; > return 0; > } Sorry for the silly question, how does the min_mtu and max_mtu stuff works? I noticed your patches but haven't looked in depth into them. When the ndo_change_mtu callback is defined, is the dev->min_mtu and dev->max_mtu checked first and if the desired mtu is not within range, ndo_change_mtu is not called? Or does ndo_change_mtu override the checks? In either case, the code does not look correct. In the first case, increasing of lowerdev MTU wouldn't allow increasing of vxlan MTU without deleting and recreating the vxlan interface. In the second case, you're missing check against the min_mtu. > > -static int vxlan_change_mtu(struct net_device *dev, int new_mtu) > -{ > - struct vxlan_dev *vxlan = netdev_priv(dev); > - struct vxlan_rdst *dst = &vxlan->default_dst; > - struct net_device *lowerdev = __dev_get_by_index(vxlan->net, > - dst->remote_ifindex); > - return __vxlan_change_mtu(dev, lowerdev, dst, new_mtu, true); > -} > - > static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff > *skb) > { > struct vxlan_dev *vxlan = netdev_priv(dev); > @@ -2795,6 +2783,10 @@ static int vxlan_dev_configure(struct net *src_net, > struct net_device *dev, > vxlan_ether_setup(dev); > } > > + /* MTU range: 68 - 65535 */ > + dev->min_mtu = 68; > + dev->max_mtu = IP_MAX_MTU; > + > vxlan->net = src_net; > > dst->remote_vni = conf->vni; > @@ -2837,8 +2829,11 @@ static int vxlan_dev_configure(struct net *src_net, > struct net_device *dev, > } > #endif > > - if (!conf->mtu) > - dev->mtu = lowerdev->mtu - (use_ipv6 ? VXLAN6_HEADROOM > : VXLAN_HEADROOM); > + if (!conf->mtu) { > + dev->mtu = lowerdev->mtu - > +(use_ipv6 ? VXLAN6_HEADROOM : > VXLAN_HEADROOM); > + dev->max_mtu = dev->mtu; > + } > > needed_headroom = lowerdev->hard_header_len; > } else if (vxlan_addr_multicast(&dst->remote_ip)) { > @@ -2847,9 +2842,14 @@ static int vxlan_dev_configure(struct net *src_net, > struct net_device *dev, > } > > if (conf->mtu) { > - err = __vxlan_change_mtu(dev, lowerdev, dst, conf->mtu, false); > - if (err) > - return err; > + if (lowerdev) > + dev->max_mtu = lowerdev->mtu; > + dev->max_mtu -= (use_ipv6 ? VXLAN6_HEADROOM : VXLAN_HEADROOM); > + > + dev->mtu = conf->mtu; > + > + if (conf->mtu > dev->max_mtu) > + dev->mtu = dev->max_mtu; > } You removed the check for min_mtu but it's needed here. The conf->mtu value comes from the user space and can be anything. > > if (use_ipv6 || conf->flags & VXLAN_F_COLLECT_METADATA) Jiri
Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
On Wed, 28 Sep 2016 19:36:45 -0700 (PDT), R. Parameswaran wrote: > I'd like to point out one difference with VXLAN - in VXLAN, the > local physical interface is directly specified at the time of > creation of the tunnel, and the data structure seems to have the ifindex > of the local interface with which it is able to directly pull up the > underlay interface device. Whereas in L2TP, we only have the IP > address of the remote tunnel end-point and thus only the socket and the > dst from which we need to derive this. Strictly speaking, VXLAN *may* know the underlying interface. It can also be set up with just local and remote IP address, or even worse, in metadata mode where we don't know the address nor the interface until we get a packet (and each packet may have those different). MTU wise, those cases are not accommodated for in the kernel. The vxlan interface gets MTU of 1500 and it's up to the administrator to set it correctly. Btw, PMTU events won't help with the metadata mode. And even in "normal" mode, it's not clear what should be done - the tunnel interface may be in a bridge, thus there may be other interfaces that depend on the same MTU, up to inside VMs. Jiri
Re: [PATCH v2 0/2] make POSIX timers optional
On Wed, 21 Sep 2016 10:38:52 +0200, Richard Cochran wrote: > Embedded people like to optimize their systems. One pattern I have > more than once is that a multihomed design designates a special PTP > interface, often with a different HW than the other ports. PTP > support adds extra code into the hot path, and for that reason people > want to turn it off on interfaces that don't need it. We really need to find some middle way. It's impossible to support every little corner case. As is not reasonable to have a single configuration only kernel. I think this scenario falls into the little corner case. If you need PTP, you just enable it. The PTP core will be much larger than the added code in the individual drivers anyway. And if there's run time impact on the fast path with time stamping switched off on an interface, it's a bug that has to be fixed. Jiri
Re: [PATCH v2 0/2] make POSIX timers optional
On Tue, 20 Sep 2016 23:09:52 +0200 (CEST), Thomas Gleixner wrote: > Now if you want to distangle PTP from a driver then you split it at the > driver level and not at the PTP level: > > DRIVER_X > tristate "Driver X" > > DRIVER_X_PTP > bool "Enable PTP support" > default y if !MAKE_IT_TINY > depends on DRIVER_X > select PTP Ouch. So, after the hassle to remove the VXLAN and GENEVE configs from tons of drivers, we'll add another one instead? That's just silly. If we did this for every thing we support in NICs, we would end up with something completely unmanageable. PTP should be really configured by a single switch. Btw., your suggestion does not work, select is not recursive. > We have already drivers following that scheme. That way you make the PTP > support in the driver conditional on DRIVER_X_PTP and have no hassle with > modules and dependencies. We've recently got rid of some of those, thankfully. Caused more harm than good. Jiri
Re: [PATCH 1/2] ptp_clock: allow for it to be optional
On Mon, 19 Sep 2016 10:10:21 -0400 (EDT), Nicolas Pitre wrote: > --- a/include/linux/ptp_clock_kernel.h > +++ b/include/linux/ptp_clock_kernel.h > @@ -207,7 +207,16 @@ int ptp_find_pin(struct ptp_clock *ptp, > #else > static inline struct ptp_clock *ptp_clock_register(struct ptp_clock_info > *info, > struct device *parent) > -{ return NULL; } > +{ > + if (IS_MODULE(CONFIG_PTP_1588_CLOCK)) { > + pr_warn("%s is built-in while PTP clock subsystem is modular, " > + "PTP clock ignored\n", KBUILD_MODNAME); > + } else { > + pr_warn("ignoring PTP clock from %s as PTP clock subsystem " > + "is configured out\n", KBUILD_MODNAME); > + } > + return NULL; > +} I think the else part is not needed. If PTP is disabled, it is disabled, nobody should be surprised by that. Looks good otherwise. Thanks, Jiri
Re: [PATCH 1/2] ptp_clock: allow for it to be optional
On Sun, 18 Sep 2016 23:51:09 -0400, Nicolas Pitre wrote: > And to make it possible for PTP to be configured out, the select statement > in the Kconfig entry for those ethernet drivers is changed from selecting > PTP_1588_CLOCK to PTP_1588_CLOCK_SELECTED whose purpose is to indicate the > default Kconfig value for the PTP subsystem. With this patch applied, the user is free to set a NIC driver as built in and PTP_1588_CLOCK as a module, right? If so, that would lead to non-functional PTP without any warning due to the use of IS_REACHABLE. That doesn't sound right. Could easily cause hours of headache to someone. Or is this handled somehow? Thanks, Jiri
Re: [PATCH] [RFC] proc connector: add namespace events
On Tue, 13 Sep 2016 16:42:43 +0200, Alban Crequy wrote: > Note that I will probably not have the chance to spend more time on > this patch soon because Iago will explore other methods with > eBPF+kprobes instead. eBPF+kprobes would not have the same API > stability though. I was curious to see if anyone would find the > namespace addition to proc connector interesting for other projects. Yes, this is a sorely missing feature. I don't care how this is done (proc connector or something else) but the feature itself is quite important for system management daemons. In particular, we need an application that monitors network configuration changes on the machine, displays the current configuration and records history of the changes. This is currently impossible to do reliably if net name spaces are in use - which they are with OpenStack and Docker and similar things in place on those machines. The current tools try to do things like monitoring /var/run/netns which is obviously unreliable and broken. There are actually two (orthogonal) problems here: apart of the one described above, it's also startup of such daemon. There's currently no way to find all current name spaces from the user space. We'll need an API for this, too. And no, eBPF is not the answer. This should just work like any other system daemon. I can't imagine that we would need llvm compiler and kernel sources/debuginfo/whatever on every machine that runs such daemon. Thanks, Jiri
Re: [PATCH] vxlan: Update tx_errors statistics if vxlan_build_skb return err.
On Sun, 4 Sep 2016 18:52:51 +0800, Haishuang Yan wrote: > If vxlan_build_skb return err < 0, tx_errors should be also increased. > > Signed-off-by: Haishuang Yan > --- > drivers/net/vxlan.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c > index f605a36..2c72dcd 100644 > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -2103,6 +2103,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct > net_device *dev, > vni, md, flags, udp_sum); > if (err < 0) { > dst_release(ndst); > + dev->stats.tx_errors++; > return; > } > udp_tunnel6_xmit_skb(ndst, sk, skb, dev, Acked-by: Jiri Benc The error path in vxlan_xmit_one deserves complete rework, though. Jiri
Re: [PATCH 0746/1285] Replace numeric parameter like 0444 with macro
On Tue, 2 Aug 2016 19:42:14 +0800, Baole Ni wrote: > I find that the developers often just specified the numeric value > when calling a macro which is defined with a parameter for access permission. > As we know, these numeric value for access permission have had the > corresponding macro, > and that using macro can improve the robustness and readability of the code, > thus, I suggest replacing the numeric parameter with the macro. > > Signed-off-by: Chuansheng Liu > Signed-off-by: Baole Ni > --- > drivers/net/geneve.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c > index cadefe4..2ddaa09 100644 > --- a/drivers/net/geneve.c > +++ b/drivers/net/geneve.c > @@ -32,7 +32,7 @@ > #define VNI_HASH_SIZE(1< > static bool log_ecn_error = true; > -module_param(log_ecn_error, bool, 0644); > +module_param(log_ecn_error, bool, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); 0644 is much more readable than the S_IGRL macros ("Incomprehensible Group of Random Letters"). Most people use the octal value with chmod and umask all the time while nobody is using the macros from the shell. As a consequence, a quick glance at the number is enough to see what the access rights are, while the macros need to be read carefully to understand what's going on. I very much dislike this patchset. It makes readability worse, not better, and increases the likelihood of errors. Jiri
[PATCH] virtio: silence compiler warning
gcc 4.8.5 emits the following false positive: drivers/virtio/virtio_ring.c: In function 'vring_create_virtqueue': drivers/virtio/virtio_ring.c:1032:5: warning: 'queue' may be used uninitialized in this function [-Wmaybe-uninitialized] Silence it. Signed-off-by: Jiri Benc --- drivers/virtio/virtio_ring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 5c802d47892c..ca6bfddaacad 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -1006,7 +1006,7 @@ struct virtqueue *vring_create_virtqueue( const char *name) { struct virtqueue *vq; - void *queue; + void *queue = NULL; dma_addr_t dma_addr; size_t queue_size_in_bytes; struct vring vring; -- 1.8.3.1
Re: [PATCH] vxlan: Refactor vxlan_udp_encap_recv() to kill compiler warning
On Fri, 4 Sep 2015 12:49:32 +0200, Geert Uytterhoeven wrote: > drivers/net/vxlan.c: In function ‘vxlan_udp_encap_recv’: > drivers/net/vxlan.c:1226: warning: ‘info’ may be used uninitialized in this > function > > While this warning is a false positive, it can be killed easily by > getting rid of the pointer intermediary and referring directly to the > ip_tunnel_info structure. > > Signed-off-by: Geert Uytterhoeven Reviewed-by: Jiri Benc -- Jiri Benc -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] route: put lwstate before freeing dst to avoid use after free
On Tue, 25 Aug 2015 14:25:14 -0400, Sasha Levin wrote: > Commit 61adedf3 ("route: move lwtunnel state to dst_entry") is trying to > release lwstate after getting rid of dst, which causes a use-after-free > trying to access dst->lwstate. > > Fixes: 61adedf3 ("route: move lwtunnel state to dst_entry") > Signed-off-by: Sasha Levin Already fixed by e252b3d1a174 in net-next. Jiri -- Jiri Benc -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Network drivers that don't suspend on interface down
On Wed, 20 Dec 2006 12:53:14 +, Matthew Garrett wrote: > The situation is more complicated for wireless. Userspace expects to be > able to get scan results from the card even if the interface is down. User space should get an error when trying to get scan results from the interface that is down. Some drivers are broken and don't do this but when they're fixed there is no problem here. > In that case, I'm pretty sure we need a third state rather than just > "up" or "down". We have that third state, it's IFF_DORMANT. Not supported yet by any wireless driver/stack, unfortunately. Thanks, Jiri -- Jiri Benc SUSE Labs - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] rfkill - Add support for input key to control wireless radio
On Wed, 6 Dec 2006 15:18:12 -0500, Dmitry Torokhov wrote: > On 12/6/06, Ivo van Doorn <[EMAIL PROTECTED]> wrote: > > Ok, so input device opening should not block the rfkill signal and the > > rfkill handler > > should still go through with its work unless a different config option > > indicates that > > userspace wants to handle the event. > > I don't think a config option is a good idea unless by config option > you mean a sysfs attribute. What about using EVIOCGRAB ioctl for this? Jiri -- Jiri Benc SUSE Labs - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [BUG] Tulip for ULi M5263: No packets transmitted
On Sat, 23 Jul 2005 20:22:04 +0100, Ricardo Bugalho wrote: > the tulip driver isn't work out for my ULi M5263 network adapter. > The driver loads and the interface receives packets but it won't > transmit any. Running a packet capture on it shows no outbound packets, > so I guess the driver thinks the card is screwed up and can't transmit. Does tulip-fixes-for-uli5261.patch from -mm tree help? http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.13-rc3/2.6.13-rc3-mm1/broken-out/tulip-fixes-for-uli5261.patch -- Jiri Benc SUSE Labs - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/