* Re: [PATCH 1/1] VSOCK: Introduce VM Sockets
From: Gerd Hoffmann @ 2013-02-13 11:06 UTC (permalink / raw)
To: Andy King; +Cc: pv-drivers, netdev, linux-kernel, virtualization, gregkh, davem
In-Reply-To: <1675407061.963254.1360682467627.JavaMail.root@vmware.com>
On 02/12/13 16:21, Andy King wrote:
> Hi Gerd,
>
>>> +struct vsock_transport {
> ...
>> Whoa. This has grown *alot*. Care to explain this please? Patch
>> creating a Documentation/virtual/vsock.txt would be cool.
>
> Yes, it grew because of the notification stuff, which I'd forgotten
> about until I removed the vmci header from the core code. You are
> free to use empty functions for these if they don't make any sense
> for virtio.
I've seen you have a notify_ops in the vmci bits. Do you have different
notify ops depending on socket type or something? Does it make sense to
move the notify ops ptr into "struct vsock_sock" maybe?
And can we make it optional please (i.e. allow the function pointers to
be NULL)?
> The alternative is for us to move the entire body of
> vsock_stream_recv/send into the transport, where we can hide the
> notification stuff, but it seems like folks will just end up
> duplicating a lot of code then.
Sounds not so good. However, it is still not very clear what this
notification stuff is all about. And it's not just notification, the
calls can return errors too.
Which problem you are trying to tackle with the notifications?
>> stream_has_data + stream_has_space + stream_rcvhiwat look like they
>> are needed for buffer management. Details please (especially for
>> stream_rcvhiwat).
>
> stream_has_data: Returns amount of data available (in bytes) in the
> socket's receive buffer, or -1 if empty.
>
> stream_has_space: Returns amount of space available (in bytes) in the
> socket's send buffer, or -1 if full.
As one would expect from the names, good.
> stream_rcvhiwat: The upper bound on the socket's receive buffer.
> Which technically is the same as the value returned by
> get_buffer_size(), so perhaps we could substitute that here and
> drop this one.
Yes, please.
>> What is stream_is_active?
>
> For the VMCI transport, it indicates if the underlying queuepair is
> still around (i.e., make sure we haven't torn it down while sleeping
> in a blocking send or receive). Perhaps it's not the best name?
How you'd hit that? Peer closing the socket while sleeping? Other
thread closing the socket wile sleeping? Both?
I think a state field in struct vsock_sock would be a better solution here.
>> What is *_allow?
>
> It's very basic filtering. We have specific addresses that we don't
> allow, and we look for them in the allow() functions. You can just
> return true if you like.
Can we make those calls optional too please?
>> What are all those notify_* calls?
>
> They're to do with signaling and flow-control. Again, they might
> not make any sense at all, but it's hard to know without having
> written another transport :)
I don't see a immediate need for them in the virto transport, but it's
hard to say without knowing what exactly they are doing.
>> Why do you need vsock_transport_{send,recv}_notify_data structs?
>> Can't this live in vsock_sock->trans?
>
> Those have to be setup on a per-call basis (per-thread), so it's
> just easier to have them on the stack of the send/recv calls. If
> you think there's a better name, or a better way to allocate them,
> I'm all ears.
Hmm. The struct content seems to be vmci-specific and it is in generic
code, which isn't that nice.
The notify_*_init could return a opaque pointer instead. But then
you'll need a notify_*_free too. And you can't place it on the stack
and thus have a allocation in the hot path, which I guess you are trying
to avoid in the first place.
No good idea offhand, but maybe that changes if the purpose of all those
notify_* calls is more clear.
cheers,
Gerd
^ permalink raw reply
* Re: [PATCH v4 4/9] rdma/cm: Update port reservation to support AF_IB
From: Or Gerlitz @ 2013-02-13 11:09 UTC (permalink / raw)
To: sean.hefty-ral2JQCrhuEAvxtiuMwx3w
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1358891797-14625-5-git-send-email-sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On 22/01/2013 23:56, sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
> The AF_IB uses a 64-bit service id (SID), which the
> user can control through the use of a mask. The rdma_cm
> will assign values to the unmasked portions of the SID
> based on the selected port space and port number.
Something here I am not sure to follow -- if AF_IB consumers are allowed
to provide 64-bit SID, how
the IB port space is expressed in their SIDs?
Wouldn't it make sense to use a simpler approach that has a dedicated
IB_PS in the port space
portion of the RDMA-CM IP IBTA annex and let the consumer provide 16
bits for their part of the SID?
Or.
> Because the IB spec divides the SID range into several regions,
> a SID/mask combination may fall into one of the existing
> port space ranges as defined by the RDMA CM IP Annex. Map
> the AF_IB SID to the correct RDMA port space.
> [...]
> --- a/include/rdma/rdma_cm.h
> +++ b/include/rdma/rdma_cm.h
> @@ -70,6 +70,11 @@ enum rdma_port_space {
> RDMA_PS_UDP = 0x0111,
> };
>
> +#define RDMA_IB_IP_PS_MASK 0xFFFFFFFFFFFF0000ULL
> +#define RDMA_IB_IP_PS_TCP 0x0000000001060000ULL
> +#define RDMA_IB_IP_PS_UDP 0x0000000001110000ULL
> +#define RDMA_IB_IP_PS_IB 0x00000000013F0000ULL
>
--
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 net-next 5/5] gianfar: Fix and cleanup Rx FCB handling
From: Claudiu Manoil @ 2013-02-13 11:34 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: netdev, David S. Miller
In-Reply-To: <511A799C.4020105@windriver.com>
On 2/12/2013 7:19 PM, Paul Gortmaker wrote:
> On 13-02-12 07:47 AM, Claudiu Manoil wrote:
>> NETIF_F_HW_VLAN_TX flag must not condition RxFCB usage.
>
> The above statement isn't 100% clear to me. Is this the intent?
The above statement is a rule, if you wish. The existing code breaks
that rule by saying: RxFCB is enabled if NETIF_F_HW_VLAN_TX; which is
a false statement. This patch corrects this error, according to the
rule above.
So, primarily, this patch is a fix (as expressed in <subj.>). I'll
resend the patch with additional comments to make this point clearer.
>
> Currently, gfar_uses_fcb() calls gfar_is_vlan_on() which in turn
> checks NETIF_F_HW_VLAN_TX. However there is no relation between
> whether FCBs are used and the VLAN transmit state.
>
>> In the case of RxBD rings, FCBs (Frame Control Block) are inserted by
>> the eTSEC whenever RCTRL[PRSDEP] is set to a non-zero value. Only one
>> FCB is inserted per frame (in the buffer pointed to by the RxBD with
>> bit F set). TOE acceleration for receive is enabled for all rx frames
>> in this case.
>> Indroduce the uses_rxfcb field to signal RxFCB insertion in accordance
>> with the specification above (leading to cleaner, less confusing code).
>
> The is_vlan_on() and uses_fcb() calls were more self documenting than
> setting/clearing a new single use variable added to priv, I think.
> Even if they get changed/simplified, perhaps it is worth keeping them?
gfar_uses_fcb() generates confusion around the FCB concept, this maybe
explains how it came to the error above. First, there are 2 types of
FCBs with different meaning, covering different use cases: Rx (receive
side) FCB and TxFCB. uses_fcb() was intended to signal RxFCB insertion,
which is not obvious from its name, and it became (subtly) erroneous
after incorporating is_vlan_on().
is_vlan_on() is also misleading because we need to differentiate b/w
hw VLAN extraction/VLEX (marked by VLAN_RX flag) and hw VLAN
insertion/VLINS (VLAN_TX flag), which are different mechanisms using
different types of FCBs.
>
> Rather than a specific priv->uses_rxfcb field, perhaps it makes sense
> to make it more future proof with priv->rctrl field, that is a cached
> value of the register, and then you keep gfar_uses_fcb() and it in
> turn checks for RCTRL_PRSDEP_INIT bit in rctrl?
>
The main purpose of the priv->uses_rxfcb field is to quickly signal, on
the hot path, that the incoming frame has a *Rx* FCB block inserted
which needs to be pulled out before passing the skb to the stack.
This is a performance critical operation, it needs to happen fast,
that's why uses_rxfcb is placed in the first cacheline of gfar_private.
This is also why I cannot use a cached rctrl instead: 1) because I
don't have 32 bits available in the first cacheline of gfar_probe
(but only 16); 2) no time for bit operations on the hot path.
> Also, the dependency/conditional on FSL_GIANFAR_DEV_HAS_TIMER seems
> to simply vanish with this patch, and it isn't clear to me if that
> was 100% intentional or not...
>
The dependency on FSL_GIANFAR_DEV_HAS_TIMER is another source of
confusion. The dependency is actually to priv->hwts_rx_en.
Upon changing priv->hwts_rx_en via IOCTL, the gfar device is being
restarted and on init_mac() the priv->hwts_rx_en conditions the RxFCB
insertion, and rctrl is programmed accordingly. The patch takes care
of this case too.
So, I'll re-spin this patch with enhanced comments.
Thanks,
Claudiu
^ permalink raw reply
* Re: [PATCH 2/2 v2] sierra_net: fix issues with SYNC/RESTART messages and interrupt pipe setup
From: Bjørn Mork @ 2013-02-13 11:44 UTC (permalink / raw)
To: Dan Williams
Cc: Oliver Neukum, Elina Pasheva, netdev, linux-usb, Rory Filer,
Phil Sutter
In-Reply-To: <1360256793.2382.27.camel@dcbw.foobar.com>
Dan Williams <dcbw@redhat.com> writes:
> It doesn't need to run exactly at probe, but it appears to need to be
> the first thing the driver does when communicating with the firmware to
> ensure clear state and whatnot. Possibly like the QMI SYNC message that
> clears all the client IDs and resets the internal stack. (the driver
> also sends a "shutdown" message to the firmware when unbinding).
>
> So I do think that somewhere around probe() is the best time to do this,
> because it's best to initialize the device when the driver binds to it
> and react to errors as soon as possible, rather than trying to set
> everything up on open/IFF_UP and then fail right before you want to
> actually use the device. Late-fail is quite unhelpful for applications.
>
> I don't really care if it happens in probe() or somewhere else right
> after the driver is bound to the device, but it should be part of the
> initialization process.
I was looking for something else in the rndis_host driver right now and
noticed that it has the same sort of problem. The generic_rndis_bind()
function will call rndis_command() which does
usb_control_msg(.., USB_CDC_SEND_ENCAPSULATED_COMMAND, ..);
usb_interrupt_msg(.., dev->status->desc.bEndpointAddress, .. );
for (count = 0; count < 10; count++) {
usb_control_msg(.., USB_CDC_GET_ENCAPSULATED_RESPONSE, ..);
if (ok)
return 0;
msleep(20);
}
Somewhat ugly, but it is a way to send and receive commands while
probing. This driver also sends a command in unbind().
Looks like your patch would allow this to be solved a lot cleaner,
replacing the loop over USB_CDC_GET_ENCAPSULATED_RESPONSE with proper
interrupt endpoint handling.
Bjørn
^ permalink raw reply
* Re: [PATCH net] net: sctp: sctp_v6_get_dst: fix boolean test in dst cache
From: Neil Horman @ 2013-02-13 12:02 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, linux-sctp, netdev
In-Reply-To: <4a07201201d7bac08468d17dea3dbc1ea9a67205.1360709645.git.dborkman@redhat.com>
On Wed, Feb 13, 2013 at 12:30:16AM +0100, Daniel Borkmann wrote:
> We walk through the bind address list and try to get the best source
> address for a given destination. However, currently, we take the
> 'continue' path of the loop when an entry is invalid (!laddr->valid)
> *and* the entry state does not equal SCTP_ADDR_SRC (laddr->state !=
> SCTP_ADDR_SRC).
>
> Thus, still, invalid entries with SCTP_ADDR_SRC might not 'continue'
> as well as valid entries with SCTP_ADDR_{NEW, SRC, DEL}, with a possible
> false baddr and matchlen as a result, causing in worst case dst route
> to be false or possibly NULL.
>
> This test should actually be a '||' instead of '&&'. But lets fix it
> and make this a bit easier to read by having the condition the same way
> as similarly done in sctp_v4_get_dst.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---
> net/sctp/ipv6.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index f3f0f4d..391a245 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -326,9 +326,10 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
> */
> rcu_read_lock();
> list_for_each_entry_rcu(laddr, &bp->address_list, list) {
> - if (!laddr->valid && laddr->state != SCTP_ADDR_SRC)
> + if (!laddr->valid)
> continue;
> - if ((laddr->a.sa.sa_family == AF_INET6) &&
> + if ((laddr->state == SCTP_ADDR_SRC) &&
> + (laddr->a.sa.sa_family == AF_INET6) &&
> (scope <= sctp_scope(&laddr->a))) {
> bmatchlen = sctp_v6_addr_match_len(daddr, &laddr->a);
> if (!baddr || (matchlen < bmatchlen)) {
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* dev_add_pack() duplicate skb ?
From: Felipe Dias @ 2013-02-13 12:11 UTC (permalink / raw)
To: netdev
Hi, someone might try to tell me what I'm doing wrong, please?
I have a simple module and I registered one packet handler, in init function:
---
pseudo_proto.type = htons(ETH_P_ALL);
pseudo_proto.dev = NULL;
pseudo_proto.func = packet_handler;
dev_add_pack(&pseudo_proto);
---
My packet_handler, is a simple function just to print in dmesg info
about the packet:
---
...
switch (ntohs(eth_hdr(skb)->h_proto))
{
case ETH_P_IPV6:
printk(KERN_INFO "%s: (%s %d %s, %s %d) Packet\n",
THIS_MODULE->name,
skb->dev->name,
skb->dev->type,
ntohs(eth_hdr(skb)->h_proto,
ipv6_hdr(skb)->nexthdr,
skb->len);
break;
...
---
When I sent a icmpv6 with a ping6 -c1 ::1 , this in my tcpdump show
only 2 packets (icmp request and icmp reply), but in my dmesg I see
4!:
---
595:[30469.099222] test: (lo 772 0x86dd, 0x3a 118) Packet
596:[30469.099226] test: (lo 772 0x86dd, 0x3a 104) Packet
597:[30469.099234] test: (lo 772 0x86dd, 0x3a 118) Packet
598:[30469.099236] test: (lo 772 0x86dd, 0x3a 104) Packet
---
This show 4 packets in ipv4 too. What I'm doing wrong ?
Best Regards,
fd.
^ permalink raw reply
* Re: [PATCH -next 1/1] net: skbuff: fix compile error in skb_panic()
From: Sedat Dilek @ 2013-02-13 12:41 UTC (permalink / raw)
To: James Hogan
Cc: Jean Sacren, David S. Miller, linux-kernel, linux-next,
Jiri Pirko, netdev
In-Reply-To: <1360754427-9051-1-git-send-email-james.hogan@imgtec.com>
On Wed, Feb 13, 2013 at 12:20 PM, James Hogan <james.hogan@imgtec.com> wrote:
> I get the following build error on next-20130213 due to the following
> commit:
>
> commit f05de73bf82fbbc00265c06d12efb7273f7dc54a ("skbuff: create
> skb_panic() function and its wrappers").
>
> It adds an argument called panic to a function that uses the BUG() macro
> which tries to call panic, but the argument masks the panic() function
> declaration, resulting in the following error (gcc 4.2.4):
>
> net/core/skbuff.c In function 'skb_panic':
> net/core/skbuff.c +126 : error: called object 'panic' is not a function
>
[ CC linux-netdev ]
Hmm, with gcc-4.6.3 on Ubuntu/precise I don't see this... compiler-specific?
$ grep skbuff build-log_3.8.0-rc7-next20130213-2-iniza-small.txt
CC net/core/skbuff.o
- Sedat -
> This is fixed by renaming the argument to msg.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Jean Sacren <sakiwit@gmail.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Cc: David S. Miller <davem@davemloft.net>
> ---
>
> Feel free to rename panic to something else or squash this into the
> offending commit.
>
> net/core/skbuff.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 8731c39..21a22cc 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -109,7 +109,7 @@ static const struct pipe_buf_operations sock_pipe_buf_ops = {
> * @skb: buffer
> * @sz: size
> * @addr: address
> - * @panic: skb_over_panic or skb_under_panic
> + * @msg: skb_over_panic or skb_under_panic
> *
> * Out-of-line support for skb_put() and skb_push().
> * Called via the wrapper skb_over_panic() or skb_under_panic().
> @@ -117,10 +117,10 @@ static const struct pipe_buf_operations sock_pipe_buf_ops = {
> * __builtin_return_address is not used because it is not always reliable.
> */
> static void skb_panic(struct sk_buff *skb, unsigned int sz, void *addr,
> - const char panic[])
> + const char msg[])
> {
> pr_emerg("%s: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx dev:%s\n",
> - panic, addr, skb->len, sz, skb->head, skb->data,
> + msg, addr, skb->len, sz, skb->head, skb->data,
> (unsigned long)skb->tail, (unsigned long)skb->end,
> skb->dev ? skb->dev->name : "<NULL>");
> BUG();
> --
> 1.8.1.2
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-next" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 7/9] rdma/cm: Add helper functions to return id address information
From: Or Gerlitz @ 2013-02-13 12:44 UTC (permalink / raw)
To: sean.hefty; +Cc: linux-rdma, netdev
In-Reply-To: <1358891797-14625-8-git-send-email-sean.hefty@intel.com>
On 22/01/2013 23:56, sean.hefty@intel.com wrote:
> Provide inline helpers to extract source and destination address data from the rdma_cm_id.
Sean,
FWIW - this patch and most of patch #6 can (and worth doing) be
introduced as cleanup/maintenance patches to the rdma-cm without direct
relation to the AF_IB effort.
Or.
^ permalink raw reply
* Re: [PATCH 1/1] VSOCK: Introduce VM Sockets
From: Gerd Hoffmann @ 2013-02-13 12:44 UTC (permalink / raw)
To: Andy King; +Cc: pv-drivers, netdev, linux-kernel, virtualization, gregkh, davem
In-Reply-To: <1360196636-9357-2-git-send-email-acking@vmware.com>
On 02/07/13 01:23, Andy King wrote:
> +static int vsock_create(struct net *net, struct socket *sock,
> + int protocol, int kern)
> +{
> + if (!sock)
> + return -EINVAL;
> +
> + if (protocol)
> + return -EPROTONOSUPPORT;
> +
IMO protocol == PF_VSOCK should not get rejected here.
cheers,
Gerd
^ permalink raw reply
* Re: [net-next (TAKE 2) 0/4] IPv6 over Firewire
From: Stephan Gatzka @ 2013-02-13 12:45 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: netdev, linux1394-devel, davem, stefanr
In-Reply-To: <5118EE78.5000508@linux-ipv6.org>
Hi Yoshi,
I just wanted to try out your patch but is does not apply to net-next.
What is the tree I should use?
Regards,
Stephan
^ permalink raw reply
* Re: [PATCH vringh 2/2] caif_virtio: Introduce caif over virtio
From: Sjur Brændeland @ 2013-02-13 12:50 UTC (permalink / raw)
To: Rusty Russell
Cc: Vikram ARV, Dmitry TARNYAGIN, netdev@vger.kernel.org,
Linus Walleij, Ido Yariv, linux-kernel@vger.kernel.org,
Erwan YVIN, virtualization@lists.linux-foundation.org,
David S. Miller
In-Reply-To: <87ehgklds9.fsf@rustcorp.com.au>
Hi Rusty,
On Wed, Feb 13, 2013 at 11:16 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Sjur BRENDELAND <sjur.brandeland@stericsson.com> writes:
>>> > +static inline void ctx_prep_iov(struct cfv_napi_context *ctx)
>>> > +{
>>> > + if (ctx->riov.allocated) {
>>> > + kfree(ctx->riov.iov);
>>> > + ctx->riov.iov = NULL;
>>> > + ctx->riov.allocated = false;
>>> > + }
>>> > + ctx->riov.iov = NULL;
>>> > + ctx->riov.i = 0;
>>> > + ctx->riov.max = 0;
>>> > +}
>>>
>>> Hmm, we should probably make sure you don't have to do this: that if
>>> allocated once, you can simply reuse it by setting i = 0.
>>
>> Yes, I had problems getting the alloc/free of iov right. This is
>> perhaps the least intuitively part of the API. I maybe it's just me, but
>> I think some more helper functions and support from vringh in this
>> area would be great.
>
> Yes, I've neatened my git tree, an in particular added a commit which
> covers this explicitly, and one for the -EPROTO when we get unexpected
> r/w bufs. I've appended them below.
Great, thanks. This helps a lot. I picked up your patch-sett yesterday
so my V2 patch-set is using this already.
...
> +static inline void vringh_iov_init(struct vringh_iov *iov,
> + struct iovec *iovec, unsigned num)
> +{
> + iov->used = iov->i = 0;
> + iov->off = 0;
> + iov->max_num = num;
> + iov->iov = iovec;
> +}
How about supporting struct vringh_kiov and struct kvec as well?
I currently get the following complaints with my V2 patch-set:
drivers/net/caif/caif_virtio.c:486:2: warning: passing argument 1 of
‘vringh_iov_init’ from incompatible pointer type [enabled by default]
...
Thanks,
Sjur
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v4 1/9] rdma/cm: define native IB address
From: Or Gerlitz @ 2013-02-13 12:51 UTC (permalink / raw)
To: Hefty, Sean
Cc: linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
David Miller (davem@davemloft.net), Roland Dreier
In-Reply-To: <1828884A29C6694DAF28B7E6B8A8237368B99D59@ORSMSX101.amr.corp.intel.com>
On 11/02/2013 20:02, Hefty, Sean wrote:
>> Define AF_IB and sockaddr_ib to allow the rdma_cm to use native IB addressing.
>>
>> Signed-off-by: Sean Hefty <sean.hefty@intel.com>
>> ---
>> include/linux/socket.h | 2 +
>> include/rdma/ib.h | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 91 insertions(+), 0 deletions(-)
>> create mode 100644 include/rdma/ib.h
>>
>> diff --git a/include/linux/socket.h b/include/linux/socket.h
>> index 9a546ff..17a33f7 100644
>> --- a/include/linux/socket.h
>> +++ b/include/linux/socket.h
>> @@ -167,6 +167,7 @@ struct ucred {
>> #define AF_PPPOX 24 /* PPPoX sockets */
>> #define AF_WANPIPE 25 /* Wanpipe API Sockets */
>> #define AF_LLC 26 /* Linux LLC */
>> +#define AF_IB 27 /* Native InfiniBand address */
> ...
>
>> diff --git a/include/rdma/ib.h b/include/rdma/ib.h
> ...
>
>> +struct sockaddr_ib {
>> + unsigned short int sib_family; /* AF_IB */
>> + __be16 sib_pkey;
>> + __be32 sib_flowinfo;
>> + struct ib_addr sib_addr;
>> + __be64 sib_sid;
>> + __be64 sib_sid_mask;
>> + __u64 sib_scope_id;
>> +};
> Dave/Roland/anyone, is there any feedback on this approach?
>
> If there's hesitation to add new address families to socket.h, I could instead use definitions local to the rdma_cm. This has the potential to result in conflicts if the rdma_cm is expanded for other address families, though such conflicts seem unlikely.
>
>
I don't see why not add new address family if it comes to serve a real
world use case, which seems to be the case from the description you
provided in the cover letter.
Or.
^ permalink raw reply
* Re: [PATCH v4 1/9] rdma/cm: define native IB address
From: Or Gerlitz @ 2013-02-13 12:56 UTC (permalink / raw)
To: sean.hefty; +Cc: linux-rdma, netdev
In-Reply-To: <1358891797-14625-2-git-send-email-sean.hefty@intel.com>
On 22/01/2013 23:56, sean.hefty@intel.com wrote:
> +
> +struct sockaddr_ib {
> + unsigned short int sib_family; /* AF_IB */
> + __be16 sib_pkey;
> + __be32 sib_flowinfo;
> + struct ib_addr sib_addr;
> + __be64 sib_sid;
> + __be64 sib_sid_mask;
> + __u64 sib_scope_id;
> +};
just a nit, maybe reorder the fields to better cope with their IPv6
buddies (where there is
such) from sockaddr_in6?
Also I see that both IPv6 header and IB GRH have a traffic class field
which is skipped in both
cases for the related sockaddr_ structure, not sure why, is this
something the kernel stack decides on and uses but not available for
applications to read/modify?
struct sockaddr_in6 {
unsigned short int sin6_family; /* AF_INET6 */
__be16 sin6_port; /* Transport layer port
# */
__be32 sin6_flowinfo; /* IPv6 flow information */
struct in6_addr sin6_addr; /* IPv6 address */
__u32 sin6_scope_id; /* scope id (new in
RFC2553) */
};
^ permalink raw reply
* Re: [PATCH net-next 1/2] gianfar: remove largely unused gfar_stats struct
From: Claudiu Manoil @ 2013-02-13 13:13 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: David Miller, netdev, Eric Dumazet
In-Reply-To: <1360715064-2689-2-git-send-email-paul.gortmaker@windriver.com>
On 2/13/2013 2:24 AM, Paul Gortmaker wrote:
> The gfar_stats struct is only used in copying out data
> via ethtool. It is declared as the extra stats, followed
> by the rmon stats. However, the rmon stats are never
> actually ever used in the driver; instead the rmon data
> is a u32 register read that is cast directly into the
> ethtool buf.
>
> It seems the only reason rmon is in the struct at all is
> to give the offset(s) at which it should be exported into
> the ethtool buffer. But note gfar_stats doesn't contain
> a gfar_extra_stats as a substruct -- instead it contains
> a u64 array of equal element count. This implicitly means
> we have two independent declarations of what gfar_extra_stats
> really is. Rather than have this duality, we already have
> defines which give us the offset directly, and hence do not
> need the struct at all.
>
> Further, since we know the extra_stats is unconditionally
> always present, we can write it out to the ethtool buf
> 1st, and then optionally write out the rmon data. There
> is no need for two independent loops, both of which are
> simply copying out the extra_stats to buf offset zero.
>
> This also helps pave the way towards allowing the extra
> stats fields to be converted to atomic64_t values, without
> having their types directly influencing the ethtool stats
> export code (gfar_fill_stats) that expects to deal with u64.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Reviewed-by: Claudiu Manoil <claudiu.manoil@freescale.com>
> ---
> drivers/net/ethernet/freescale/gianfar.h | 8 +-------
> drivers/net/ethernet/freescale/gianfar_ethtool.c | 15 ++++++---------
> 2 files changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
> index 71793f4..61b1785 100644
> --- a/drivers/net/ethernet/freescale/gianfar.h
> +++ b/drivers/net/ethernet/freescale/gianfar.h
> @@ -646,15 +646,9 @@ struct gfar_extra_stats {
> #define GFAR_RMON_LEN ((sizeof(struct rmon_mib) - 16)/sizeof(u32))
> #define GFAR_EXTRA_STATS_LEN (sizeof(struct gfar_extra_stats)/sizeof(u64))
>
> -/* Number of stats in the stats structure (ignore car and cam regs)*/
> +/* Number of stats exported via ethtool */
> #define GFAR_STATS_LEN (GFAR_RMON_LEN + GFAR_EXTRA_STATS_LEN)
>
> -struct gfar_stats {
> - u64 extra[GFAR_EXTRA_STATS_LEN];
> - u64 rmon[GFAR_RMON_LEN];
> -};
> -
> -
> struct gfar {
> u32 tsec_id; /* 0x.000 - Controller ID register */
> u32 tsec_id2; /* 0x.004 - Controller ID2 register */
> diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
> index 45e59d5..172acb9 100644
> --- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
> +++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
> @@ -151,18 +151,15 @@ static void gfar_fill_stats(struct net_device *dev, struct ethtool_stats *dummy,
> struct gfar __iomem *regs = priv->gfargrp[0].regs;
> u64 *extra = (u64 *) & priv->extra_stats;
>
> + for (i = 0; i < GFAR_EXTRA_STATS_LEN; i++)
> + buf[i] = extra[i];
> +
> if (priv->device_flags & FSL_GIANFAR_DEV_HAS_RMON) {
> u32 __iomem *rmon = (u32 __iomem *) ®s->rmon;
> - struct gfar_stats *stats = (struct gfar_stats *) buf;
> -
> - for (i = 0; i < GFAR_RMON_LEN; i++)
> - stats->rmon[i] = (u64) gfar_read(&rmon[i]);
>
> - for (i = 0; i < GFAR_EXTRA_STATS_LEN; i++)
> - stats->extra[i] = extra[i];
> - } else
> - for (i = 0; i < GFAR_EXTRA_STATS_LEN; i++)
> - buf[i] = extra[i];
> + for (; i < GFAR_STATS_LEN; i++, rmon++)
> + buf[i] = (u64) gfar_read(rmon);
> + }
> }
>
> static int gfar_sset_count(struct net_device *dev, int sset)
>
^ permalink raw reply
* Re: [PATCH] cfg80211: configuration for WoWLAN over TCP
From: Johannes Berg @ 2013-02-13 13:33 UTC (permalink / raw)
To: linux-wireless; +Cc: netdev
In-Reply-To: <1360596853-13610-1-git-send-email-johannes@sipsolutions.net>
On Mon, 2013-02-11 at 16:34 +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Intel Wireless devices are able to make a TCP connection
> after suspending, sending some data and waking up when
> the connection receives wakeup data (or breaks). Add the
> WoWLAN configuration and feature advertising API for it.
Applied.
johannes
^ permalink raw reply
* Re: [PATCH net-next 5/5] gianfar: Fix and cleanup Rx FCB handling
From: Paul Gortmaker @ 2013-02-13 14:32 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, David S. Miller
In-Reply-To: <511B7A53.3060800@freescale.com>
On 13-02-13 06:34 AM, Claudiu Manoil wrote:
> On 2/12/2013 7:19 PM, Paul Gortmaker wrote:
>> On 13-02-12 07:47 AM, Claudiu Manoil wrote:
>>> NETIF_F_HW_VLAN_TX flag must not condition RxFCB usage.
>>
>> The above statement isn't 100% clear to me. Is this the intent?
>
> The above statement is a rule, if you wish. The existing code breaks
> that rule by saying: RxFCB is enabled if NETIF_F_HW_VLAN_TX; which is
> a false statement. This patch corrects this error, according to the
> rule above.
> So, primarily, this patch is a fix (as expressed in <subj.>). I'll
> resend the patch with additional comments to make this point clearer.
OK, perhaps just:
"...must not condition RxFCB..."
--> "...must not be conditional on RxFCB..."
would have helped; otherwise it reads as if somehow VLAN_TX is
altering/shaping/conditioning the RxFCB behaviour.
>
>>
>> Currently, gfar_uses_fcb() calls gfar_is_vlan_on() which in turn
>> checks NETIF_F_HW_VLAN_TX. However there is no relation between
>> whether FCBs are used and the VLAN transmit state.
>>
>>> In the case of RxBD rings, FCBs (Frame Control Block) are inserted by
>>> the eTSEC whenever RCTRL[PRSDEP] is set to a non-zero value. Only one
>>> FCB is inserted per frame (in the buffer pointed to by the RxBD with
>>> bit F set). TOE acceleration for receive is enabled for all rx frames
>>> in this case.
>>> Indroduce the uses_rxfcb field to signal RxFCB insertion in accordance
>>> with the specification above (leading to cleaner, less confusing code).
>>
>> The is_vlan_on() and uses_fcb() calls were more self documenting than
>> setting/clearing a new single use variable added to priv, I think.
>> Even if they get changed/simplified, perhaps it is worth keeping them?
>
> gfar_uses_fcb() generates confusion around the FCB concept, this maybe
> explains how it came to the error above. First, there are 2 types of
> FCBs with different meaning, covering different use cases: Rx (receive
> side) FCB and TxFCB. uses_fcb() was intended to signal RxFCB insertion,
> which is not obvious from its name, and it became (subtly) erroneous
> after incorporating is_vlan_on().
> is_vlan_on() is also misleading because we need to differentiate b/w
> hw VLAN extraction/VLEX (marked by VLAN_RX flag) and hw VLAN
> insertion/VLINS (VLAN_TX flag), which are different mechanisms using
> different types of FCBs.
OK, so perhaps keep the names, but add prefix (eg. uses_fcb --> uses_rxfcb)
so there is no confusion, and people without the in-depth hardware
knowledge won't fall into the old trap.
>
>>
>> Rather than a specific priv->uses_rxfcb field, perhaps it makes sense
>> to make it more future proof with priv->rctrl field, that is a cached
>> value of the register, and then you keep gfar_uses_fcb() and it in
>> turn checks for RCTRL_PRSDEP_INIT bit in rctrl?
>>
>
> The main purpose of the priv->uses_rxfcb field is to quickly signal, on
> the hot path, that the incoming frame has a *Rx* FCB block inserted
> which needs to be pulled out before passing the skb to the stack.
> This is a performance critical operation, it needs to happen fast,
> that's why uses_rxfcb is placed in the first cacheline of gfar_private.
> This is also why I cannot use a cached rctrl instead: 1) because I
> don't have 32 bits available in the first cacheline of gfar_probe
> (but only 16); 2) no time for bit operations on the hot path.
OK, but don't forget that inlining will still allow you to keep
self documenting names if desired and worthwhile.
>
>> Also, the dependency/conditional on FSL_GIANFAR_DEV_HAS_TIMER seems
>> to simply vanish with this patch, and it isn't clear to me if that
>> was 100% intentional or not...
>>
>
> The dependency on FSL_GIANFAR_DEV_HAS_TIMER is another source of
> confusion. The dependency is actually to priv->hwts_rx_en.
> Upon changing priv->hwts_rx_en via IOCTL, the gfar device is being
> restarted and on init_mac() the priv->hwts_rx_en conditions the RxFCB
> insertion, and rctrl is programmed accordingly. The patch takes care
> of this case too.
>
> So, I'll re-spin this patch with enhanced comments.
Great, that should help a lot.
Thanks,
Paul.
--
>
> Thanks,
> Claudiu
>
>
>
^ permalink raw reply
* Re: dev_add_pack() duplicate skb ?
From: Eric Dumazet @ 2013-02-13 15:05 UTC (permalink / raw)
To: Felipe Dias; +Cc: netdev
In-Reply-To: <CAJX4=r08yC7PPtO7KA-3rsY5SMsZueJ=tVDdi2HT6MvnuO1ung@mail.gmail.com>
On Wed, 2013-02-13 at 10:11 -0200, Felipe Dias wrote:
> Hi, someone might try to tell me what I'm doing wrong, please?
>
>
> I have a simple module and I registered one packet handler, in init function:
> ---
> pseudo_proto.type = htons(ETH_P_ALL);
> pseudo_proto.dev = NULL;
> pseudo_proto.func = packet_handler;
> dev_add_pack(&pseudo_proto);
> ---
>
> My packet_handler, is a simple function just to print in dmesg info
> about the packet:
>
> ---
> ...
>
> switch (ntohs(eth_hdr(skb)->h_proto))
> {
> case ETH_P_IPV6:
> printk(KERN_INFO "%s: (%s %d %s, %s %d) Packet\n",
> THIS_MODULE->name,
> skb->dev->name,
> skb->dev->type,
> ntohs(eth_hdr(skb)->h_proto,
> ipv6_hdr(skb)->nexthdr,
> skb->len);
> break;
> ...
> ---
>
> When I sent a icmpv6 with a ping6 -c1 ::1 , this in my tcpdump show
> only 2 packets (icmp request and icmp reply), but in my dmesg I see
> 4!:
>
> ---
> 595:[30469.099222] test: (lo 772 0x86dd, 0x3a 118) Packet
> 596:[30469.099226] test: (lo 772 0x86dd, 0x3a 104) Packet
> 597:[30469.099234] test: (lo 772 0x86dd, 0x3a 118) Packet
> 598:[30469.099236] test: (lo 772 0x86dd, 0x3a 104) Packet
> ---
>
> This show 4 packets in ipv4 too. What I'm doing wrong ?
The ICMP request is sent, _and_ received : 2 packets
The ICMP response is sent, _and_ received : 2 packets
Total : 4 packets.
af_packet has special code to avoid seeing packets twice on loopback, as
it would be a bit annoying.
^ permalink raw reply
* Re: [PATCH net-next 2/2] gianfar: convert u64 status counters to atomic64_t
From: Claudiu Manoil @ 2013-02-13 15:22 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: David Miller, netdev, Eric Dumazet
In-Reply-To: <1360715064-2689-3-git-send-email-paul.gortmaker@windriver.com>
On 2/13/2013 2:24 AM, Paul Gortmaker wrote:
> While looking at some asm dump for an unrelated change, Eric
> noticed in the following stats count increment code:
>
> 50b8: 81 3c 01 f8 lwz r9,504(r28)
> 50bc: 81 5c 01 fc lwz r10,508(r28)
> 50c0: 31 4a 00 01 addic r10,r10,1
> 50c4: 7d 29 01 94 addze r9,r9
> 50c8: 91 3c 01 f8 stw r9,504(r28)
> 50cc: 91 5c 01 fc stw r10,508(r28)
>
> that a 64 bit counter was used on ppc-32 without sync
> and hence the "ethtool -S" output was racy.
>
> Here we convert all the values to use atomic64_t so that
> the output will always be consistent.
>
At least it seems that this conversion results in fewer asm
instructions, as apparently addze and the double lwz/stw are
not generated anymore. Hopefully it's faster too :P
Reviewed-by: Claudiu Manoil <claudiu.manoil@freescale.com>
Thanks,
Claudiu
^ permalink raw reply
* Re: dev_add_pack() duplicate skb ?
From: Felipe Dias @ 2013-02-13 16:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1360767925.6884.22.camel@edumazet-glaptop>
On Wed, Feb 13, 2013 at 1:05 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2013-02-13 at 10:11 -0200, Felipe Dias wrote:
>> This show 4 packets in ipv4 too. What I'm doing wrong ?
>
> The ICMP request is sent, _and_ received : 2 packets
> The ICMP response is sent, _and_ received : 2 packets
>
> Total : 4 packets.
>
> af_packet has special code to avoid seeing packets twice on loopback, as
> it would be a bit annoying.
Of course. Thank you so much.
Att
fd.
^ permalink raw reply
* Re: [PATCH net-next 2/2] gianfar: convert u64 status counters to atomic64_t
From: Eric Dumazet @ 2013-02-13 16:14 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: Paul Gortmaker, David Miller, netdev
In-Reply-To: <511BAFC4.9060006@freescale.com>
On Wed, 2013-02-13 at 17:22 +0200, Claudiu Manoil wrote:
> At least it seems that this conversion results in fewer asm
> instructions, as apparently addze and the double lwz/stw are
> not generated anymore. Hopefully it's faster too :P
Strange, could you show us these asm instructions ?
^ permalink raw reply
* [PATCH] netpoll: fix smatch warnings in netpoll core code
From: Neil Horman @ 2013-02-13 16:32 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, Dan Carpenter, David S. Miller
Dan Carpenter contacted me with some notes regarding some smatch warnings in the
netpoll code, some of which I introduced with my recent netpoll locking fixes,
some which were there prior. Specifically they were:
net-next/net/core/netpoll.c:243 netpoll_poll_dev() warn: inconsistent
returns mutex:&ni->dev_lock: locked (213,217) unlocked (210,243)
net-next/net/core/netpoll.c:706 netpoll_neigh_reply() warn: potential
pointer math issue ('skb_transport_header(send_skb)' is a 128 bit pointer)
This patch corrects the locking imbalance (the first error), and adds some
parenthesis to correct the second error. Tested by myself. Applies to net-next
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Dan Carpenter <dan.carpenter@oracle.com>
CC: "David S. Miller" <davem@davemloft.net>
---
net/core/netpoll.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index bcfd4f4..fa32899 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -209,12 +209,16 @@ static void netpoll_poll_dev(struct net_device *dev)
if (!mutex_trylock(&ni->dev_lock))
return;
- if (!dev || !netif_running(dev))
+ if (!netif_running(dev)) {
+ mutex_unlock(&ni->dev_lock);
return;
+ }
ops = dev->netdev_ops;
- if (!ops->ndo_poll_controller)
+ if (!ops->ndo_poll_controller) {
+ mutex_unlock(&ni->dev_lock);
return;
+ }
/* Process pending work on NIC */
ops->ndo_poll_controller(dev);
@@ -703,7 +707,7 @@ static void netpoll_neigh_reply(struct sk_buff *skb, struct netpoll_info *npinfo
icmp6h->icmp6_type = NDISC_NEIGHBOUR_ADVERTISEMENT;
icmp6h->icmp6_router = 0;
icmp6h->icmp6_solicited = 1;
- target = (struct in6_addr *)skb_transport_header(send_skb) + sizeof(struct icmp6hdr);
+ target = (struct in6_addr *)(skb_transport_header(send_skb) + sizeof(struct icmp6hdr));
*target = msg->target;
icmp6h->icmp6_cksum = csum_ipv6_magic(saddr, daddr, size,
IPPROTO_ICMPV6,
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH 2/2 v2] sierra_net: fix issues with SYNC/RESTART messages and interrupt pipe setup
From: Dan Williams @ 2013-02-13 16:34 UTC (permalink / raw)
To: Bjørn Mork
Cc: Oliver Neukum, Elina Pasheva, netdev, linux-usb, Rory Filer,
Phil Sutter
In-Reply-To: <87a9r8cub8.fsf@nemi.mork.no>
On Wed, 2013-02-13 at 12:44 +0100, Bjørn Mork wrote:
> Dan Williams <dcbw@redhat.com> writes:
>
> > It doesn't need to run exactly at probe, but it appears to need to be
> > the first thing the driver does when communicating with the firmware to
> > ensure clear state and whatnot. Possibly like the QMI SYNC message that
> > clears all the client IDs and resets the internal stack. (the driver
> > also sends a "shutdown" message to the firmware when unbinding).
> >
> > So I do think that somewhere around probe() is the best time to do this,
> > because it's best to initialize the device when the driver binds to it
> > and react to errors as soon as possible, rather than trying to set
> > everything up on open/IFF_UP and then fail right before you want to
> > actually use the device. Late-fail is quite unhelpful for applications.
> >
> > I don't really care if it happens in probe() or somewhere else right
> > after the driver is bound to the device, but it should be part of the
> > initialization process.
>
> I was looking for something else in the rndis_host driver right now and
> noticed that it has the same sort of problem. The generic_rndis_bind()
> function will call rndis_command() which does
>
> usb_control_msg(.., USB_CDC_SEND_ENCAPSULATED_COMMAND, ..);
> usb_interrupt_msg(.., dev->status->desc.bEndpointAddress, .. );
> for (count = 0; count < 10; count++) {
> usb_control_msg(.., USB_CDC_GET_ENCAPSULATED_RESPONSE, ..);
> if (ok)
> return 0;
> msleep(20);
> }
>
> Somewhat ugly, but it is a way to send and receive commands while
> probing. This driver also sends a command in unbind().
>
> Looks like your patch would allow this to be solved a lot cleaner,
> replacing the loop over USB_CDC_GET_ENCAPSULATED_RESPONSE with proper
> interrupt endpoint handling.
It would end up being more "correct" but more complicated, because you'd
need to have rndis_command() block on a semaphore or something until the
response was processed by a "status" handler, which neither rndis_host.c
or rndis_wlan.c actually implement. The status handler would have to
know that something was waiting on it, and then package up the response
in some way that the rndis_command() (which is now blocking on the
status interrupt) can read it and return it to the caller.
More correct, more code, more complicated... but probably still
worthwhile?
Dan
^ permalink raw reply
* Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id
From: YOSHIFUJI Hideaki @ 2013-02-13 16:47 UTC (permalink / raw)
To: Brian Haley; +Cc: hannes, netdev, YOSHIFUJI Hideaki
In-Reply-To: <511AFF9A.8040506@hp.com>
Brian Haley wrote:
> On 02/12/2013 07:13 PM, Hannes Frederic Sowa wrote:
>>> --- a/net/ipv6/af_inet6.c
>>> +++ b/net/ipv6/af_inet6.c
>>> @@ -323,7 +323,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
>>> struct net_device *dev = NULL;
>>>
>>> rcu_read_lock();
>>> - if (addr_type & IPV6_ADDR_LINKLOCAL) {
>>> + if (__ipv6_addr_needs_scope_id(addr_type)) {
>>> if (addr_len >= sizeof(struct sockaddr_in6) &&
>>> addr->sin6_scope_id) {
>>> /* Override any existing binding, if another one
>>
>> By trying to setup the multicast interface scoped routes by default I
>> just found a bug in this patch essentially breaking ipv6 multicast. I
>> overlooked that ipv6_addr_type strips off the scopes, thus my check if
>> a multicast address needs a scope_id always returns true. I'll check
>> if I can convert the ipv6_addr_type calls to __ipv6_addr_type and will
>> reroll the patch. Sorry, my tests were too focused on interface/local
>> multicast. :(
>
> I'd always thought of adding helper inlines like these in net/ipv6.h:
>
> static inline bool ipv6_addr_linklocal(const struct in6_addr *a)
> {
> return ((a->s6_addr32[0] & htonl(0xFFC00000)) == htonl(0xFE800000));
> }
>
> static inline bool ipv6_addr_mc_linklocal(const struct in6_addr *a)
> {
> return (((a->s6_addr32[0] & htonl(0xFF000000)) == htonl(0xFF000000)) &&
> ((a->s6_addr32[1] & 0x0F) == IPV6_ADDR_SCOPE_LINKLOCAL));
> }
>
> Maybe something like that would help here?
If you have several address checks around, please use ipv6_addr_type()
(or __ipv6_addr_type()). Above "direct" checks should be used only for
single-shot test. But well, I have to agree that ipv6_addr_type and
friends is becoming complex. In mid-term, I would like to take look
at it. I might think of having addr_type for src/dst in skb->cb
after all.
--yoshfuji
^ permalink raw reply
* Re: [PATCH -next 1/1] net: skbuff: fix compile error in skb_panic()
From: David Miller @ 2013-02-13 16:52 UTC (permalink / raw)
To: sedat.dilek; +Cc: james.hogan, sakiwit, linux-kernel, linux-next, jiri, netdev
In-Reply-To: <CA+icZUVB9QaZfnKGSz_Dr=EADBtYULOHqEHDDocdXO5-Dy=uYA@mail.gmail.com>
From: Sedat Dilek <sedat.dilek@gmail.com>
Date: Wed, 13 Feb 2013 13:41:21 +0100
> On Wed, Feb 13, 2013 at 12:20 PM, James Hogan <james.hogan@imgtec.com> wrote:
>> I get the following build error on next-20130213 due to the following
>> commit:
>>
>> commit f05de73bf82fbbc00265c06d12efb7273f7dc54a ("skbuff: create
>> skb_panic() function and its wrappers").
>>
>> It adds an argument called panic to a function that uses the BUG() macro
>> which tries to call panic, but the argument masks the panic() function
>> declaration, resulting in the following error (gcc 4.2.4):
>>
>> net/core/skbuff.c In function 'skb_panic':
>> net/core/skbuff.c +126 : error: called object 'panic' is not a function
>>
>
> [ CC linux-netdev ]
>
> Hmm, with gcc-4.6.3 on Ubuntu/precise I don't see this... compiler-specific?
Kconfig option specific.
Applied, thanks James.
^ permalink raw reply
* Re: [PATCH] netpoll: fix smatch warnings in netpoll core code
From: David Miller @ 2013-02-13 16:57 UTC (permalink / raw)
To: nhorman; +Cc: netdev, dan.carpenter
In-Reply-To: <1360773162-11949-1-git-send-email-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Wed, 13 Feb 2013 11:32:42 -0500
> Dan Carpenter contacted me with some notes regarding some smatch warnings in the
> netpoll code, some of which I introduced with my recent netpoll locking fixes,
> some which were there prior. Specifically they were:
>
> net-next/net/core/netpoll.c:243 netpoll_poll_dev() warn: inconsistent
> returns mutex:&ni->dev_lock: locked (213,217) unlocked (210,243)
> net-next/net/core/netpoll.c:706 netpoll_neigh_reply() warn: potential
> pointer math issue ('skb_transport_header(send_skb)' is a 128 bit pointer)
>
> This patch corrects the locking imbalance (the first error), and adds some
> parenthesis to correct the second error. Tested by myself. Applies to net-next
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks Neil.
Again, please specify in the Subject line the destination tree.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox