Re: [PATCH bpf-next v2 0/6] selftests/bpf: Various sockmap-related fixes
On Fri, Sep 27, 2024 at 12:54 AM +02, Michal Luczaj wrote: > On 9/24/24 12:25, Michal Luczaj wrote: >> On 8/19/24 22:05, Jakub Sitnicki wrote: >>> On Wed, Aug 14, 2024 at 06:14 PM +02, Michal Luczaj wrote: >>>> On 8/6/24 19:45, Jakub Sitnicki wrote: >>>>> On Tue, Aug 06, 2024 at 07:18 PM +02, Michal Luczaj wrote: >>>>>> Great, thanks for the review. With this completed, I guess we can unwind >>>>>> the (mail) stack to [1]. Is that ingress-to-local et al. something you >>>>>> wanted to take care of yourself or can I give it a try? >>>>>> [1] https://lore.kernel.org/netdev/87msmqn9ws@cloudflare.com/ >>>>> >>>>> I haven't stated any work on. You're welcome to tackle that. >>>>> >>>>> All I have is a toy test that I've used to generate the redirect matrix. >>>>> Perhaps it can serve as inspiration: >>>>> >>>>> https://github.com/jsitnicki/sockmap-redir-matrix >>>> >>>> All right, please let me know if this is more or less what you meant and >>>> I'll post the whole series for a review (+patch to purge sockmap_listen of >>>> redir tests, fix misnomers). [...] >>> >>> Gave it a look as promised. It makes sense to me as well to put these >>> tests in a new module. There will be some overlap with sockmap_listen, >>> which has diverged from its inital scope, but we can dedup that later. >>> >>> One thought that I had is that it could make sense to test the not >>> supported redirect combos (and expect an error). Sometimes folks make >>> changes and enable some parts of the API by accient. >> >> All right, so I did what sockmap_listen does: check >> test_sockmap_listen.c:verdict_map[SK_PASS] to see if the redirect took >> place for a given combo. And that works well... except for skb/msg to >> ingress af_vsock. Even though this is unsupported and no redirect >> actually happens, verdict appears to be SK_PASS. Is this correct? > > Here's a follow up: my guess is that some checks are missing. I'm not sure > if it's the best approach, but this fixes things for me: So you have already found a bug with a negative test. Nice. Your patch makes sense to me. FYI, I've started a GH repo for anciallary materials for sockmap. Code samples, pointers to resources, a backlog of stuff to do (?). Inspired by the xdp-project repo: https://github.com/xdp-project/xdp-project We can move it to a dedicated project namespace, right now it's at: https://github.com/jsitnicki/sockmap-project/ Feel free to add stuff there. > diff --git a/include/net/sock.h b/include/net/sock.h > index c58ca8dd561b..c87295f3476d 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -2715,6 +2715,11 @@ static inline bool sk_is_stream_unix(const struct sock > *sk) > return sk->sk_family == AF_UNIX && sk->sk_type == SOCK_STREAM; > } > > +static inline bool sk_is_vsock(const struct sock *sk) > +{ > + return sk->sk_family == AF_VSOCK; > +} > + > /** > * sk_eat_skb - Release a skb if it is no longer needed > * @sk: socket to eat this skb from > diff --git a/net/core/sock_map.c b/net/core/sock_map.c > index 242c91a6e3d3..07d6aa4e39ef 100644 > --- a/net/core/sock_map.c > +++ b/net/core/sock_map.c > @@ -647,6 +647,8 @@ BPF_CALL_4(bpf_sk_redirect_map, struct sk_buff *, skb, > sk = __sock_map_lookup_elem(map, key); > if (unlikely(!sk || !sock_map_redirect_allowed(sk))) > return SK_DROP; > + if ((flags & BPF_F_INGRESS) && sk_is_vsock(sk)) > + return SK_DROP; > > skb_bpf_set_redir(skb, sk, flags & BPF_F_INGRESS); > return SK_PASS; > @@ -675,6 +677,8 @@ BPF_CALL_4(bpf_msg_redirect_map, struct sk_msg *, msg, > return SK_DROP; > if (!(flags & BPF_F_INGRESS) && !sk_is_tcp(sk)) > return SK_DROP; > + if (sk_is_vsock(sk)) > + return SK_DROP; > > msg->flags = flags; > msg->sk_redir = sk; > @@ -1249,6 +1253,8 @@ BPF_CALL_4(bpf_sk_redirect_hash, struct sk_buff *, skb, > sk = __sock_hash_lookup_elem(map, key); > if (unlikely(!sk || !sock_map_redirect_allowed(sk))) > return SK_DROP; > + if ((flags & BPF_F_INGRESS) && sk_is_vsock(sk)) > + return SK_DROP; > > skb_bpf_set_redir(skb, sk, flags & BPF_F_INGRESS); > return SK_PASS; > @@ -1277,6 +1283,8 @@ BPF_CALL_4(bpf_msg_redirect_hash, struct sk_msg *, msg, > return SK_DROP; > if (!(flags & BPF_F_INGRESS) && !sk_is_tcp(sk)) > return SK_DROP; > + if (sk_is_vsock(sk)) > + return SK_DROP; > > msg->flags = flags; > msg->sk_redir = sk;
Re: [RFC PATCH v2 2/3] ipv6: Support setting src port in sendmsg().
On Mon, Sep 23, 2024 at 03:45 PM GMT, David Laight wrote: > From: Jakub Sitnicki >> Sent: 23 September 2024 15:56 >> >> On Mon, Sep 23, 2024 at 01:08 PM GMT, David Laight wrote: >> > From: Tiago Lam >> >> [...] >> >> >> To limit its usage, a reverse socket lookup is performed to check if the >> >> configured egress source address and/or port have any ingress sk_lookup >> >> match. If it does, traffic is allowed to proceed, otherwise it falls >> >> back to the regular egress path. >> > >> > Is that really useful/necessary? >> >> We've been asking ourselves the same question during Plumbers with >> Martin. >> >> Unprivileges processes can already source UDP traffic from (almost) any >> IP & port by binding a socket to the desired source port and passing >> IP_PKTINFO. So perhaps having a reverse socket lookup is an overkill. > > Traditionally you'd need to bind to the source port on any local IP > (or the wildcard IP) that didn't have another socket bound to that port. Right. Linux IP_PKTINFO extension relaxes this requirement. You can bind to some local IP (whichever is free, plently to choose from in 127/8 local subnet), and specify the source address to use OOB at sendmsg() time (as long as the address is local to the host, otherwise you need additional capabilities). > Modern Linux might have more restrictions and SO_REUSADDR muddies things. > > And I don't think you can do a connect() on an unbound UDP socket to > set the source port at the same time as the destination IP+port. > (That would actually be useful.) You can. It's somewhat recent (v6.3+) [1]: https://manpages.debian.org/unstable/manpages/ip.7.en.html#IP_LOCAL_PORT_RANGE It's not on par with TCP when it comes to local port sharing because we hash UDP sockets only by 2-tuple. Though, some effort to improve that is taking place I see. The recipe is: 1. delay the auto-bind until connect() time with IP_BIND_ADDRESS_NO_PORT socket option, and 2. tell the udp stack to consider only a single local port during the free port search with IP_LOCAL_PORT_RANGE option. That amounts to something like (in pseudocode): s = socket(AF_INET, SOCK_DGRAM) s.setsockopt(SOL_IP, IP_BIND_ADDRESS_NO_PORT, 1) s.setsockopt(SOL_IP, IP_LOCAL_PORT_RANGE, 44_444 << 16 | 44_444) s.bind(("192.0.2.42", 0)) s.connect(("1.1.1.1", 53)) You can combine it with SO_REUSEADDR to share the local address between sockets, but you have to ensure manually that you don't run into conflicts between sockets (two sockets using the same 4-tuple). That's something we're hoping to improve in the future. > OTOH if you just want to send a UDP message you can just use another > system on the same network. > You might need to spoof the source mac - but that isn't hard (although > it might confuse any ethernet switches). > >> We should probably respect net.ipv4.ip_local_reserved_ports and >> net.ipv4.ip_unprivileged_port_start system settings, though, or check >> for relevant caps. > > True. > >> Open question if it is acceptable to disregard exclusive UDP port >> ownership by sockets binding to a wildcard address without SO_REUSEADDR? > > We've often suffered from the opposite - a program binds to the wildcard > IP and everything works until something else binds to the same port and > a specific local IP. Let me see if I understand - what would happen today for UDP is: app #1 - bind(("0.0.0.0", 53)) -> OK app #2 - bind(("192.0.2.1", 53)) -> EADDRINUSE ... unless both are setting SO_REUSEADDR (or SO_REUSEPORT and run under same UID). That is why if we allow selecting the source port at sendmsg() time, we would be relaxing the existing UDP port ownership guarantees for wildcard binds. Perhaps this merits a sysctl, so the admin can decide if it is an acceptable trade-off in their environment. > I'm sure this is grief some on both TCP and UDP - especially since you > often need to set SO_REUSADDR to stop other things breaking. > > David > > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 > 1PT, UK > Registration No: 1397386 (Wales)
Re: [RFC PATCH v2 2/3] ipv6: Support setting src port in sendmsg().
On Mon, Sep 23, 2024 at 01:08 PM GMT, David Laight wrote: > From: Tiago Lam [...] >> To limit its usage, a reverse socket lookup is performed to check if the >> configured egress source address and/or port have any ingress sk_lookup >> match. If it does, traffic is allowed to proceed, otherwise it falls >> back to the regular egress path. > > Is that really useful/necessary? We've been asking ourselves the same question during Plumbers with Martin. Unprivileges processes can already source UDP traffic from (almost) any IP & port by binding a socket to the desired source port and passing IP_PKTINFO. So perhaps having a reverse socket lookup is an overkill. We should probably respect net.ipv4.ip_local_reserved_ports and net.ipv4.ip_unprivileged_port_start system settings, though, or check for relevant caps. Open question if it is acceptable to disregard exclusive UDP port ownership by sockets binding to a wildcard address without SO_REUSEADDR? [...] > The check (but not the commit message) implies that some 'bpf thingy' > also needs to be enabled. > Any check would need to include the test that the program sending the packet > has the ability to send a packet through the ingress socket. > Additionally a check for the sending process having (IIRC) CAP_NET_ADMIN > (which would let the process send the message by other means) would save the > slow path. > > The code we have sends a lot of UDP RTP (typically 160 bytes of audio every > 20ms). > There is actually no reason for there to be a valid matching ingress path. > (That code would benefit from being able to bind a lot of ports to the same > UDP socket.) > > David > >> >> Suggested-by: Jakub Sitnicki >> Signed-off-by: Tiago Lam >> --- >> net/ipv6/datagram.c | 79 >> + >> net/ipv6/udp.c | 8 -- >> 2 files changed, 85 insertions(+), 2 deletions(-) >> >> diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c >> index fff78496803d..369c64a478ec 100644 >> --- a/net/ipv6/datagram.c >> +++ b/net/ipv6/datagram.c >> @@ -756,6 +756,29 @@ void ip6_datagram_recv_ctl(struct sock *sk, struct >> msghdr *msg, >> } >> EXPORT_SYMBOL_GPL(ip6_datagram_recv_ctl); >> >> +static inline bool reverse_sk_lookup(struct flowi6 *fl6, struct sock *sk, >> + struct in6_addr *saddr, __be16 sport) >> +{ >> +if (static_branch_unlikely(&bpf_sk_lookup_enabled) && >> +(saddr && sport) && >> +(ipv6_addr_cmp(&sk->sk_v6_rcv_saddr, saddr) || >> +inet_sk(sk)->inet_sport != sport)) { >> +struct sock *sk_egress; >> + >> +bpf_sk_lookup_run_v6(sock_net(sk), IPPROTO_UDP, &fl6->daddr, >> + fl6->fl6_dport, saddr, ntohs(sport), 0, >> + &sk_egress); >> +if (!IS_ERR_OR_NULL(sk_egress) && sk_egress == sk) >> +return true; >> + >> +net_info_ratelimited("No reverse socket lookup match for local >> addr %pI6:%d remote addr >> %pI6:%d\n", >> + &saddr, ntohs(sport), &fl6->daddr, >> + ntohs(fl6->fl6_dport)); >> +} >> + >> +return false; >> +} >> + >> int ip6_datagram_send_ctl(struct net *net, struct sock *sk, >>struct msghdr *msg, struct flowi6 *fl6, >>struct ipcm6_cookie *ipc6) >> @@ -844,7 +867,63 @@ int ip6_datagram_send_ctl(struct net *net, struct sock >> *sk, >> >> break; >> } >> +case IPV6_ORIGDSTADDR: >> +{ >> +struct sockaddr_in6 *sockaddr_in; >> +struct net_device *dev = NULL; >> + >> +if (cmsg->cmsg_len < CMSG_LEN(sizeof(struct >> sockaddr_in6))) { >> +err = -EINVAL; >> +goto exit_f; >> +} >> + >> +sockaddr_in = (struct sockaddr_in6 *)CMSG_DATA(cmsg); >> + >> +addr_type = __ipv6_addr_type(&sockaddr_in->sin6_addr); >> + >> +if (addr_type & IPV6_ADDR_LINKLOCAL) >> +return -EINVAL; >> + >> +/* If we're egressing with a different source address >> + *
Re: [RFC PATCH v2 0/3] Allow sk_lookup UDP return traffic to egress when setting src port/address.
Just an FYI to the reviewers - Tiago is out this week, so his reponses will be delayed.
Re: [PATCH net] flow_dissector: fix byteorder of dissected ICMP ID
On Fri, Mar 12, 2021 at 09:08 PM CET, Alexander Lobakin wrote: > flow_dissector_key_icmp::id is of type u16 (CPU byteorder), > ICMP header has its ID field in network byteorder obviously. > Sparse says: > > net/core/flow_dissector.c:178:43: warning: restricted __be16 degrades to > integer > > Convert ID value to CPU byteorder when storing it into > flow_dissector_key_icmp. > > Fixes: 5dec597e5cd0 ("flow_dissector: extract more ICMP information") > Signed-off-by: Alexander Lobakin > --- > net/core/flow_dissector.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c > index 2ef2224b3bff..a96a4f5de0ce 100644 > --- a/net/core/flow_dissector.c > +++ b/net/core/flow_dissector.c > @@ -176,7 +176,7 @@ void skb_flow_get_icmp_tci(const struct sk_buff *skb, >* avoid confusion with packets without such field >*/ > if (icmp_has_id(ih->type)) > - key_icmp->id = ih->un.echo.id ? : 1; > + key_icmp->id = ih->un.echo.id ? ntohs(ih->un.echo.id) : 1; > else > key_icmp->id = 0; > } Smells like a breaking change for existing consumers of this value. How about we change the type of flow_dissector_key_icmp{}.id to __be16 instead to make sparse happy?
Re: [PATCH] net: sockmap: Don't call bpf_prog_put() on NULL pointer
On Thu, Oct 15, 2020 at 06:43 AM CEST, John Fastabend wrote: [...] > Jakub, any opinions on if we should just throw an error if users try to > add a sock to a map with a parser but no verdict? At the moment we fall > through and add the socket, but it wont do any receive parsing/verdict. > At the moment I think its fine with above fix. The useful cases for RX > are parser+verdict, verdict, and empty. Where empty is just used for > redirects or other socket account tricks. Just something to keep in mind. IMO we should not fail because map updates can interleave with sk_skb prog attachments, like so: update_map(map_fd, sock_fd); attach_prog(parser_fd, map_fd, BPF_SK_SKB_STREAM_PARSER); update_map(map_fd, sock_fd); // OK attach_prog(verdict_fd, map_fd, BPF_SK_SKB_STREAM_VERDICT); update_map(map_fd, sock_fd); In practice, I would expect one process/thread to attach the programs, while another is allowed to update the map at the same time.
Re: [PATCH] net: sockmap: Don't call bpf_prog_put() on NULL pointer
On Mon, Oct 12, 2020 at 07:09 PM CEST, Alex Dewar wrote: > If bpf_prog_inc_not_zero() fails for skb_parser, then bpf_prog_put() is > called unconditionally on skb_verdict, even though it may be NULL. Fix > and tidy up error path. > > Addresses-Coverity-ID: 1497799: Null pointer dereferences (FORWARD_NULL) > Fixes: 743df8b7749f ("bpf, sockmap: Check skb_verdict and skb_parser programs > explicitly") > Signed-off-by: Alex Dewar > --- Note to maintainers: the issue exists only in bpf-next where we have: https://lore.kernel.org/bpf/160239294756.8495.5796595770890272219.stgit@john-Precision-5820-Tower/ The patch also looks like it is supposed to be applied on top of the above.
Re: [PATCH] net: sockmap: Don't call bpf_prog_put() on NULL pointer
On Mon, Oct 12, 2020 at 07:09 PM CEST, Alex Dewar wrote: > If bpf_prog_inc_not_zero() fails for skb_parser, then bpf_prog_put() is > called unconditionally on skb_verdict, even though it may be NULL. Fix > and tidy up error path. > > Addresses-Coverity-ID: 1497799: Null pointer dereferences (FORWARD_NULL) > Fixes: 743df8b7749f ("bpf, sockmap: Check skb_verdict and skb_parser programs > explicitly") > Signed-off-by: Alex Dewar > --- Acked-by: Jakub Sitnicki
Re: [PATCH bpf] bpf: sockmap: add locking annotations to iterator
On Mon, Oct 12, 2020 at 11:18 AM CEST, Lorenz Bauer wrote: > The sparse checker currently outputs the following warnings: > > include/linux/rcupdate.h:632:9: sparse: sparse: context imbalance in > 'sock_hash_seq_start' - wrong count at exit > include/linux/rcupdate.h:632:9: sparse: sparse: context imbalance in > 'sock_map_seq_start' - wrong count at exit > > Add the necessary __acquires and __release annotations to make the > iterator locking schema palatable to sparse. Also add __must_hold > for good measure. > > The kernel codebase uses both __acquires(rcu) and __acquires(RCU). > I couldn't find any guidance which one is preferred, so I used > what is easier to type out. > > Fixes: 0365351524d7 ("net: Allow iterating sockmap and sockhash") > Reported-by: kernel test robot > Signed-off-by: Lorenz Bauer > --- Acked-by: Jakub Sitnicki
[PATCH] bpf: sk_lookup: Add user documentation
Describe the purpose of BPF sk_lookup program, how it can be attached, when it gets invoked, and what information gets passed to it. Point the reader to examples and further documentation. Signed-off-by: Jakub Sitnicki --- Documentation/bpf/index.rst | 1 + Documentation/bpf/prog_sk_lookup.rst | 98 2 files changed, 99 insertions(+) create mode 100644 Documentation/bpf/prog_sk_lookup.rst diff --git a/Documentation/bpf/index.rst b/Documentation/bpf/index.rst index 7df2465fd108..4f2874b729c3 100644 --- a/Documentation/bpf/index.rst +++ b/Documentation/bpf/index.rst @@ -52,6 +52,7 @@ Program types prog_cgroup_sysctl prog_flow_dissector bpf_lsm + prog_sk_lookup Map types diff --git a/Documentation/bpf/prog_sk_lookup.rst b/Documentation/bpf/prog_sk_lookup.rst new file mode 100644 index ..85a305c19bcd --- /dev/null +++ b/Documentation/bpf/prog_sk_lookup.rst @@ -0,0 +1,98 @@ +.. SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) + += +BPF sk_lookup program += + +BPF sk_lookup program type (``BPF_PROG_TYPE_SK_LOOKUP``) introduces programmability +into the socket lookup performed by the transport layer when a packet is to be +delivered locally. + +When invoked BPF sk_lookup program can select a socket that will receive the +incoming packet by calling the ``bpf_sk_assign()`` BPF helper function. + +Hooks for a common attach point (``BPF_SK_LOOKUP``) exist for both TCP and UDP. + +Motivation +== + +BPF sk_lookup program type was introduced to address setup scenarios where +binding sockets to an address with ``bind()`` socket call is impractical, such +as: + +1. receiving connections on a range of IP addresses, e.g. 192.0.2.0/24, when + binding to a wildcard address ``INADRR_ANY`` is not possible due to a port + conflict, +2. receiving connections on all or a wide range of ports, i.e. an L7 proxy use + case. + +Such setups would require creating and ``bind()``'ing one socket to each of the +IP address/port in the range, leading to resource consumption and potential +latency spikes during socket lookup. + +Attachment +== + +BPF sk_lookup program can be attached to a network namespace with +``bpf(BPF_LINK_CREATE, ...)`` syscall using the ``BPF_SK_LOOKUP`` attach type and a +netns FD as attachment ``target_fd``. + +Multiple programs can be attached to one network namespace. Programs will be +invoked in the same order as they were attached. + +Hooks += + +The attached BPF sk_lookup programs run whenever the transport layer needs to +find a listening (TCP) or an unconnected (UDP) socket for an incoming packet. + +Incoming traffic to established (TCP) and connected (UDP) sockets is delivered +as usual without triggering the BPF sk_lookup hook. + +The attached BPF programs must return with either ``SK_PASS`` or ``SK_DROP`` +verdict code. As for other BPF program types that are network filters, +``SK_PASS`` signifies that the socket lookup should continue on to regular +hashtable-based lookup, while ``SK_DROP`` causes the transport layer to drop the +packet. + +A BPF sk_lookup program can also select a socket to receive the packet by +calling ``bpf_sk_assign()`` BPF helper. Typically, the program looks up a socket +in a map holding sockets, such as ``SOCKMAP`` or ``SOCKHASH``, and passes a +``struct bpf_sock *`` to ``bpf_sk_assign()`` helper to record the +selection. Selecting a socket only takes effect if the program has terminated +with ``SK_PASS`` code. + +When multiple programs are attached, the end result is determined from return +codes of all the programs according to the following rules: + +1. If any program returned ``SK_PASS`` and selected a valid socket, the socket + is used as the result of the socket lookup. +2. If more than one program returned ``SK_PASS`` and selected a socket, the last + selection takes effect. +3. If any program returned ``SK_DROP``, and no program returned ``SK_PASS`` and + selected a socket, socket lookup fails. +4. If all programs returned ``SK_PASS`` and none of them selected a socket, + socket lookup continues on. + +API +=== + +In its context, an instance of ``struct bpf_sk_lookup``, BPF sk_lookup program +receives information about the packet that triggered the socket lookup. Namely: + +* IP version (``AF_INET`` or ``AF_INET6``), +* L4 protocol identifier (``IPPROTO_TCP`` or ``IPPROTO_UDP``), +* source and destination IP address, +* source and destination L4 port, +* the socket that has been selected with ``bpf_sk_assign()``. + +Refer to ``struct bpf_sk_lookup`` declaration in ``linux/bpf.h`` user API +header, and `bpf-helpers(7) +<https://man7.org/linux/man-pages/man7/bpf-helpers.7.html>`_ man-page section +for ``bpf_sk_assign()`` for details. + +Example +=== + +See ``tools/testing/selftests/bpf/prog_tests/sk_lookup.c`` for the reference +implementation. -- 2.25.4
Re: linux-next: manual merge of the bpf-next tree with the net tree
On Wed, Jul 22, 2020 at 05:05 PM CEST, Willem de Bruijn wrote: > On Wed, Jul 22, 2020 at 11:02 AM Jakub Sitnicki wrote: >> >> On Wed, Jul 22, 2020 at 04:42 PM CEST, Kuniyuki Iwashima wrote: >> > Can I submit a patch to net tree that rewrites udp[46]_lib_lookup2() to >> > use only 'result' ? >> >> Feel free. That should make the conflict resolution even easier later >> on. > > Thanks for the detailed analysis, Jakub. > > Would it be easier to fix this wholly in bpf-next, by introducing > reuseport_result there? Did you mean replicating the Kuniyuki fix in bpf-next, or just introducing the intermediate 'reuseport_result' var? I'm assuming the former, so that the conflict resolving later on will reduce to selecting everything from bpf-next side. TBH, I don't what is the preferred way to handle it. Perhaps DaveM or Alexei/Daniel can say what would make their life easiest?
Re: linux-next: manual merge of the bpf-next tree with the net tree
On Wed, Jul 22, 2020 at 04:42 PM CEST, Kuniyuki Iwashima wrote: > Can I submit a patch to net tree that rewrites udp[46]_lib_lookup2() to > use only 'result' ? Feel free. That should make the conflict resolution even easier later on. Thanks, -jkbs
Re: linux-next: manual merge of the bpf-next tree with the net tree
On Wed, Jul 22, 2020 at 05:21 AM CEST, Stephen Rothwell wrote: > Hi all, > > Today's linux-next merge of the bpf-next tree got conflicts in: > > net/ipv4/udp.c > net/ipv6/udp.c > > between commit: > > efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") > > from the net tree and commits: > > 7629c73a1466 ("udp: Extract helper for selecting socket from reuseport > group") > 2a08748cd384 ("udp6: Extract helper for selecting socket from reuseport > group") > > from the bpf-next tree. > > I fixed it up (I wasn't sure how to proceed, so I used the latter > version) and can carry the fix as necessary. This is now fixed as far > as linux-next is concerned, but any non trivial conflicts should be > mentioned to your upstream maintainer when your tree is submitted for > merging. You may also want to consider cooperating with the maintainer > of the conflicting tree to minimise any particularly complex conflicts. This one is a bit tricky. Looking at how code in udp[46]_lib_lookup2 evolved, first: acdcecc61285 ("udp: correct reuseport selection with connected sockets") 1) exluded connected UDP sockets from reuseport group during lookup, and 2) limited fast reuseport return to groups with no connected sockets, The second change had an uninteded side-effect of discarding reuseport socket selection when reuseport group contained connected sockets. Then, recent efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") rectified it by recording reuseport socket selection as lookup result candidate, in case fast reuseport return did not happen because reuseport group had connected sockets. I belive that changes in commit efc6b6f6c311 can be rewritten as below to the same effect, by realizing that we are always setting the 'result' if 'score > badness'. Either to what reuseport_select_sock() returned or to 'sk' that scored higher than current 'badness' threshold. ---8<--- static struct sock *udp4_lib_lookup2(struct net *net, __be32 saddr, __be16 sport, __be32 daddr, unsigned int hnum, int dif, int sdif, struct udp_hslot *hslot2, struct sk_buff *skb) { struct sock *sk, *result; int score, badness; u32 hash = 0; result = NULL; badness = 0; udp_portaddr_for_each_entry_rcu(sk, &hslot2->head) { score = compute_score(sk, net, saddr, sport, daddr, hnum, dif, sdif); if (score > badness) { result = NULL; if (sk->sk_reuseport && sk->sk_state != TCP_ESTABLISHED) { hash = udp_ehashfn(net, daddr, hnum, saddr, sport); result = reuseport_select_sock(sk, hash, skb, sizeof(struct udphdr)); if (result && !reuseport_has_conns(sk, false)) return result; } if (!result) result = sk; badness = score; } } return result; } ---8<--- >From there, it is now easier to resolve the conflict with 7629c73a1466 ("udp: Extract helper for selecting socket from reuseport group") 2a08748cd384 ("udp6: Extract helper for selecting socket from reuseport group") which extract the 'if (sk->sk_reuseport && sk->sk_state != TCP_ESTABLISHED)' block into a helper called lookup_reuseport(). To merge the two, we need to pull the reuseport_has_conns() check up from lookup_reuseport() and back into udp[46]_lib_lookup2(), because now we want to record reuseport socket selection even if reuseport group has connections. The only other call site of lookup_reuseport() is in udp[46]_lookup_run_bpf(). We don't want to discard the reuseport selected socket if group has connections there either, so no changes are needed. And, now that I think about it, the current behavior in udp[46]_lookup_run_bpf() is not right. The end result for udp4 will look like: ---8<--- static inline struct sock *lookup_reuseport(struct net *net, struct sock *sk, struct sk_buff *skb, __be32 saddr, __be16 sport, __be32 daddr, unsigned short hnum) { struct sock *reuse_sk = NULL; u32 hash; if (sk->sk_reuseport && sk->sk_state != TCP_ESTABLISHED) { hash = udp_ehashfn(net, daddr, hnum, saddr, sport); reuse_sk = reuseport_select_sock(sk, hash, skb, sizeof(struct udphdr));
Re: linux-next: Tree for Jul 20 (kernel/bpf/net_namespace)
On Mon, Jul 20, 2020 at 09:37 PM CEST, Alexei Starovoitov wrote: > On Mon, Jul 20, 2020 at 11:49 AM Stephen Rothwell > wrote: >> >> Hi all, >> >> On Mon, 20 Jul 2020 08:51:54 -0700 Randy Dunlap >> wrote: >> > >> > on i386 or x86_64: >> > >> > # CONFIG_INET is not set >> > # CONFIG_NET_NS is not set >> > >> > ld: kernel/bpf/net_namespace.o: in function `bpf_netns_link_release': >> > net_namespace.c:(.text+0x32c): undefined reference to >> > `bpf_sk_lookup_enabled' >> > ld: kernel/bpf/net_namespace.o: in function `netns_bpf_link_create': >> > net_namespace.c:(.text+0x8b7): undefined reference to >> > `bpf_sk_lookup_enabled' >> > ld: kernel/bpf/net_namespace.o: in function `netns_bpf_pernet_pre_exit': >> > net_namespace.c:(.ref.text+0xa3): undefined reference to >> > `bpf_sk_lookup_enabled' >> >> Caused by commit >> >> 1559b4aa1db4 ("inet: Run SK_LOOKUP BPF program on socket lookup") >> >> from the bpf-next tree. > > Jakub, please take a look. Sorry about that. Proposed fix: https://lore.kernel.org/bpf/20200721100716.720477-1-ja...@cloudflare.com/
Re: [PATCH bpf] selftests: bpf: fix detach from sockmap tests
On Thu, Jul 09, 2020 at 01:51 PM CEST, Lorenz Bauer wrote: > Fix sockmap tests which rely on old bpf_prog_dispatch behaviour. > In the first case, the tests check that detaching without giving > a program succeeds. Since these are not the desired semantics, > invert the condition. In the second case, the clean up code doesn't > supply the necessary program fds. > > Reported-by: Martin KaFai Lau > Signed-off-by: Lorenz Bauer > Fixes: bb0de3131f4c ("bpf: sockmap: Require attach_bpf_fd when detaching a > program") > --- > tools/testing/selftests/bpf/test_maps.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/tools/testing/selftests/bpf/test_maps.c > b/tools/testing/selftests/bpf/test_maps.c > index 6a12a0e01e07..754cf611723e 100644 > --- a/tools/testing/selftests/bpf/test_maps.c > +++ b/tools/testing/selftests/bpf/test_maps.c > @@ -789,19 +789,19 @@ static void test_sockmap(unsigned int tasks, void *data) > } > > err = bpf_prog_detach(fd, BPF_SK_SKB_STREAM_PARSER); > - if (err) { > + if (!err) { > printf("Failed empty parser prog detach\n"); > goto out_sockmap; > } > > err = bpf_prog_detach(fd, BPF_SK_SKB_STREAM_VERDICT); > - if (err) { > + if (!err) { > printf("Failed empty verdict prog detach\n"); > goto out_sockmap; > } > > err = bpf_prog_detach(fd, BPF_SK_MSG_VERDICT); > - if (err) { > + if (!err) { > printf("Failed empty msg verdict prog detach\n"); > goto out_sockmap; > } > @@ -1090,19 +1090,19 @@ static void test_sockmap(unsigned int tasks, void > *data) > assert(status == 0); > } > > - err = bpf_prog_detach(map_fd_rx, __MAX_BPF_ATTACH_TYPE); > + err = bpf_prog_detach2(parse_prog, map_fd_rx, __MAX_BPF_ATTACH_TYPE); > if (!err) { > printf("Detached an invalid prog type.\n"); > goto out_sockmap; > } > > - err = bpf_prog_detach(map_fd_rx, BPF_SK_SKB_STREAM_PARSER); > + err = bpf_prog_detach2(parse_prog, map_fd_rx, BPF_SK_SKB_STREAM_PARSER); > if (err) { > printf("Failed parser prog detach\n"); > goto out_sockmap; > } > > - err = bpf_prog_detach(map_fd_rx, BPF_SK_SKB_STREAM_VERDICT); > + err = bpf_prog_detach2(verdict_prog, map_fd_rx, > BPF_SK_SKB_STREAM_VERDICT); > if (err) { > printf("Failed parser prog detach\n"); > goto out_sockmap; Reviewed-by: Jakub Sitnicki
Re: [PATCH bpf] bpf: sockmap: don't attach programs to UDP sockets
On Thu, 11 Jun 2020 18:25:20 +0100 Lorenz Bauer wrote: > The stream parser infrastructure isn't set up to deal with UDP > sockets, so we mustn't try to attach programs to them. > > I remember making this change at some point, but I must have lost > it while rebasing or something similar. > > Fixes: 7b98cd42b049 ("bpf: sockmap: Add UDP support") > Signed-off-by: Lorenz Bauer > --- > net/core/sock_map.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/net/core/sock_map.c b/net/core/sock_map.c > index 00a26cf2cfe9..35cea36f3892 100644 > --- a/net/core/sock_map.c > +++ b/net/core/sock_map.c > @@ -424,10 +424,7 @@ static int sock_map_get_next_key(struct bpf_map *map, > void *key, void *next) > return 0; > } > > -static bool sock_map_redirect_allowed(const struct sock *sk) > -{ > - return sk->sk_state != TCP_LISTEN; > -} > +static bool sock_map_redirect_allowed(const struct sock *sk); > > static int sock_map_update_common(struct bpf_map *map, u32 idx, > struct sock *sk, u64 flags) > @@ -508,6 +505,11 @@ static bool sk_is_udp(const struct sock *sk) > sk->sk_protocol == IPPROTO_UDP; > } > > +static bool sock_map_redirect_allowed(const struct sock *sk) > +{ > + return sk_is_tcp(sk) && sk->sk_state != TCP_LISTEN; > +} > + > static bool sock_map_sk_is_suitable(const struct sock *sk) > { > return sk_is_tcp(sk) || sk_is_udp(sk); Acked-by: Jakub Sitnicki
Re: [PATCH] bpf/sockmap: fix kernel panic at __tcp_bpf_recvmsg
On Fri, May 29, 2020 at 11:05 AM CEST, dihu wrote: > On 2020/5/27 5:10, John Fastabend wrote: >> dihu wrote: >>> From 865a45747de6b68fd02a0ff128a69a5c8feb73c3 Mon Sep 17 00:00:00 2001 >>> From: dihu >>> Date: Mon, 25 May 2020 17:23:16 +0800 >>> Subject: [PATCH] bpf/sockmap: fix kernel panic at __tcp_bpf_recvmsg >>> >>> When user application calls read() with MSG_PEEK flag to read data >>> of bpf sockmap socket, kernel panic happens at >>> __tcp_bpf_recvmsg+0x12c/0x350. sk_msg is not removed from ingress_msg >>> queue after read out under MSG_PEEK flag is set. Because it's not >>> judged whether sk_msg is the last msg of ingress_msg queue, the next >>> sk_msg may be the head of ingress_msg queue, whose memory address of >>> sg page is invalid. So it's necessary to add check codes to prevent >>> this problem. >>> >>> [20759.125457] BUG: kernel NULL pointer dereference, address: >>> 0008 >>> [20759.132118] CPU: 53 PID: 51378 Comm: envoy Tainted: GE >>> 5.4.32 #1 >>> [20759.140890] Hardware name: Inspur SA5212M4/YZMB-00370-109, BIOS >>> 4.1.12 06/18/2017 >>> [20759.149734] RIP: 0010:copy_page_to_iter+0xad/0x300 >>> [20759.270877] __tcp_bpf_recvmsg+0x12c/0x350 >>> [20759.276099] tcp_bpf_recvmsg+0x113/0x370 >>> [20759.281137] inet_recvmsg+0x55/0xc0 >>> [20759.285734] __sys_recvfrom+0xc8/0x130 >>> [20759.290566] ? __audit_syscall_entry+0x103/0x130 >>> [20759.296227] ? syscall_trace_enter+0x1d2/0x2d0 >>> [20759.301700] ? __audit_syscall_exit+0x1e4/0x290 >>> [20759.307235] __x64_sys_recvfrom+0x24/0x30 >>> [20759.312226] do_syscall_64+0x55/0x1b0 >>> [20759.316852] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >>> >>> Signed-off-by: dihu >>> --- >>> net/ipv4/tcp_bpf.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c >>> index 5a05327..c0d4624 100644 >>> --- a/net/ipv4/tcp_bpf.c >>> +++ b/net/ipv4/tcp_bpf.c >>> @@ -64,6 +64,9 @@ int __tcp_bpf_recvmsg(struct sock *sk, struct sk_psock >>> *psock, >>> } while (i != msg_rx->sg.end); >>> >>> if (unlikely(peek)) { >>> + if (msg_rx == list_last_entry(&psock->ingress_msg, >>> + struct sk_msg, list)) >>> +break; >> >> Thanks. Change looks good but spacing is a bit off . Can we >> turn those spaces into tabs? Otherwise adding fixes tag and >> my ack would be great. >> >> Fixes: 02c558b2d5d67 ("bpf: sockmap, support for msg_peek in sk_msg with >> redirect ingress") >> Acked-by: John Fastabend > > > From 127a334fa5e5d029353ceb1a0414886c527f4be5 Mon Sep 17 00:00:00 2001 > From: dihu > Date: Fri, 29 May 2020 16:38:50 +0800 > Subject: [PATCH] bpf/sockmap: fix kernel panic at __tcp_bpf_recvmsg > > When user application calls read() with MSG_PEEK flag to read data > of bpf sockmap socket, kernel panic happens at > __tcp_bpf_recvmsg+0x12c/0x350. sk_msg is not removed from ingress_msg > queue after read out under MSG_PEEK flag is set. Because it's not > judged whether sk_msg is the last msg of ingress_msg queue, the next > sk_msg may be the head of ingress_msg queue, whose memory address of > sg page is invalid. So it's necessary to add check codes to prevent > this problem. > > [20759.125457] BUG: kernel NULL pointer dereference, address: > 0008 > [20759.132118] CPU: 53 PID: 51378 Comm: envoy Tainted: G E > 5.4.32 #1 > [20759.140890] Hardware name: Inspur SA5212M4/YZMB-00370-109, BIOS > 4.1.12 06/18/2017 > [20759.149734] RIP: 0010:copy_page_to_iter+0xad/0x300 > [20759.270877] __tcp_bpf_recvmsg+0x12c/0x350 > [20759.276099] tcp_bpf_recvmsg+0x113/0x370 > [20759.281137] inet_recvmsg+0x55/0xc0 > [20759.285734] __sys_recvfrom+0xc8/0x130 > [20759.290566] ? __audit_syscall_entry+0x103/0x130 > [20759.296227] ? syscall_trace_enter+0x1d2/0x2d0 > [20759.301700] ? __audit_syscall_exit+0x1e4/0x290 > [20759.307235] __x64_sys_recvfrom+0x24/0x30 > [20759.312226] do_syscall_64+0x55/0x1b0 > [20759.316852] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > Signed-off-by: dihu > --- > net/ipv4/tcp_bpf.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c > index 5a05327..b82e4c3 100644 > --- a/net/ipv4/tcp_bpf.c > +++ b/net/ipv4/tcp_bpf.c > @@ -64,6 +64,9 @@ int __tcp_bpf_recvmsg(struct sock *sk, struct sk_psock > *psock, > } while (i != msg_rx->sg.end); > > if (unlikely(peek)) { > + if (msg_rx == list_last_entry(&psock->ingress_msg, > + struct sk_msg, list)) > +break; >msg_rx = list_next_entry(msg_rx, list); >continue; > } Looks like the patch is garbled. I suspect due to copy-paste to an e-mail client. Context line got wrapped and there are non-breaking spaces instead of tabs in the body. Crash fix is important so could you resend it with `git send-email`? git send-email --to b...@vger.kernel.org --cc net...@vger.kernel.org file.patch You might find the documentation below helpful: https://www.kernel.org/doc/html/latest/process/email-clients.html
Re: [LKP] [sk_msg] 1d79895aef: WARNING:at_net/strparser/strparser.c:#strp_done
On Mon, Mar 04, 2019 at 03:05 PM CET, kernel test robot wrote: > FYI, we noticed the following commit (built with gcc-7): > > commit: 1d79895aef18fa05789995d86d523c9b2ee58a02 ("sk_msg: Always cancel strp > work before freeing the psock") > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git pending-fixes > > in testcase: kernel_selftests > with following parameters: > > group: kselftests-00 > > test-description: The kernel contains a set of "self tests" under the > tools/testing/selftests/ directory. These are intended to be small unit tests > to exercise individual code paths in the kernel. > test-url: https://www.kernel.org/doc/Documentation/kselftest.txt > > > on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 4G > > caused below changes (please refer to attached dmesg/kmsg for entire > log/backtrace): > > > +-+++ > | | 8c79b35693 | 1d79895aef | > +-+++ > | boot_successes | 8 | 4 | > | boot_failures | 0 | 4 | > | WARNING:at_net/strparser/strparser.c:#strp_done | 0 | 4 | > | RIP:strp_done | 0 | 4 | > +-+++ > > > > [ 103.644327] WARNING: CPU: 1 PID: 9646 at net/strparser/strparser.c:526 > strp_done+0x3c/0x40 > [ 103.644435] Fork 100 tasks to 'test_hashmap_walk' > [ 103.644438] > [ 103.646486] Modules linked in: binfmt_misc crct10dif_pclmul crc32_pclmul > crc32c_intel ghash_clmulni_intel sr_mod cdrom sg ppdev ata_generic pata_acpi > aesni_intel crypto_simd cryptd glue_helper snd_pcm snd_timer snd soundcore > pcspkr ata_piix serio_raw i2c_piix4 libata floppy parport_pc parport ip_tables > [ 103.653572] CPU: 1 PID: 9646 Comm: kworker/1:26 Not tainted > 5.0.0-rc3-00017-g1d79895 #1 > [ 103.655420] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > 1.10.2-1 04/01/2014 > [ 103.657321] Workqueue: events sk_psock_destroy_deferred > [ 103.658499] RIP: 0010:strp_done+0x3c/0x40 > [ 103.659471] Code: 28 e8 b8 2d 6c ff 48 8d bb 80 00 00 00 e8 9c 2d 6c ff 48 > 8b 7b 18 48 85 ff 74 0d e8 ae 9b e8 ff 48 c7 43 18 00 00 00 00 5b c3 <0f> 0b > eb cf 66 66 66 66 90 55 53 48 89 fb 48 83 c7 28 89 f5 e8 0b > [ 103.660968] Fork 100 tasks to 'test_arraymap' > [ 103.660972] > [ 103.663347] RSP: 0018:c90005263e48 EFLAGS: 00010246 > [ 103.663350] RAX: 818b46d0 RBX: 8880867eba40 RCX: > 8880867ea1e8 > [ 103.663351] RDX: 88813fd22620 RSI: 0040 RDI: > 8880867eba40 > [ 103.663353] RBP: 88813fd22600 R08: 73746e657665 R09: > 8080808080808080 > [ 103.663354] R10: c96b3d88 R11: fefefefefefefeff R12: > 8880867eba00 > [ 103.663355] R13: R14: 8880867ebbe0 R15: > 8880867ebbe8 > [ 103.663357] FS: () GS:88813fd0() > knlGS: > [ 103.663358] CS: 0010 DS: ES: CR0: 80050033 > [ 103.663359] CR2: 7f47d7f26cb0 CR3: 88b38000 CR4: > 000406e0 > [ 103.663363] DR0: DR1: DR2: > > [ 103.667182] Fork 100 tasks to 'test_arraymap_percpu' > [ 103.667186] > [ 103.667670] DR3: DR6: fffe0ff0 DR7: > 0400 > [ 103.667671] Call Trace: > [ 103.667678] sk_psock_destroy_deferred+0x27/0x1c0 > [ 103.667683] ? __schedule+0x268/0x870 > [ 103.671331] Fork 1024 tasks to 'test_update_delete' > [ 103.671334] > [ 103.672256] process_one_work+0x19c/0x3b0 > [ 103.672259] worker_thread+0x3c/0x3b0 > [ 103.672261] ? process_one_work+0x3b0/0x3b0 > [ 103.672263] kthread+0x11e/0x140 > [ 103.672266] ? kthread_park+0x90/0x90 > [ 103.672268] ret_from_fork+0x35/0x40 > [ 103.672271] ---[ end trace efe23fd08e655cc6 ]--- Addressed in: https://lore.kernel.org/netdev/20190307103543.15151-1-ja...@cloudflare.com/ Sorry about the oversight. -Jakub
Re: [PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
On Tue, Nov 01, 2016 at 03:35 PM GMT, David Miller wrote: > From: Jakub Sitnicki > Date: Tue, 01 Nov 2016 16:13:51 +0100 > >> On Mon, Oct 31, 2016 at 07:15 PM GMT, David Miller wrote: >>> From: Jakub Sitnicki >>> Date: Sun, 30 Oct 2016 14:03:11 +0100 >>> >>>> 2) ensure the flow labels used in both directions are the same (either >>>>reflected by one side, or fixed, e.g. not used and set to 0), so that >>>>the 4-tuple we hash over when forwarding, >>>label, next hdr>, is the same both ways, modulo the order of >>>>addresses. >>> >>> Even Linux, by default, does not do reflection. >>> >>> See the flowlabel_consistency sysctl, which we set by default to '1'. >> >> Yes, unfortunately, if Linux-based hosts are used as sending/receiving >> IPv6, ICMP error forwarding will not work out of the box. Users will be >> burdened with adjusting the runtime network stack config, as you point >> out, or otherwise instructing the apps to set the flow label, >> e.g. traceroute6 -I ... > > I'm pretty sure that sysctl default was choosen intentionally, and we > actively are _encouraging_ the world to not depend upon reflection in > any way, shape, or form. > > And it's kind of pointless to suggest a facility that can't work with > Linux endpoints out of the box. > > This was the point I'm trying to make. > > If the intentions of that sysctl default does pan out, the idea is for > the world to move towards arbitrary flow label settings, even perhaps > changing over time. The intention is to make this more, not less, > common. And the idea is to give maximum flexibility for endpoints to > set these flow labels, in order to increase entropy wherever possible. > > I have a really hard time accepting a "fix" that depends upon behavior > that the Linux ipv6 stack doesn't even have. Fair enough. I'm not questioning the defaults or the benefits of widespread use of flow labels. I was trying to do this without changing as to how we hash the packets and balance traffic over multiple paths, but that does yield a solution that does not work out of the box with Linux endpoints. Hard to sell, I agree. As a potential way out, I can rework it so that we exclude the flow label from the multipath hash. That way we lose some entropy (not worse than IPv4), but do not depend any more on how flow labels are set (flexible). This could be made runtime configurable, as it changes existing behavior. Thanks, Jakub
Re: [PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
On Mon, Oct 31, 2016 at 07:25 PM GMT, Tom Herbert wrote: > On Sun, Oct 30, 2016 at 6:03 AM, Jakub Sitnicki wrote: >> On Fri, Oct 28, 2016 at 02:25 PM GMT, Tom Herbert wrote: >>> >>> If this patch is being done to be compatible with IPv4 I guess that's >>> okay, but it would be false advertisement to say this makes ICMP >>> follow the same path as the flow being targeted in an error. >>> Fortunately, I doubt anyone can have a dependency on this for ICMP. >> >> I wouldn't want to propose anything that would be useless. If you think >> that this is the case here, I would very much like to understand what >> and why cannot work in practice. >> > The normal hash for TCP or UDP using ECMP is over dstIP, srcPort, dstPort>. For an ICMP packet ECMP would most likely be > done over . There really is no way to ensure > that an ICMP packet will follow the same path as TCP or any other > protocol. Fortunately, this is really isn't so terrible. The Internet > has worked this way ever since routers started using ports as input to > ECMP and that hasn't caused any major meltdown. Ahh, I see the problem now. Thank you for clearing it up for me. You are right, for locally generated TCP/UDP traffic we are computing an L4 hash (over the 5-tuple that you mentioned) that drives the multipath routing. While when sending locally generated ICMP errors we are only computing an L3 hash (over the mentioned 3-tuple). I believe that is a problem affects both IPv6 and IPv4, and manifests itself only when the offending packet that triggers the error is destined for the ECMP router. When an ECMP router is: (i) sending an ICMP error in reaction to a packet that was to be forwarded, or (ii) forwarding an ICMP error, everything works as expected. That is because when forwarding traffic we limit ourselves to computing an L3 hash so that the IP fragments are routed together. Right? So, my understanding is that, with these changes, things are not perfect but we are not worse than IPv4 right now. Would you be okay with this series if I update the patch 4/5 description to highlight the existing problem that you point out? A fix for this IPv4/6 common issue could follow afterwards. Thanks, Jakub
Re: [PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
On Mon, Oct 31, 2016 at 07:15 PM GMT, David Miller wrote: > From: Jakub Sitnicki > Date: Sun, 30 Oct 2016 14:03:11 +0100 > >> 2) ensure the flow labels used in both directions are the same (either >>reflected by one side, or fixed, e.g. not used and set to 0), so that >>the 4-tuple we hash over when forwarding, >label, next hdr>, is the same both ways, modulo the order of >>addresses. > > Even Linux, by default, does not do reflection. > > See the flowlabel_consistency sysctl, which we set by default to '1'. Yes, unfortunately, if Linux-based hosts are used as sending/receiving IPv6, ICMP error forwarding will not work out of the box. Users will be burdened with adjusting the runtime network stack config, as you point out, or otherwise instructing the apps to set the flow label, e.g. traceroute6 -I ... > I think we need to think a lot more about how systems actually set and > use flowlabels. The only alternative I can think of, only for ECMP routing, is having a toggle option that would exclude the flow label from the input to the multipath hash. We would be sacrificing the entropy that potentially comes from hashing over the flow label, leading to better flow balancing. But we wouldn't be making IPv6 multipath routing any worse than IPv4 is in that regard. And user-space apps wouldn't need to resort to reflecting/setting the label, just like with IPv4. Is that something that you would consider a viable option? > Also, one issue I also had with this series was adding a new member > to the flow label. Is it possible to implement this like the ipv4 > side did, by simply passing a new parameter around to the necessary > functions? This was my initial approach, i.e. to mimic the IPv4 and pass the multipath hash down the call chain via a parameter. However, I gave up on it, thinking it will cause too much disturbance in the involved functions' interfaces, when I realized that one of the call paths the multipath hash would have to also be passed through is: ip6_route_input_lookup fib6_rule_lookup fib_rules_lookup fib6_rule_action ip6_pol_route_input To be honest, I was thinking that if extending flowi6 structure would find acceptance, then maybe the new member could be at some point moved to flowi_common and also used by IPv4 to avoid parameter passing there as well. Thanks, Jakub
Re: [PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
On Fri, Oct 28, 2016 at 02:25 PM GMT, Tom Herbert wrote: > On Fri, Oct 28, 2016 at 1:32 AM, Jakub Sitnicki wrote: >> On Thu, Oct 27, 2016 at 10:35 PM GMT, Tom Herbert wrote: >>> On Mon, Oct 24, 2016 at 2:28 AM, Jakub Sitnicki wrote: >>>> Same as for the transmit path, let's do our best to ensure that received >>>> ICMP errors that may be subject to forwarding will be routed the same >>>> path as flow that triggered the error, if it was going in the opposite >>>> direction. >>>> >>> Unfortunately our ability to do this is generally quite limited. This >>> patch will select the route for multipath, but I don't believe sets >>> the same link in LAG and definitely can't help switches doing ECMP to >>> route the ICMP packet in the same way as the flow would be. Did you >>> see a problem that warrants solving this case? >> >> The motivation here is to bring IPv6 ECMP routing on par with IPv4 to >> enable its wider use, targeting anycast services. Forwarding ICMP errors >> back to the source host, at the L3 layer, is what we thought would be a >> step forward. >> >> Similar to change in IPv4 routing introduced in commit 79a131592dbb >> ("ipv4: ICMP packet inspection for multipath", [1]) we do our best at >> L3, leaving any potential problems with LAG at lower layer (L2) >> unaddressed. >> > ICMP will almost certainly take a different path in the network than > TCP or UDP due to ECMP. If we ever get proper flow label support for > ECMP then that could solve the problem if all the devices do a hash > just on . Sorry for my late reply, I have been traveling. I think that either I am missing something here, or the proposed changes address just the problem that you have described. Yes, if we compute the hash that drives the route choice over the IP header of the ICMP error, then there is no guarantee it will travel back to the sender of the offending packet that triggered the error. That is why, we look at the offending packet carried by an ICMP error and hash over its fields, instead. We need, however, to take care of two things: 1) swap the source with the destination address, because we are forwarding the ICMP error in the opposite direction than the offending packet was going (see icmpv6_multipath_hash() introduced in patch 4/5); and 2) ensure the flow labels used in both directions are the same (either reflected by one side, or fixed, e.g. not used and set to 0), so that the 4-tuple we hash over when forwarding, , is the same both ways, modulo the order of addresses. > If this patch is being done to be compatible with IPv4 I guess that's > okay, but it would be false advertisement to say this makes ICMP > follow the same path as the flow being targeted in an error. > Fortunately, I doubt anyone can have a dependency on this for ICMP. I wouldn't want to propose anything that would be useless. If you think that this is the case here, I would very much like to understand what and why cannot work in practice. Thanks for reviewing this series, Jakub
Re: [PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
On Thu, Oct 27, 2016 at 10:35 PM GMT, Tom Herbert wrote: > On Mon, Oct 24, 2016 at 2:28 AM, Jakub Sitnicki wrote: >> Same as for the transmit path, let's do our best to ensure that received >> ICMP errors that may be subject to forwarding will be routed the same >> path as flow that triggered the error, if it was going in the opposite >> direction. >> > Unfortunately our ability to do this is generally quite limited. This > patch will select the route for multipath, but I don't believe sets > the same link in LAG and definitely can't help switches doing ECMP to > route the ICMP packet in the same way as the flow would be. Did you > see a problem that warrants solving this case? The motivation here is to bring IPv6 ECMP routing on par with IPv4 to enable its wider use, targeting anycast services. Forwarding ICMP errors back to the source host, at the L3 layer, is what we thought would be a step forward. Similar to change in IPv4 routing introduced in commit 79a131592dbb ("ipv4: ICMP packet inspection for multipath", [1]) we do our best at L3, leaving any potential problems with LAG at lower layer (L2) unaddressed. >> diff --git a/net/ipv6/route.c b/net/ipv6/route.c >> index 1184c2b..c0f38ea 100644 >> --- a/net/ipv6/route.c >> +++ b/net/ipv6/route.c [...] >> @@ -1168,6 +1192,8 @@ void ip6_route_input(struct sk_buff *skb) >> tun_info = skb_tunnel_info(skb); >> if (tun_info && !(tun_info->mode & IP_TUNNEL_INFO_TX)) >> fl6.flowi6_tun_key.tun_id = tun_info->key.tun_id; >> + if (unlikely(fl6.flowi6_proto == IPPROTO_ICMPV6)) >> + fl6.mp_hash = ip6_multipath_icmp_hash(skb); > > I will point out that this is only Sorry, looks like part of your reply got cut short. Could you repost? -Jakub [1] https://git.kernel.org/torvalds/c/79a131592dbb81a2dba208622a2ffbfc53f28bc0
Re: [PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
On Thu, Oct 27, 2016 at 03:25 PM GMT, David Miller wrote: > From: Jakub Sitnicki > Date: Mon, 24 Oct 2016 11:28:52 +0200 > >> +inner_iph = skb_header_pointer( >> +skb, skb_transport_offset(skb) + sizeof(*icmph), >> +sizeof(_inner_iph), &_inner_iph); > > Please do not style this call like this, put as many arguments as > you can on the first line. > > inner_iph = skb_header_pointer(skb, > skb_transport_offset(skb) + > sizeof(*icmph), > sizeof(_inner_iph), &_inner_iph); > > And on the second and subsequent lines, indent to the first column after > the openning parenthesis of the first line. FWIW, I had it styled like that and then changed it. Will change back. In my defense - checkpatch.pl made me do it, Your Honor! (line too long)
Re: [PATCH net-next 4/5] ipv6: Compute multipath hash for sent ICMP errors from offending packet
On Thu, Oct 27, 2016 at 03:24 PM GMT, David Miller wrote: > From: Jakub Sitnicki > Date: Mon, 24 Oct 2016 11:28:51 +0200 > >> diff --git a/include/linux/icmpv6.h b/include/linux/icmpv6.h >> index 57086e9..6282e03 100644 >> --- a/include/linux/icmpv6.h >> +++ b/include/linux/icmpv6.h >> @@ -45,4 +45,6 @@ extern void >> icmpv6_flow_init(struct sock *sk, >> const struct in6_addr >> *saddr, >> const struct in6_addr >> *daddr, >> int oif); >> +struct ipv6hdr; >> +extern u32 icmpv6_multipath_hash(const struct >> ipv6hdr *iph); >> #endif > > We do not use "extern" in external function declarations in header file any > more. My mistake, will remote it.
Re: [PATCH] rtl8xxxu: mark symbol static where possible
On Wed, Oct 26, 2016 at 09:32 AM GMT, Baoyou Xie wrote: > We get 1 warning when building kernel with W=1: > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c:1557:6: warning: no > previous prototype for 'rtl8192eu_power_off' [-Wmissing-prototypes] > > In fact, this function is only used in the file in which it is > declared and don't need a declaration, but can be made static. > So this patch marks this function with 'static'. > > Signed-off-by: Baoyou Xie > --- > drivers/net/dsa/mv88e6xxx/chip.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c > b/drivers/net/dsa/mv88e6xxx/chip.c > index 883fd98..4d975f0 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -2701,7 +2701,7 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip > *chip, int port) > return mv88e6xxx_port_write(chip, port, PORT_DEFAULT_VLAN, 0x); > } > > -int mv88e6xxx_g1_set_switch_mac(struct mv88e6xxx_chip *chip, u8 *addr) > +static int mv88e6xxx_g1_set_switch_mac(struct mv88e6xxx_chip *chip, u8 *addr) > { > int err; Probably a mix-up - your patch doesn't match the description. Thanks, Jakub
[PATCH net-next 2/5] net: Extend struct flowi6 with multipath hash
Allow for functions that fill out the IPv6 flow info to also pass a hash computed over the skb contents. The hash value will drive the multipath routing decisions. This is intended for special treatment of ICMPv6 errors, where we would like to make a routing decision based on the flow identifying the offending IPv6 datagram that triggered the error, rather than the flow of the ICMP error itself. Signed-off-by: Jakub Sitnicki --- include/net/flow.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/net/flow.h b/include/net/flow.h index 035aa77..73ee3aa 100644 --- a/include/net/flow.h +++ b/include/net/flow.h @@ -143,6 +143,7 @@ struct flowi6 { #define fl6_ipsec_spi uli.spi #define fl6_mh_typeuli.mht.type #define fl6_gre_keyuli.gre_key + __u32 mp_hash; } __attribute__((__aligned__(BITS_PER_LONG/8))); struct flowidn { -- 2.7.4
[PATCH net-next 1/5] ipv6: Fold rt6_info_hash_nhsfn() into its only caller
Commit 644d0e656958 ("ipv6 Use get_hash_from_flowi6 for rt6 hash") has turned rt6_info_hash_nhsfn() into a one-liner, so it no longer makes sense to keep it around. Also the accompanying documentation comment has become outdated, so just remove it altogether. Signed-off-by: Jakub Sitnicki Acked-by: Hannes Frederic Sowa --- net/ipv6/route.c | 12 +--- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/net/ipv6/route.c b/net/ipv6/route.c index bdbc38e..0514b35 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -425,16 +425,6 @@ static bool rt6_check_expired(const struct rt6_info *rt) return false; } -/* Multipath route selection: - * Hash based function using packet header and flowlabel. - * Adapted from fib_info_hashfn() - */ -static int rt6_info_hash_nhsfn(unsigned int candidate_count, - const struct flowi6 *fl6) -{ - return get_hash_from_flowi6(fl6) % candidate_count; -} - static struct rt6_info *rt6_multipath_select(struct rt6_info *match, struct flowi6 *fl6, int oif, int strict) @@ -442,7 +432,7 @@ static struct rt6_info *rt6_multipath_select(struct rt6_info *match, struct rt6_info *sibling, *next_sibling; int route_choosen; - route_choosen = rt6_info_hash_nhsfn(match->rt6i_nsiblings + 1, fl6); + route_choosen = get_hash_from_flowi6(fl6) % (match->rt6i_nsiblings + 1); /* Don't change the route, if route_choosen == 0 * (siblings does not include ourself) */ -- 2.7.4
[PATCH net-next 4/5] ipv6: Compute multipath hash for sent ICMP errors from offending packet
Improve debuggability with tools like traceroute and make PMTUD work in setups that make use of ECMP routing by sending ICMP errors down the same path as the offending packet would travel, if it was going in the opposite direction. There is a caveat, flows in both directions need use the same label. Otherwise packets from flow in the opposite direction and ICMP errors will not be routed over the same ECMP link. Export the function for calculating the multipath hash so that we can use it also on receive side, when forwarding ICMP errors. Signed-off-by: Jakub Sitnicki --- include/linux/icmpv6.h | 2 ++ net/ipv6/icmp.c| 21 + 2 files changed, 23 insertions(+) diff --git a/include/linux/icmpv6.h b/include/linux/icmpv6.h index 57086e9..6282e03 100644 --- a/include/linux/icmpv6.h +++ b/include/linux/icmpv6.h @@ -45,4 +45,6 @@ extern void icmpv6_flow_init(struct sock *sk, const struct in6_addr *saddr, const struct in6_addr *daddr, int oif); +struct ipv6hdr; +extern u32 icmpv6_multipath_hash(const struct ipv6hdr *iph); #endif diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c index bd59c34..ab376b3d1 100644 --- a/net/ipv6/icmp.c +++ b/net/ipv6/icmp.c @@ -385,6 +385,26 @@ static struct dst_entry *icmpv6_route_lookup(struct net *net, return ERR_PTR(err); } +u32 icmpv6_multipath_hash(const struct ipv6hdr *iph) +{ + struct flowi6 fl6; + + /* Calculate the multipath hash from the offending IP datagram that +* triggered the ICMP error. The source and destination addresses are +* swapped as we do our best to route the ICMP message together with the +* flow it belongs to. However, flows in both directions have to have +* the same label (e.g. by using flow label reflection) for it to +* happen. +*/ + memset(&fl6, 0, sizeof(fl6)); + fl6.daddr = iph->saddr; + fl6.saddr = iph->daddr; + fl6.flowlabel = ip6_flowinfo(iph); + fl6.flowi6_proto = iph->nexthdr; + + return get_hash_from_flowi6(&fl6); +} + /* * Send an ICMP message in response to a packet in error */ @@ -484,6 +504,7 @@ static void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info, fl6.flowi6_oif = iif; fl6.fl6_icmp_type = type; fl6.fl6_icmp_code = code; + fl6.mp_hash = icmpv6_multipath_hash(hdr); security_skb_classify_flow(skb, flowi6_to_flowi(&fl6)); sk = icmpv6_xmit_lock(net); -- 2.7.4
[PATCH net-next 3/5] ipv6: Use multipath hash from flow info if available
Allow our callers to influence the choice of ECMP link by honoring the hash passed together with the flow info. This will allow for special treatment of ICMP errors which we would like to route over the same link as the IP datagram that triggered the error. Signed-off-by: Jakub Sitnicki --- net/ipv6/route.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 0514b35..1184c2b 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -430,9 +430,11 @@ static struct rt6_info *rt6_multipath_select(struct rt6_info *match, int strict) { struct rt6_info *sibling, *next_sibling; + unsigned int hash; int route_choosen; - route_choosen = get_hash_from_flowi6(fl6) % (match->rt6i_nsiblings + 1); + hash = fl6->mp_hash ? : get_hash_from_flowi6(fl6); + route_choosen = hash % (match->rt6i_nsiblings + 1); /* Don't change the route, if route_choosen == 0 * (siblings does not include ourself) */ -- 2.7.4
[PATCH net-next 0/5] Route ICMPv6 errors with the flow when ECMP in use
The motivation for this series is to route ICMPv6 error messages together with the flow they belong to when multipath routing is in use. It intends to bring the ECMP routing in IPv6 stack on par with IPv4. This enables the use of tools that rely on ICMP error messages such as traceroute and makes PMTU discovery work both ways. However, for it to work IPv6 flow labels have to be same in both directions (i.e. reflected) or need to be chosen in a manner that ensures that the flow going in the opposite direction would actually be routed to a given path. Changes have been tested in a virtual setup with a topology as below: Re1 --- Hs1 / Hc --- Ri --- Rc \ Re1 --- Hs2 Hc - client host HsX - server host Rc - core router ReX - edge router Ri - intermediate router To test the changes, traceroute in UDP mode to the client host, with flow label set, has been run from one of the server hosts. Full test is available at [1]. -Jakub [1] https://github.com/jsitnicki/tools/blob/master/net/tests/ecmp/test-ecmp-icmpv6-error-routing.sh Jakub Sitnicki (5): ipv6: Fold rt6_info_hash_nhsfn() into its only caller net: Extend struct flowi6 with multipath hash ipv6: Use multipath hash from flow info if available ipv6: Compute multipath hash for sent ICMP errors from offending packet ipv6: Compute multipath hash for forwarded ICMP errors from offending packet include/linux/icmpv6.h | 2 ++ include/net/flow.h | 1 + net/ipv6/icmp.c| 21 + net/ipv6/route.c | 40 +--- 4 files changed, 53 insertions(+), 11 deletions(-) -- 2.7.4
[PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
Same as for the transmit path, let's do our best to ensure that received ICMP errors that may be subject to forwarding will be routed the same path as flow that triggered the error, if it was going in the opposite direction. Signed-off-by: Jakub Sitnicki --- net/ipv6/route.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 1184c2b..c0f38ea 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -1150,6 +1150,30 @@ struct dst_entry *ip6_route_input_lookup(struct net *net, } EXPORT_SYMBOL_GPL(ip6_route_input_lookup); +static u32 ip6_multipath_icmp_hash(const struct sk_buff *skb) +{ + const struct icmp6hdr *icmph = icmp6_hdr(skb); + const struct ipv6hdr *inner_iph; + struct ipv6hdr _inner_iph; + + if (icmph->icmp6_type != ICMPV6_DEST_UNREACH && + icmph->icmp6_type != ICMPV6_PKT_TOOBIG && + icmph->icmp6_type != ICMPV6_TIME_EXCEED && + icmph->icmp6_type != ICMPV6_PARAMPROB) + goto standard_hash; + + inner_iph = skb_header_pointer( + skb, skb_transport_offset(skb) + sizeof(*icmph), + sizeof(_inner_iph), &_inner_iph); + if (!inner_iph) + goto standard_hash; + + return icmpv6_multipath_hash(inner_iph); + +standard_hash: + return 0; /* compute it later, if needed */ +} + void ip6_route_input(struct sk_buff *skb) { const struct ipv6hdr *iph = ipv6_hdr(skb); @@ -1168,6 +1192,8 @@ void ip6_route_input(struct sk_buff *skb) tun_info = skb_tunnel_info(skb); if (tun_info && !(tun_info->mode & IP_TUNNEL_INFO_TX)) fl6.flowi6_tun_key.tun_id = tun_info->key.tun_id; + if (unlikely(fl6.flowi6_proto == IPPROTO_ICMPV6)) + fl6.mp_hash = ip6_multipath_icmp_hash(skb); skb_dst_drop(skb); skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags)); } -- 2.7.4
Re: [PATCH v2 net-next 1/2] net: centralize net_device min/max MTU checking
On Wed, Sep 28, 2016 at 10:20 PM GMT, Jarod Wilson wrote: > While looking into an MTU issue with sfc, I started noticing that almost > every NIC driver with an ndo_change_mtu function implemented almost > exactly the same range checks, and in many cases, that was the only > practical thing their ndo_change_mtu function was doing. Quite a few > drivers have either 68, 64, 60 or 46 as their minimum MTU value checked, > and then various sizes from 1500 to 65535 for their maximum MTU value. We > can remove a whole lot of redundant code here if we simple store min_mtu > and max_mtu in net_device, and check against those in net/core/dev.c's > dev_set_mtu(). > > In theory, there should be zero functional change with this patch, it just > puts the infrastructure in place. Subsequent patches will attempt to start > using said infrastructure, with theoretically zero change in > functionality. > > CC: "David S. Miller" > CC: net...@vger.kernel.org > Signed-off-by: Jarod Wilson > --- [...] > diff --git a/net/core/dev.c b/net/core/dev.c > index c0c291f..5343799 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -6493,9 +6493,17 @@ int dev_set_mtu(struct net_device *dev, int new_mtu) > if (new_mtu == dev->mtu) > return 0; > > - /* MTU must be positive.*/ > - if (new_mtu < 0) > + if (new_mtu < dev->min_mtu) { Ouch, integral promotions. Looks like you need to keep the < 0 check. Otherwise new_mtu gets promoted to unsigned int and negative values will pass the check. Thanks, Jakub
[PATCH] staging: rtl8188eu: Fix build error when CFG80211 is not selected
The kbuild test robot reports the following build error for i386-randconfig-c0-09230740: >> ERROR: "ieee80211_hdrlen" [drivers/staging/rtl8188eu/r8188eu.ko] undefined! Add a dependency on CFG80211 to fix it. Signed-off-by: Jakub Sitnicki --- Same issue has also been reported by Jim Davis for randconfig-1442987837.txt. drivers/staging/rtl8188eu/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/Kconfig b/drivers/staging/rtl8188eu/Kconfig index 5a38b41..94f3879 100644 --- a/drivers/staging/rtl8188eu/Kconfig +++ b/drivers/staging/rtl8188eu/Kconfig @@ -1,6 +1,6 @@ config R8188EU tristate "Realtek RTL8188EU Wireless LAN NIC driver" - depends on WLAN && USB + depends on WLAN && USB && CFG80211 select WIRELESS_EXT select WEXT_PRIV ---help--- -- 2.1.0 -- 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/
[PATCH] staging: rtl8188eu: Introduce monitor interface for IEEE 802.11 frames
This adds support for monitoring IEEE 802.11 Data and Management frames received or transmitted by a RTL8188EU-based device handled by this driver. The monitor interface is not enabled by default and will be registered only if monitor_enable module parameter is set to 1. When enabled it will show up as a monX network device, which can be used by the userspace programs for monitoring network traffic. It is intended as an exploratory/debugging tool for rtl8188eu driver. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/Makefile | 1 + drivers/staging/rtl8188eu/core/rtw_recv.c | 14 ++ drivers/staging/rtl8188eu/core/rtw_xmit.c | 4 + drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c | 4 + drivers/staging/rtl8188eu/include/drv_types.h | 2 + drivers/staging/rtl8188eu/include/mon.h| 36 + drivers/staging/rtl8188eu/os_dep/mon.c | 194 + drivers/staging/rtl8188eu/os_dep/os_intfs.c| 5 + drivers/staging/rtl8188eu/os_dep/usb_intf.c| 10 ++ 9 files changed, 270 insertions(+) create mode 100644 drivers/staging/rtl8188eu/include/mon.h create mode 100644 drivers/staging/rtl8188eu/os_dep/mon.c diff --git a/drivers/staging/rtl8188eu/Makefile b/drivers/staging/rtl8188eu/Makefile index 31ac159..ed72358 100644 --- a/drivers/staging/rtl8188eu/Makefile +++ b/drivers/staging/rtl8188eu/Makefile @@ -42,6 +42,7 @@ r8188eu-y := \ hal/usb_halinit.o \ os_dep/ioctl_linux.o\ os_dep/mlme_linux.o \ + os_dep/mon.o\ os_dep/os_intfs.o \ os_dep/osdep_service.o \ os_dep/recv_linux.o \ diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c index 44eeb03..cb90ad5 100644 --- a/drivers/staging/rtl8188eu/core/rtw_recv.c +++ b/drivers/staging/rtl8188eu/core/rtw_recv.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -1329,6 +1330,19 @@ static int validate_recv_frame(struct adapter *adapter, break; } + /* +* This is the last moment before management and control frames get +* discarded. So we need to forward them to the monitor now or never. +* +* At the same time data frames can still be encrypted if software +* decryption is in use. However, decryption can occur not until later +* (see recv_func()). +* +* Hence forward the frame to the monitor anyway to preserve the order +* in which frames were received. +*/ + rtl88eu_mon_recv_hook(adapter->pmondev, precv_frame); + exit: return retval; diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c index 5dc0b90..cabb810 100644 --- a/drivers/staging/rtl8188eu/core/rtw_xmit.c +++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -1100,6 +1101,9 @@ s32 rtw_xmitframe_coalesce(struct adapter *padapter, struct sk_buff *pkt, struct memcpy(mem_start, pbuf_start + hw_hdr_offset, pattrib->hdrlen); } + /* Frame is about to be encrypted. Forward it to the monitor first. */ + rtl88eu_mon_xmit_hook(padapter->pmondev, pxmitframe, frg_len); + if (xmitframe_addmic(padapter, pxmitframe) == _FAIL) { RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_, ("xmitframe_addmic(padapter, pxmitframe) == _FAIL\n")); DBG_88E("xmitframe_addmic(padapter, pxmitframe) == _FAIL\n"); diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c index 4433560..7c5086e 100644 --- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c +++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c @@ -20,6 +20,7 @@ #define _RTL8188E_XMIT_C_ #include #include +#include #include #include #include @@ -684,6 +685,9 @@ enqueue: s32 rtl8188eu_mgnt_xmit(struct adapter *adapt, struct xmit_frame *pmgntframe) { + struct xmit_priv *xmitpriv = &adapt->xmitpriv; + + rtl88eu_mon_xmit_hook(adapt->pmondev, pmgntframe, xmitpriv->frag_len); return rtw_dump_xframe(adapt, pmgntframe); } diff --git a/drivers/staging/rtl8188eu/include/drv_types.h b/drivers/staging/rtl8188eu/include/drv_types.h index bcc74dc..0729bd4 100644 --- a/drivers/staging/rtl8188eu/include/drv_types.h +++ b/drivers/staging/rtl8188eu/include/drv_types.h @@ -131,6 +131,7 @@ struct registry_priv { u8 if2name[16]; u8 notch_filter; + boolmonitor_enable; }; /* For registry parameters */ @@ -209,6 +210,7 @@ struct adapter { void (*intf_start)(struct adapter *adapter); void (*intf_stop)(struct adapter *adapter); struct net_device *pn
[PATCH] Monitor interface for rtl8188eu
Hello, This was previously posted as a RFC[1] to linux-wireless. Following Larry Finger's suggestion[2] I'm resending it as a proposed patch. This patch is intended as a debugging aid for people working on the rtl8188eu driver. I started working on it because debug logs from rtl8188eu driver got me nowhere when I wanted to see what was "going in and out". It has reached a working state where you can use TShark/Wireshark to analyze the flow of 802.11 frames: modprobe r8188eu monitor_enable=1 ip link set mon0 up tshark -i mon0 I've been testing it against recent staging-next (6dd19f1), in managed mode, with hardware encryption, and only with CCMP (AES) cipher in use, but it should work with any. Performance implications? A throughput test ran with iperf for 20 minutes before and after the changes (with monitor inferace enabled and up) showed: before: 43.0 Mbits/sec after: 41.6 Mbits/sec [1] https://lkml.kernel.org/g/1441383439-27007-1-git-send-email-jsitni...@gmail.com [2] https://github.com/lwfinger/rtl8188eu/issues/73#issuecomment-138588729 Jakub Sitnicki (1): staging: rtl8188eu: Introduce monitor interface for IEEE 802.11 frames drivers/staging/rtl8188eu/Makefile | 1 + drivers/staging/rtl8188eu/core/rtw_recv.c | 14 ++ drivers/staging/rtl8188eu/core/rtw_xmit.c | 4 + drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c | 4 + drivers/staging/rtl8188eu/include/drv_types.h | 2 + drivers/staging/rtl8188eu/include/mon.h| 36 + drivers/staging/rtl8188eu/os_dep/mon.c | 194 + drivers/staging/rtl8188eu/os_dep/os_intfs.c| 5 + drivers/staging/rtl8188eu/os_dep/usb_intf.c| 10 ++ 9 files changed, 270 insertions(+) create mode 100644 drivers/staging/rtl8188eu/include/mon.h create mode 100644 drivers/staging/rtl8188eu/os_dep/mon.c -- 2.1.0 -- 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/
[PATCH v2 0/4] staging: rtl8188eu: clean up duplicated WLAN_* constants
This short series was initially a patch[1] that had to be rebased and split (patches 1 & 2). Since then it has been extended to clean up other WLAN_* constants duplicated in rtl8188eu driver's headers (patches 3 & 4). [1] http://lkml.kernel.org/g/1437750758-17120-1-git-send-email-jsitni...@gmail.com Jakub Sitnicki (4): staging: rtl8188eu: don't duplicate ieee80211 WLAN_EID_* constants staging: rtl8188eu: wrap a long if condition and remove extra parenthesis staging: rtl8188eu: don't duplicate ieee80211 WLAN_AUTH_* constants staging: rtl8188eu: don't duplicate ieee80211 WLAN_HT_CAP_SM_PS_* constants drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 2 +- drivers/staging/rtl8188eu/include/ieee80211.h | 44 -- drivers/staging/rtl8188eu/include/wifi.h | 7 drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 3 +- 4 files changed, 3 insertions(+), 53 deletions(-) -- 2.1.0 -- 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/
[PATCH v2 2/4] staging: rtl8188eu: wrap a long if condition and remove extra parenthesis
Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c index 554c04d..844ed6f 100644 --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c @@ -2666,7 +2666,8 @@ static int rtw_get_sta_wpaie(struct net_device *dev, struct ieee_param *param) psta = rtw_get_stainfo(pstapriv, param->sta_addr); if (psta) { - if ((psta->wpa_ie[0] == WLAN_EID_RSN) || (psta->wpa_ie[0] == WLAN_EID_VENDOR_SPECIFIC)) { + if (psta->wpa_ie[0] == WLAN_EID_RSN || + psta->wpa_ie[0] == WLAN_EID_VENDOR_SPECIFIC) { int wpa_ie_len; int copy_len; -- 2.1.0 -- 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/
[PATCH v2 3/4] staging: rtl8188eu: don't duplicate ieee80211 WLAN_AUTH_* constants
linux/ieee80211.h already defines constants for authentication algorithms. Remove the duplicated definitions. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/include/ieee80211.h | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/staging/rtl8188eu/include/ieee80211.h b/drivers/staging/rtl8188eu/include/ieee80211.h index cf3f64d..6400f75 100644 --- a/drivers/staging/rtl8188eu/include/ieee80211.h +++ b/drivers/staging/rtl8188eu/include/ieee80211.h @@ -477,12 +477,6 @@ struct ieee80211_snap_hdr { #define WLAN_GET_SEQ_FRAG(seq) ((seq) & RTW_IEEE80211_SCTL_FRAG) #define WLAN_GET_SEQ_SEQ(seq) ((seq) & RTW_IEEE80211_SCTL_SEQ) -/* Authentication algorithms */ -#define WLAN_AUTH_OPEN 0 -#define WLAN_AUTH_SHARED_KEY 1 - -#define WLAN_AUTH_CHALLENGE_LEN 128 - /* Non standard? Not in */ #define WLAN_REASON_EXPIRATION_CHK 65535 -- 2.1.0 -- 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/
[PATCH v2 1/4] staging: rtl8188eu: don't duplicate ieee80211 WLAN_EID_* constants
linux/ieee80211.h already defines constants for information element IDs. Resolve discrepancies in naming and remove the duplicated definitions. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 2 +- drivers/staging/rtl8188eu/include/ieee80211.h | 38 -- drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 2 +- 3 files changed, 2 insertions(+), 40 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c index d43e867..c3c5828 100644 --- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c +++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c @@ -1044,7 +1044,7 @@ enum parse_res rtw_ieee802_11_parse_elems(u8 *start, uint len, elems->timeout_int = pos; elems->timeout_int_len = elen; break; - case WLAN_EID_HT_CAP: + case WLAN_EID_HT_CAPABILITY: elems->ht_capabilities = pos; elems->ht_capabilities_len = elen; break; diff --git a/drivers/staging/rtl8188eu/include/ieee80211.h b/drivers/staging/rtl8188eu/include/ieee80211.h index fabb959..cf3f64d 100644 --- a/drivers/staging/rtl8188eu/include/ieee80211.h +++ b/drivers/staging/rtl8188eu/include/ieee80211.h @@ -486,44 +486,6 @@ struct ieee80211_snap_hdr { /* Non standard? Not in */ #define WLAN_REASON_EXPIRATION_CHK 65535 -/* Information Element IDs */ -#define WLAN_EID_SSID 0 -#define WLAN_EID_SUPP_RATES 1 -#define WLAN_EID_FH_PARAMS 2 -#define WLAN_EID_DS_PARAMS 3 -#define WLAN_EID_CF_PARAMS 4 -#define WLAN_EID_TIM 5 -#define WLAN_EID_IBSS_PARAMS 6 -#define WLAN_EID_CHALLENGE 16 -/* EIDs defined by IEEE 802.11h - START */ -#define WLAN_EID_PWR_CONSTRAINT 32 -#define WLAN_EID_PWR_CAPABILITY 33 -#define WLAN_EID_TPC_REQUEST 34 -#define WLAN_EID_TPC_REPORT 35 -#define WLAN_EID_SUPPORTED_CHANNELS 36 -#define WLAN_EID_CHANNEL_SWITCH 37 -#define WLAN_EID_MEASURE_REQUEST 38 -#define WLAN_EID_MEASURE_REPORT 39 -#define WLAN_EID_QUITE 40 -#define WLAN_EID_IBSS_DFS 41 -/* EIDs defined by IEEE 802.11h - END */ -#define WLAN_EID_ERP_INFO 42 -#define WLAN_EID_HT_CAP 45 -#define WLAN_EID_RSN 48 -#define WLAN_EID_EXT_SUPP_RATES 50 -#define WLAN_EID_MOBILITY_DOMAIN 54 -#define WLAN_EID_FAST_BSS_TRANSITION 55 -#define WLAN_EID_TIMEOUT_INTERVAL 56 -#define WLAN_EID_RIC_DATA 57 -#define WLAN_EID_HT_OPERATION 61 -#define WLAN_EID_SECONDARY_CHANNEL_OFFSET 62 -#define WLAN_EID_20_40_BSS_COEXISTENCE 72 -#define WLAN_EID_20_40_BSS_INTOLERANT 73 -#define WLAN_EID_OVERLAPPING_BSS_SCAN_PARAMS 74 -#define WLAN_EID_MMIE 76 -#define WLAN_EID_VENDOR_SPECIFIC 221 -#define WLAN_EID_GENERIC (WLAN_EID_VENDOR_SPECIFIC) - #define IEEE80211_MGMT_HDR_LEN 24 #define IEEE80211_DATA_HDR3_LEN 24 #define IEEE80211_DATA_HDR4_LEN 30 diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c index 2db86fb..554c04d 100644 --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c @@ -2666,7 +2666,7 @@ static int rtw_get_sta_wpaie(struct net_device *dev, struct ieee_param *param) psta = rtw_get_stainfo(pstapriv, param->sta_addr); if (psta) { - if ((psta->wpa_ie[0] == WLAN_EID_RSN) || (psta->wpa_ie[0] == WLAN_EID_GENERIC)) { + if ((psta->wpa_ie[0] == WLAN_EID_RSN) || (psta->wpa_ie[0] == WLAN_EID_VENDOR_SPECIFIC)) { int wpa_ie_len; int copy_len; -- 2.1.0 -- 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/
[PATCH v2 4/4] staging: rtl8188eu: don't duplicate ieee80211 WLAN_HT_CAP_SM_PS_* constants
linux/ieee80211.h already defines constants for spatial multiplexing power save modes. Remove the duplicated definitions. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/include/wifi.h | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/staging/rtl8188eu/include/wifi.h b/drivers/staging/rtl8188eu/include/wifi.h index a08a2e0..dba8af1 100644 --- a/drivers/staging/rtl8188eu/include/wifi.h +++ b/drivers/staging/rtl8188eu/include/wifi.h @@ -649,13 +649,6 @@ enum ht_cap_ampdu_factor { #define IEEE80211_MAX_AMPDU_BUF 0x40 -/* Spatial Multiplexing Power Save Modes */ -#define WLAN_HT_CAP_SM_PS_STATIC 0 -#define WLAN_HT_CAP_SM_PS_DYNAMIC 1 -#define WLAN_HT_CAP_SM_PS_INVALID 2 -#define WLAN_HT_CAP_SM_PS_DISABLED 3 - - #define OP_MODE_PURE0 #define OP_MODE_MAY_BE_LEGACY_STAS 1 #define OP_MODE_20MHZ_HT_STA_ASSOCED2 -- 2.1.0 -- 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 RESEND] staging: rtl8188eu: don't duplicate ieee80211 WLAN_EID_* constants
On Sat, Jul 25, 2015 at 04:30 PM CEST, Dan Carpenter wrote: > On Sat, Jul 25, 2015 at 04:05:52PM +0200, Jakub Sitnicki wrote: >> On Fri, Jul 24, 2015 at 10:39 PM CEST, Greg Kroah-Hartman >> wrote: >> > On Fri, Jul 24, 2015 at 05:12:38PM +0200, Jakub Sitnicki wrote: >> >> linux/ieee80211.h already defines constants for information element IDs. >> >> Include it where needed, resolve discrepancies in naming, and remove the >> >> duplicated definitions. >> >> >> >> While at it, wrap a line that was too long and remove extra parentheses >> >> in an expression that mixes only equality and logical operators. >> > >> > This patch doesn't apply at all. >> >> My mistake, sorry. I've generated it against v4.2-rc3. >> > > Do it against > > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git staging-next Will do. Thanks, Jakub -- 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 RESEND] staging: rtl8188eu: don't duplicate ieee80211 WLAN_EID_* constants
On Fri, Jul 24, 2015 at 10:39 PM CEST, Greg Kroah-Hartman wrote: > On Fri, Jul 24, 2015 at 05:12:38PM +0200, Jakub Sitnicki wrote: >> linux/ieee80211.h already defines constants for information element IDs. >> Include it where needed, resolve discrepancies in naming, and remove the >> duplicated definitions. >> >> While at it, wrap a line that was too long and remove extra parentheses >> in an expression that mixes only equality and logical operators. > > This patch doesn't apply at all. My mistake, sorry. I've generated it against v4.2-rc3. > Also, don't do more than one thing in a single patch, this should be > broken up into multiple ones. OK, I'll redo it with style fixes split out into a separate patch. Cheers, Jakub -- 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/
[PATCH RESEND] staging: rtl8188eu: don't duplicate ieee80211 WLAN_EID_* constants
linux/ieee80211.h already defines constants for information element IDs. Include it where needed, resolve discrepancies in naming, and remove the duplicated definitions. While at it, wrap a line that was too long and remove extra parentheses in an expression that mixes only equality and logical operators. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 4 ++- drivers/staging/rtl8188eu/include/ieee80211.h | 38 -- drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 3 +- 3 files changed, 5 insertions(+), 40 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c index 11b780d..c3c5828 100644 --- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c +++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c @@ -19,6 +19,8 @@ **/ #define _IEEE80211_C +#include + #include #include #include @@ -1042,7 +1044,7 @@ enum parse_res rtw_ieee802_11_parse_elems(u8 *start, uint len, elems->timeout_int = pos; elems->timeout_int_len = elen; break; - case WLAN_EID_HT_CAP: + case WLAN_EID_HT_CAPABILITY: elems->ht_capabilities = pos; elems->ht_capabilities_len = elen; break; diff --git a/drivers/staging/rtl8188eu/include/ieee80211.h b/drivers/staging/rtl8188eu/include/ieee80211.h index b129ad1..611877c 100644 --- a/drivers/staging/rtl8188eu/include/ieee80211.h +++ b/drivers/staging/rtl8188eu/include/ieee80211.h @@ -496,44 +496,6 @@ struct ieee80211_snap_hdr { /* Non standard? Not in */ #define WLAN_REASON_EXPIRATION_CHK 65535 -/* Information Element IDs */ -#define WLAN_EID_SSID 0 -#define WLAN_EID_SUPP_RATES 1 -#define WLAN_EID_FH_PARAMS 2 -#define WLAN_EID_DS_PARAMS 3 -#define WLAN_EID_CF_PARAMS 4 -#define WLAN_EID_TIM 5 -#define WLAN_EID_IBSS_PARAMS 6 -#define WLAN_EID_CHALLENGE 16 -/* EIDs defined by IEEE 802.11h - START */ -#define WLAN_EID_PWR_CONSTRAINT 32 -#define WLAN_EID_PWR_CAPABILITY 33 -#define WLAN_EID_TPC_REQUEST 34 -#define WLAN_EID_TPC_REPORT 35 -#define WLAN_EID_SUPPORTED_CHANNELS 36 -#define WLAN_EID_CHANNEL_SWITCH 37 -#define WLAN_EID_MEASURE_REQUEST 38 -#define WLAN_EID_MEASURE_REPORT 39 -#define WLAN_EID_QUITE 40 -#define WLAN_EID_IBSS_DFS 41 -/* EIDs defined by IEEE 802.11h - END */ -#define WLAN_EID_ERP_INFO 42 -#define WLAN_EID_HT_CAP 45 -#define WLAN_EID_RSN 48 -#define WLAN_EID_EXT_SUPP_RATES 50 -#define WLAN_EID_MOBILITY_DOMAIN 54 -#define WLAN_EID_FAST_BSS_TRANSITION 55 -#define WLAN_EID_TIMEOUT_INTERVAL 56 -#define WLAN_EID_RIC_DATA 57 -#define WLAN_EID_HT_OPERATION 61 -#define WLAN_EID_SECONDARY_CHANNEL_OFFSET 62 -#define WLAN_EID_20_40_BSS_COEXISTENCE 72 -#define WLAN_EID_20_40_BSS_INTOLERANT 73 -#define WLAN_EID_OVERLAPPING_BSS_SCAN_PARAMS 74 -#define WLAN_EID_MMIE 76 -#define WLAN_EID_VENDOR_SPECIFIC 221 -#define WLAN_EID_GENERIC (WLAN_EID_VENDOR_SPECIFIC) - #define IEEE80211_MGMT_HDR_LEN 24 #define IEEE80211_DATA_HDR3_LEN 24 #define IEEE80211_DATA_HDR4_LEN 30 diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c index 38dba14..dec089d 100644 --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c @@ -2666,7 +2666,8 @@ static int rtw_get_sta_wpaie(struct net_device *dev, struct ieee_param *param) psta = rtw_get_stainfo(pstapriv, param->sta_addr); if (psta) { - if ((psta->wpa_ie[0] == WLAN_EID_RSN) || (psta->wpa_ie[0] == WLAN_EID_GENERIC)) { + if (psta->wpa_ie[0] == WLAN_EID_RSN || + psta->wpa_ie[0] == WLAN_EID_VENDOR_SPECIFIC) { int wpa_ie_len; int copy_len; -- 2.1.0 -- 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/
[PATCH RESEND] staging: rtl8188eu: kill unused hal_data_8188e::fw_ractrl flag
Flag is never set. Remove it and the code that is dead because of it. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/hal/odm.c | 11 -- drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c | 21 -- drivers/staging/rtl8188eu/hal/usb_halinit.c | 27 ++-- drivers/staging/rtl8188eu/include/rtl8188e_cmd.h | 1 - drivers/staging/rtl8188eu/include/rtl8188e_hal.h | 1 - drivers/staging/rtl8188eu/include/sta_info.h | 1 - 6 files changed, 6 insertions(+), 56 deletions(-) diff --git a/drivers/staging/rtl8188eu/hal/odm.c b/drivers/staging/rtl8188eu/hal/odm.c index 28b5e7b..710fdc3 100644 --- a/drivers/staging/rtl8188eu/hal/odm.c +++ b/drivers/staging/rtl8188eu/hal/odm.c @@ -1170,13 +1170,10 @@ void odm_RSSIMonitorCheckCE(struct odm_dm_struct *pDM_Odm) } for (i = 0; i < sta_cnt; i++) { - if (PWDB_rssi[i] != (0)) { - if (pHalData->fw_ractrl) { - /* Report every sta's RSSI to FW */ - } else { - ODM_RA_SetRSSI_8188E( - &(pHalData->odmpriv), (PWDB_rssi[i]&0xFF), (u8)((PWDB_rssi[i]>>16) & 0xFF)); - } + if (PWDB_rssi[i] != 0) { + ODM_RA_SetRSSI_8188E(&pHalData->odmpriv, +PWDB_rssi[i] & 0xFF, +(PWDB_rssi[i] >> 16) & 0xFF); } } diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c index 86347f2..0a62bfa 100644 --- a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c @@ -127,27 +127,6 @@ exit: return ret; } -u8 rtl8188e_set_raid_cmd(struct adapter *adapt, u32 mask) -{ - u8 buf[3]; - u8 res = _SUCCESS; - struct hal_data_8188e *haldata = GET_HAL_DATA(adapt); - - if (haldata->fw_ractrl) { - - memset(buf, 0, 3); - put_unaligned_le32(mask, buf); - - FillH2CCmd_88E(adapt, H2C_DM_MACID_CFG, 3, buf); - } else { - DBG_88E("==>%s fw dont support RA\n", __func__); - res = _FAIL; - } - - - return res; -} - /* bitmap[0:27] = tx_rate_bitmap */ /* bitmap[28:31]= Rate Adaptive id */ /* arg[0:4] = macid */ diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c b/drivers/staging/rtl8188eu/hal/usb_halinit.c index 8726222..bdbc78f 100644 --- a/drivers/staging/rtl8188eu/hal/usb_halinit.c +++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c @@ -743,19 +743,16 @@ static u32 rtl8188eu_hal_init(struct adapter *Adapter) if (Adapter->registrypriv.mp_mode == 1) { _InitRxSetting(Adapter); Adapter->bFWReady = false; - haldata->fw_ractrl = false; } else { status = rtl88eu_download_fw(Adapter); if (status) { DBG_88E("%s: Download Firmware failed!!\n", __func__); Adapter->bFWReady = false; - haldata->fw_ractrl = false; return status; } else { RT_TRACE(_module_hci_hal_init_c_, _drv_info_, ("Initializeadapt8192CSdio(): Download Firmware Success!!\n")); Adapter->bFWReady = true; - haldata->fw_ractrl = false; } } rtl8188e_InitializeFirmwareVars(Adapter); @@ -2085,28 +2082,9 @@ static void UpdateHalRAMask8188EUsb(struct adapter *adapt, u32 mac_id, u8 rssi_l init_rate = get_highest_rate_idx(mask)&0x3f; - if (haldata->fw_ractrl) { - u8 arg; + ODM_RA_UpdateRateInfo_8188E(&haldata->odmpriv, mac_id, + raid, mask, shortGIrate); - arg = mac_id & 0x1f;/* MACID */ - arg |= BIT(7); - if (shortGIrate) - arg |= BIT(5); - mask |= ((raid << 28) & 0xf000); - DBG_88E("update raid entry, mask=0x%x, arg=0x%x\n", mask, arg); - psta->ra_mask = mask; - mask |= ((raid << 28) & 0xf000); - - /* to do ,for 8188E-SMIC */ - rtl8188e_set_raid_cmd(adapt, mask); - } else { - ODM_RA_UpdateRateInfo_8188E(&(haldata->odmpriv), - mac_id, - raid, - mask, - shortGIrate - ); - } /* set ra_id */ psta->raid = raid; psta->init_rate = init_rate; @@ -2156,7 +2134,6 @@ sta
Re: [PATCH] staging: rtl8188eu: don't duplicate ieee80211 WLAN_EID_* constants
On Wed, Jun 24, 2015 at 07:23 AM CEST, Jakub Sitnicki wrote: > linux/ieee80211.h already defines constants for information element IDs. > Include it where needed, resolve discrepancies in naming, and remove the > duplicated definitions. > > While at it, wrap a line that was too long and remove extra parentheses > in an expression that mixes only equality and logical operators. > > Signed-off-by: Jakub Sitnicki > --- Looks like this patch got lost in the noise. It still applies & builds. Cheers, Jakub -- 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 5/6] staging: rtl8188eu: stop using DBG_88E
On Sat, Jul 18, 2015 at 06:46 AM CEST, Sudip Mukherjee wrote: > On Fri, Jul 17, 2015 at 05:33:55PM +0200, Jakub Sitnicki wrote: >> On Thu, Jul 16, 2015 at 01:28 PM CEST, Sudip Mukherjee >> wrote: >> > Stop using DBG_88E which is a custom macro for printing debugging >> > messages. Instead start using pr_debug and in the process define >> > pr_fmt. >> >> In the end, don't we want to use netdev_dbg() everywhere where we work >> with a struct net_device? And use dev_dbg() everywhere where we work >> with a struct device (or a struct usb_interface)? > Looks like in some places we can get net_device from usb_interface. > > struct dvobj_priv *dvobj = usb_get_intfdata(pusb_intf); > struct adapter *padapter = dvobj->if1; > struct net_device *pnetdev = padapter->pnetdev; You're right providing that the net_device has been successfully allocated and hasn't been deallocated yet. There are at least a couple of pr_debug() calls that couldn't be converted to netdev_dbg(), one in rtw_drv_init() and another in rtw_dev_remove(), because the net_device is not available, if I'm not wrong. >> At least that's how I understand commit 8f26b8376faa ("checkpatch: >> update suggested printk conversions") description: >> >> Direct conversion of printk(KERN_... to pr_ isn't the >> preferred conversion when a struct net_device or struct device is >> available. >> >> Do you think it is worth going straight for netdev_dbg()/dev_dbg() to >> avoid redoing it later? > At the end it should be netdev_* or dev_* and if both are not available > then pr_*. Here my main intention was to remove the custom defined > macro. And while doing this it is easier to use a script to reduce the > chances of error. Now that the custom macro is out of the way we can > concentrate on converting it to netdev_* or dev_*. >> >> > > >> > >> > +#define pr_fmt(fmt) "R8188EU: " fmt >> > #include >> > #include >> > #include >> >> If we're going to stay with pr_debug(), using KBUILD_MODNAME seems to be >> the convention among drivers when defining pr_fmt(): >> >> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > yes, KBUILD_MODNAME is the usual convention but you will also find many > places where that has not been used. Here I have used R8188EU to keep it > same for all the messages that will be printed from the other files of > this driver else it might be confusing for the user. I see your point. If consistent log prefix is the goal do you think it would make sense to change it to: #define pr_fmt(fmt) DRIVER_PREFIX ": " fmt ... so the prefix is not defined in two places? Cheers, Jakub -- 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 5/6] staging: rtl8188eu: stop using DBG_88E
On Thu, Jul 16, 2015 at 01:28 PM CEST, Sudip Mukherjee wrote: > Stop using DBG_88E which is a custom macro for printing debugging > messages. Instead start using pr_debug and in the process define > pr_fmt. In the end, don't we want to use netdev_dbg() everywhere where we work with a struct net_device? And use dev_dbg() everywhere where we work with a struct device (or a struct usb_interface)? At least that's how I understand commit 8f26b8376faa ("checkpatch: update suggested printk conversions") description: Direct conversion of printk(KERN_... to pr_ isn't the preferred conversion when a struct net_device or struct device is available. Do you think it is worth going straight for netdev_dbg()/dev_dbg() to avoid redoing it later? > > Signed-off-by: Sudip Mukherjee > --- > drivers/staging/rtl8188eu/os_dep/usb_intf.c | 39 > +++-- > 1 file changed, 20 insertions(+), 19 deletions(-) > > diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c > b/drivers/staging/rtl8188eu/os_dep/usb_intf.c > index 2d75c77..b245e9c 100644 > --- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c > +++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c > @@ -19,6 +19,7 @@ > > **/ > #define _HCI_INTF_C_ > > +#define pr_fmt(fmt) "R8188EU: " fmt > #include > #include > #include If we're going to stay with pr_debug(), using KBUILD_MODNAME seems to be the convention among drivers when defining pr_fmt(): #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt Cheers, Jakub -- 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] staging: rtl8188eu: core: find and remove code valid only for 5 HGz.
On Thu, Jul 16, 2015 at 04:04 AM CEST, Sreenath Madasu wrote: > This one of the TODO tasks for staging rtl8188eu driver. I have removed > the code referring to channel > 14 for rtw_ap.c, rtw_ieee80211.c and > rtw_mlme.c files. Please review. > > Signed-off-by: Sreenath Madasu I would consider rewording the subject and the description to say what the change does ("remove ...") as opposed to saying what you did ("find and remove..."). The "Description, description, description" section from the article below, if you haven't seen it yet, explains it better than I can: https://github.com/gregkh/kernel-tutorial/blob/master/lxf_article/write_kernel_patch.txt > --- > drivers/staging/rtl8188eu/core/rtw_ap.c| 31 > +++--- > drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 24 ++-- > drivers/staging/rtl8188eu/core/rtw_mlme.c | 5 + > 3 files changed, 16 insertions(+), 44 deletions(-) > [snip] > @@ -584,15 +576,8 @@ static void update_bmc_sta(struct adapter *padapter) > tx_ra_bitmap |= > rtw_get_bit_value_from_ieee_value(psta->bssrateset[i]&0x7f); > } > > - if (pcur_network->Configuration.DSConfig > 14) { > - /* force to A mode. 5G doesn't support CCK rates */ > - network_type = WIRELESS_11A; > - tx_ra_bitmap = 0x150; /* 6, 12, 24 Mbps */ > - } else { > - /* force to b mode */ > - network_type = WIRELESS_11B; > - tx_ra_bitmap = 0xf; > - } > + network_type = WIRELESS_11B; > + tx_ra_bitmap = 0xf; > > raid = networktype_to_raid(network_type); > init_rate = get_highest_rate_idx(tx_ra_bitmap&0x0fff)&0x3f; Is the dropped comment ("force to b mode") worth leaving just to draw attention? There is something suspicious going on in update_bmc_sta() because just a few lines above we determine the network_type: network_type = rtw_check_network_type((u8 *)&pcur_network->SupportedRates, supportRateNum, 1); ... only to force to it WIRELESS_11B later on. Unfortunately I'm not familiar enough with the driver to know why. > diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c > b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c > index 11b780d..f55dae1 100644 > --- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c > +++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c > @@ -113,19 +113,12 @@ uintrtw_is_cckratesonly_included(u8 *rate) > > int rtw_check_network_type(unsigned char *rate, int ratelen, int channel) > { > - if (channel > 14) { > - if ((rtw_is_cckrates_included(rate)) == true) > - return WIRELESS_INVALID; > - else > - return WIRELESS_11A; > - } else { /* could be pure B, pure G, or B/G */ > - if ((rtw_is_cckratesonly_included(rate)) == true) > - return WIRELESS_11B; > - else if ((rtw_is_cckrates_included(rate)) == true) > - return WIRELESS_11BG; > - else > - return WIRELESS_11G; > - } > + if ((rtw_is_cckratesonly_included(rate)) == true) > + return WIRELESS_11B; > + else if ((rtw_is_cckrates_included(rate)) == true) > + return WIRELESS_11BG; > + else > + return WIRELESS_11G; > } > That makes the 'channel' parameter unused. A candidate for a clean-up together with 'ratelen'. Cheers, Jakub -- 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 v2 04/23] staging: rtl8192e: Remove unused enums
On Tue, Jul 14, 2015 at 10:04 PM CEST, Mateusz Kulikowski wrote: > Remove ack_policy enum and some unused RTL_DEBUG enums. > > Signed-off-by: Mateusz Kulikowski > --- [snip] > diff --git a/drivers/staging/rtl8192e/rtllib_debug.h > b/drivers/staging/rtl8192e/rtllib_debug.h > index 42e88d6..2f47a7c 100644 > --- a/drivers/staging/rtl8192e/rtllib_debug.h > +++ b/drivers/staging/rtl8192e/rtllib_debug.h > @@ -40,10 +40,7 @@ enum RTL_DEBUG { > COMP_DBG= (1 << 1), > COMP_INIT = (1 << 2), > COMP_RECV = (1 << 3), > - COMP_SEND = (1 << 4), > - COMP_CMD= (1 << 5), > COMP_POWER = (1 << 6), > - COMP_EPROM = (1 << 7), > COMP_SWBW = (1 << 8), > COMP_SEC= (1 << 9), > COMP_LPS= (1 << 10), > @@ -58,15 +55,12 @@ enum RTL_DEBUG { > COMP_CH = (1 << 19), > COMP_RF = (1 << 20), > COMP_FIRMWARE = (1 << 21), > - COMP_HT = (1 << 22), > COMP_RESET = (1 << 23), > COMP_CMDPKT = (1 << 24), > COMP_SCAN = (1 << 25), > COMP_PS = (1 << 26), > COMP_DOWN = (1 << 27), > COMP_INTR = (1 << 28), > - COMP_LED= (1 << 29), > - COMP_MLME = (1 << 30), > COMP_ERR= (1 << 31) > }; Is it possible that this change will make future readers wonder why there are holes in the enum values, and hence hurts readability? Cheers, Jakub -- 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/
[PATCH 3/5] staging: rtl8188eu: remove RFtype from struct HAL_VERSION
RFtype in struct HAL_VERSION duplicates rf_type in struct hal_data_8188e, and does not change. Remove it and the macros that test it. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/hal/hal_com.c | 12 +--- drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c | 15 ++- drivers/staging/rtl8188eu/include/HalVerDef.h | 21 - 3 files changed, 3 insertions(+), 45 deletions(-) diff --git a/drivers/staging/rtl8188eu/hal/hal_com.c b/drivers/staging/rtl8188eu/hal/hal_com.c index 62ffa7c..a296648 100644 --- a/drivers/staging/rtl8188eu/hal/hal_com.c +++ b/drivers/staging/rtl8188eu/hal/hal_com.c @@ -49,17 +49,7 @@ void dump_chip_info(struct HAL_VERSION chip_vers) else cnt += sprintf((buf+cnt), "UNKNOWN_CUT(%d)_", chip_vers.CUTVersion); - - if (IS_1T1R(chip_vers)) - cnt += sprintf((buf+cnt), "1T1R_"); - else if (IS_1T2R(chip_vers)) - cnt += sprintf((buf+cnt), "1T2R_"); - else if (IS_2T2R(chip_vers)) - cnt += sprintf((buf+cnt), "2T2R_"); - else - cnt += sprintf((buf+cnt), "UNKNOWN_RFTYPE(%d)_", - chip_vers.RFType); - + cnt += sprintf((buf+cnt), "1T1R_"); cnt += sprintf((buf+cnt), "RomVer(%d)\n", chip_vers.ROMVer); pr_info("%s", buf); diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c index af757ff..ea97030 100644 --- a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c @@ -138,8 +138,6 @@ static struct HAL_VERSION ReadChipVersion8188E(struct adapter *padapter) value32 = usb_read32(padapter, REG_SYS_CFG); ChipVersion.ChipType = ((value32 & RTL_ID) ? TEST_CHIP : NORMAL_CHIP); - - ChipVersion.RFType = RF_TYPE_1T1R; ChipVersion.VendorType = ((value32 & VENDOR_ID) ? CHIP_VENDOR_UMC : CHIP_VENDOR_TSMC); ChipVersion.CUTVersion = (value32 & CHIP_VER_RTL_MASK)>>CHIP_VER_RTL_SHIFT; /* IC version (CUT) */ @@ -148,17 +146,8 @@ static struct HAL_VERSION ReadChipVersion8188E(struct adapter *padapter) dump_chip_info(ChipVersion); pHalData->VersionID = ChipVersion; - - if (IS_1T2R(ChipVersion)) { - pHalData->rf_type = RF_1T2R; - pHalData->NumTotalRFPath = 2; - } else if (IS_2T2R(ChipVersion)) { - pHalData->rf_type = RF_2T2R; - pHalData->NumTotalRFPath = 2; - } else{ - pHalData->rf_type = RF_1T1R; - pHalData->NumTotalRFPath = 1; - } + pHalData->rf_type = RF_1T1R; + pHalData->NumTotalRFPath = 1; MSG_88E("RF_Type is %x!!\n", pHalData->rf_type); diff --git a/drivers/staging/rtl8188eu/include/HalVerDef.h b/drivers/staging/rtl8188eu/include/HalVerDef.h index c542f7e..2c24814 100644 --- a/drivers/staging/rtl8188eu/include/HalVerDef.h +++ b/drivers/staging/rtl8188eu/include/HalVerDef.h @@ -41,28 +41,15 @@ enum HAL_VENDOR { CHIP_VENDOR_UMC = 1, }; -enum HAL_RF_TYPE { - RF_TYPE_1T1R= 0, - RF_TYPE_1T2R= 1, - RF_TYPE_2T2R= 2, - RF_TYPE_2T3R= 3, - RF_TYPE_2T4R= 4, - RF_TYPE_3T3R= 5, - RF_TYPE_3T4R= 6, - RF_TYPE_4T4R= 7, -}; - struct HAL_VERSION { enum HAL_CHIP_TYPE ChipType; enum HAL_CUT_VERSIONCUTVersion; enum HAL_VENDOR VendorType; - enum HAL_RF_TYPERFType; u8 ROMVer; }; /* Get element */ #define GET_CVID_CHIP_TYPE(version)(((version).ChipType)) -#define GET_CVID_RF_TYPE(version) (((version).RFType)) #define GET_CVID_MANUFACTUER(version) (((version).VendorType)) #define GET_CVID_CUT_VERSION(version) (((version).CUTVersion)) #define GET_CVID_ROM_VERSION(version) (((version).ROMVer) & ROM_VERSION_MASK) @@ -95,12 +82,4 @@ struct HAL_VERSION { #define IS_CHIP_VENDOR_UMC(version)\ ((GET_CVID_MANUFACTUER(version) == CHIP_VENDOR_UMC) ? true : false) -/* HAL_RF_TYPE_E */ -#define IS_1T1R(version) \ - ((GET_CVID_RF_TYPE(version) == RF_TYPE_1T1R) ? true : false) -#define IS_1T2R(version) \ - ((GET_CVID_RF_TYPE(version) == RF_TYPE_1T2R) ? true : false) -#define IS_2T2R(version) \ - ((GET_CVID_RF_TYPE(version) == RF_TYPE_2T2R) ? true : false) - #endif -- 2.1.0 -- 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/
[PATCH 5/5] staging: rtl8188eu: fold rtl8188e_read_chip_version() into rtl8188e_SetHalODMVar()
Both rtl8188e_read_chip_version() and ReadChipVersion8188E() are used only in one place. Make ReadChipVersion8188E() a void function and eliminate its wrapper - rtl8188e_read_chip_version(). Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c index 49b6b9c..b05da9d 100644 --- a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c @@ -128,7 +128,7 @@ static void rtl8188e_free_hal_data(struct adapter *padapter) padapter->HalData = NULL; } -static struct HAL_VERSION ReadChipVersion8188E(struct adapter *padapter) +static void ReadChipVersion8188E(struct adapter *padapter) { u32 value32; struct HAL_VERSION ChipVersion; @@ -148,13 +148,6 @@ static struct HAL_VERSION ReadChipVersion8188E(struct adapter *padapter) pHalData->NumTotalRFPath = 1; MSG_88E("RF_Type is %x!!\n", pHalData->rf_type); - - return ChipVersion; -} - -static void rtl8188e_read_chip_version(struct adapter *padapter) -{ - ReadChipVersion8188E(padapter); } static void rtl8188e_SetHalODMVar(struct adapter *Adapter, enum hal_odm_variable eVariable, void *pValue1, bool bSet) @@ -203,7 +196,7 @@ void rtl8188e_set_hal_ops(struct hal_ops *pHalFunc) pHalFunc->dm_init = &rtl8188e_init_dm_priv; - pHalFunc->read_chip_version = &rtl8188e_read_chip_version; + pHalFunc->read_chip_version = &ReadChipVersion8188E; pHalFunc->set_bwmode_handler = &phy_set_bw_mode; pHalFunc->set_channel_handler = &phy_sw_chnl; -- 2.1.0 -- 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/
[PATCH 4/5] staging: rtl8188eu: remove ROMVer from struct HAL_VERSION
ROM version on RTL8188EU is always 0. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/hal/hal_com.c | 2 +- drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c | 2 -- drivers/staging/rtl8188eu/include/HalVerDef.h | 2 -- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/staging/rtl8188eu/hal/hal_com.c b/drivers/staging/rtl8188eu/hal/hal_com.c index a296648..38e9fdc 100644 --- a/drivers/staging/rtl8188eu/hal/hal_com.c +++ b/drivers/staging/rtl8188eu/hal/hal_com.c @@ -50,7 +50,7 @@ void dump_chip_info(struct HAL_VERSIONchip_vers) cnt += sprintf((buf+cnt), "UNKNOWN_CUT(%d)_", chip_vers.CUTVersion); cnt += sprintf((buf+cnt), "1T1R_"); - cnt += sprintf((buf+cnt), "RomVer(%d)\n", chip_vers.ROMVer); + cnt += sprintf((buf+cnt), "RomVer(0)\n"); pr_info("%s", buf); } diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c index ea97030..49b6b9c 100644 --- a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c @@ -141,8 +141,6 @@ static struct HAL_VERSION ReadChipVersion8188E(struct adapter *padapter) ChipVersion.VendorType = ((value32 & VENDOR_ID) ? CHIP_VENDOR_UMC : CHIP_VENDOR_TSMC); ChipVersion.CUTVersion = (value32 & CHIP_VER_RTL_MASK)>>CHIP_VER_RTL_SHIFT; /* IC version (CUT) */ - ChipVersion.ROMVer = 0; /* ROM code version. */ - dump_chip_info(ChipVersion); pHalData->VersionID = ChipVersion; diff --git a/drivers/staging/rtl8188eu/include/HalVerDef.h b/drivers/staging/rtl8188eu/include/HalVerDef.h index 2c24814..56b4ff0 100644 --- a/drivers/staging/rtl8188eu/include/HalVerDef.h +++ b/drivers/staging/rtl8188eu/include/HalVerDef.h @@ -45,14 +45,12 @@ struct HAL_VERSION { enum HAL_CHIP_TYPE ChipType; enum HAL_CUT_VERSIONCUTVersion; enum HAL_VENDOR VendorType; - u8 ROMVer; }; /* Get element */ #define GET_CVID_CHIP_TYPE(version)(((version).ChipType)) #define GET_CVID_MANUFACTUER(version) (((version).VendorType)) #define GET_CVID_CUT_VERSION(version) (((version).CUTVersion)) -#define GET_CVID_ROM_VERSION(version) (((version).ROMVer) & ROM_VERSION_MASK) /* Common Macro. -- */ /* HAL_VERSION VersionID */ -- 2.1.0 -- 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/
[PATCH 1/5] staging: rtl8188eu: remove RegulatorMode from struct hal_data_8188e
This field is not used anywhere. Also, kill the rt_regulator_mode enum which exists just for this field. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c | 3 --- drivers/staging/rtl8188eu/include/rtl8188e_hal.h | 7 --- 2 files changed, 10 deletions(-) diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c index 7904d22..f6c1591 100644 --- a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c @@ -144,9 +144,6 @@ static struct HAL_VERSION ReadChipVersion8188E(struct adapter *padapter) ChipVersion.VendorType = ((value32 & VENDOR_ID) ? CHIP_VENDOR_UMC : CHIP_VENDOR_TSMC); ChipVersion.CUTVersion = (value32 & CHIP_VER_RTL_MASK)>>CHIP_VER_RTL_SHIFT; /* IC version (CUT) */ - /* For regulator mode. by tynli. 2011.01.14 */ - pHalData->RegulatorMode = ((value32 & TRP_BT_EN) ? RT_LDO_REGULATOR : RT_SWITCHING_REGULATOR); - ChipVersion.ROMVer = 0; /* ROM code version. */ dump_chip_info(ChipVersion); diff --git a/drivers/staging/rtl8188eu/include/rtl8188e_hal.h b/drivers/staging/rtl8188eu/include/rtl8188e_hal.h index 7d8e022..5aa2adc 100644 --- a/drivers/staging/rtl8188eu/include/rtl8188e_hal.h +++ b/drivers/staging/rtl8188eu/include/rtl8188e_hal.h @@ -188,15 +188,8 @@ struct txpowerinfo24g { #define EFUSE_PROTECT_BYTES_BANK 16 -/* For RTL8723 regulator mode. */ -enum rt_regulator_mode { - RT_SWITCHING_REGULATOR = 0, - RT_LDO_REGULATOR = 1, -}; - struct hal_data_8188e { struct HAL_VERSION VersionID; - enum rt_regulator_mode RegulatorMode; /* switching regulator or LDO */ u16 CustomerID; u8 *pfirmware; u32 fwsize; -- 2.1.0 -- 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/
[PATCH 2/5] staging: rtl8188eu: remove ICtype from struct HAL_VERSION
IC type on RTL8188EU is always 8188E. Remove it and all the macros that check it. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/hal/hal_com.c | 13 + drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c | 1 - drivers/staging/rtl8188eu/hal/usb_halinit.c | 2 +- drivers/staging/rtl8188eu/include/HalVerDef.h | 61 --- 4 files changed, 2 insertions(+), 75 deletions(-) diff --git a/drivers/staging/rtl8188eu/hal/hal_com.c b/drivers/staging/rtl8188eu/hal/hal_com.c index 170e3de..62ffa7c 100644 --- a/drivers/staging/rtl8188eu/hal/hal_com.c +++ b/drivers/staging/rtl8188eu/hal/hal_com.c @@ -31,18 +31,7 @@ void dump_chip_info(struct HAL_VERSION chip_vers) uint cnt = 0; char buf[128]; - if (IS_81XXC(chip_vers)) { - cnt += sprintf((buf+cnt), "Chip Version Info: %s_", - IS_92C_SERIAL(chip_vers) ? - "CHIP_8192C" : "CHIP_8188C"); - } else if (IS_92D(chip_vers)) { - cnt += sprintf((buf+cnt), "Chip Version Info: CHIP_8192D_"); - } else if (IS_8723_SERIES(chip_vers)) { - cnt += sprintf((buf+cnt), "Chip Version Info: CHIP_8723A_"); - } else if (IS_8188E(chip_vers)) { - cnt += sprintf((buf+cnt), "Chip Version Info: CHIP_8188E_"); - } - + cnt += sprintf((buf+cnt), "Chip Version Info: CHIP_8188E_"); cnt += sprintf((buf+cnt), "%s_", IS_NORMAL_CHIP(chip_vers) ? "Normal_Chip" : "Test_Chip"); cnt += sprintf((buf+cnt), "%s_", IS_CHIP_VENDOR_TSMC(chip_vers) ? diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c index f6c1591..af757ff 100644 --- a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c @@ -137,7 +137,6 @@ static struct HAL_VERSION ReadChipVersion8188E(struct adapter *padapter) pHalData = GET_HAL_DATA(padapter); value32 = usb_read32(padapter, REG_SYS_CFG); - ChipVersion.ICType = CHIP_8188E; ChipVersion.ChipType = ((value32 & RTL_ID) ? TEST_CHIP : NORMAL_CHIP); ChipVersion.RFType = RF_TYPE_1T1R; diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c b/drivers/staging/rtl8188eu/hal/usb_halinit.c index 8726222..ffcca6b 100644 --- a/drivers/staging/rtl8188eu/hal/usb_halinit.c +++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c @@ -1703,7 +1703,7 @@ static void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val) /* Forece leave RF low power mode for 1T1R to prevent conficting setting in Fw power */ /* saving sequence. 2010.06.07. Added by tynli. Suggested by SD3 yschang. */ - if ((psmode != PS_MODE_ACTIVE) && (!IS_92C_SERIAL(haldata->VersionID))) + if (psmode != PS_MODE_ACTIVE) ODM_RF_Saving(podmpriv, true); rtl8188e_set_FwPwrMode_cmd(Adapter, psmode); } diff --git a/drivers/staging/rtl8188eu/include/HalVerDef.h b/drivers/staging/rtl8188eu/include/HalVerDef.h index 97047cf..c542f7e 100644 --- a/drivers/staging/rtl8188eu/include/HalVerDef.h +++ b/drivers/staging/rtl8188eu/include/HalVerDef.h @@ -20,20 +20,6 @@ #ifndef __HAL_VERSION_DEF_H__ #define __HAL_VERSION_DEF_H__ -enum HAL_IC_TYPE { - CHIP_8192S = 0, - CHIP_8188C = 1, - CHIP_8192C = 2, - CHIP_8192D = 3, - CHIP_8723A = 4, - CHIP_8188E = 5, - CHIP_8881A = 6, - CHIP_8812A = 7, - CHIP_8821A = 8, - CHIP_8723B = 9, - CHIP_8192E = 10, -}; - enum HAL_CHIP_TYPE { TEST_CHIP = 0, NORMAL_CHIP = 1, @@ -67,7 +53,6 @@ enum HAL_RF_TYPE { }; struct HAL_VERSION { - enum HAL_IC_TYPEICType; enum HAL_CHIP_TYPE ChipType; enum HAL_CUT_VERSIONCUTVersion; enum HAL_VENDOR VendorType; @@ -76,7 +61,6 @@ struct HAL_VERSION { }; /* Get element */ -#define GET_CVID_IC_TYPE(version) (((version).ICType)) #define GET_CVID_CHIP_TYPE(version)(((version).ChipType)) #define GET_CVID_RF_TYPE(version) (((version).RFType)) #define GET_CVID_MANUFACTUER(version) (((version).VendorType)) @@ -86,17 +70,6 @@ struct HAL_VERSION { /* Common Macro. -- */ /* HAL_VERSION VersionID */ -/* HAL_IC_TYPE_E */ -#define IS_81XXC(version) \ - (((GET_CVID_IC_TYPE(version) == CHIP_8192C) || \ -(GET_CVID_IC_TYPE(version) == CHIP_8188C)) ? true : false) -#define IS_8723_SERIES(version
[PATCH 0/5] staging: rtl8188eu: Clean up rtl8188e_read_chip_version()
This series of clean-up patches is a result of a review of rtl8188e_read_chip_version(). The motivation for getting it in shape is that this function could serve as read_chip_version function callback in struct rtl_hal_ops, if this driver gets converted to use rtl_usb (rtlwifi). Jakub Sitnicki (5): staging: rtl8188eu: remove RegulatorMode from struct hal_data_8188e staging: rtl8188eu: remove ICtype from struct HAL_VERSION staging: rtl8188eu: remove RFtype from struct HAL_VERSION staging: rtl8188eu: remove ROMVer from struct HAL_VERSION staging: rtl8188eu: fold rtl8188e_read_chip_version() into rtl8188e_SetHalODMVar() drivers/staging/rtl8188eu/hal/hal_com.c | 27 +--- drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c | 32 ++--- drivers/staging/rtl8188eu/hal/usb_halinit.c | 2 +- drivers/staging/rtl8188eu/include/HalVerDef.h | 84 --- drivers/staging/rtl8188eu/include/rtl8188e_hal.h | 7 -- 5 files changed, 8 insertions(+), 144 deletions(-) -- 2.1.0 -- 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/
[PATCH] staging: rtl8188eu: don't duplicate ieee80211 WLAN_CAPABILITY_* constants
linux/ieee80211.h already defines constants for capability bits. Include it where needed, resolve discrepancies in naming, and remove the duplicated definitions. Also, make use of WLAN_CAPABILITY_IS_STA_BSS() macro to check if neither ESS nor IBSS capability bits are set. Signed-off-by: Jakub Sitnicki --- This patch depends on commit 04fbf979b39b ("rtl8188eu: don't duplicate ieee80211 constants for status/reason") in staging-next branch. drivers/staging/rtl8188eu/core/rtw_ap.c| 2 +- drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 2 ++ drivers/staging/rtl8188eu/core/rtw_mlme.c | 5 +++-- drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 2 ++ drivers/staging/rtl8188eu/include/ieee80211.h | 10 -- drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 4 ++-- 6 files changed, 10 insertions(+), 15 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c index 1d3f728..f4376d5 100644 --- a/drivers/staging/rtl8188eu/core/rtw_ap.c +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c @@ -1564,7 +1564,7 @@ void bss_cap_update_on_sta_join(struct adapter *padapter, struct sta_info *psta) } } - if (!(psta->capability & WLAN_CAPABILITY_SHORT_SLOT)) { + if (!(psta->capability & WLAN_CAPABILITY_SHORT_SLOT_TIME)) { if (!psta->no_short_slot_time_set) { psta->no_short_slot_time_set = 1; diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c index 11b780d..d43e867 100644 --- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c +++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c @@ -19,6 +19,8 @@ **/ #define _IEEE80211_C +#include + #include #include #include diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c index 6c91aa5..d398151 100644 --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c @@ -19,6 +19,7 @@ **/ #define _RTW_MLME_C_ +#include #include #include @@ -352,8 +353,8 @@ int is_same_network(struct wlan_bssid_ex *src, struct wlan_bssid_ex *dst) ((!memcmp(src->Ssid.Ssid, dst->Ssid.Ssid, src->Ssid.SsidLength)) == true) && ((s_cap & WLAN_CAPABILITY_IBSS) == (d_cap & WLAN_CAPABILITY_IBSS)) && - ((s_cap & WLAN_CAPABILITY_BSS) == - (d_cap & WLAN_CAPABILITY_BSS))); + ((s_cap & WLAN_CAPABILITY_ESS) == + (d_cap & WLAN_CAPABILITY_ESS))); } struct wlan_network*rtw_get_oldest_wlan_network(struct __queue *scanned_queue) diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c index 2b37175..c0e4751 100644 --- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c +++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c @@ -19,6 +19,8 @@ **/ #define _RTW_WLAN_UTIL_C_ +#include + #include #include #include diff --git a/drivers/staging/rtl8188eu/include/ieee80211.h b/drivers/staging/rtl8188eu/include/ieee80211.h index b129ad1..fabb959 100644 --- a/drivers/staging/rtl8188eu/include/ieee80211.h +++ b/drivers/staging/rtl8188eu/include/ieee80211.h @@ -483,16 +483,6 @@ struct ieee80211_snap_hdr { #define WLAN_AUTH_CHALLENGE_LEN 128 -#define WLAN_CAPABILITY_BSS (1<<0) -#define WLAN_CAPABILITY_IBSS (1<<1) -#define WLAN_CAPABILITY_CF_POLLABLE (1<<2) -#define WLAN_CAPABILITY_CF_POLL_REQUEST (1<<3) -#define WLAN_CAPABILITY_PRIVACY (1<<4) -#define WLAN_CAPABILITY_SHORT_PREAMBLE (1<<5) -#define WLAN_CAPABILITY_PBCC (1<<6) -#define WLAN_CAPABILITY_CHANNEL_AGILITY (1<<7) -#define WLAN_CAPABILITY_SHORT_SLOT (1<<10) - /* Non standard? Not in */ #define WLAN_REASON_EXPIRATION_CHK 65535 diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c index 0bde2887..a80cf31 100644 --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c @@ -179,8 +179,8 @@ static char *translate_scan(struct adapter *padapter, cap = le16_to_cpu(le_tmp); - if (cap & (WLAN_CAPABILITY_IBSS | WLAN_CAPABILITY_BSS)) { - if (cap & WLAN_CAPABILITY_BSS) + if (!WLAN_CAPABILITY_IS_STA_BSS(cap)) { + if (cap & WLAN_CAPABILITY_ESS) iwe.u.mode = IW_MODE_MASTER; else iwe.u.mode = IW_MODE_ADHOC; -- 2.1.0 -- 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/
[PATCH] staging: rtl8188eu: don't duplicate ieee80211 WLAN_EID_* constants
linux/ieee80211.h already defines constants for information element IDs. Include it where needed, resolve discrepancies in naming, and remove the duplicated definitions. While at it, wrap a line that was too long and remove extra parentheses in an expression that mixes only equality and logical operators. Signed-off-by: Jakub Sitnicki --- This patch depends on commit 04fbf979b39b ("rtl8188eu: don't duplicate ieee80211 constants for status/reason") in staging-next branch. drivers/staging/rtl8188eu/core/rtw_ieee80211.c | 4 ++- drivers/staging/rtl8188eu/include/ieee80211.h | 38 -- drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 3 +- 3 files changed, 5 insertions(+), 40 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c index 11b780d..c3c5828 100644 --- a/drivers/staging/rtl8188eu/core/rtw_ieee80211.c +++ b/drivers/staging/rtl8188eu/core/rtw_ieee80211.c @@ -19,6 +19,8 @@ **/ #define _IEEE80211_C +#include + #include #include #include @@ -1042,7 +1044,7 @@ enum parse_res rtw_ieee802_11_parse_elems(u8 *start, uint len, elems->timeout_int = pos; elems->timeout_int_len = elen; break; - case WLAN_EID_HT_CAP: + case WLAN_EID_HT_CAPABILITY: elems->ht_capabilities = pos; elems->ht_capabilities_len = elen; break; diff --git a/drivers/staging/rtl8188eu/include/ieee80211.h b/drivers/staging/rtl8188eu/include/ieee80211.h index b129ad1..611877c 100644 --- a/drivers/staging/rtl8188eu/include/ieee80211.h +++ b/drivers/staging/rtl8188eu/include/ieee80211.h @@ -496,44 +496,6 @@ struct ieee80211_snap_hdr { /* Non standard? Not in */ #define WLAN_REASON_EXPIRATION_CHK 65535 -/* Information Element IDs */ -#define WLAN_EID_SSID 0 -#define WLAN_EID_SUPP_RATES 1 -#define WLAN_EID_FH_PARAMS 2 -#define WLAN_EID_DS_PARAMS 3 -#define WLAN_EID_CF_PARAMS 4 -#define WLAN_EID_TIM 5 -#define WLAN_EID_IBSS_PARAMS 6 -#define WLAN_EID_CHALLENGE 16 -/* EIDs defined by IEEE 802.11h - START */ -#define WLAN_EID_PWR_CONSTRAINT 32 -#define WLAN_EID_PWR_CAPABILITY 33 -#define WLAN_EID_TPC_REQUEST 34 -#define WLAN_EID_TPC_REPORT 35 -#define WLAN_EID_SUPPORTED_CHANNELS 36 -#define WLAN_EID_CHANNEL_SWITCH 37 -#define WLAN_EID_MEASURE_REQUEST 38 -#define WLAN_EID_MEASURE_REPORT 39 -#define WLAN_EID_QUITE 40 -#define WLAN_EID_IBSS_DFS 41 -/* EIDs defined by IEEE 802.11h - END */ -#define WLAN_EID_ERP_INFO 42 -#define WLAN_EID_HT_CAP 45 -#define WLAN_EID_RSN 48 -#define WLAN_EID_EXT_SUPP_RATES 50 -#define WLAN_EID_MOBILITY_DOMAIN 54 -#define WLAN_EID_FAST_BSS_TRANSITION 55 -#define WLAN_EID_TIMEOUT_INTERVAL 56 -#define WLAN_EID_RIC_DATA 57 -#define WLAN_EID_HT_OPERATION 61 -#define WLAN_EID_SECONDARY_CHANNEL_OFFSET 62 -#define WLAN_EID_20_40_BSS_COEXISTENCE 72 -#define WLAN_EID_20_40_BSS_INTOLERANT 73 -#define WLAN_EID_OVERLAPPING_BSS_SCAN_PARAMS 74 -#define WLAN_EID_MMIE 76 -#define WLAN_EID_VENDOR_SPECIFIC 221 -#define WLAN_EID_GENERIC (WLAN_EID_VENDOR_SPECIFIC) - #define IEEE80211_MGMT_HDR_LEN 24 #define IEEE80211_DATA_HDR3_LEN 24 #define IEEE80211_DATA_HDR4_LEN 30 diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c index 0bde2887..3aeb00a 100644 --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c @@ -2666,7 +2666,8 @@ static int rtw_get_sta_wpaie(struct net_device *dev, struct ieee_param *param) psta = rtw_get_stainfo(pstapriv, param->sta_addr); if (psta) { - if ((psta->wpa_ie[0] == WLAN_EID_RSN) || (psta->wpa_ie[0] == WLAN_EID_GENERIC)) { + if (psta->wpa_ie[0] == WLAN_EID_RSN || + psta->wpa_ie[0] == WLAN_EID_VENDOR_SPECIFIC) { int wpa_ie_len; int copy_len; -- 2.1.0 -- 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] staging: rtl8188eu: kill unused hal_data_8188e::fw_ractrl flag
On Thu, Jun 18, 2015 at 10:30 AM CEST, Sudip Mukherjee wrote: > On Thu, Jun 18, 2015 at 08:31:59AM +0200, Jakub Sitnicki wrote: >> Flag is never set. Remove it and the code that is dead because of it. >> >> Signed-off-by: Jakub Sitnicki >> --- > >> >> diff --git a/drivers/staging/rtl8188eu/hal/odm.c >> b/drivers/staging/rtl8188eu/hal/odm.c >> index 28b5e7b..710fdc3 100644 >> --- a/drivers/staging/rtl8188eu/hal/odm.c >> +++ b/drivers/staging/rtl8188eu/hal/odm.c >> @@ -1170,13 +1170,10 @@ void odm_RSSIMonitorCheckCE(struct odm_dm_struct >> *pDM_Odm) >> } >> >> for (i = 0; i < sta_cnt; i++) { >> -if (PWDB_rssi[i] != (0)) { >> -if (pHalData->fw_ractrl) { >> -/* Report every sta's RSSI to FW */ >> -} else { >> -ODM_RA_SetRSSI_8188E( >> -&(pHalData->odmpriv), (PWDB_rssi[i]&0xFF), >> (u8)((PWDB_rssi[i]>>16) & 0xFF)); >> -} >> +if (PWDB_rssi[i] != 0) { >> +ODM_RA_SetRSSI_8188E(&pHalData->odmpriv, >> + PWDB_rssi[i] & 0xFF, >> + (PWDB_rssi[i] >> 16) & 0xFF); > and you are also removing an extra unneeded () and a typecast. True. I could have mentioned that in the commit message as it was intentional. Thanks for reviewing it. Regards, Jakub -- 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/
[PATCH] staging: rtl8188eu: kill unused hal_data_8188e::fw_ractrl flag
Flag is never set. Remove it and the code that is dead because of it. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/hal/odm.c | 11 -- drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c | 21 -- drivers/staging/rtl8188eu/hal/usb_halinit.c | 27 ++-- drivers/staging/rtl8188eu/include/rtl8188e_cmd.h | 1 - drivers/staging/rtl8188eu/include/rtl8188e_hal.h | 1 - drivers/staging/rtl8188eu/include/sta_info.h | 1 - 6 files changed, 6 insertions(+), 56 deletions(-) diff --git a/drivers/staging/rtl8188eu/hal/odm.c b/drivers/staging/rtl8188eu/hal/odm.c index 28b5e7b..710fdc3 100644 --- a/drivers/staging/rtl8188eu/hal/odm.c +++ b/drivers/staging/rtl8188eu/hal/odm.c @@ -1170,13 +1170,10 @@ void odm_RSSIMonitorCheckCE(struct odm_dm_struct *pDM_Odm) } for (i = 0; i < sta_cnt; i++) { - if (PWDB_rssi[i] != (0)) { - if (pHalData->fw_ractrl) { - /* Report every sta's RSSI to FW */ - } else { - ODM_RA_SetRSSI_8188E( - &(pHalData->odmpriv), (PWDB_rssi[i]&0xFF), (u8)((PWDB_rssi[i]>>16) & 0xFF)); - } + if (PWDB_rssi[i] != 0) { + ODM_RA_SetRSSI_8188E(&pHalData->odmpriv, +PWDB_rssi[i] & 0xFF, +(PWDB_rssi[i] >> 16) & 0xFF); } } diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c index 86347f2..0a62bfa 100644 --- a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c @@ -127,27 +127,6 @@ exit: return ret; } -u8 rtl8188e_set_raid_cmd(struct adapter *adapt, u32 mask) -{ - u8 buf[3]; - u8 res = _SUCCESS; - struct hal_data_8188e *haldata = GET_HAL_DATA(adapt); - - if (haldata->fw_ractrl) { - - memset(buf, 0, 3); - put_unaligned_le32(mask, buf); - - FillH2CCmd_88E(adapt, H2C_DM_MACID_CFG, 3, buf); - } else { - DBG_88E("==>%s fw dont support RA\n", __func__); - res = _FAIL; - } - - - return res; -} - /* bitmap[0:27] = tx_rate_bitmap */ /* bitmap[28:31]= Rate Adaptive id */ /* arg[0:4] = macid */ diff --git a/drivers/staging/rtl8188eu/hal/usb_halinit.c b/drivers/staging/rtl8188eu/hal/usb_halinit.c index 7b01d5a..caf3731 100644 --- a/drivers/staging/rtl8188eu/hal/usb_halinit.c +++ b/drivers/staging/rtl8188eu/hal/usb_halinit.c @@ -743,19 +743,16 @@ static u32 rtl8188eu_hal_init(struct adapter *Adapter) if (Adapter->registrypriv.mp_mode == 1) { _InitRxSetting(Adapter); Adapter->bFWReady = false; - haldata->fw_ractrl = false; } else { status = rtl88eu_download_fw(Adapter); if (status) { DBG_88E("%s: Download Firmware failed!!\n", __func__); Adapter->bFWReady = false; - haldata->fw_ractrl = false; return status; } else { RT_TRACE(_module_hci_hal_init_c_, _drv_info_, ("Initializeadapt8192CSdio(): Download Firmware Success!!\n")); Adapter->bFWReady = true; - haldata->fw_ractrl = false; } } rtl8188e_InitializeFirmwareVars(Adapter); @@ -2086,28 +2083,9 @@ static void UpdateHalRAMask8188EUsb(struct adapter *adapt, u32 mac_id, u8 rssi_l init_rate = get_highest_rate_idx(mask)&0x3f; - if (haldata->fw_ractrl) { - u8 arg; + ODM_RA_UpdateRateInfo_8188E(&haldata->odmpriv, mac_id, + raid, mask, shortGIrate); - arg = mac_id & 0x1f;/* MACID */ - arg |= BIT(7); - if (shortGIrate) - arg |= BIT(5); - mask |= ((raid << 28) & 0xf000); - DBG_88E("update raid entry, mask=0x%x, arg=0x%x\n", mask, arg); - psta->ra_mask = mask; - mask |= ((raid << 28) & 0xf000); - - /* to do ,for 8188E-SMIC */ - rtl8188e_set_raid_cmd(adapt, mask); - } else { - ODM_RA_UpdateRateInfo_8188E(&(haldata->odmpriv), - mac_id, - raid, - mask, - shortGIrate - ); - } /* set ra_id */ psta->raid = raid; psta->init_rate = init_rate; @@ -2157,7 +2135,6 @@ sta
Re: [PATCH 0/8] staging/rtl8xxx: delete ieee80211 constant duplication
On Mon, Apr 27, 2015 at 07:12 PM CEST, Larry Finger wrote: > These drivers do not use mac80211, which is a prerequisite for inclusion in > the > regular wifi tree. A complete rewrite will be needed to get them out of > staging. > Nonetheless, cleanups are in order. Regarding the rtl8188eu driver, would it be acceptable to rewrite it to use the rtlwifi interface instead of using mac80211 directly? I can't help but notice some similarities between the structure of rtl8188eu and the drivers from the rtlwifi family. Then again, perhaps I'm missing something that makes it a bad idea. Thanks, Jakub -- 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/
[PATCH] staging: rtl8188eu: kill unused INCLUDE_MULTI_FUNC_* macros
Also, remove rt_multi_func enum used exclusively by the killed macros. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/include/rtl8188e_hal.h | 13 - 1 file changed, 13 deletions(-) diff --git a/drivers/staging/rtl8188eu/include/rtl8188e_hal.h b/drivers/staging/rtl8188eu/include/rtl8188e_hal.h index b8c42ee..de1e1c8 100644 --- a/drivers/staging/rtl8188eu/include/rtl8188e_hal.h +++ b/drivers/staging/rtl8188eu/include/rtl8188e_hal.h @@ -188,14 +188,6 @@ struct txpowerinfo24g { #define EFUSE_PROTECT_BYTES_BANK 16 -/* For RTL8723 WiFi/BT/GPS multi-function configuration. */ -enum rt_multi_func { - RT_MULTI_FUNC_NONE = 0x00, - RT_MULTI_FUNC_WIFI = 0x01, - RT_MULTI_FUNC_BT = 0x02, - RT_MULTI_FUNC_GPS = 0x04, -}; - /* For RTL8723 regulator mode. */ enum rt_regulator_mode { RT_SWITCHING_REGULATOR = 0, @@ -378,11 +370,6 @@ struct hal_data_8188e { ((struct hal_data_8188e *)((__pAdapter)->HalData)) #define GET_RF_TYPE(priv) (GET_HAL_DATA(priv)->rf_type) -#define INCLUDE_MULTI_FUNC_BT(_Adapter)\ - (GET_HAL_DATA(_Adapter)->MultiFunc & RT_MULTI_FUNC_BT) -#define INCLUDE_MULTI_FUNC_GPS(_Adapter) \ - (GET_HAL_DATA(_Adapter)->MultiFunc & RT_MULTI_FUNC_GPS) - /* rtl8188e_hal_init.c */ void _8051Reset88E(struct adapter *padapter); void rtl8188e_InitializeFirmwareVars(struct adapter *padapter); -- 2.1.0 -- 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/
[PATCH] staging: rtl8188eu: Kill dead calls to kill_pid()
There is no interface to register PIDs of processes the driver should send a signal to. Remove it. Signed-off-by: Jakub Sitnicki --- drivers/staging/rtl8188eu/include/drv_types.h | 1 - drivers/staging/rtl8188eu/include/osdep_service.h | 2 -- drivers/staging/rtl8188eu/include/rtw_ioctl.h | 2 -- drivers/staging/rtl8188eu/os_dep/mlme_linux.c | 2 -- drivers/staging/rtl8188eu/os_dep/usb_intf.c | 12 5 files changed, 19 deletions(-) diff --git a/drivers/staging/rtl8188eu/include/drv_types.h b/drivers/staging/rtl8188eu/include/drv_types.h index c813179..bcc74dc 100644 --- a/drivers/staging/rtl8188eu/include/drv_types.h +++ b/drivers/staging/rtl8188eu/include/drv_types.h @@ -175,7 +175,6 @@ static inline struct device *dvobj_to_dev(struct dvobj_priv *dvobj) }; struct adapter { - int pid[3];/* process id from UI, 0:wps, 1:hostapd, 2:dhcpcd */ u16 chip_type; struct dvobj_priv *dvobj; diff --git a/drivers/staging/rtl8188eu/include/osdep_service.h b/drivers/staging/rtl8188eu/include/osdep_service.h index 515e949..00472e0 100644 --- a/drivers/staging/rtl8188eu/include/osdep_service.h +++ b/drivers/staging/rtl8188eu/include/osdep_service.h @@ -157,8 +157,6 @@ void rtw_free_netdev(struct net_device *netdev); #define FUNC_ADPT_FMT "%s(%s)" #define FUNC_ADPT_ARG(adapter) __func__, adapter->pnetdev->name -#define rtw_signal_process(pid, sig) kill_pid(find_vpid((pid)), (sig), 1) - u64 rtw_modular64(u64 x, u64 y); /* Macros for handling unaligned memory accesses */ diff --git a/drivers/staging/rtl8188eu/include/rtw_ioctl.h b/drivers/staging/rtl8188eu/include/rtw_ioctl.h index f3aa924..ee2cb54 100644 --- a/drivers/staging/rtl8188eu/include/rtw_ioctl.h +++ b/drivers/staging/rtl8188eu/include/rtw_ioctl.h @@ -117,6 +117,4 @@ int drv_set_info(struct net_device *MiniportAdapterContext, u32 informationbufferlength, u32 *bytesread, u32 *bytesneeded); -extern int ui_pid[3]; - #endif /* #ifndef __INC_CEINFO_ */ diff --git a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c index baff1e2..9099241 100644 --- a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c +++ b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c @@ -41,8 +41,6 @@ void rtw_os_indicate_connect(struct adapter *adapter) { rtw_indicate_wx_assoc_event(adapter); netif_carrier_on(adapter->pnetdev); - if (adapter->pid[2] != 0) - rtw_signal_process(adapter->pid[2], SIGALRM); } void rtw_os_indicate_scan_done(struct adapter *padapter, bool aborted) diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c b/drivers/staging/rtl8188eu/os_dep/usb_intf.c index ef3c73e..d0d4335 100644 --- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c +++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c @@ -32,8 +32,6 @@ #include #include -int ui_pid[3] = {0, 0, 0}; - #define USB_VENDER_ID_REALTEK 0x0bda /* DID_USB_v916_20130116 */ @@ -330,11 +328,6 @@ static int rtw_resume_process(struct adapter *padapter) _exit_pwrlock(&pwrpriv->lock); - if (padapter->pid[1] != 0) { - DBG_88E("pid[1]:%d\n", padapter->pid[1]); - rtw_signal_process(padapter->pid[1], SIGUSR2); - } - rtw_roaming(padapter, NULL); ret = 0; @@ -511,11 +504,6 @@ static int rtw_drv_init(struct usb_interface *pusb_intf, const struct usb_device goto free_dvobj; } - if (ui_pid[1] != 0) { - DBG_88E("ui_pid[1]:%d\n", ui_pid[1]); - rtw_signal_process(ui_pid[1], SIGUSR2); - } - RT_TRACE(_module_hci_intfs_c_, _drv_err_, ("-871x_drv - drv_init, success!\n")); status = _SUCCESS; -- 2.1.0 -- 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/
[PATCH] staging: rtl8188eu: Remove redundant CONFIG_88EU_AP_MODE tests
Remove #ifdef's enclosed by an #ifdef test for the same macro to improve readability. No code changes: md5, CONFIG_88EU_AP_MODE=y: b819a33f65133607ebc33b8999ee3a79 r8188eu.o.before b819a33f65133607ebc33b8999ee3a79 r8188eu.o.after md5, CONFIG_88EU_AP_MODE=n: 94c84035d59285408b866a57b442276d r8188eu.o.before 94c84035d59285408b866a57b442276d r8188eu.o.after Signed-off-by: Jakub Sitnicki --- Patch generated with more context to hopefully make the review easier. drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 14 +- drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 2 -- drivers/staging/rtl8188eu/include/rtw_ap.h| 2 -- 3 files changed, 1 insertion(+), 17 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c index be9e34a..79fbf7d 100644 --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c @@ -787,40 +787,36 @@ unsigned int OnAuth(struct adapter *padapter, struct recv_frame *precv_frame) status = _STATS_OUT_OF_AUTH_SEQ_; goto auth_fail; } } /* Now, we are going to issue_auth... */ pstat->auth_seq = seq + 1; -#ifdef CONFIG_88EU_AP_MODE issue_auth(padapter, pstat, (unsigned short)(_STATS_SUCCESSFUL_)); -#endif if (pstat->state & WIFI_FW_AUTH_SUCCESS) pstat->auth_seq = 0; return _SUCCESS; auth_fail: if (pstat) rtw_free_stainfo(padapter , pstat); pstat = &stat; memset((char *)pstat, '\0', sizeof(stat)); pstat->auth_seq = 2; memcpy(pstat->hwaddr, sa, 6); -#ifdef CONFIG_88EU_AP_MODE issue_auth(padapter, pstat, (unsigned short)status); -#endif -#endif +#endif /* CONFIG_88EU_AP_MODE */ return _FAIL; } unsigned int OnAuthClient(struct adapter *padapter, struct recv_frame *precv_frame) { unsigned intseq, len, status, offset; unsigned char *p; unsigned intgo2asoc = 0; @@ -1288,57 +1284,49 @@ unsigned int OnAssocReq(struct adapter *padapter, struct recv_frame *precv_frame pstat->expire_to = pstapriv->expire_to; list_add_tail(&pstat->asoc_list, &pstapriv->asoc_list); pstapriv->asoc_list_cnt++; } spin_unlock_bh(&pstapriv->asoc_list_lock); /* now the station is qualified to join our BSS... */ if (pstat && (pstat->state & WIFI_FW_ASSOC_SUCCESS) && (_STATS_SUCCESSFUL_ == status)) { -#ifdef CONFIG_88EU_AP_MODE /* 1 bss_cap_update & sta_info_update */ bss_cap_update_on_sta_join(padapter, pstat); sta_info_update(padapter, pstat); /* issue assoc rsp before notify station join event. */ if (frame_type == WIFI_ASSOCREQ) issue_asocrsp(padapter, status, pstat, WIFI_ASSOCRSP); else issue_asocrsp(padapter, status, pstat, WIFI_REASSOCRSP); /* 2 - report to upper layer */ DBG_88E("indicate_sta_join_event to upper layer - hostapd\n"); rtw_indicate_sta_assoc_event(padapter, pstat); /* 3-(1) report sta add event */ report_add_sta_event(padapter, pstat->hwaddr, pstat->aid); -#endif } return _SUCCESS; asoc_class2_error: -#ifdef CONFIG_88EU_AP_MODE issue_deauth(padapter, (void *)GetAddr2Ptr(pframe), status); -#endif return _FAIL; OnAssocReqFail: - -#ifdef CONFIG_88EU_AP_MODE pstat->aid = 0; if (frame_type == WIFI_ASSOCREQ) issue_asocrsp(padapter, status, pstat, WIFI_ASSOCRSP); else issue_asocrsp(padapter, status, pstat, WIFI_REASSOCRSP); -#endif - #endif /* CONFIG_88EU_AP_MODE */ return _FAIL; } unsigned int OnAssocRsp(struct adapter *padapter, struct recv_frame *precv_frame) { diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c index dc9d0dd..0b1cb03 100644 --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c @@ -49,24 +49,22 @@ static void _rtw_init_stainfo(struct sta_info *psta) psta->expire_to = 0; psta->flags = 0; psta->capability = 0; psta->bpairwise_key_installed = false; -#ifdef CONFIG_88EU_AP_MODE psta->nonerp_set = 0; psta->no_short_slot_time_set = 0; psta->no_short_preamble_set = 0; psta->no_ht_gf_set = 0; psta->no_ht_set = 0; psta->ht_20mhz_set = 0; -#endif psta->under_exist_checking = 0; psta->keep_alive_trycnt = 0; #end
[PATCH] resource: remove deprecated __check_region() and friends
All users of __check_region(), check_region(), and check_mem_region() are gone. We got rid of the last user in v4.0-rc1. Remove them. bloat-o-meter on x86_64 shows: add/remove: 0/3 grow/shrink: 0/0 up/down: 0/-102 (-102) function old new delta __kstrtab___check_region 15 - -15 __ksymtab___check_region 16 - -16 __check_region71 - -71 Signed-off-by: Jakub Sitnicki --- include/linux/ioport.h | 8 kernel/resource.c | 32 2 files changed, 40 deletions(-) diff --git a/include/linux/ioport.h b/include/linux/ioport.h index 2c525022..388e3ae 100644 --- a/include/linux/ioport.h +++ b/include/linux/ioport.h @@ -196,10 +196,8 @@ extern struct resource * __request_region(struct resource *, /* Compatibility cruft */ #define release_region(start,n)__release_region(&ioport_resource, (start), (n)) -#define check_mem_region(start,n) __check_region(&iomem_resource, (start), (n)) #define release_mem_region(start,n)__release_region(&iomem_resource, (start), (n)) -extern int __check_region(struct resource *, resource_size_t, resource_size_t); extern void __release_region(struct resource *, resource_size_t, resource_size_t); #ifdef CONFIG_MEMORY_HOTREMOVE @@ -207,12 +205,6 @@ extern int release_mem_region_adjustable(struct resource *, resource_size_t, resource_size_t); #endif -static inline int __deprecated check_region(resource_size_t s, - resource_size_t n) -{ - return __check_region(&ioport_resource, s, n); -} - /* Wrappers for managed devices */ struct device; diff --git a/kernel/resource.c b/kernel/resource.c index 19f2357..90552aa 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -1034,8 +1034,6 @@ resource_size_t resource_alignment(struct resource *res) * * request_region creates a new busy region. * - * check_region returns non-zero if the area is already busy. - * * release_region releases a matching busy region. */ @@ -1098,36 +1096,6 @@ struct resource * __request_region(struct resource *parent, EXPORT_SYMBOL(__request_region); /** - * __check_region - check if a resource region is busy or free - * @parent: parent resource descriptor - * @start: resource start address - * @n: resource region size - * - * Returns 0 if the region is free at the moment it is checked, - * returns %-EBUSY if the region is busy. - * - * NOTE: - * This function is deprecated because its use is racy. - * Even if it returns 0, a subsequent call to request_region() - * may fail because another driver etc. just allocated the region. - * Do NOT use it. It will be removed from the kernel. - */ -int __check_region(struct resource *parent, resource_size_t start, - resource_size_t n) -{ - struct resource * res; - - res = __request_region(parent, start, n, "check-region", 0); - if (!res) - return -EBUSY; - - release_resource(res); - free_resource(res); - return 0; -} -EXPORT_SYMBOL(__check_region); - -/** * __release_region - release a previously reserved resource region * @parent: parent resource descriptor * @start: resource start address -- 2.1.0 -- 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/
[PATCH] HID: microsoft: Add ID for NE7K wireless keyboard
Microsoft Natural Wireless Ergonomic Keyboard 7000 has special My Favorites 1..5 keys which are handled through a vendor-defined usage page (0xff05). Apply MS_ERGONOMY quirks handling to USB PID 0x071d (Microsoft Microsoft 2.4GHz Transceiver V1.0) so that the My Favorites 1..5 keys are reported as KEY_F14..18 events. Link: https://bugzilla.kernel.org/show_bug.cgi?id=52841 Signed-off-by: Jakub Sitnicki --- drivers/hid/hid-core.c | 1 + drivers/hid/hid-ids.h | 1 + drivers/hid/hid-microsoft.c | 2 ++ 3 files changed, 4 insertions(+) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 8b63879..13320c1 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1855,6 +1855,7 @@ static const struct hid_device_id hid_have_special_driver[] = { { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_SIDEWINDER_GV) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_NE4K) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_NE4K_JP) }, + { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_NE7K) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_LK6K) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_PRESENTER_8K_USB) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_3K) }, diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 9243359..4e77ce1 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -648,6 +648,7 @@ #define USB_DEVICE_ID_MS_LK6K 0x00f9 #define USB_DEVICE_ID_MS_PRESENTER_8K_BT 0x0701 #define USB_DEVICE_ID_MS_PRESENTER_8K_USB 0x0713 +#define USB_DEVICE_ID_MS_NE7K 0x071d #define USB_DEVICE_ID_MS_DIGITAL_MEDIA_3K 0x0730 #define USB_DEVICE_ID_MS_COMFORT_MOUSE_45000x076c #define USB_DEVICE_ID_MS_SURFACE_PRO_2 0x0799 diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c index cacda43..b962df4 100644 --- a/drivers/hid/hid-microsoft.c +++ b/drivers/hid/hid-microsoft.c @@ -264,6 +264,8 @@ static const struct hid_device_id ms_devices[] = { .driver_data = MS_ERGONOMY }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_NE4K_JP), .driver_data = MS_ERGONOMY }, + { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_NE7K), + .driver_data = MS_ERGONOMY }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_LK6K), .driver_data = MS_ERGONOMY | MS_RDESC }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_PRESENTER_8K_USB), -- 2.1.0 -- 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] PNP: Switch from __check_region() to __request_region()
On Fri, Jan 30, 2015 at 01:01 AM CET, Rafael J. Wysocki wrote: > On Monday, December 22, 2014 11:34:48 PM Rafael J. Wysocki wrote: >> On Monday, December 22, 2014 12:19:44 PM Jakub Sitnicki wrote: >> > On Fri, Dec 12, 2014 at 08:47 AM CET, Jakub Sitnicki wrote: >> > > Rafael J. Wysocki writes: >> > > >> > >> On Monday, December 08, 2014 10:01:57 PM Jakub Sitnicki wrote: >> > >>> diff --git a/drivers/pnp/resource.c b/drivers/pnp/resource.c >> > >>> index 782e822..f980ff7 100644 >> > >>> --- a/drivers/pnp/resource.c >> > >>> +++ b/drivers/pnp/resource.c >> > >>> @@ -179,8 +179,9 @@ int pnp_check_port(struct pnp_dev *dev, struct >> > >>> resource *res) >> > >>>/* check if the resource is already in use, skip if the >> > >>> * device is active because it itself may be in use */ >> > >>>if (!dev->active) { >> > >>> - if (__check_region(&ioport_resource, *port, >> > >>> length(port, end))) >> > >>> + if (!request_region(*port, length(port, end), "pnp")) >> > >>>return 0; >> > >>> + release_region(*port, length(port, end)); >> > >> >> > >> Shouldn't we also release the resource returned by request_region() if >> > >> it is >> > >> not NULL? >> > >> >> > > >> > > Thanks for taking a look at this. I think we're good here. If you please >> > > bear with me for a moment: >> > > >> > > release_resource() removes an element from the list of resource parent's >> > > children (and makes it an orphan): >> > > >> > > p = &old->parent->child; >> > > for (;;) { >> > > tmp = *p; >> > > if (!tmp) >> > > break; >> > > if (tmp == old) { >> > > *p = tmp->sibling; >> > > old->parent = NULL; >> > > return 0; >> > > } >> > > p = &tmp->sibling; >> > > } >> > > >> > > release_region() does the same but with additional checks, and also >> > > frees the resource: >> > > >> > > p = &parent->child; >> > > /* ... */ >> > > for (;;) { >> > > struct resource *res = *p; >> > > >> > > if (!res) >> > > break; >> > > if (res->start <= start && res->end >= end) { >> > > /* ... */ >> > > *p = res->sibling; >> > > /* ... */ >> > > free_resource(res); >> > > return; >> > > } >> > > p = &res->sibling; >> > > } >> > > >> > > When making the change I've based on other code in the kernel which also >> > > make use of request_region(). >> > > >> > > To quote one example, drivers/net/ethernet/8390/ne2k-pci.c cleans up its >> > > I/O port region when initialization fails like so: >> > > >> > > static int ne2k_pci_init_one(struct pci_dev *pdev, >> > > const struct pci_device_id *ent) >> > > { >> > > /* ... */ >> > > >> > > if (request_region (ioaddr, NE_IO_EXTENT, DRV_NAME) == NULL) { >> > > dev_err(&pdev->dev, "I/O resource 0x%x @ 0x%lx busy\n", >> > > NE_IO_EXTENT, ioaddr); >> > > return -EBUSY; >> > > } >> > > >> > > /* ... */ >> > > >> > > dev = alloc_ei_netdev(); >> > > if (!dev) { >> > > dev_err(&pdev->dev, "cannot allocate ethernet device\n"); >> > > goto err_out_free_res; >> > > } >> > > >> > > /* ... */ >> > > >> > > err_out_free_res: >> > > release_region (ioaddr, NE_IO_EXTENT); >> > > return -ENODEV; >> > > } >> > >> > Just wondering, do you have any further thoughts on this? >> >> I'll queue it up for 3.20 later in January. > > Done now. If you don't mind me asking, is this going to go through the linux-pm tree? Thanks, Jakub -- 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] PNP: Switch from __check_region() to __request_region()
On Fri, Dec 12, 2014 at 08:47 AM CET, Jakub Sitnicki wrote: > Rafael J. Wysocki writes: > >> On Monday, December 08, 2014 10:01:57 PM Jakub Sitnicki wrote: >>> diff --git a/drivers/pnp/resource.c b/drivers/pnp/resource.c >>> index 782e822..f980ff7 100644 >>> --- a/drivers/pnp/resource.c >>> +++ b/drivers/pnp/resource.c >>> @@ -179,8 +179,9 @@ int pnp_check_port(struct pnp_dev *dev, struct resource >>> *res) >>> /* check if the resource is already in use, skip if the >>> * device is active because it itself may be in use */ >>> if (!dev->active) { >>> - if (__check_region(&ioport_resource, *port, length(port, end))) >>> + if (!request_region(*port, length(port, end), "pnp")) >>> return 0; >>> + release_region(*port, length(port, end)); >> >> Shouldn't we also release the resource returned by request_region() if it is >> not NULL? >> > > Thanks for taking a look at this. I think we're good here. If you please > bear with me for a moment: > > release_resource() removes an element from the list of resource parent's > children (and makes it an orphan): > > p = &old->parent->child; > for (;;) { > tmp = *p; > if (!tmp) > break; > if (tmp == old) { > *p = tmp->sibling; > old->parent = NULL; > return 0; > } > p = &tmp->sibling; > } > > release_region() does the same but with additional checks, and also > frees the resource: > > p = &parent->child; > /* ... */ > for (;;) { > struct resource *res = *p; > > if (!res) > break; > if (res->start <= start && res->end >= end) { > /* ... */ > *p = res->sibling; > /* ... */ > free_resource(res); > return; > } > p = &res->sibling; > } > > When making the change I've based on other code in the kernel which also > make use of request_region(). > > To quote one example, drivers/net/ethernet/8390/ne2k-pci.c cleans up its > I/O port region when initialization fails like so: > > static int ne2k_pci_init_one(struct pci_dev *pdev, >const struct pci_device_id *ent) > { > /* ... */ > > if (request_region (ioaddr, NE_IO_EXTENT, DRV_NAME) == NULL) { > dev_err(&pdev->dev, "I/O resource 0x%x @ 0x%lx busy\n", > NE_IO_EXTENT, ioaddr); > return -EBUSY; > } > > /* ... */ > > dev = alloc_ei_netdev(); > if (!dev) { > dev_err(&pdev->dev, "cannot allocate ethernet device\n"); > goto err_out_free_res; > } > > /* ... */ > > err_out_free_res: > release_region (ioaddr, NE_IO_EXTENT); > return -ENODEV; > } Just wondering, do you have any further thoughts on this? Thanks, Jakub -- 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] PNP: Switch from __check_region() to __request_region()
Rafael J. Wysocki writes: > On Monday, December 08, 2014 10:01:57 PM Jakub Sitnicki wrote: >> diff --git a/drivers/pnp/resource.c b/drivers/pnp/resource.c >> index 782e822..f980ff7 100644 >> --- a/drivers/pnp/resource.c >> +++ b/drivers/pnp/resource.c >> @@ -179,8 +179,9 @@ int pnp_check_port(struct pnp_dev *dev, struct resource >> *res) >> /* check if the resource is already in use, skip if the >> * device is active because it itself may be in use */ >> if (!dev->active) { >> -if (__check_region(&ioport_resource, *port, length(port, end))) >> +if (!request_region(*port, length(port, end), "pnp")) >> return 0; >> +release_region(*port, length(port, end)); > > Shouldn't we also release the resource returned by request_region() if it is > not NULL? > Thanks for taking a look at this. I think we're good here. If you please bear with me for a moment: release_resource() removes an element from the list of resource parent's children (and makes it an orphan): p = &old->parent->child; for (;;) { tmp = *p; if (!tmp) break; if (tmp == old) { *p = tmp->sibling; old->parent = NULL; return 0; } p = &tmp->sibling; } release_region() does the same but with additional checks, and also frees the resource: p = &parent->child; /* ... */ for (;;) { struct resource *res = *p; if (!res) break; if (res->start <= start && res->end >= end) { /* ... */ *p = res->sibling; /* ... */ free_resource(res); return; } p = &res->sibling; } When making the change I've based on other code in the kernel which also make use of request_region(). To quote one example, drivers/net/ethernet/8390/ne2k-pci.c cleans up its I/O port region when initialization fails like so: static int ne2k_pci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) { /* ... */ if (request_region (ioaddr, NE_IO_EXTENT, DRV_NAME) == NULL) { dev_err(&pdev->dev, "I/O resource 0x%x @ 0x%lx busy\n", NE_IO_EXTENT, ioaddr); return -EBUSY; } /* ... */ dev = alloc_ei_netdev(); if (!dev) { dev_err(&pdev->dev, "cannot allocate ethernet device\n"); goto err_out_free_res; } /* ... */ err_out_free_res: release_region (ioaddr, NE_IO_EXTENT); return -ENODEV; } >> } >> >> /* check if the resource is reserved */ >> @@ -241,8 +242,9 @@ int pnp_check_mem(struct pnp_dev *dev, struct resource >> *res) >> /* check if the resource is already in use, skip if the >> * device is active because it itself may be in use */ >> if (!dev->active) { >> -if (check_mem_region(*addr, length(addr, end))) >> +if (!request_mem_region(*addr, length(addr, end), "pnp")) >> return 0; >> +release_mem_region(*addr, length(addr, end)); >> } >> >> /* check if the resource is reserved */ >> -- 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/
[PATCH] PNP: Switch from __check_region() to __request_region()
PNP core is the last user of the __check_region() which has been deprecated for almost 12 years (since v2.5.54). Replace it with a combo of __request_region() followed by __release_region(). pnp_check_port() and pnp_check_mem() remain racy after this change. Signed-off-by: Jakub Sitnicki --- There was a previous attempt at making this change 3 years ago but the author has never addressed the review comments: https://lkml.org/lkml/2011/8/12/216 The end goal here is to get rid of __check_region() which lands in every kernel image because of the PNP core. It has been previously pointed out that replacing __check_region() with request_region() obscures the fact that pnp_check_port() is racy: https://lkml.org/lkml/2011/8/11/466 Because of that I've also considered just moving __check_region() to PNP core. However, that would require making free_resource() an exported symbol in kernel/resource.c. On the other hand, a switch to request/release_region() makes pnp_check_port() and pnp_check_mem() follow the same pattern as found in pnp_check_irq() and pnp_check_dma(): if (!dev->active) { if (request_(...)) return 0; free_(...); } Admittedly, I was not able to exercise the touched code paths on a commodity x86_64 laptop or under QEMU / VirtualBox (which lack ISA PnP support, AFAIK). To my understanding, the correct way to test pnp_check_port() or pnp_check_mem() would be by issuing either: $ echo fill >/sys/bus/pnp/devices/XX:YY/resources or $ echo auto >/sys/bus/pnp/devices/XX:YY/resources ... but only if the device is not attached or active, which is not the case for ACPI PnP devices on my machines. If anyone can provide hints on steps to test this, I will be glad to do so. drivers/pnp/resource.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/pnp/resource.c b/drivers/pnp/resource.c index 782e822..f980ff7 100644 --- a/drivers/pnp/resource.c +++ b/drivers/pnp/resource.c @@ -179,8 +179,9 @@ int pnp_check_port(struct pnp_dev *dev, struct resource *res) /* check if the resource is already in use, skip if the * device is active because it itself may be in use */ if (!dev->active) { - if (__check_region(&ioport_resource, *port, length(port, end))) + if (!request_region(*port, length(port, end), "pnp")) return 0; + release_region(*port, length(port, end)); } /* check if the resource is reserved */ @@ -241,8 +242,9 @@ int pnp_check_mem(struct pnp_dev *dev, struct resource *res) /* check if the resource is already in use, skip if the * device is active because it itself may be in use */ if (!dev->active) { - if (check_mem_region(*addr, length(addr, end))) + if (!request_mem_region(*addr, length(addr, end), "pnp")) return 0; + release_mem_region(*addr, length(addr, end)); } /* check if the resource is reserved */ -- 1.9.3 -- 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 2/2] ext4: acl: remove unneeded include of linux/capability.h
Please ignore the patch sequence number. It is a single patch, not a part of a series. Newbie mistake. Please let me know if I should resend the patch. Thanks, Jakub -- 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 1/2] ext2: acl: remove unneeded include of linux/capability.h
Please ignore the patch sequence number. It is a single patch, not a part of a series. Newbie mistake. Please let me know if I should resend the patch. Thanks, Jakub -- 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/
[PATCH 1/2] ext2: acl: remove unneeded include of linux/capability.h
acl.c has not been (directly) using the interface defined by linux/capability.h header since commit 3bd858ab1c451725c07a ("Introduce is_owner_or_cap() to wrap CAP_FOWNER use with fsuid check"). Remove it. Signed-off-by: Jakub Sitnicki --- fs/ext2/acl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/ext2/acl.c b/fs/ext2/acl.c index 1b8001b..27695e6 100644 --- a/fs/ext2/acl.c +++ b/fs/ext2/acl.c @@ -4,7 +4,6 @@ * Copyright (C) 2001-2003 Andreas Gruenbacher, */ -#include #include #include #include -- 1.8.5.3 -- 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/
[PATCH 2/2] ext4: acl: remove unneeded include of linux/capability.h
acl.c has not been (directly) using the interface defined by linux/capability.h header since commit 3bd858ab1c451725c07a ("Introduce is_owner_or_cap() to wrap CAP_FOWNER use with fsuid check"). Remove it. Signed-off-by: Jakub Sitnicki --- fs/ext4/acl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c index d40c8db..26e880e 100644 --- a/fs/ext4/acl.c +++ b/fs/ext4/acl.c @@ -7,7 +7,6 @@ #include #include #include -#include #include #include "ext4_jbd2.h" #include "ext4.h" -- 1.8.5.3 -- 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/