Netdev List
 help / color / mirror / Atom feed
* Re: Question about SOCK_SEQPACKET
From: Steven Whitehouse @ 2017-05-05 10:34 UTC (permalink / raw)
  To: Sowmini Varadhan; +Cc: Sam Kumar, linux-kernel, netdev@vger.kernel.org
In-Reply-To: <20170505100926.GD1547@oracle.com>

Hi,


On 05/05/17 11:09, Sowmini Varadhan wrote:
> On (05/05/17 10:45), Steven Whitehouse wrote:
>> I do wonder if the man page for recvmsg is wrong, or at least a bit
>> confusing. SOCK_SEQPACKET is stream based not message based - it just
>> happens to have EOR markers in the stream. There is no reason that the whole
>> message needs to be returned in a single read, and in fact that would be
>> impossible if the sender didn't insert any EOR markers but kept sending data
>> beyond the size that the socket could buffer.
>>
>> I notice that man 7 socket says SOCK_SEQPACKET is for datagrams of fixed
>> maximum length which is definitely wrong, as is the statement that a
>> consumer has to read an entire packet with each system call.
> Which man page do you think is wrong here? The POSIX definition is here
>
> http://pubs.opengroup.org/onlinepubs/009695399/functions/recvmsg.html
>
> The description in
>
> http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_10.html
>
> says, "It is protocol-specific whether a maximum record size is imposed."
> In my machine (Ubuntu 4.4.0-72, and it is in socket(2), not socket(7), btw)
> doesnt have any references to max length, but I'm not sure I'd boldly assert
> "definitely wrong" about the requirement of having to read entire
> packet in a system call (see POSIX man page)
>
> --Sowmini
>
Just before the part that you've quoted, the description for 
SOCK_SEQPACKET says:
"The SOCK_SEQPACKET socket type is similar to the SOCK_STREAM type, and 
is also connection-oriented. The only difference between these types is 
that record boundaries are maintained using the SOCK_SEQPACKET type. A 
record can be sent using one or more output operations and received 
using one or more input operations, but a single operation never 
transfers parts of more than one record."

The man page for socket says SOCK_SEQPACKET "Provides  a sequenced,  
reliable,  two-way connection-based data transmission path  for  
datagrams  of  fixed maximum  length" which is not true, because while 
there may be a length restriction, it is quite possible that there is 
not a length restriction (as per DECnet). It also says "a  consumer  is  
required  to read an entire packet with each input system call" which is 
also contradicted by POSIX which says that a record can be "received 
using one or more input operations". So both statements in the man page 
are wrong, I think.

I have to say that I'd not spotted the POSIX recvmsg wording before, 
which says "For message-based sockets, such as SOCK_DGRAM and 
SOCK_SEQPACKET, the entire message shall be read in a single operation" 
however that does contradict the earlier wording, where it explicitly 
says that multiple receive operations per record are ok for 
SOCK_SEQPACKET - at least if we assume that record == message in this 
case. Also, if this restriction was true (one message per recvmsg call) 
then MSG_EOR would never be needed on receive, since every recvmsg would 
be a single message/record only, and that same document does say that 
MSG_EOR can be set on receive for protocols which support it,

Steve.

^ permalink raw reply

* RE: SSE instructions for fast packet copy?
From: David Laight @ 2017-05-05 10:14 UTC (permalink / raw)
  To: 'Tom Herbert', Linux Kernel Network Developers
In-Reply-To: <CALx6S34r6Q_wCKqzv5YjKJyuETyYmY1y=M7uDYiLfpnXzqbheg@mail.gmail.com>

From: Tom Herbert
> Sent: 05 May 2017 06:51
> To: Linux Kernel Network Developers
> Subject: SSE instructions for fast packet copy?
> 
> Hi,
> 
> I am thinking about the possibility of using SSE in kernel for
> speeding up the kernel memcpy particularly for copy to userspace
> emeory, and maybe even using the string instructions (like if we
> supported regex in something like eBPF). AFAIK we don't use SSE in
> kernel because of xmm register state needing to be saved across
> context switch. However, if we start busy-polling a CPU in kernel on
> network queues then there might not be any context switches to worry
> about. In this model we'd want to enable SSE per CPU.
> 
> Has this ever been tried before? Is this at all feasible? :-) Is it
> possible to enable SSE for kernel for just one CPU? (I found CPUID
> will return SSE supported, but don't see how to enable other than
> -msse for compiling).

Not even worth thinking about.
With recent intel cpus 'rep movsb' is optimised in the hardware
(for cached memory) and will run as fast as any other copy.

(There is a related fubar that memcopytoio() is implemented
as memcpy() and then as 'rep movsb' so generates repeated
byte accesses to io memory.)

I'm pretty sure the FP registers are 'lazy saved'.
The cpu's sse registers (the entire FP register set) might
contain life values for a process that is running on a different cpu.
If that process executes an FP instruction it will fault and an IPI
issued to get the registers written to the processes fp save area
from where they can be loaded.
Any use of the sse registers would have to interact correctly
with that IPI code.

	David


^ permalink raw reply

* Re: [PATCH 0/9] net: thunderx: Adds XDP support
From: Sunil Kovvuri @ 2017-05-05 10:10 UTC (permalink / raw)
  To: Rami Rosen; +Cc: Sunil Goutham, LAKML, Netdev, LKML, David Miller
In-Reply-To: <CAKoUArku6tRuV=Y3fteY-R6eHM4daq-0ct6w1_unjkGA+yPTFQ@mail.gmail.com>

On Thu, May 4, 2017 at 2:09 AM, Rami Rosen <roszenrami@gmail.com> wrote:
> Thanks, Sunil.
>
>>with network stack: 0.32 Mpps
>>with XDP (XDP_TX): 3 Mpps
>>and XDP_DROP: 3.8 Mpps
>
> Interesting; May I ask - which packet size did you use ?

Packet size is 64bytes.

Thanks,
Sunil.

>
> Regards,
> Rami Rosen

^ permalink raw reply

* Re: Question about SOCK_SEQPACKET
From: Sowmini Varadhan @ 2017-05-05 10:09 UTC (permalink / raw)
  To: Steven Whitehouse; +Cc: Sam Kumar, linux-kernel, netdev@vger.kernel.org
In-Reply-To: <af19f11a-1bc3-bb8f-faf6-65c11b632bbf@redhat.com>

On (05/05/17 10:45), Steven Whitehouse wrote:
> 
> I do wonder if the man page for recvmsg is wrong, or at least a bit
> confusing. SOCK_SEQPACKET is stream based not message based - it just
> happens to have EOR markers in the stream. There is no reason that the whole
> message needs to be returned in a single read, and in fact that would be
> impossible if the sender didn't insert any EOR markers but kept sending data
> beyond the size that the socket could buffer.
> 
> I notice that man 7 socket says SOCK_SEQPACKET is for datagrams of fixed
> maximum length which is definitely wrong, as is the statement that a
> consumer has to read an entire packet with each system call.

Which man page do you think is wrong here? The POSIX definition is here

http://pubs.opengroup.org/onlinepubs/009695399/functions/recvmsg.html

The description in

http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_10.html

says, "It is protocol-specific whether a maximum record size is imposed."
In my machine (Ubuntu 4.4.0-72, and it is in socket(2), not socket(7), btw)
doesnt have any references to max length, but I'm not sure I'd boldly assert
"definitely wrong" about the requirement of having to read entire
packet in a system call (see POSIX man page)

--Sowmini

^ permalink raw reply

* Re: [PATCH v1] ACPI: Switch to use generic UUID API
From: Amir Goldstein @ 2017-05-05 10:02 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Andy Shevchenko, Dan Williams, Linux ACPI, tpmdd-devel, intel-gfx,
	nouveau, linux-input, iommu, linux-mmc, Netdev, linux-pci,
	USB list, alsa-devel, linux-kernel@vger.kernel.org,
	Rafael J . Wysocki, Mika Westerberg, Borislav Petkov,
	Jarkko Sakkinen, Jani Nikula, Ben Skeggs, Benj
In-Reply-To: <20170505095739.GB6762@lst.de>

On Fri, May 5, 2017 at 12:57 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Fri, May 05, 2017 at 12:50:31PM +0300, Amir Goldstein wrote:
>> To complete the picture for folks not cc'ed on my patches,
>> xfs use case suggests there is also justification for the additional helpers:
>>
>> uuid_is_null() / uuid_equal()
>> guid_is_null() / guid_equal()
>
> The is_null is useful and I bet we'll find other instances. I'm
> not sure _equals really adds much value over the existing _cmp
> helpers, but on the other hand they are so trivial that we might as
> well add them.

Exactly. The fact that not only xfs used the same helper name
(drivers/md/md.c) suggests that it useful.

>
> The other thing XFS has is uuid_copy.

Andy already listed uuid_copy.

^ permalink raw reply

* Re: [PATCH v1] ACPI: Switch to use generic UUID API
From: Christoph Hellwig @ 2017-05-05  9:57 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: alsa-devel, Heikki Krogerus, Liam Girdwood, linux-pci,
	Jarkko Sakkinen, Adrian Hunter, Benjamin Tissoires,
	Christoph Hellwig, Joerg Roedel, Linux ACPI, tpmdd-devel,
	Ben Skeggs, nouveau, linux-mmc, Zhang Rui, Borislav Petkov,
	Yisen Zhuang, Mathias Nyman, intel-gfx, linux-input, Mark Brown,
	Bjorn Helgaas, Dan Williams, Andy Shevchenko
In-Reply-To: <CAOQ4uxjErce01+xyDnM=nCJS+avQ4g6hQVvH3fVotj7Ws2moCQ@mail.gmail.com>

On Fri, May 05, 2017 at 12:50:31PM +0300, Amir Goldstein wrote:
> To complete the picture for folks not cc'ed on my patches,
> xfs use case suggests there is also justification for the additional helpers:
> 
> uuid_is_null() / uuid_equal()
> guid_is_null() / guid_equal()

The is_null is useful and I bet we'll find other instances.  I'm
not sure _equals really adds much value over the existing _cmp
helpers, but on the other hand they are so trivial that we might as
well add them.

The other thing XFS has is uuid_copy.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH v1] ACPI: Switch to use generic UUID API
From: Amir Goldstein @ 2017-05-05  9:50 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Dan Williams, Linux ACPI, tpmdd-devel, intel-gfx, nouveau,
	linux-input, iommu, linux-mmc, Netdev, linux-pci, USB list,
	alsa-devel, linux-kernel@vger.kernel.org, Rafael J . Wysocki,
	Mika Westerberg, Borislav Petkov, Jarkko Sakkinen, Jani Nikula,
	Ben Skeggs, Benjamin Tissoires, Joerg
In-Reply-To: <1493976265.30052.21.camel@linux.intel.com>

On Fri, May 5, 2017 at 12:24 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Fri, 2017-05-05 at 10:06 +0300, Amir Goldstein wrote:
[...]
>> I think with this semantic change, our proposals can reach common
>> grounds
>> and satisfy a wider group of users (i.e. filesystem developers).
>>
>> Christoph also suggested a similar treatment to typedef guid_t to
>> struct uuid_le.
>> I don't know the use cases enough to comment on that.
>
> We may go this way. But I wouldn't prevent current users of uuid_le to
> continue using it without conversion (it may be done case by case after
> we settle an API)
>
> So, summarize what Christoph said it will look like
>
> typedef uuid_be uuid_t;
> typedef uuid_le guid_t
>
> uuid_cmp() / uuid_copy() / uuid_to_bin() / etc
> guid_cmp() / guid_copy() / guid_to_bin() / etc
>
> Correct? Christoph?
>

That looks right to me.

To complete the picture for folks not cc'ed on my patches,
xfs use case suggests there is also justification for the additional helpers:

uuid_is_null() / uuid_equal()
guid_is_null() / guid_equal()

Cheers,
Amir.

^ permalink raw reply

* Re: [PATCH] mac80211: Create ieee80211_if_process_skb from ieee80211_iface_work
From: Joe Perches @ 2017-05-05  9:47 UTC (permalink / raw)
  To: Johannes Berg; +Cc: David S. Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <1493977157.2500.2.camel@sipsolutions.net>

On Fri, 2017-05-05 at 11:39 +0200, Johannes Berg wrote:
> On Fri, 2017-05-05 at 02:34 -0700, Joe Perches wrote:
> > On Fri, 2017-05-05 at 11:06 +0200, Johannes Berg wrote:
> > > > o Use explicit casts to proper types instead of casts to (void *)
> > > >   and have the compiler do the implicit cast
> > > 
> > > I see no advantage in this, why? All it does is make the code
> > > longer,
> > > and if anything changes, you have to change it in multiple places
> > > now.
> > 
> > It makes use of the casted to types consistent within net/mac80211
> > 
> > Here are the current uses.  I changed iface .c to match the others.
> 
> Well, OK. I'd rather change the others I guess, don't really see the
> point.
> 
> > $ grep -P --include=*.[ch] "\((\w++\s*){1,2}\s*\*\).*skb->cb" net/mac80211
> > net/mac80211/iface.c:			ra_tid = (void *)&skb->cb;
> > net/mac80211/iface.c:			ra_tid = (void *)&skb->cb;
> > net/mac80211/iface.c:			rx_agg = (void *)&skb->cb;
> > net/mac80211/iface.c:			rx_agg = (void *)&skb->cb;
> > net/mac80211/agg-tx.c:	ra_tid = (struct ieee80211_ra_tid *) &skb->cb;
> > net/mac80211/agg-tx.c:	ra_tid = (struct ieee80211_ra_tid *) &skb->cb;
> > net/mac80211/agg-rx.c:	rx_agg = (struct ieee80211_rx_agg *) &skb->cb;
> 
> It's really just these three that are related to the iface.c ones
> anyway.
> 
> > net/mac80211/mlme.c:	struct ieee80211_rx_status *rx_status = (void *) skb->cb;
> > net/mac80211/mlme.c:	rx_status = (struct ieee80211_rx_status *) skb->cb;
> > 
> 
> These should be using IEEE80211_SKB_RXCB(skb) :)

patches welcome... :)

^ permalink raw reply

* Re: Question about SOCK_SEQPACKET
From: Steven Whitehouse @ 2017-05-05  9:45 UTC (permalink / raw)
  To: Sam Kumar, linux-kernel, netdev@vger.kernel.org
In-Reply-To: <CAGtMfeCt95bj3-J5+TNb5gxVJUBDrgRjMXqMz-3jk9cOh0Z=tA@mail.gmail.com>

Hi,


On 05/05/17 06:18, Sam Kumar wrote:
> Hello,
> I have recently had occasion to use SOCK_SEQPACKET sockets on Linux,
> and noticed some odd behavior. When using sendmsg and recvmsg with
> these sockets, it seems that the "end-of-record" flag (MSG_EOR) is not
> being propagated correctly.
It depends which protocol you are using as to whether that is true. 
SOCK_SEQPACKET is supposed to be identical to SOCK_STREAM except for the 
record separators. That is true for DECnet (but whether DECnet is still 
functional is another thing!) and not true for ax.25 which uses 
SOCK_SEQPACKET incorrectly. For AF_UNIX that you are using I'm not quite 
sure what would be expected.

> The man page for recvmsg(2) states:
>> The  msg_flags  field  in the msghdr is set on return of recvmsg().  It
>>         can contain several flags:
>>
>>         MSG_EOR
>>                indicates end-of-record; the data returned  completed  a  record
>>                (generally used with sockets of type SOCK_SEQPACKET).
>>
> The man page for recvmsg(3) states:
>> For
>>        message-based  sockets,  such as SOCK_DGRAM and SOCK_SEQPACKET, the entire
>>        message shall be read in a single operation.
>
>
> This leads me to believe that MSG_EOR should be set in the msghdr
> struct whenever recvmsg() returns data. However, I am not observing
> this flag ever being set, whether or not I set the MSG_EOR when
> sending the messages.
>
> If it helps you can take a look at the code I'm using. It is at
> https://github.com/samkumar/seqpacket-test/, commit
> 2a7dbc1f94bafce6950ee726bdd54da96945d083 (HEAD of master at the time
> of writing). Look at server.c and client.c (don't bother with
> goclient.go).
>
> The reason that I need to check MSG_EOR is that I need to distinguish
> between EOF and messages of length 0. For SOCK_STREAM sockets, a
> return value of 0 unambiguously means EOF, and for SOCK_DGRAM sockets
> a return value of 0 unambiguously means that a datagram of length 0
> was received.
>
> Because SOCK_SEQPACKET is both connection-based and message-oriented,
> a return value of 0 is ambiguous. Based on my reading of the man
> pages, reading the MSG_EOR bit would let me disambiguate between EOF
> and a zero-length datagram, because MSG_EOR would be set for a
> zero-length datagram, but would not be set for EOF.
>
> If someone could please help me understand MSG_EOR, and how to
> distinguish between EOF and zero-length messages in a SOCK_SEQPACKET
> connection, I would definitely appreciate it!
>
> Thanks,
> Sam Kumar
That would be my expectation of how it should work - if you ignore 
MSG_EOR on recvmsg then what you get is identical to a SOCK_STREAM, and 
that every call to recvmsg will return data from (at most) a single 
message, with MSG_EOR set if the end of that message has been reached. 
That is what POSIX says should happen anyway.

I do wonder if the man page for recvmsg is wrong, or at least a bit 
confusing. SOCK_SEQPACKET is stream based not message based - it just 
happens to have EOR markers in the stream. There is no reason that the 
whole message needs to be returned in a single read, and in fact that 
would be impossible if the sender didn't insert any EOR markers but kept 
sending data beyond the size that the socket could buffer.

I notice that man 7 socket says SOCK_SEQPACKET is for datagrams of fixed 
maximum length which is definitely wrong, as is the statement that a 
consumer has to read an entire packet with each system call.

Also it is probably better to ask this question on netdev where it is 
likely to get more attention from the net developers, so I'm copying my 
reply there too,

Steve.

^ permalink raw reply

* Re: [PATCH] mac80211: Create ieee80211_if_process_skb from ieee80211_iface_work
From: Johannes Berg @ 2017-05-05  9:39 UTC (permalink / raw)
  To: Joe Perches; +Cc: David S. Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <1493976873.31950.15.camel@perches.com>

On Fri, 2017-05-05 at 02:34 -0700, Joe Perches wrote:
> On Fri, 2017-05-05 at 11:06 +0200, Johannes Berg wrote:
> > > o Use explicit casts to proper types instead of casts to (void *)
> > >   and have the compiler do the implicit cast
> > 
> > I see no advantage in this, why? All it does is make the code
> > longer,
> > and if anything changes, you have to change it in multiple places
> > now.
> 
> It makes use of the casted to types consistent within net/mac80211
> 
> Here are the current uses.  I changed iface .c to match the others.

Well, OK. I'd rather change the others I guess, don't really see the
point.

> $ grep -P --include=*.[ch] "\((\w++\s*){1,2}\s*\*\).*skb->cb" net/mac80211
> net/mac80211/iface.c:			ra_tid = (void *)&skb->cb;
> net/mac80211/iface.c:			ra_tid = (void *)&skb->cb;
> net/mac80211/iface.c:			rx_agg = (void *)&skb->cb;
> net/mac80211/iface.c:			rx_agg = (void *)&skb->cb;
> net/mac80211/agg-tx.c:	ra_tid = (struct ieee80211_ra_tid *) &skb->cb;
> net/mac80211/agg-tx.c:	ra_tid = (struct ieee80211_ra_tid *) &skb->cb;
> net/mac80211/agg-rx.c:	rx_agg = (struct ieee80211_rx_agg *) &skb->cb;

It's really just these three that are related to the iface.c ones
anyway.

> net/mac80211/mlme.c:	struct ieee80211_rx_status *rx_status = (void *) skb->cb;
> net/mac80211/mlme.c:	rx_status = (struct ieee80211_rx_status *) skb->cb;
> 

These should be using IEEE80211_SKB_RXCB(skb) :)

johannes

^ permalink raw reply

* Re: [PATCH v2 net] tcp: randomize timestamps on syncookies
From: Florian Westphal @ 2017-05-05  9:36 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Florian Westphal, David Miller, netdev, Yuchung Cheng
In-Reply-To: <1493950957.7796.36.camel@edumazet-glaptop3.roam.corp.google.com>

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Whole point of randomization was to hide server uptime, but an attacker
> can simply start a syn flood and TCP generates 'old style' timestamps,
> directly revealing server jiffies value.
> 
> Also, TSval sent by the server to a particular remote address vary
> depending on syncookies being sent or not, potentially triggering PAWS
> drops for innocent clients.
> 
> Lets implement proper randomization, including for SYNcookies.


Thanks a lot Eric, this works for me (I also tested ipv6 this time ;) )

Minor nit:
net/ipv4/tcp_ipv4.c:154:6: warning: unused variable 'seq' [-Wunused-variable]

Other than this:
Reviewed-by: Florian Westphal <fw@strlen.de>
Tested-by: Florian Westphal <fw@strlen.de>

^ permalink raw reply

* Re: [PATCH] mac80211: Create ieee80211_if_process_skb from ieee80211_iface_work
From: Joe Perches @ 2017-05-05  9:34 UTC (permalink / raw)
  To: Johannes Berg; +Cc: David S. Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <1493975178.2500.0.camel@sipsolutions.net>

On Fri, 2017-05-05 at 11:06 +0200, Johannes Berg wrote:
> > o Use explicit casts to proper types instead of casts to (void *)
> >   and have the compiler do the implicit cast
> 
> I see no advantage in this, why? All it does is make the code longer,
> and if anything changes, you have to change it in multiple places now.

It makes use of the casted to types consistent within net/mac80211

Here are the current uses.  I changed iface .c to match the others.

$ grep -P --include=*.[ch] "\((\w++\s*){1,2}\s*\*\).*skb->cb" net/mac80211
net/mac80211/iface.c:			ra_tid = (void *)&skb->cb;
net/mac80211/iface.c:			ra_tid = (void *)&skb->cb;
net/mac80211/iface.c:			rx_agg = (void *)&skb->cb;
net/mac80211/iface.c:			rx_agg = (void *)&skb->cb;
net/mac80211/agg-tx.c:	ra_tid = (struct ieee80211_ra_tid *) &skb->cb;
net/mac80211/agg-tx.c:	ra_tid = (struct ieee80211_ra_tid *) &skb->cb;
net/mac80211/mlme.c:	struct ieee80211_rx_status *rx_status = (void *) skb->cb;
net/mac80211/mlme.c:	rx_status = (struct ieee80211_rx_status *) skb->cb;
net/mac80211/agg-rx.c:	rx_agg = (struct ieee80211_rx_agg *) &skb->cb;

^ permalink raw reply

* Re: [PATCH v1] ACPI: Switch to use generic UUID API
From: Andy Shevchenko @ 2017-05-05  9:24 UTC (permalink / raw)
  To: Amir Goldstein, Dan Williams
  Cc: alsa-devel, Heikki Krogerus, Liam Girdwood, linux-pci,
	Jarkko Sakkinen, Adrian Hunter, Benjamin Tissoires,
	Christoph Hellwig, Joerg Roedel, Linux ACPI, tpmdd-devel,
	Ben Skeggs, nouveau, linux-mmc, Zhang Rui, Borislav Petkov,
	Yisen Zhuang, Mathias Nyman, intel-gfx, linux-input, Mark Brown,
	Bjorn Helgaas, Mika Westerberg, Felipe Balbi, Ne
In-Reply-To: <CAOQ4uxjha8_2_HwgdgBN=G-vNytAW5Gk155E9z8OWyFiGNEZVw@mail.gmail.com>

On Fri, 2017-05-05 at 10:06 +0300, Amir Goldstein wrote:
> On Fri, May 5, 2017 at 9:20 AM, Dan Williams <dan.j.williams@intel.com
> > wrote:
> > On Thu, May 4, 2017 at 2:21 AM, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:


> > >         for (i = 0; i < NFIT_UUID_MAX; i++)
> > > -               if (memcmp(to_nfit_uuid(i), spa->range_guid, 16)
> > > == 0)
> > > +               if (!uuid_le_cmp_pp(to_nfit_uuid(i), (uuid_le
> > > *)spa->range_guid))
> > 
> > What is _cmp_pp? Why not _compare?

Dan, it's a typo. In this patch it should be just ..._cmp(), which is
already a part of API.

> > 
> 
> I second that.
> 
> Andy,

Amir, just to be clear. This patch can be applied without any addons to
an existing API. Above is just a typo due to rebase in my tree. I will
replace it to just uuid_le_cmp().

> I much rather that you sort out uuid helpers in a way that will
> satisfy the filesystem
> needs (just provide the helpers don't need to convert filesystems
> code).

> The only reason I took a swing at hoisting the xfs uuid helpers is
> because it didn't
> seem like your proposal was going to be posted soon or wasn't going to
> satisfy
> the filesystems use case.

> 
> My opinion now, is that your suggestion is probably much closer to the
> real deal
> than mine.
> 
> IMO, you should acknowledge that the common use case for filesystems
> is
> to handle an opaque char[16] which most likely holds a uuid_be and you
> should provide 'neutral' helpers to satisfy this use case.
> 
> The simplest would be to typedef uuid_t to struct uuid_be and to name
> 'neutral'
> helpers' uuid_cmp/uuid_copy(uuid_t *, uuid_t *), similar to my
> proposal.

> I think with this semantic change, our proposals can reach common
> grounds
> and satisfy a wider group of users (i.e. filesystem developers).
> 
> Christoph also suggested a similar treatment to typedef guid_t to
> struct uuid_le.
> I don't know the use cases enough to comment on that.

We may go this way. But I wouldn't prevent current users of uuid_le to
continue using it without conversion (it may be done case by case after
we settle an API)

So, summarize what Christoph said it will look like

typedef uuid_be uuid_t;
typedef uuid_le guid_t

uuid_cmp() / uuid_copy() / uuid_to_bin() / etc
guid_cmp() / guid_copy() / guid_to_bin() / etc

Correct? Christoph?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: net/key: slab-out-of-bounds in pfkey_compile_policy
From: Steffen Klassert @ 2017-05-05  9:11 UTC (permalink / raw)
  To: Andrey Konovalov; +Cc: Herbert Xu, David S. Miller, netdev, LKML
In-Reply-To: <CAAeHK+xqxvEWMXS9sDNi2seUqFzJkijKJhivB8JDehpCJvBAaw@mail.gmail.com>

On Tue, May 02, 2017 at 06:45:03PM +0200, Andrey Konovalov wrote:
> Hi,
> 
> I've got the following error report while fuzzing the kernel with syzkaller.
> 
> On commit d3b5d35290d729a2518af00feca867385a1b08fa (4.11).
> 
> A reproducer and .config are attached.
> 
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in pfkey_compile_policy+0x8e6/0xd40 at
> addr ffff88006701f798
> Read of size 1280 by task a.out/4181


This bug was introduced twelve years ago...

This patch is based just on code review, I don't have an option to
function test this. But I see that we now exit with -EINVAL before the
memcpy that causes the slab-out-of-bounds when using your reproducer,
so it should at least fix the bug.

Subject: [PATCH RFC] af_key: Fix slab-out-of-bounds in pfkey_compile_policy.

The sadb_x_sec_len is stored in the unit 'byte divided by eight'.
So we have to multiply this value by eight before we can do
size checks. Otherwise we may get a slab-out-of-bounds when
we memcpy the user sec_ctx.

Fixes: df71837d502 ("[LSM-IPSec]: Security association restriction.")
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/key/af_key.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index c1950bb..512dc43 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3285,7 +3285,7 @@ static struct xfrm_policy *pfkey_compile_policy(struct sock *sk, int opt,
 		p += pol->sadb_x_policy_len*8;
 		sec_ctx = (struct sadb_x_sec_ctx *)p;
 		if (len < pol->sadb_x_policy_len*8 +
-		    sec_ctx->sadb_x_sec_len) {
+		    sec_ctx->sadb_x_sec_len*8) {
 			*dir = -EINVAL;
 			goto out;
 		}
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] mac80211: Create ieee80211_if_process_skb from ieee80211_iface_work
From: Johannes Berg @ 2017-05-05  9:06 UTC (permalink / raw)
  To: Joe Perches; +Cc: David S. Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <401d68b9015dc036589c99a26ef4664000cc821e.1493919003.git.joe@perches.com>


> o Use explicit casts to proper types instead of casts to (void *)
>   and have the compiler do the implicit cast

I see no advantage in this, why? All it does is make the code longer,
and if anything changes, you have to change it in multiple places now.

johannes

^ permalink raw reply

* how to insert new option header into ipv6 hop by hop for tx
From: JANARDHANACHARI KELLA @ 2017-05-05  8:40 UTC (permalink / raw)
  To: netdev

Hi,

i wish to insert a new option like pad1, padn and rpl option into hop
by hop option payload of ipv6 transmission. how can insert into the
ipv6 module? can i get suggestion?


Thanks & Regards

-- 

Janardhanachari Kella
E-mail: janardhanachari.kella@smartron.com

^ permalink raw reply

* [tip:x86/urgent] x86/asm: Don't use RBP as a temporary register in csum_partial_copy_generic()
From: tip-bot for Josh Poimboeuf @ 2017-05-05  8:10 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, jpoimboe, nhorman, syzkaller, netdev, kcc,
	marcelo.leitner, edumazet, torvalds, andreyknvl, mingo,
	xiyou.wangcong, peterz, davem, dvyukov, tglx, vyasevich, hpa
In-Reply-To: <4b03a961efda5ec9bfe46b7b9c9ad72d1efad343.1493909486.git.jpoimboe@redhat.com>

Commit-ID:  42fc6c6cb1662ba2fa727dd01c9473c63be4e3b6
Gitweb:     http://git.kernel.org/tip/42fc6c6cb1662ba2fa727dd01c9473c63be4e3b6
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Thu, 4 May 2017 09:51:40 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 5 May 2017 07:59:24 +0200

x86/asm: Don't use RBP as a temporary register in csum_partial_copy_generic()

Andrey Konovalov reported the following warning while fuzzing the kernel
with syzkaller:

  WARNING: kernel stack regs at ffff8800686869f8 in a.out:4933 has bad 'bp' value c3fc855a10167ec0

The unwinder dump revealed that RBP had a bad value when an interrupt
occurred in csum_partial_copy_generic().

That function saves RBP on the stack and then overwrites it, using it as
a scratch register.  That's problematic because it breaks stack traces
if an interrupt occurs in the middle of the function.

Replace the usage of RBP with another callee-saved register (R15) so
stack traces are no longer affected.

Reported-by: Andrey Konovalov <andreyknvl@google.com>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: linux-sctp@vger.kernel.org
Cc: netdev <netdev@vger.kernel.org>
Cc: syzkaller <syzkaller@googlegroups.com>
Link: http://lkml.kernel.org/r/4b03a961efda5ec9bfe46b7b9c9ad72d1efad343.1493909486.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/lib/csum-copy_64.S | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/lib/csum-copy_64.S b/arch/x86/lib/csum-copy_64.S
index 7e48807..45a53df 100644
--- a/arch/x86/lib/csum-copy_64.S
+++ b/arch/x86/lib/csum-copy_64.S
@@ -55,7 +55,7 @@ ENTRY(csum_partial_copy_generic)
 	movq  %r12, 3*8(%rsp)
 	movq  %r14, 4*8(%rsp)
 	movq  %r13, 5*8(%rsp)
-	movq  %rbp, 6*8(%rsp)
+	movq  %r15, 6*8(%rsp)
 
 	movq  %r8, (%rsp)
 	movq  %r9, 1*8(%rsp)
@@ -74,7 +74,7 @@ ENTRY(csum_partial_copy_generic)
 	/* main loop. clear in 64 byte blocks */
 	/* r9: zero, r8: temp2, rbx: temp1, rax: sum, rcx: saved length */
 	/* r11:	temp3, rdx: temp4, r12 loopcnt */
-	/* r10:	temp5, rbp: temp6, r14 temp7, r13 temp8 */
+	/* r10:	temp5, r15: temp6, r14 temp7, r13 temp8 */
 	.p2align 4
 .Lloop:
 	source
@@ -89,7 +89,7 @@ ENTRY(csum_partial_copy_generic)
 	source
 	movq  32(%rdi), %r10
 	source
-	movq  40(%rdi), %rbp
+	movq  40(%rdi), %r15
 	source
 	movq  48(%rdi), %r14
 	source
@@ -103,7 +103,7 @@ ENTRY(csum_partial_copy_generic)
 	adcq  %r11, %rax
 	adcq  %rdx, %rax
 	adcq  %r10, %rax
-	adcq  %rbp, %rax
+	adcq  %r15, %rax
 	adcq  %r14, %rax
 	adcq  %r13, %rax
 
@@ -121,7 +121,7 @@ ENTRY(csum_partial_copy_generic)
 	dest
 	movq %r10, 32(%rsi)
 	dest
-	movq %rbp, 40(%rsi)
+	movq %r15, 40(%rsi)
 	dest
 	movq %r14, 48(%rsi)
 	dest
@@ -203,7 +203,7 @@ ENTRY(csum_partial_copy_generic)
 	movq 3*8(%rsp), %r12
 	movq 4*8(%rsp), %r14
 	movq 5*8(%rsp), %r13
-	movq 6*8(%rsp), %rbp
+	movq 6*8(%rsp), %r15
 	addq $7*8, %rsp
 	ret
 

^ permalink raw reply related

* Re: struct ip vs struct iphdr
From: Oleg @ 2017-05-05  8:02 UTC (permalink / raw)
  To: Girish Moodalbail; +Cc: netdev
In-Reply-To: <d56b2182-3c81-5c52-5d77-8be128088e1b@oracle.com>

On Thu, May 04, 2017 at 10:08:46AM -0700, Girish Moodalbail wrote:
> Also, see this:
> 
> http://stackoverflow.com/questions/42840636/difference-between-struct-ip-and-struct-iphdr

  I saw this. But the answer say that struct ip and struct iphdr defined in
different places. However i see struct iphdr together with struct ip in
my netinet/ip.h. So, i thought that this answer may be inexact or incorrect.

  Thanks!

-- 
Олег Неманов (Oleg Nemanov)

^ permalink raw reply

* Re: [Unstrung-hackers] [RFC net-next] ipv6: ext_header: add function to handle RPL extension header option 0x63
From: Andreas Bardoutsos @ 2017-05-05  7:55 UTC (permalink / raw)
  To: JANARDHANACHARI KELLA
  Cc: Jiri Pirko, Michael Richardson, netdev, netdev-owner,
	linux-bluetooth, linux-wpan, unstrung-hackers
In-Reply-To: <CAAK7Ti9iFFqM7vMyveNfaNeCi6PmAGptXdTYgEzh2gsMptqi4w@mail.gmail.com>

Yes I think we have faced the same problem,communication with RPL 
supporting devices was failing otherwise.Your patch is also more 
complete since it also implements #ifdef .About the comment,yes I have 
run checkpatch twice with no errors,but ok :)

Στις 2017-05-05 08:59, JANARDHANACHARI KELLA έγραψε:
> I was inserted this patch manually. It was working. on 4.9 kernel.
> 
> check this bellow link for your ref.
> 
> https://github.com/mwasilak/bluetooth-next/commit/f29c632ef6a6a1777815c97fd2f326faccc704f7
> [2]
> 
> On Thu, May 4, 2017 at 9:30 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> 
>> Thu, May 04, 2017 at 05:17:18PM CEST, bardoutsos@ceid.upatras.gr
>> wrote:
>>> Signed-off-by: Andreas Bardoutsos <bardoutsos@ceid.upatras.gr>
>>> ---
>>> Hi all!
>>> 
>>> I have added a dump function(always return true) to recognise RPL
>> extension
>>> header(RFC6553)
>>> Otherwise packet was dropped by kernel resulting in failing
>> communication in
>>> RPL DAG's between
>>> linux running border routers and devices in the graph.For example
>>> communication
>>> with contiki OS running devices was previously impossible.
>>> 
>>> include/uapi/linux/in6.h | 1 +
>>> net/ipv6/exthdrs.c | 13 +++++++++++++
>>> 2 files changed, 14 insertions(+)
>>> 
>>> diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h
>>> index 46444f8fbee4..5cc12d309dfe 100644
>>> --- a/include/uapi/linux/in6.h
>>> +++ b/include/uapi/linux/in6.h
>>> @@ -146,6 +146,7 @@ struct in6_flowlabel_req {
>>> #define IPV6_TLV_CALIPSO 7 /* RFC 5570 */
>>> #define IPV6_TLV_JUMBO 194
>>> #define IPV6_TLV_HAO 201 /* home address option */
>>> +#define IPV6_TLV_RPL 99 /* RFC 6553 */
>>> 
>>> /*
>>> * IPV6 socket options
>>> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
>>> index b636f1da9aec..82ed60d3180e 100644
>>> --- a/net/ipv6/exthdrs.c
>>> +++ b/net/ipv6/exthdrs.c
>>> @@ -785,6 +785,15 @@ static bool ipv6_hop_calipso(struct sk_buff
>> *skb, int
>>> optoff)
>>> return false;
>>> }
>>> 
>>> +/* RPL RFC 6553 */
>>> +
>>> +static bool ipv6_hop_rpl(struct sk_buff *skb, int optoff)
>>> +{
>>> + /*Dump function which always return true
>>> + *when rpl option is detected*/
>> 
>> This is definitelly wrong formatting of comment. Did you run
>> checkpatch?
>> 
>>> + return true;
>>> +}
>>> +
>>> static const struct tlvtype_proc tlvprochopopt_lst[] = {
>>> {
>>> .type = IPV6_TLV_ROUTERALERT,
>>> @@ -798,6 +807,10 @@ static const struct tlvtype_proc
>> tlvprochopopt_lst[] = {
>>> .type = IPV6_TLV_CALIPSO,
>>> .func = ipv6_hop_calipso,
>>> },
>>> + {
>>> + .type = IPV6_TLV_RPL,
>>> + .func = ipv6_hop_rpl,
>>> + },
>>> { -1, }
>>> };
>>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-wpan" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> [1]
> 
> --
> 
> Sincerely Your's
> 
> Janardhanachari Kella
> 
> Contact:+91-9908469599
> E-mail: eni.chari@gmail.com
> 
> 
> Links:
> ------
> [1] http://vger.kernel.org/majordomo-info.html
> [2]
> https://github.com/mwasilak/bluetooth-next/commit/f29c632ef6a6a1777815c97fd2f326faccc704f7
> 
> _______________________________________________
> Unstrung-hackers mailing list
> Unstrung-hackers@lists.sandelman.ca
> https://lists.sandelman.ca/mailman/listinfo/unstrung-hackers

^ permalink raw reply

* Re: struct ip vs struct iphdr
From: Oleg @ 2017-05-05  7:48 UTC (permalink / raw)
  To: Sowmini Varadhan; +Cc: netdev
In-Reply-To: <20170504165248.GI7300@oracle.com>

On Thu, May 04, 2017 at 12:52:48PM -0400, Sowmini Varadhan wrote:
> BSD vs linux?
> 
> struct ip is a BSD-ism, intended to be used if you were porting
> some BSD app.

  Thanks!

-- 
Олег Неманов (Oleg Nemanov)

^ permalink raw reply

* Re: [PATCH] iproute2: hide devices starting with period by default
From: Nicolas Dichtel @ 2017-05-05  7:42 UTC (permalink / raw)
  To: David Ahern, Florian Fainelli; +Cc: David Miller, stephen, netdev
In-Reply-To: <01aa60dc-8bf2-3207-1abe-158100f52960@gmail.com>

Le 04/05/2017 à 21:47, David Ahern a écrit :
> On 5/4/17 1:10 PM, Florian Fainelli wrote:
>> On 05/04/2017 09:37 AM, David Ahern wrote:
>>> On 5/4/17 9:15 AM, Nicolas Dichtel wrote:
>>>> Le 24/02/2017 à 16:52, David Ahern a écrit :
>>>>> On 2/23/17 8:12 PM, David Miller wrote:
>>>>>> This really need to be a fundamental facility, so that it transparently
>>>>>> works for NetworkManager, router daemons, everything.  Not just iproute2
>>>>>> and "ls".
>>>>>
>>>>> I'll rebase my patch and send out as RFC.
>>>>>
>>>> David, did you finally send those patches?
>>>>
>>>
>>> No, but for a few reasons.
>>>
>>> It is easy to hide devices in a dump:
>>>
>>> https://github.com/dsahern/linux/commit/48a80a00eac284e58bae04af10a5a932dd7aee00
>>>
>>>
>>> But I think those devices should also not exist in sysfs or procfs which
>>> overlaps what I would like to see for lightweight netdevices:
>>>
>>> https://github.com/dsahern/linux/commit/70574be699cf252e77f71e3df11192438689f976
>>
>> Interesting that does indeed solve the same problems as the L2 only
>> patch set intended. I am not exactly sure if hiding the devices from
>> procfs/sysfs would be appropriate in my case (dumb L2 only switch that
>> only does 802.1q for instance), but why not.
>>
>>
>>>
>>>
>>> and to be complete, hidden devices should not be allowed to have a
>>> network address or transmit packets which is the L2 only intent from
>>> Florian:
>>>     https://www.spinics.net/lists/netdev/msg340808.html
>>>
>>
>> Do you plan on submitting the LWT patch set at some point?
> 
> Definitely. Maybe I can find some time this weekend.
> 
Ok, thank you for the details.

I agree with Jiri that the name should be something different than lwt.


Regards,
Nicolas

^ permalink raw reply

* Re: [PATCH v1] ACPI: Switch to use generic UUID API
From: Christoph Hellwig @ 2017-05-05  7:30 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Heikki Krogerus, Liam Girdwood,
	linux-pci-u79uwXL29TY76Z2rM5mHXA, Jarkko Sakkinen, Adrian Hunter,
	Benjamin Tissoires, Christoph Hellwig, Linux ACPI,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Ben Skeggs,
	nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA, Zhang Rui, Borislav Petkov,
	Yisen Zhuang, Mathias Nyman,
	intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Jani Nikula, Mark Brown,
	Bjorn Helgaas, Dan Williams, Andy Shevchenko <andriy.shevchenk
In-Reply-To: <CAOQ4uxjha8_2_HwgdgBN=G-vNytAW5Gk155E9z8OWyFiGNEZVw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, May 05, 2017 at 10:06:06AM +0300, Amir Goldstein wrote:
> I much rather that you sort out uuid helpers in a way that will
> satisfy the filesystem
> needs (just provide the helpers don't need to convert filesystems code).

Yeah.

> IMO, you should acknowledge that the common use case for filesystems is
> to handle an opaque char[16] which most likely holds a uuid_be and you
> should provide 'neutral' helpers to satisfy this use case.
> 
> The simplest would be to typedef uuid_t to struct uuid_be and to name 'neutral'
> helpers' uuid_cmp/uuid_copy(uuid_t *, uuid_t *), similar to my proposal.

It's not jut neutral, it's the right thing to to.  The Apollo / DCE
uuids (later also specified in RFC 4122) are big endian, so we should
use the name there.

> Christoph also suggested a similar treatment to typedef guid_t to
> struct uuid_le.

Exactly.  The whole idea of "little endian UUIDs" comes from the
Wintel world, and if you look at the relevant specs they are almost
exclusively referred to as GUIDs.

The magic XFS and AFS types for specific interpretations of one of
the RFC4122 formats don't really help, but I'll just send a patch
to kill them off for XFS ASAP to at least get that out, and we
probably should revert at least

"afs: Move UUID struct to linux/uuid.h"

That moved the AFS mess to common code as a start, and then
also clean up the way we generate random UUIDs, where we currently
have le helper, a be helper and then also generate_random_uuid
just to confuse the heck out of people.  With no description of
either of them.

^ permalink raw reply

* Re: [PATCH v1] ACPI: Switch to use generic UUID API
From: Amir Goldstein @ 2017-05-05  7:06 UTC (permalink / raw)
  To: Dan Williams
  Cc: Andy Shevchenko, Linux ACPI, tpmdd-devel, intel-gfx, nouveau,
	linux-input, iommu, linux-mmc, Netdev, linux-pci, USB list,
	alsa-devel, linux-kernel@vger.kernel.org, Rafael J . Wysocki,
	Mika Westerberg, Borislav Petkov, Jarkko Sakkinen, Jani Nikula,
	Ben Skeggs, Benjamin Tissoires
In-Reply-To: <CAPcyv4jzD0MEBRNTwTokVzgdDWas682ArUjRAGT1hp=QRE8Pjg@mail.gmail.com>

On Fri, May 5, 2017 at 9:20 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Thu, May 4, 2017 at 2:21 AM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>> acpi_evaluate_dsm() and friends take a pointer to a raw buffer of 16
>> bytes. Instead we convert them to use uuid_le type. At the same time we
>> convert current users.
>>
>> acpi_str_to_uuid() becomes useless after the conversion and it's safe to
>> get rid of it.
>>
>> The conversion fixes a potential bug in int340x_thermal as well since
>> we have to use memcmp() on binary data.
>>
>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Cc: Amir Goldstein <amir73il@gmail.com>
>> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Ben Skeggs <bskeggs@redhat.com>
>> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> Cc: Joerg Roedel <joro@8bytes.org>
>> Cc: Adrian Hunter <adrian.hunter@intel.com>
>> Cc: Yisen Zhuang <yisen.zhuang@huawei.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Zhang Rui <rui.zhang@intel.com>
>> Cc: Felipe Balbi <balbi@kernel.org>
>> Cc: Mathias Nyman <mathias.nyman@intel.com>
>> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> Cc: Liam Girdwood <lgirdwood@gmail.com>
>> Cc: Mark Brown <broonie@kernel.org>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> [..]
>> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
>> index 0f7982a1caaf..bd3e45ede056 100644
>> --- a/drivers/acpi/nfit/core.c
>> +++ b/drivers/acpi/nfit/core.c
>> @@ -74,11 +74,11 @@ struct nfit_table_prev {
>>         struct list_head flushes;
>>  };
>>
>> -static u8 nfit_uuid[NFIT_UUID_MAX][16];
>> +static uuid_le nfit_uuid[NFIT_UUID_MAX];
>>
>> -const u8 *to_nfit_uuid(enum nfit_uuids id)
>> +const uuid_le *to_nfit_uuid(enum nfit_uuids id)
>>  {
>> -       return nfit_uuid[id];
>> +       return &nfit_uuid[id];
>>  }
>>  EXPORT_SYMBOL(to_nfit_uuid);
>>
>> @@ -207,7 +207,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
>>         u32 offset, fw_status = 0;
>>         acpi_handle handle;
>>         unsigned int func;
>> -       const u8 *uuid;
>> +       const uuid_le *uuid;
>>         int rc, i;
>>
>>         func = cmd;
>> @@ -394,7 +394,7 @@ int nfit_spa_type(struct acpi_nfit_system_address *spa)
>>         int i;
>>
>>         for (i = 0; i < NFIT_UUID_MAX; i++)
>> -               if (memcmp(to_nfit_uuid(i), spa->range_guid, 16) == 0)
>> +               if (!uuid_le_cmp_pp(to_nfit_uuid(i), (uuid_le *)spa->range_guid))
>
> What is _cmp_pp? Why not _compare?
>

I second that.

Andy,

I much rather that you sort out uuid helpers in a way that will
satisfy the filesystem
needs (just provide the helpers don't need to convert filesystems code).

The only reason I took a swing at hoisting the xfs uuid helpers is
because it didn't
seem like your proposal was going to be posted soon or wasn't going to satisfy
the filesystems use case.

My opinion now, is that your suggestion is probably much closer to the real deal
than mine.

IMO, you should acknowledge that the common use case for filesystems is
to handle an opaque char[16] which most likely holds a uuid_be and you
should provide 'neutral' helpers to satisfy this use case.

The simplest would be to typedef uuid_t to struct uuid_be and to name 'neutral'
helpers' uuid_cmp/uuid_copy(uuid_t *, uuid_t *), similar to my proposal.

I think with this semantic change, our proposals can reach common grounds
and satisfy a wider group of users (i.e. filesystem developers).

Christoph also suggested a similar treatment to typedef guid_t to
struct uuid_le.
I don't know the use cases enough to comment on that.

Cheers,
Amir.

^ permalink raw reply

* Re: [RFC iproute2 0/8] RDMA tool
From: Jiri Benc @ 2017-05-05  6:54 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Stephen Hemminger, Doug Ledford, Leon Romanovsky, Jiri Pirko,
	Ariel Almog, Dennis Dalessandro, Ram Amrani, Bart Van Assche,
	Sagi Grimberg, Jason Gunthorpe, Christoph Hellwig, Or Gerlitz,
	Linux RDMA, Linux Netdev
In-Reply-To: <20170504180216.7665-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Thu,  4 May 2017 21:02:08 +0300, Leon Romanovsky wrote:
> In order to close object model, ensure reuse of existing code and make this
> tool usable from day one, we decided to implement wrappers over legacy sysfs
> prior to implementing netlink functionality. As a nice bonus, it will allow
> to use this tool with old kernels too.

This sounds wrong. We don't support legacy ioctl interface for the 'ip'
command, either. I think rdma should be converted to netlink first and
the new tool should only use netlink.

 Jiri
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] iproute2: hide devices starting with period by default
From: Jiri Benc @ 2017-05-05  6:47 UTC (permalink / raw)
  To: David Ahern
  Cc: Florian Fainelli, nicolas.dichtel, David Miller, stephen, netdev
In-Reply-To: <01aa60dc-8bf2-3207-1abe-158100f52960@gmail.com>

On Thu, 4 May 2017 13:47:36 -0600, David Ahern wrote:
> On 5/4/17 1:10 PM, Florian Fainelli wrote:
> > On 05/04/2017 09:37 AM, David Ahern wrote:
> > Do you plan on submitting the LWT patch set at some point?
> 
> Definitely. Maybe I can find some time this weekend.

I suggest to change the name to "lwd" or so. "lwt" name is too similar
to the existing "lwtunnel" infrastructure and would be very confusing.

Thanks,

 Jiri

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox