* Re: Soft lockup in tc_classify
From: Cong Wang @ 2016-12-19 17:58 UTC (permalink / raw)
To: Shahar Klein
Cc: Or Gerlitz, Daniel Borkmann, Linux Netdev List, Roi Dayan,
David Miller, Jiri Pirko, John Fastabend, Hadar Hen Zion
In-Reply-To: <18a64d65-1241-6c72-8333-47b0ae933139@mellanox.com>
[-- Attachment #1: Type: text/plain, Size: 1719 bytes --]
Hello,
On Mon, Dec 19, 2016 at 8:39 AM, Shahar Klein <shahark@mellanox.com> wrote:
>
>
> On 12/13/2016 12:51 AM, Cong Wang wrote:
>>
>> On Mon, Dec 12, 2016 at 1:18 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>
>>> On Mon, Dec 12, 2016 at 3:28 PM, Daniel Borkmann <daniel@iogearbox.net>
>>> wrote:
>>>
>>>> Note that there's still the RCU fix missing for the deletion race that
>>>> Cong will still send out, but you say that the only thing you do is to
>>>> add a single rule, but no other operation in involved during that test?
>>>
>>>
>>> What's missing to have the deletion race fixed? making a patch or
>>> testing to a patch which was sent?
>>
>>
>> If you think it would help for this problem, here is my patch rebased
>> on the latest net-next.
>>
>> Again, I don't see how it could help this case yet, especially I don't
>> see how we could have a loop in this singly linked list.
>>
>
> I've applied cong's patch and hit a different lockup(full log attached):
Are you sure this is really different? For me, it is still inside the loop
in tc_classify(), with only a slightly different offset.
>
> Daniel suggested I'll add a print:
> case RTM_DELTFILTER:
> - err = tp->ops->delete(tp, fh);
> + printk(KERN_ERR "DEBUGG:SK %s:%d\n", __func__, __LINE__);
> + err = tp->ops->delete(tp, fh, &last);
> if (err == 0) {
>
> and I couldn't see this print in the output.....
Hmm, that is odd, if this never prints, then my patch should not make any
difference.
There are still two other cases where we could change tp->next, so do you
mind to add two more printk's for debugging?
Attached is the delta patch.
Thanks!
[-- Attachment #2: tc-filter-debug.diff --]
[-- Type: text/plain, Size: 880 bytes --]
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index f9179e0..45bfe9f 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -317,6 +317,8 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
if (n->nlmsg_type == RTM_DELTFILTER && t->tcm_handle == 0) {
struct tcf_proto *next = rtnl_dereference(tp->next);
+ printk(KERN_ERR "DEBUGG:SK delete filter by: %pf\n", tp->ops->get);
+
RCU_INIT_POINTER(*back, next);
tfilter_notify(net, skb, n, tp, fh,
@@ -370,6 +372,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
n->nlmsg_flags & NLM_F_CREATE ? TCA_ACT_NOREPLACE : TCA_ACT_REPLACE);
if (err == 0) {
if (tp_created) {
+ printk(KERN_ERR "DEBUGG:SK add/change filter by: %pf\n", tp->ops->change);
RCU_INIT_POINTER(tp->next, rtnl_dereference(*back));
rcu_assign_pointer(*back, tp);
}
^ permalink raw reply related
* Re: ovs internal_dev mtu
From: Cong Wang @ 2016-12-19 18:01 UTC (permalink / raw)
To: nickcooper-zhangtonghao; +Cc: Linux Kernel Network Developers
In-Reply-To: <E0899132-14F7-45F9-8F02-94421B595D4E@gmail.com>
On Mon, Dec 19, 2016 at 12:59 AM, nickcooper-zhangtonghao
<nic.tongh@gmail.com> wrote:
> The upstream net-next has removed the “internal_dev_change_mtu”.
> It will be ok for ovs(e.g. mtu_request) ? I think that code should
> be keep. Can you provide more information ？
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=91572088e3fdbf4fe31cf397926d8b890fdb3237
^ permalink raw reply
* RE: [PATCH v5 1/4] siphash: add cryptographically secure PRF
From: George Spelvin @ 2016-12-19 18:10 UTC (permalink / raw)
To: David.Laight, linux, tom
Cc: ak, davem, djb, ebiggers3, hannes, Jason, jeanphilippe.aumasson,
kernel-hardening, linux-crypto, linux-kernel, luto, netdev,
torvalds, tytso, vegard.nossum
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0242669@AcuExch.aculab.com>
David Laight wrote:
> From: George Spelvin
...
>> uint32_t
>> hsiphash24(char const *in, size_t len, uint32_t const key[2])
>> {
>> uint32_t c = key[0];
>> uint32_t d = key[1];
>> uint32_t a = 0x6c796765 ^ 0x736f6d65;
>> uint32_t b = d ^ 0x74656462 ^ 0x646f7261;
> I've not looked closely, but is that (in some sense) duplicating
> the key length?
> So you could set a = key[2] and b = key[3] and still have an
> working hash - albeit not exactly the one specified.
That's tempting, but not necessarily effective. (A similar unsuccesful
idea can be found in discussions of "DES with independent round keys".
Or see the design discussion of Salsa20 and the constants in its input.)
You can increase the key size, but that might not increase the *security*
any.
The big issue is that there are a *lot* of square root attack in
cryptanalysis. Because SipHash's state is twice the size of the key,
such an attack will have the same complexity as key exhaustion and need
not be considered. To make a stronger security claim, you need to start
working through them all and show that they don't apply.
For SipHash in particular, an important property is asymmetry of the
internal state. That's what duplicating the key with XORs guarantees.
If the two halves of the state end up identical, the mixing is much
weaker.
Now the probability of ending up in a "mirror state" is the square
root of the state size (1 in 2^64 for HalfSipHash's 128-bit state),
which is the same probability as guessing a key, so it's not a
problem that has to be considered when making a 64-bit security claim.
But if you want a higher security level, you have to think about
what can happen.
That said, I have been thinking very hard about
a = c ^ 0x48536970; /* 'HSip' */
d = key[2];
By guaranteeing that a and c are different, we get the desired
asymmetry, and the XOR of b and d is determined by the first word of
the message anyway, so this isn't weakening anything.
96 bits is far beyond the reach of any brute-force attack, and if a
more sophisticated 64-bit attack exists, it's at least out of the reach
of the script kiddies, and will almost certainly have a non-negligible
constant factor and more limits in when it can be applied.
> Is it worth using the 32bit hash for IP addresses on 64bit systems that
> can't do misaligned accessed?
Not a good idea. To hash 64 bits of input:
* Full SipHash has to do two loads, a shift, an or, and two rounds of mixing.
* HalfSipHash has to do a load, two rounds, another load, and two more rounds.
In other words, in addition to being less secure, it's half the speed.
Also, what systems are you thinking about? x86, ARMv8, PowerPC, and
S390 (and ia64, if anyone cares) all handle unaligned loads. MIPS has
efficient support. Alpha and HPPA are for retrocomputing fans, not
people who care about performance.
So you're down to SPARC. Which conveniently has the same maintainer as
the networking code, so I figure DaveM can take care of that himself. :-)
^ permalink raw reply
* Re: [upstream-release] [PATCH net v3 2/4] powerpc: fsl/fman: remove fsl, fman from of_device_ids[]
From: Scott Wood @ 2016-12-19 19:46 UTC (permalink / raw)
To: madalin.bucur, netdev; +Cc: mpe, linuxppc-dev, linux-kernel, davem
In-Reply-To: <1482164013-6111-3-git-send-email-madalin.bucur@nxp.com>
On Mon, 2016-12-19 at 18:13 +0200, Madalin Bucur wrote:
> The fsl/fman drivers will use of_platform_populate() on all
> supported platforms. Call of_platform_populate() to probe the
> FMan sub-nodes.
>
> Signed-off-by: Igal Liberman <igal.liberman@freescale.com>
> Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> ---
> arch/powerpc/platforms/85xx/corenet_generic.c | 3 ---
> drivers/net/ethernet/freescale/fman/fman.c | 8 ++++++++
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c
> b/arch/powerpc/platforms/85xx/corenet_generic.c
> index 1179115..824b7f1 100644
> --- a/arch/powerpc/platforms/85xx/corenet_generic.c
> +++ b/arch/powerpc/platforms/85xx/corenet_generic.c
> @@ -117,9 +117,6 @@ static const struct of_device_id of_device_ids[] = {
> {
> .compatible = "fsl,qe",
> },
> - {
> - .compatible = "fsl,fman",
> - },
> /* The following two are for the Freescale hypervisor */
> {
> .name = "hypervisor",
For this part:
Acked-by: Scott Wood <oss@buserror.net>
> diff --git a/drivers/net/ethernet/freescale/fman/fman.c
> b/drivers/net/ethernet/freescale/fman/fman.c
> index dafd9e1..0b7f711 100644
> --- a/drivers/net/ethernet/freescale/fman/fman.c
> +++ b/drivers/net/ethernet/freescale/fman/fman.c
> @@ -2868,6 +2868,14 @@ static struct fman *read_dts_node(struct
> platform_device *of_dev)
>
> fman->dev = &of_dev->dev;
>
> + /* call of_platform_populate in order to probe sub-nodes on arm64
> */
> + err = of_platform_populate(fm_node, NULL, NULL, &of_dev->dev);
> + if (err) {
> + dev_err(&of_dev->dev, "%s: of_platform_populate()
> failed\n",
> + __func__);
> + goto fman_free;
> + }
The "on arm64" comment is no longer accurate (and the rest of the comment
seems unnecessary).
-Scott
^ permalink raw reply
* Re: ipv6: handle -EFAULT from skb_copy_bits
From: David Miller @ 2016-12-19 19:48 UTC (permalink / raw)
To: davej; +Cc: netdev
In-Reply-To: <20161219170320.nnezhql2vj5mrrrp@codemonkey.org.uk>
From: Dave Jones <davej@codemonkey.org.uk>
Date: Mon, 19 Dec 2016 12:03:20 -0500
> On Sat, Dec 17, 2016 at 10:41:20AM -0500, David Miller wrote:
>
> > > It seems to be possible to craft a packet for sendmsg that triggers
> > > the -EFAULT path in skb_copy_bits resulting in a BUG_ON that looks like:
> > >
> > > RIP: 0010:[<ffffffff817c6390>] [<ffffffff817c6390>] rawv6_sendmsg+0xc30/0xc40
> > > RSP: 0018:ffff881f6c4a7c18 EFLAGS: 00010282
> > > RAX: 00000000fffffff2 RBX: ffff881f6c681680 RCX: 0000000000000002
> > > RDX: ffff881f6c4a7cf8 RSI: 0000000000000030 RDI: ffff881fed0f6a00
> > > RBP: ffff881f6c4a7da8 R08: 0000000000000000 R09: 0000000000000009
> > > R10: ffff881fed0f6a00 R11: 0000000000000009 R12: 0000000000000030
> > > R13: ffff881fed0f6a00 R14: ffff881fee39ba00 R15: ffff881fefa93a80
> > >
> > > Call Trace:
> > > [<ffffffff8118ba23>] ? unmap_page_range+0x693/0x830
> > > [<ffffffff81772697>] inet_sendmsg+0x67/0xa0
> > > [<ffffffff816d93f8>] sock_sendmsg+0x38/0x50
> > > [<ffffffff816d982f>] SYSC_sendto+0xef/0x170
> > > [<ffffffff816da27e>] SyS_sendto+0xe/0x10
> > > [<ffffffff81002910>] do_syscall_64+0x50/0xa0
> > > [<ffffffff817f7cbc>] entry_SYSCALL64_slow_path+0x25/0x25
> > >
> > > Handle this in rawv6_push_pending_frames and jump to the failure path.
> > >
> > > Signed-off-by: Dave Jones <davej@codemonkey.org.uk>
> >
> > Hmmm, that's interesting. Becaue the code in __ip6_append_data(), which
> > sets up the ->cork.base.length value, seems to be defensively trying to
> > avoid this possibility.
> >
> > For example, it checks things like:
> >
> > if (cork->length + length > mtu - headersize && ipc6->dontfrag &&
> > (sk->sk_protocol == IPPROTO_UDP ||
> > sk->sk_protocol == IPPROTO_RAW)) {
> >
> > This is why the transport offset plus the length should never exceed
> > the total length for that skb_copy_bits() call.
> >
> > Perhaps this protocol check in the code above is incomplete? Do you
> > know what the sk->sk_protocol value was when that BUG triggered? That
> > might shine some light on what is really happening here.
>
> Hm.
> sk_protocol = 7,
So, some arbitrary value.
Obviously, the test above intends to handle RAW sockets and that is exactly
what we have here.
A raw socket gets whatever the user specified as 'protocol' at create
time as the sk->sk_protocol value.
One thing that's interesting is that if the user picks "IPPROTO_RAW"
as the value of 'protocol' we set inet->hdrincl to 1.
The user can also set inet->hdrincl to 1 or 0 via setsockopt().
I think this is part of the problem. The test above means to check
for "RAW socket with hdrincl set" and is trying to do this more simply.
But because setsockopt() can set this arbitrarily, testing sk_protocol
alone isn't enough.
So changing:
sk->sk_protocol == IPPROTO_RAW
into something like:
(sk->sk_socket->type == SOCK_RAW && inet_sk(sk)->hdrincl)
should correct the test.
That is because this hdrincl thing is what controls which code path we
take in rawv6_sendmsg():
if (inet->hdrincl)
err = rawv6_send_hdrinc(sk, msg, len, &fl6, &dst, msg->msg_flags);
else {
ipc6.opt = opt;
lock_sock(sk);
err = ip6_append_data(sk, raw6_getfrag, &rfv,
len, 0, &ipc6, &fl6, (struct rt6_info *)dst,
msg->msg_flags, &sockc);
if (err)
ip6_flush_pending_frames(sk);
else if (!(msg->msg_flags & MSG_MORE))
err = rawv6_push_pending_frames(sk, &fl6, rp);
release_sock(sk);
}
You can test if the change I suggest above works.
^ permalink raw reply
* Re: [PATCH net] virtio_net: reject XDP programs using header adjustment
From: David Miller @ 2016-12-19 20:08 UTC (permalink / raw)
To: john.fastabend
Cc: jakub.kicinski, netdev, kafai, daniel, alexei.starovoitov, mst
In-Reply-To: <58581E01.7070902@gmail.com>
From: John Fastabend <john.fastabend@gmail.com>
Date: Mon, 19 Dec 2016 09:50:57 -0800
> On 16-12-19 07:05 AM, Jakub Kicinski wrote:
>> commit 17bedab27231 ("bpf: xdp: Allow head adjustment in XDP prog")
>> added a new XDP helper to prepend and remove data from a frame.
>> Make virtio_net reject programs making use of this helper until
>> proper support is added.
>>
>> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>> ---
>> drivers/net/virtio_net.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 08327e005ccc..db761f37783e 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1677,6 +1677,11 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
>> u16 xdp_qp = 0, curr_qp;
>> int i, err;
>>
>> + if (prog && prog->xdp_adjust_head) {
>> + netdev_warn(dev, "Does not support bpf_xdp_adjust_head()\n");
>> + return -EOPNOTSUPP;
>> + }
>> +
>> if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
>> virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6)) {
>> netdev_warn(dev, "can't set XDP while host is implementing LRO, disable LRO first\n");
>>
>
> Acked-by: John Fastabend <john.r.fastabend@intel.com>
>
> Thanks patch looks good. Alternatively we could push a "bug fix" to
> support the adjust header feature depending on how DaveM and MST feel
> about that. I don't have a strong opinion but I have the patch on my
> queue it just needs some more testing. The adjust header bits were
> merged in between some of the later versions of the patch which is how
> this check got missed.
I would like to avoid inconsistent XDP feature support amongst drivers
as much as possible.
^ permalink raw reply
* Re: Soft lockup in tc_classify
From: Shahar Klein @ 2016-12-19 16:39 UTC (permalink / raw)
To: Cong Wang, Or Gerlitz
Cc: shahark, Daniel Borkmann, Linux Netdev List, Roi Dayan,
David Miller, Jiri Pirko, John Fastabend, Hadar Hen Zion,
Daniel Borkmann
In-Reply-To: <CAM_iQpVJ_Y5bB-RP2S2tK7sPNo6Atwcz5Ud8sG6bwDOSnq4NnA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]
On 12/13/2016 12:51 AM, Cong Wang wrote:
> On Mon, Dec 12, 2016 at 1:18 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> On Mon, Dec 12, 2016 at 3:28 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>>> Note that there's still the RCU fix missing for the deletion race that
>>> Cong will still send out, but you say that the only thing you do is to
>>> add a single rule, but no other operation in involved during that test?
>>
>> What's missing to have the deletion race fixed? making a patch or
>> testing to a patch which was sent?
>
> If you think it would help for this problem, here is my patch rebased
> on the latest net-next.
>
> Again, I don't see how it could help this case yet, especially I don't
> see how we could have a loop in this singly linked list.
>
I've applied cong's patch and hit a different lockup(full log attached):
[ 264.725414] RIP: 0010:fl_classify+0x1f1/0x2b0 [cls_flower]
(gdb) list *(fl_classify+0x1f1)
0x1591 is in fl_classify (net/sched/cls_flower.c:187).
182 if (f && !tc_skip_sw(f->flags)) {
183 *res = f->res;
184 return tcf_exts_exec(skb, &f->exts, res);
185 }
186 return -1;
187 }
188
189 static int fl_init(struct tcf_proto *tp)
190 {
191 struct cls_fl_head *head;
(gdb)
Daniel suggested I'll add a print:
case RTM_DELTFILTER:
- err = tp->ops->delete(tp, fh);
+ printk(KERN_ERR "DEBUGG:SK %s:%d\n", __func__, __LINE__);
+ err = tp->ops->delete(tp, fh, &last);
if (err == 0) {
and I couldn't see this print in the output.....
[-- Attachment #2: lockup_with_congs_patch --]
[-- Type: text/plain, Size: 16806 bytes --]
[ 238.507670] GACT probability on
[ 264.573942] NMI watchdog: BUG: soft lockup - CPU#0 stuck for 22s! [modprobe:4867]
[ 264.582875] Modules linked in: act_gact act_mirred openvswitch nf_conntrack_ipv6 nf_nat_ipv6 nf_defrag_ipv6 vfio_pci vfio_virqfd vfio_iommu_type1 vfio cls_flower mlx5_ib mlx5_core devlink sch_ingress nfsv3 nfs fscache xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat libcrc32c nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack tun ebtable_filter ebtables ip6table_filter ip6_tables netconsole bridge stp llc rpcrdma ib_isert iscsi_target_mod ib_iser libiscsi scsi_transport_iscsi ib_srpt ib_srp scsi_transport_srp ib_ipoib rdma_ucm ib_ucm ib_uverbs ib_umad rdma_cm ib_cm iw_cm intel_rapl sb_edac edac_core x86_pkg_temp_thermal coretemp kvm_intel kvm ib_core irqbypass crct10dif_pclmul crc32_pclmul igb ptp iTCO_wdt pps_core crc32c_intel joydev i2c_algo_bit ghash_clmulni_intel iTCO_vendor_support pcspkr i2c_i801 mei_me ipmi_ssif ioatdma shpchp tpm_tis i2c_smbus tpm_tis_core dca tpm mei lpc_ich ipmi_si ipmi_msghandler wmi nfsd target_core_mod auth_rpcgss nfs_acl lockd grace sunrpc isci libsas serio_raw scsi_transport_sas [last unloaded: devlink]
[ 264.703539] CPU: 0 PID: 4867 Comm: modprobe Not tainted 4.9.0+ #27
[ 264.710766] Hardware name: Supermicro X9DRW/X9DRW, BIOS 3.0a 08/08/2013
[ 264.718475] task: ffff89916ad09d80 task.stack: ffffa36f44510000
[ 264.725414] RIP: 0010:fl_classify+0x1f1/0x2b0 [cls_flower]
[ 264.731863] RSP: 0018:ffff89916fa03ad8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff10
[ 264.740971] RAX: 00000000ffffffff RBX: ffff89913f522100 RCX: 0000000000000000
[ 264.749265] RDX: ffff89916fa03c98 RSI: ffff89915bd743c0 RDI: ffff89913f522100
[ 264.757558] RBP: ffff89916fa03c28 R08: 000000000000270f R09: 0000000000000000
[ 264.765852] R10: 0000000000000000 R11: 0000000000000004 R12: ffff89916fa03c98
[ 264.774144] R13: ffff89913ecc7c00 R14: ffff89915bd743c0 R15: 0000000000000001
[ 264.782440] FS: 00007f74f2f5e700(0000) GS:ffff89916fa00000(0000) knlGS:0000000000000000
[ 264.792066] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 264.798809] CR2: 00007f74f2491480 CR3: 000000043f3f8000 CR4: 00000000000426f0
[ 264.807102] Call Trace:
[ 264.810151] <IRQ>
[ 264.812717] ? handle_edge_irq+0x87/0x130
[ 264.817504] ? handle_irq+0xab/0x130
[ 264.821808] ? irq_exit+0x77/0xe0
[ 264.825828] ? irq_exit+0x77/0xe0
[ 264.829849] ? do_IRQ+0x51/0xd0
[ 264.833664] ? common_interrupt+0x93/0x93
[ 264.838462] tc_classify+0x78/0x120
[ 264.842679] __netif_receive_skb_core+0x623/0xa00
[ 264.848264] ? udp4_gro_receive+0x10b/0x2d0
[ 264.853253] __netif_receive_skb+0x18/0x60
[ 264.858150] netif_receive_skb_internal+0x40/0xb0
[ 264.863723] napi_gro_receive+0xcd/0x120
[ 264.868437] mlx5e_handle_rx_cqe_rep+0x61b/0x890 [mlx5_core]
[ 264.875105] ? kvm_irq_delivery_to_apic+0x2c0/0x2c0 [kvm]
[ 264.881462] mlx5e_poll_rx_cq+0x83/0x840 [mlx5_core]
[ 264.887333] mlx5e_napi_poll+0x89/0x480 [mlx5_core]
[ 264.893102] net_rx_action+0x260/0x3c0
[ 264.897610] __do_softirq+0xc9/0x28c
[ 264.901922] irq_exit+0xd7/0xe0
[ 264.905748] do_IRQ+0x51/0xd0
[ 264.909380] common_interrupt+0x93/0x93
[ 264.913984] RIP: 0010:mpihelp_submul_1+0x91/0xe0
[ 264.919460] RSP: 0018:ffffa36f44513940 EFLAGS: 00000206 ORIG_RAX: ffffffffffffffa2
[ 264.928565] RAX: 5414d8fd4127cc59 RBX: 0000000100000000 RCX: ffffffffffffff50
[ 264.936860] RDX: 00000000ffffffea RSI: 0000000019ba7e4d RDI: 0000000068ef6b86
[ 264.945146] RBP: ffffa36f44513960 R08: 352cbbd4573982a8 R09: 64589019128fea22
[ 264.953440] R10: 128fea2200000000 R11: 352cbbd5573982a8 R12: ffff89955c585c00
[ 264.961732] R13: ffff899134dbfbd0 R14: 66a4c31fb80cf128 R15: d7554d389539f6c8
[ 264.970028] </IRQ>
[ 264.972688] mpihelp_divrem+0x23b/0x740
[ 264.977291] mpi_powm+0x471/0xa10
[ 264.981312] rsa_verify+0xa5/0x130
[ 264.985421] pkcs1pad_verify+0xb9/0xf0
[ 264.989925] public_key_verify_signature+0x1f7/0x280
[ 264.995790] public_key_verify_signature_2+0x15/0x20
[ 265.001656] verify_signature+0x3c/0x50
[ 265.006259] pkcs7_validate_trust+0xa1/0x200
[ 265.011352] verify_pkcs7_signature+0x9a/0x140
[ 265.016636] mod_verify_sig+0x92/0xe0
[ 265.021046] load_module+0x171/0x2810
[ 265.025456] ? vfs_read+0x113/0x130
[ 265.029668] ? kernel_read_file+0x158/0x190
[ 265.034659] ? kernel_read_file_from_fd+0x49/0x80
[ 265.040248] SYSC_finit_module+0xa6/0xf0
[ 265.044947] SyS_finit_module+0xe/0x10
[ 265.049452] entry_SYSCALL_64_fastpath+0x1a/0xa9
[ 265.054931] RIP: 0033:0x7f74f2439239
[ 265.059241] RSP: 002b:00007ffeb8b23218 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[ 265.068373] RAX: ffffffffffffffda RBX: 00007ffeb8b21f10 RCX: 00007f74f2439239
[ 265.076665] RDX: 0000000000000000 RSI: 00005578b61d02a6 RDI: 0000000000000000
[ 265.084958] RBP: 00007ffeb8b21f00 R08: 0000000000000000 R09: 0000000000000000
[ 265.098996] R10: 0000000000000000 R11: 0000000000000246 R12: 00005578b61d1be7
[ 265.107289] R13: 00007ffeb8b22088 R14: 0000000000000000 R15: 00005578b61d1bf3
[ 265.115582] Code: 02 75 e2 48 8b 81 98 00 00 00 48 8b 91 a0 00 00 00 48 8b 7c 24 18 48 89 07 48 89 57 08 8b 81 84 00 00 00 85 c0 0f 85 86 00 00 00 <48> 8b 9c 24 20 01 00 00 65 48 33 1c 25 28 00 00 00 0f 85 a1 00
[ 265.143394] Kernel panic - not syncing: softlockup: hung tasks
[ 265.150231] CPU: 0 PID: 4867 Comm: modprobe Tainted: G L 4.9.0+ #27
[ 265.159057] Hardware name: Supermicro X9DRW/X9DRW, BIOS 3.0a 08/08/2013
[ 265.166766] Call Trace:
[ 265.169811] <IRQ>
[ 265.172374] dump_stack+0x63/0x8c
[ 265.176393] panic+0xeb/0x239
[ 265.180047] watchdog_timer_fn+0x1e5/0x1f0
[ 265.184941] ? watchdog+0x40/0x40
[ 265.188960] __hrtimer_run_queues+0xee/0x270
[ 265.194050] hrtimer_interrupt+0xa8/0x190
[ 265.198849] local_apic_timer_interrupt+0x35/0x60
[ 265.204422] smp_apic_timer_interrupt+0x38/0x50
[ 265.209802] apic_timer_interrupt+0x93/0xa0
[ 265.214794] RIP: 0010:fl_classify+0x1f1/0x2b0 [cls_flower]
[ 265.221242] RSP: 0018:ffff89916fa03ad8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff10
[ 265.230339] RAX: 00000000ffffffff RBX: ffff89913f522100 RCX: 0000000000000000
[ 265.238635] RDX: ffff89916fa03c98 RSI: ffff89915bd743c0 RDI: ffff89913f522100
[ 265.246929] RBP: ffff89916fa03c28 R08: 000000000000270f R09: 0000000000000000
[ 265.255223] R10: 0000000000000000 R11: 0000000000000004 R12: ffff89916fa03c98
[ 265.263516] R13: ffff89913ecc7c00 R14: ffff89915bd743c0 R15: 0000000000000001
[ 265.271812] ? handle_edge_irq+0x87/0x130
[ 265.276599] ? handle_irq+0xab/0x130
[ 265.280902] ? irq_exit+0x77/0xe0
[ 265.284911] ? irq_exit+0x77/0xe0
[ 265.288927] ? do_IRQ+0x51/0xd0
[ 265.292752] ? common_interrupt+0x93/0x93
[ 265.297549] tc_classify+0x78/0x120
[ 265.301762] __netif_receive_skb_core+0x623/0xa00
[ 265.307336] ? udp4_gro_receive+0x10b/0x2d0
[ 265.312328] __netif_receive_skb+0x18/0x60
[ 265.317221] netif_receive_skb_internal+0x40/0xb0
[ 265.322795] napi_gro_receive+0xcd/0x120
[ 265.327502] mlx5e_handle_rx_cqe_rep+0x61b/0x890 [mlx5_core]
[ 265.334153] ? kvm_irq_delivery_to_apic+0x2c0/0x2c0 [kvm]
[ 265.340510] mlx5e_poll_rx_cq+0x83/0x840 [mlx5_core]
[ 265.346379] mlx5e_napi_poll+0x89/0x480 [mlx5_core]
[ 265.352170] net_rx_action+0x260/0x3c0
[ 265.356675] __do_softirq+0xc9/0x28c
[ 265.360987] irq_exit+0xd7/0xe0
[ 265.364814] do_IRQ+0x51/0xd0
[ 265.368440] common_interrupt+0x93/0x93
[ 265.373041] RIP: 0010:mpihelp_submul_1+0x91/0xe0
[ 265.378516] RSP: 0018:ffffa36f44513940 EFLAGS: 00000206 ORIG_RAX: ffffffffffffffa2
[ 265.387614] RAX: 5414d8fd4127cc59 RBX: 0000000100000000 RCX: ffffffffffffff50
[ 265.395921] RDX: 00000000ffffffea RSI: 0000000019ba7e4d RDI: 0000000068ef6b86
[ 265.404235] RBP: ffffa36f44513960 R08: 352cbbd4573982a8 R09: 64589019128fea22
[ 265.412526] R10: 128fea2200000000 R11: 352cbbd5573982a8 R12: ffff89955c585c00
[ 265.420819] R13: ffff899134dbfbd0 R14: 66a4c31fb80cf128 R15: d7554d389539f6c8
[ 265.429113] </IRQ>
[ 265.431769] mpihelp_divrem+0x23b/0x740
[ 265.436373] mpi_powm+0x471/0xa10
[ 265.440394] rsa_verify+0xa5/0x130
[ 265.444508] pkcs1pad_verify+0xb9/0xf0
[ 265.449014] public_key_verify_signature+0x1f7/0x280
[ 265.454878] public_key_verify_signature_2+0x15/0x20
[ 265.460741] verify_signature+0x3c/0x50
[ 265.465343] pkcs7_validate_trust+0xa1/0x200
[ 265.470430] verify_pkcs7_signature+0x9a/0x140
[ 265.475715] mod_verify_sig+0x92/0xe0
[ 265.480147] load_module+0x171/0x2810
[ 265.484555] ? vfs_read+0x113/0x130
[ 265.488758] ? kernel_read_file+0x158/0x190
[ 265.493751] ? kernel_read_file_from_fd+0x49/0x80
[ 265.499328] SYSC_finit_module+0xa6/0xf0
[ 265.504051] SyS_finit_module+0xe/0x10
[ 265.508555] entry_SYSCALL_64_fastpath+0x1a/0xa9
[ 265.514030] RIP: 0033:0x7f74f2439239
[ 265.518340] RSP: 002b:00007ffeb8b23218 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[ 265.527446] RAX: ffffffffffffffda RBX: 00007ffeb8b21f10 RCX: 00007f74f2439239
[ 265.535740] RDX: 0000000000000000 RSI: 00005578b61d02a6 RDI: 0000000000000000
[ 265.544056] RBP: 00007ffeb8b21f00 R08: 0000000000000000 R09: 0000000000000000
[ 265.552354] R10: 0000000000000000 R11: 0000000000000246 R12: 00005578b61d1be7
[ 265.560648] R13: 00007ffeb8b22088 R14: 0000000000000000 R15: 00005578b61d1bf3
[ 265.568994] Kernel Offset: 0x3b000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[ 266.845267] ---[ end Kernel panic - not syncing: softlockup: hung tasks
[ 266.853000] ------------[ cut here ]------------
[ 266.858490] WARNING: CPU: 0 PID: 4867 at arch/x86/kernel/smp.c:127 native_smp_send_reschedule+0x3f/0x50
[ 266.869551] Modules linked in: act_gact act_mirred openvswitch nf_conntrack_ipv6 nf_nat_ipv6 nf_defrag_ipv6 vfio_pci vfio_virqfd vfio_iommu_type1 vfio cls_flower mlx5_ib mlx5_core devlink sch_ingress nfsv3 nfs fscache xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat libcrc32c nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack tun ebtable_filter ebtables ip6table_filter ip6_tables netconsole bridge stp llc rpcrdma ib_isert iscsi_target_mod ib_iser libiscsi scsi_transport_iscsi ib_srpt ib_srp scsi_transport_srp ib_ipoib rdma_ucm ib_ucm ib_uverbs ib_umad rdma_cm ib_cm iw_cm intel_rapl sb_edac edac_core x86_pkg_temp_thermal coretemp kvm_intel kvm ib_core irqbypass crct10dif_pclmul crc32_pclmul igb ptp iTCO_wdt pps_core crc32c_intel joydev i2c_algo_bit ghash_clmulni_intel iTCO_vendor_support pcspkr i2c_i801 mei_me ipmi_ssif ioatdma shpchp tpm_tis i2c_smbus tpm_tis_core dca tpm mei lpc_ich ipmi_si ipmi_msghandler wmi nfsd target_core_mod auth_rpcgss nfs_acl lockd grace sunrpc isci libsas serio_raw scsi_transport_sas [last unloaded: devlink]
[ 266.990566] CPU: 0 PID: 4867 Comm: modprobe Tainted: G L 4.9.0+ #27
[ 266.999401] Hardware name: Supermicro X9DRW/X9DRW, BIOS 3.0a 08/08/2013
[ 267.007118] Call Trace:
[ 267.010179] <IRQ>
[ 267.012748] dump_stack+0x63/0x8c
[ 267.016783] __warn+0xd1/0xf0
[ 267.020420] warn_slowpath_null+0x1d/0x20
[ 267.025314] native_smp_send_reschedule+0x3f/0x50
[ 267.030895] try_to_wake_up+0x312/0x390
[ 267.035501] wake_up_process+0x15/0x20
[ 267.040009] swake_up_locked+0x24/0x50
[ 267.044519] swake_up+0x2c/0x40
[ 267.048364] kvm_vcpu_kick+0x32/0x80 [kvm]
[ 267.053276] __apic_accept_irq+0x1b6/0x330 [kvm]
[ 267.058774] kvm_apic_set_irq+0x2a/0x30 [kvm]
[ 267.063978] kvm_irq_delivery_to_apic_fast+0xf5/0x3c0 [kvm]
[ 267.070544] kvm_arch_set_irq_inatomic+0x99/0xb0 [kvm]
[ 267.076621] irqfd_wakeup+0x111/0x160 [kvm]
[ 267.081628] ? kvm_irq_delivery_to_apic+0x2c0/0x2c0 [kvm]
[ 267.087984] __wake_up_common+0x55/0x90
[ 267.092583] __wake_up_locked_key+0x18/0x20
[ 267.097580] eventfd_signal+0x5c/0x80
[ 267.102001] vfio_msihandler+0x16/0x20 [vfio_pci]
[ 267.107578] __handle_irq_event_percpu+0x3c/0x1a0
[ 267.113164] handle_irq_event_percpu+0x32/0x80
[ 267.118453] handle_irq_event+0x2c/0x50
[ 267.123068] handle_edge_irq+0x87/0x130
[ 267.127677] handle_irq+0xab/0x130
[ 267.131801] do_IRQ+0x48/0xd0
[ 267.135435] common_interrupt+0x93/0x93
[ 267.140042] RIP: 0010:panic+0x1f5/0x239
[ 267.144647] RSP: 0018:ffff89916fa03898 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff73
[ 267.153760] RAX: 000000000000003b RBX: 0000000000000000 RCX: 0000000000000006
[ 267.162062] RDX: 0000000000000000 RSI: 0000000000000046 RDI: ffff89916fa0e060
[ 267.170362] RBP: ffff89916fa03908 R08: 0000000000000691 R09: ffff898d000bc4c0
[ 267.178659] R10: 000000000000010a R11: 0000000000000198 R12: ffffffffbcc4a459
[ 267.186964] R13: 0000000000000000 R14: 0000000000000000 R15: ffff89916fa03a28
[ 267.195266] ? panic+0x1f1/0x239
[ 267.199201] watchdog_timer_fn+0x1e5/0x1f0
[ 267.204104] ? watchdog+0x40/0x40
[ 267.208130] __hrtimer_run_queues+0xee/0x270
[ 267.213224] hrtimer_interrupt+0xa8/0x190
[ 267.218036] local_apic_timer_interrupt+0x35/0x60
[ 267.223612] smp_apic_timer_interrupt+0x38/0x50
[ 267.228999] apic_timer_interrupt+0x93/0xa0
[ 267.233987] RIP: 0010:fl_classify+0x1f1/0x2b0 [cls_flower]
[ 267.240440] RSP: 0018:ffff89916fa03ad8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff10
[ 267.249539] RAX: 00000000ffffffff RBX: ffff89913f522100 RCX: 0000000000000000
[ 267.257829] RDX: ffff89916fa03c98 RSI: ffff89915bd743c0 RDI: ffff89913f522100
[ 267.266128] RBP: ffff89916fa03c28 R08: 000000000000270f R09: 0000000000000000
[ 267.274425] R10: 0000000000000000 R11: 0000000000000004 R12: ffff89916fa03c98
[ 267.282721] R13: ffff89913ecc7c00 R14: ffff89915bd743c0 R15: 0000000000000001
[ 267.291019] ? handle_edge_irq+0x87/0x130
[ 267.295818] ? handle_irq+0xab/0x130
[ 267.300133] ? irq_exit+0x77/0xe0
[ 267.304160] ? irq_exit+0x77/0xe0
[ 267.308184] ? do_IRQ+0x51/0xd0
[ 267.312015] ? common_interrupt+0x93/0x93
[ 267.316818] tc_classify+0x78/0x120
[ 267.321038] __netif_receive_skb_core+0x623/0xa00
[ 267.326618] ? udp4_gro_receive+0x10b/0x2d0
[ 267.331616] __netif_receive_skb+0x18/0x60
[ 267.336514] netif_receive_skb_internal+0x40/0xb0
[ 267.342091] napi_gro_receive+0xcd/0x120
[ 267.346809] mlx5e_handle_rx_cqe_rep+0x61b/0x890 [mlx5_core]
[ 267.353467] ? kvm_irq_delivery_to_apic+0x2c0/0x2c0 [kvm]
[ 267.359837] mlx5e_poll_rx_cq+0x83/0x840 [mlx5_core]
[ 267.365716] mlx5e_napi_poll+0x89/0x480 [mlx5_core]
[ 267.371500] net_rx_action+0x260/0x3c0
[ 267.376008] __do_softirq+0xc9/0x28c
[ 267.380330] irq_exit+0xd7/0xe0
[ 267.384168] do_IRQ+0x51/0xd0
[ 267.387810] common_interrupt+0x93/0x93
[ 267.392426] RIP: 0010:mpihelp_submul_1+0x91/0xe0
[ 267.403605] RSP: 0018:ffffa36f44513940 EFLAGS: 00000206 ORIG_RAX: ffffffffffffffa2
[ 267.412818] RAX: 5414d8fd4127cc59 RBX: 0000000100000000 RCX: ffffffffffffff50
[ 267.421114] RDX: 00000000ffffffea RSI: 0000000019ba7e4d RDI: 0000000068ef6b86
[ 267.429418] RBP: ffffa36f44513960 R08: 352cbbd4573982a8 R09: 64589019128fea22
[ 267.437713] R10: 128fea2200000000 R11: 352cbbd5573982a8 R12: ffff89955c585c00
[ 267.446014] R13: ffff899134dbfbd0 R14: 66a4c31fb80cf128 R15: d7554d389539f6c8
[ 267.454311] </IRQ>
[ 267.456981] mpihelp_divrem+0x23b/0x740
[ 267.461598] mpi_powm+0x471/0xa10
[ 267.465630] rsa_verify+0xa5/0x130
[ 267.469759] pkcs1pad_verify+0xb9/0xf0
[ 267.474277] public_key_verify_signature+0x1f7/0x280
[ 267.480148] public_key_verify_signature_2+0x15/0x20
[ 267.486011] verify_signature+0x3c/0x50
[ 267.490619] pkcs7_validate_trust+0xa1/0x200
[ 267.495710] verify_pkcs7_signature+0x9a/0x140
[ 267.501000] mod_verify_sig+0x92/0xe0
[ 267.505404] load_module+0x171/0x2810
[ 267.509822] ? vfs_read+0x113/0x130
[ 267.514047] ? kernel_read_file+0x158/0x190
[ 267.519041] ? kernel_read_file_from_fd+0x49/0x80
[ 267.524620] SYSC_finit_module+0xa6/0xf0
[ 267.529325] SyS_finit_module+0xe/0x10
[ 267.533844] entry_SYSCALL_64_fastpath+0x1a/0xa9
[ 267.539325] RIP: 0033:0x7f74f2439239
[ 267.543639] RSP: 002b:00007ffeb8b23218 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[ 267.552740] RAX: ffffffffffffffda RBX: 00007ffeb8b21f10 RCX: 00007f74f2439239
[ 267.561042] RDX: 0000000000000000 RSI: 00005578b61d02a6 RDI: 0000000000000000
[ 267.569341] RBP: 00007ffeb8b21f00 R08: 0000000000000000 R09: 0000000000000000
[ 267.577638] R10: 0000000000000000 R11: 0000000000000246 R12: 00005578b61d1be7
[ 267.585935] R13: 00007ffeb8b22088 R14: 0000000000000000 R15: 00005578b61d1bf3
[ 267.594231] ---[ end trace 2eb46a584b6ba420 ]---
^ permalink raw reply
* Re: [PATCH v5 1/4] siphash: add cryptographically secure PRF
From: Jean-Philippe Aumasson @ 2016-12-19 20:18 UTC (permalink / raw)
To: George Spelvin, David.Laight, tom
Cc: ak, davem, djb, ebiggers3, hannes, Jason, kernel-hardening,
linux-crypto, linux-kernel, luto, netdev, torvalds, tytso,
vegard.nossum
In-Reply-To: <20161219181040.25441.qmail@ns.sciencehorizons.net>
[-- Attachment #1: Type: text/plain, Size: 3984 bytes --]
FTR, I agree that SipHash may benefit from security/performance
optimizations (there's always optimizations), but I'm note sure that it's
the right time/place to discuss it. Cryptanalysis is hard, and any major
change should be backed by a rigorous security analysis and/or security
proof. For example, the redundancy in the initial state prevents trivial
weaknesses that would occur if four key words were used (something I've
seen proposed in this thread).
In the paper at http://aumasson.jp/siphash/siphash.pdf we explain rationale
behind most of our design choices. The cryptanalysis paper at
https://eprint.iacr.org/2014/722.pdf analyzes differences propagations and
presents attacks on reduced versions of SipHash.
On Mon, Dec 19, 2016 at 7:10 PM George Spelvin <linux@sciencehorizons.net>
wrote:
> David Laight wrote:
> > From: George Spelvin
> ...
> >> uint32_t
> >> hsiphash24(char const *in, size_t len, uint32_t const key[2])
> >> {
> >> uint32_t c = key[0];
> >> uint32_t d = key[1];
> >> uint32_t a = 0x6c796765 ^ 0x736f6d65;
> >> uint32_t b = d ^ 0x74656462 ^ 0x646f7261;
>
> > I've not looked closely, but is that (in some sense) duplicating
> > the key length?
> > So you could set a = key[2] and b = key[3] and still have an
> > working hash - albeit not exactly the one specified.
>
> That's tempting, but not necessarily effective. (A similar unsuccesful
> idea can be found in discussions of "DES with independent round keys".
> Or see the design discussion of Salsa20 and the constants in its input.)
>
> You can increase the key size, but that might not increase the *security*
> any.
>
> The big issue is that there are a *lot* of square root attack in
> cryptanalysis. Because SipHash's state is twice the size of the key,
> such an attack will have the same complexity as key exhaustion and need
> not be considered. To make a stronger security claim, you need to start
> working through them all and show that they don't apply.
>
> For SipHash in particular, an important property is asymmetry of the
> internal state. That's what duplicating the key with XORs guarantees.
> If the two halves of the state end up identical, the mixing is much
> weaker.
>
> Now the probability of ending up in a "mirror state" is the square
> root of the state size (1 in 2^64 for HalfSipHash's 128-bit state),
> which is the same probability as guessing a key, so it's not a
> problem that has to be considered when making a 64-bit security claim.
>
> But if you want a higher security level, you have to think about
> what can happen.
>
> That said, I have been thinking very hard about
>
> a = c ^ 0x48536970; /* 'HSip' */
> d = key[2];
>
> By guaranteeing that a and c are different, we get the desired
> asymmetry, and the XOR of b and d is determined by the first word of
> the message anyway, so this isn't weakening anything.
>
> 96 bits is far beyond the reach of any brute-force attack, and if a
> more sophisticated 64-bit attack exists, it's at least out of the reach
> of the script kiddies, and will almost certainly have a non-negligible
> constant factor and more limits in when it can be applied.
>
> > Is it worth using the 32bit hash for IP addresses on 64bit systems that
> > can't do misaligned accessed?
>
> Not a good idea. To hash 64 bits of input:
>
> * Full SipHash has to do two loads, a shift, an or, and two rounds of
> mixing.
> * HalfSipHash has to do a load, two rounds, another load, and two more
> rounds.
>
> In other words, in addition to being less secure, it's half the speed.
>
> Also, what systems are you thinking about? x86, ARMv8, PowerPC, and
> S390 (and ia64, if anyone cares) all handle unaligned loads. MIPS has
> efficient support. Alpha and HPPA are for retrocomputing fans, not
> people who care about performance.
>
> So you're down to SPARC. Which conveniently has the same maintainer as
> the networking code, so I figure DaveM can take care of that himself. :-)
>
[-- Attachment #2: Type: text/html, Size: 6131 bytes --]
^ permalink raw reply
* Re: [PATCH RFC net-next 2/7] net: add dst_pending_confirm flag to skbuff
From: Julian Anastasov @ 2016-12-19 20:37 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, linux-sctp, YueHaibing
In-Reply-To: <1482164252.1521.7.camel@edumazet-glaptop3.roam.corp.google.com>
Hello,
On Mon, 19 Dec 2016, Eric Dumazet wrote:
> I am still digesting this awesome patch series ;)
Thanks. I don't feel quite comfortable with some
of the changes (mostly XFRM, dst_confirm usage in CXGB) and
I hope the discussion can provide adequate solution.
> Not sure why you used an unlikely() here. TCP for example would hit this
> path quite often.
I was not sure, may be because ACKs can come with lower
rate than the sent packets. Also because non-TCP rarely uses
MSG_CONFIRM. If you still think it is better without unlikely,
I'll remove it.
> So considering sk_dst_pending_confirm might be dirtied quite often,
>
> I am not sure why you placed it in the cache line that contains
> sk_rx_dst (in 1st patch)
I saw your recent changes and was worried if the
sk_dst_confirm() calling on RX can cause unwanted dirtying of
additional cache line. My preliminary analyze pointed
sk_omem_alloc as good candidate for moving to next cache
line. I know how critical is to properly place the new flags,
so I really need recommendations about this.
Regards
^ permalink raw reply
* [PATCH net v4 1/4] fsl/fman: fix 1G support for QSGMII interfaces
From: Madalin Bucur @ 2016-12-19 20:42 UTC (permalink / raw)
To: netdev; +Cc: linuxppc-dev, linux-kernel, davem, scott.wood, mpe
In-Reply-To: <1482180166-10677-1-git-send-email-madalin.bucur@nxp.com>
QSGMII ports were not advertising 1G speed.
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
Reviewed-by: Camelia Groza <camelia.groza@nxp.com>
---
drivers/net/ethernet/freescale/fman/mac.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 69ca42c..0b31f85 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -594,6 +594,7 @@ static const u16 phy2speed[] = {
[PHY_INTERFACE_MODE_RGMII_RXID] = SPEED_1000,
[PHY_INTERFACE_MODE_RGMII_TXID] = SPEED_1000,
[PHY_INTERFACE_MODE_RTBI] = SPEED_1000,
+ [PHY_INTERFACE_MODE_QSGMII] = SPEED_1000,
[PHY_INTERFACE_MODE_XGMII] = SPEED_10000
};
--
2.1.0
^ permalink raw reply related
* [PATCH net v4 2/4] powerpc: fsl/fman: remove fsl,fman from of_device_ids[]
From: Madalin Bucur @ 2016-12-19 20:42 UTC (permalink / raw)
To: netdev; +Cc: linuxppc-dev, linux-kernel, davem, scott.wood, mpe
In-Reply-To: <1482180166-10677-1-git-send-email-madalin.bucur@nxp.com>
The fsl/fman drivers will use of_platform_populate() on all
supported platforms. Call of_platform_populate() to probe the
FMan sub-nodes.
Signed-off-by: Igal Liberman <igal.liberman@freescale.com>
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
Acked-by: Scott Wood <oss@buserror.net>
---
arch/powerpc/platforms/85xx/corenet_generic.c | 3 ---
drivers/net/ethernet/freescale/fman/fman.c | 7 +++++++
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c
index 1179115..824b7f1 100644
--- a/arch/powerpc/platforms/85xx/corenet_generic.c
+++ b/arch/powerpc/platforms/85xx/corenet_generic.c
@@ -117,9 +117,6 @@ static const struct of_device_id of_device_ids[] = {
{
.compatible = "fsl,qe",
},
- {
- .compatible = "fsl,fman",
- },
/* The following two are for the Freescale hypervisor */
{
.name = "hypervisor",
diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index dafd9e1..4b83263 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -2868,6 +2868,13 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
fman->dev = &of_dev->dev;
+ err = of_platform_populate(fm_node, NULL, NULL, &of_dev->dev);
+ if (err) {
+ dev_err(&of_dev->dev, "%s: of_platform_populate() failed\n",
+ __func__);
+ goto fman_free;
+ }
+
return fman;
fman_node_put:
--
2.1.0
^ permalink raw reply related
* [PATCH net v4 4/4] fsl/fman: enable compilation on ARM64
From: Madalin Bucur @ 2016-12-19 20:42 UTC (permalink / raw)
To: netdev; +Cc: scott.wood, linuxppc-dev, linux-kernel, davem
In-Reply-To: <1482180166-10677-1-git-send-email-madalin.bucur@nxp.com>
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
drivers/net/ethernet/freescale/fman/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fman/Kconfig b/drivers/net/ethernet/freescale/fman/Kconfig
index 79b7c84..dc0850b 100644
--- a/drivers/net/ethernet/freescale/fman/Kconfig
+++ b/drivers/net/ethernet/freescale/fman/Kconfig
@@ -1,6 +1,6 @@
config FSL_FMAN
tristate "FMan support"
- depends on FSL_SOC || COMPILE_TEST
+ depends on FSL_SOC || ARCH_LAYERSCAPE || COMPILE_TEST
select GENERIC_ALLOCATOR
select PHYLIB
default n
--
2.1.0
^ permalink raw reply related
* [PATCH net v4 0/4] fsl/fman: fixes for ARM
From: Madalin Bucur @ 2016-12-19 20:42 UTC (permalink / raw)
To: netdev; +Cc: linuxppc-dev, linux-kernel, davem, scott.wood, mpe
The patch set fixes advertised speeds for QSGMII interfaces, disables
A007273 erratum workaround on non-PowerPC platforms where it does not
apply, enables compilation on ARM64 and addresses a probing issue on
non PPC platforms.
Changes from v3: removed redundant comment, added ack by Scott
Changes from v2: merged fsl/fman changes to avoid a point of failure
Changes from v1: unifying probing on all supported platforms
Madalin Bucur (4):
fsl/fman: fix 1G support for QSGMII interfaces
powerpc: fsl/fman: remove fsl,fman from of_device_ids[]
fsl/fman: A007273 only applies to PPC SoCs
fsl/fman: enable compilation on ARM64
arch/powerpc/platforms/85xx/corenet_generic.c | 3 ---
drivers/net/ethernet/freescale/fman/Kconfig | 2 +-
drivers/net/ethernet/freescale/fman/fman.c | 15 +++++++++++++++
drivers/net/ethernet/freescale/fman/mac.c | 1 +
4 files changed, 17 insertions(+), 4 deletions(-)
--
2.1.0
^ permalink raw reply
* [PATCH net v4 3/4] fsl/fman: A007273 only applies to PPC SoCs
From: Madalin Bucur @ 2016-12-19 20:42 UTC (permalink / raw)
To: netdev; +Cc: linuxppc-dev, linux-kernel, davem, scott.wood, mpe
In-Reply-To: <1482180166-10677-1-git-send-email-madalin.bucur@nxp.com>
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
Reviewed-by: Camelia Groza <camelia.groza@nxp.com>
---
drivers/net/ethernet/freescale/fman/fman.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index 4b83263..f60845f 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -1890,6 +1890,7 @@ static int fman_reset(struct fman *fman)
goto _return;
} else {
+#ifdef CONFIG_PPC
struct device_node *guts_node;
struct ccsr_guts __iomem *guts_regs;
u32 devdisr2, reg;
@@ -1921,6 +1922,7 @@ static int fman_reset(struct fman *fman)
/* Enable all MACs */
iowrite32be(reg, &guts_regs->devdisr2);
+#endif
/* Perform FMan reset */
iowrite32be(FPM_RSTC_FM_RESET, &fman->fpm_regs->fm_rstc);
@@ -1932,25 +1934,31 @@ static int fman_reset(struct fman *fman)
} while (((ioread32be(&fman->fpm_regs->fm_rstc)) &
FPM_RSTC_FM_RESET) && --count);
if (count == 0) {
+#ifdef CONFIG_PPC
iounmap(guts_regs);
of_node_put(guts_node);
+#endif
err = -EBUSY;
goto _return;
}
+#ifdef CONFIG_PPC
/* Restore devdisr2 value */
iowrite32be(devdisr2, &guts_regs->devdisr2);
iounmap(guts_regs);
of_node_put(guts_node);
+#endif
goto _return;
+#ifdef CONFIG_PPC
guts_regs:
of_node_put(guts_node);
guts_node:
dev_dbg(fman->dev, "%s: Didn't perform FManV3 reset due to Errata A007273!\n",
__func__);
+#endif
}
_return:
return err;
--
2.1.0
^ permalink raw reply related
* RE: [upstream-release] [PATCH net v3 2/4] powerpc: fsl/fman: remove fsl, fman from of_device_ids[]
From: Madalin-Cristian Bucur @ 2016-12-19 20:43 UTC (permalink / raw)
To: Scott Wood, netdev@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
davem@davemloft.net
In-Reply-To: <1482176783.32531.28.camel@buserror.net>
> From: Scott Wood [mailto:oss@buserror.net]
> Sent: Monday, December 19, 2016 9:46 PM
>
> On Mon, 2016-12-19 at 18:13 +0200, Madalin Bucur wrote:
> > The fsl/fman drivers will use of_platform_populate() on all
> > supported platforms. Call of_platform_populate() to probe the
> > FMan sub-nodes.
> >
> > Signed-off-by: Igal Liberman <igal.liberman@freescale.com>
> > Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> > ---
> > arch/powerpc/platforms/85xx/corenet_generic.c | 3 ---
> > drivers/net/ethernet/freescale/fman/fman.c | 8 ++++++++
> > 2 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c
> > b/arch/powerpc/platforms/85xx/corenet_generic.c
> > index 1179115..824b7f1 100644
> > --- a/arch/powerpc/platforms/85xx/corenet_generic.c
> > +++ b/arch/powerpc/platforms/85xx/corenet_generic.c
> > @@ -117,9 +117,6 @@ static const struct of_device_id of_device_ids[] = {
> > {
> > .compatible = "fsl,qe",
> > },
> > - {
> > - .compatible = "fsl,fman",
> > - },
> > /* The following two are for the Freescale hypervisor */
> > {
> > .name = "hypervisor",
>
> For this part:
>
> Acked-by: Scott Wood <oss@buserror.net>
Thank you, added to v4.
> > diff --git a/drivers/net/ethernet/freescale/fman/fman.c
> > b/drivers/net/ethernet/freescale/fman/fman.c
> > index dafd9e1..0b7f711 100644
> > --- a/drivers/net/ethernet/freescale/fman/fman.c
> > +++ b/drivers/net/ethernet/freescale/fman/fman.c
> > @@ -2868,6 +2868,14 @@ static struct fman *read_dts_node(struct
> > platform_device *of_dev)
> >
> > fman->dev = &of_dev->dev;
> >
> > + /* call of_platform_populate in order to probe sub-nodes on arm64
> > */
> > + err = of_platform_populate(fm_node, NULL, NULL, &of_dev->dev);
> > + if (err) {
> > + dev_err(&of_dev->dev, "%s: of_platform_populate()
> > failed\n",
> > + __func__);
> > + goto fman_free;
> > + }
>
> The "on arm64" comment is no longer accurate (and the rest of the comment
> seems unnecessary).
>
> -Scott
Removed in v4.
^ permalink raw reply
* Re: HalfSipHash Acceptable Usage
From: Jean-Philippe Aumasson @ 2016-12-19 20:49 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Theodore Ts'o, Hannes Frederic Sowa, LKML, Eric Biggers,
Daniel J . Bernstein, David Laight, David Miller, Andi Kleen,
George Spelvin, kernel-hardening, Andy Lutomirski,
Linux Crypto Mailing List, Tom Herbert, Vegard Nossum, Netdev,
Linus Torvalds
In-Reply-To: <CAHmME9rPmH=wP_eHYopt8ZPG9TSN7bos3fGOuqKL2HjQW-2SWA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4442 bytes --]
On Mon, Dec 19, 2016 at 6:32 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Hi JP,
>
> With the threads getting confusing, I've been urged to try and keep
> the topics and threads more closely constrained. Here's where we're
> at, and here's the current pressing security concern. It'd be helpful
> to have a definitive statement on what you think is best, so we can
> just build on top of that, instead of getting lost in the chorus of
> opinions.
>
> 1) Anything that requires actual long-term security will use
> SipHash2-4, with the 64-bit output and the 128-bit key. This includes
> things like TCP sequence numbers. This seems pretty uncontroversial to
> me. Seem okay to you?
>
Right, since 2012 when we published SipHash many cryptanalysts attempted to
break SipHash-2-4 with a 128-bit key, for various notions of "break", and
nothing worth worrying was ever found. I'm totally confident that
SipHash-2-4 will live up to its security promises.
Don't use something weaker for things like TCP sequence numbers or RNGs.
Use SipHash2-4 for those. That is the correct choice.
>
> 2) People seem to want something competitive, performance-wise, with
> jhash if it's going to replace jhash. The kernel community
> instinctively pushes back on anything that could harm performance,
> especially in networking and in critical data structures, so there
> have been some calls for something faster than SipHash. So, questions
> regarding this:
>
>
No free lunch I guess: either go with a cryptographically secure,
time-proved keyed hash such as SipHash, or go with some simpler hash deemed
secure cos its designer can't break it :) #DontRollYourOwnCrypto
2a) George thinks that HalfSipHash on 32-bit systems will have roughly
> comparable speed as SipHash on 64-bit systems, so the idea would be to
> use HalfSipHash on 32-bit systems' hash tables and SipHash on 64-bit
> systems' hash tables. The big obvious question is: does HalfSipHash
> have a sufficient security margin for hashtable usage and hashtable
> attacks? I'm not wondering about the security margin for other usages,
> but just of the hashtable usage. In your opinion, does HalfSipHash cut
> it?
>
HalfSipHash takes its core function from Chaskey and uses the same
construction as SipHash, so it *should* be secure. Nonetheless it hasn't
received the same amount of attention as 64-bit SipHash did. So I'm less
confident about its security than about SipHash's, but it obviously
inspires a lot more confidence than non-crypto hashes.
Too, HalfSipHash only has a 64-bit key, not a 128-bit key like SipHash, so
only use this as a mitigation for hash-flooding attacks, where the output
of the hash function is never directly shown to the caller. Do not use
HalfSipHash for TCP sequence numbers or RNGs.
>
> 2b) While I certainly wouldn't consider making the use case in
> question (1) employ a weaker function, for this question (2), there
> has been some discussion about using HalfSipHash1-3 (or SipHash1-3 on
> 64-bit) instead of 2-4. So, the same question is therefore posed:
> would using HalfSipHash1-3 give a sufficient security margin for
> hashtable usage and hashtable attacks?
>
My educated guess is that yes, it will, but that it may not withhold
cryptanalysis as a pseudorandom function (PRF). For example I wouldn't be
surprised if there were a "distinguishing attack" that detects non-random
patterns in HalfSipHash-1-3's output. But most of the non-crypto hashes
I've seen have obvious distinguishing attacks. So the upshot is that HSH
will get you better security that AnyWeakHash even with 1 & 3 rounds.
So, if you're willing to compromise on security, but still want something
not completely unreasonable, you might be able to get away with using
HalfSipHash1-3 as a replacement for jhash—in circumstances where the output
of the hash function is kept secret—in order to mitigate hash-flooding
attacks.
>
> My plan is essentially to implement things according to your security
> recommendation. The thread started with me pushing a heavy duty
> security solution -- SipHash2-4 -- for _everything_. I've received
> understandable push back on that notion for certain use cases. So now
> I'm trying to discover what the most acceptable compromise is. Your
> answers on (2a) and (2b) will direct that compromise.
>
> Thanks again,
> Jason
>
[-- Attachment #2: Type: text/html, Size: 6561 bytes --]
^ permalink raw reply
* Re: Potential issues (security and otherwise) with the current cgroup-bpf API
From: Alexei Starovoitov @ 2016-12-19 20:56 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Daniel Mack, Mickaël Salaün, Kees Cook, Jann Horn,
Tejun Heo, David Ahern, David S. Miller, Thomas Graf,
Michael Kerrisk, Peter Zijlstra, Linux API,
linux-kernel@vger.kernel.org, Network Development
In-Reply-To: <CALCETrV81oFwq2AgeRsN54HA1jR=b5cOZfAgve8H8zhx83DTyA@mail.gmail.com>
On Sat, Dec 17, 2016 at 10:18:44AM -0800, Andy Lutomirski wrote:
> Hi all-
>
> I apologize for being rather late with this. I didn't realize that
> cgroup-bpf was going to be submitted for Linux 4.10, and I didn't see
> it on the linux-api list, so I missed the discussion.
>
> I think that the inet ingress, egress etc filters are a neat feature,
> but I think the API has some issues that will bite us down the road
> if it becomes stable in its current form.
>
> Most of the problems I see are summarized in this transcript:
>
> # mkdir cg2
> # mount -t cgroup2 none cg2
> # mkdir cg2/nosockets
> # strace cgrp_socket_rule cg2/nosockets/ 0
> ...
> open("cg2/nosockets/", O_RDONLY|O_DIRECTORY) = 3
>
> ^^^^ You can modify a cgroup after opening it O_RDONLY?
>
> bpf(BPF_PROG_LOAD, {prog_type=0x9 /* BPF_PROG_TYPE_??? */, insn_cnt=2,
> insns=0x7fffe3568c10, license="GPL", log_level=1, log_size=262144,
> log_buf=0x6020c0, kern_version=0}, 48) = 4
>
> ^^^^ This is fine. The bpf() syscall manipulates bpf objects.
>
> bpf(0x8 /* BPF_??? */, 0x7fffe3568bf0, 48) = 0
>
> ^^^^ This is not so good:
> ^^^^
> ^^^^ a) The bpf() syscall is supposed to manipulate bpf objects. This
> ^^^^ is manipulating a cgroup. There's no reason that a socket creation
> ^^^^ filter couldn't be written in a different language (new iptables
> ^^^^ table? Simple list of address families?), but if that happened,
> ^^^^ then using bpf() to install it would be entirely nonsensical.
I don't see why it's _modifing_ the cgroup. I'm looking at it as
network stack is using cgroup as an application group that should
invoke bpf program at the certain point in the stack.
imo cgroup management is orthogonal.
I certainly don't mind adding 'cat cgroup.bpf' control file that will
print the program digest for debugging, just like we do for fdinfo,
but cgroup file interface shouldn't be used to control networking.
If there was something like networking-wide ioctl, I think it could
have been an alternative way to attach a program to a hook to a cgroup.
Adding a control file to a cgroup for the purpose of attaching bpf,
just doesn't make sense to me, since it's dragging bpf and networking
details into cgroup land. Imagine in the future somebody would want
to have nft to perform similar BPF_CGROUP_INET_INGRESS functionality.
I'd think that the position of the hook within the networking stack
would remain the same, but enum id will be different, since
other ids in that enum (like BPF_CGROUP_INET_SOCK_CREATE) are not
applicable to nft. Similarly the concept of program_fd doesn't exist
there either. So it would be using its own netlink based mechanism
and its own way of enumerating hook id. And cgroup could be passed
as a string into netlink message instead of fd.
So if somebody adds a control file to a cgroup api that takes bpf's hookid
and bpf's program_fd, such control file will only be applicable to bpf
and not usable for anything else. Hence I see no reason to go that route.
> ^^^^
> ^^^^ b) This is starting to be an excessively ugly multiplexer. Among
> ^^^^ other things, it's very unfriendly to seccomp.
>
> # echo $$ >cg2/nosockets/cgroup.procs
> # ping 127.0.0.1
> ping: socket: Operation not permitted
> # ls cg2/nosockets/
> cgroup.controllers cgroup.events cgroup.procs cgroup.subtree_control
> # cat cg2/nosockets/cgroup.controllers
>
> ^^^^ Something in cgroupfs should give an indication that this cgroup
> ^^^^ filters socket creation, but there's nothing there. You should also
> ^^^^ be able to turn the filter off from cgroupfs.
Doing 'cat cg2/nosockets/cgroup.bpf' here would be useful for debugging
to see which program is attached to which hook in this cgroup.
Detaching programs via cgroup control file is a lot less clear,
since it will be confusing to a running application that attached the program
in the first place, since it won't have any indication that external entity
detached the program.
One can argue that it could be useful for curious human/admin
to detach all bpf progs from all hooks for this cgroup, but
then it probably needs dmesg warning and only usable in extreme cases
as debugging and not something control plane should ever use.
> # mkdir cg2/nosockets/sockets
> # /home/luto/apps/linux/samples/bpf/cgrp_socket_rule cg2/nosockets/sockets/ 1
>
> ^^^^ This succeeded, which means that, if this feature is enabled in 4.10,
> ^^^^ then we're stuck with its semantics. If it returned -EINVAL instead,
> ^^^^ there would be a chance to refine it.
i don't see the problem with this behavior. bpf and cgroup are indepedent.
Imange that socket accounting program is attached to cg2/nosockets.
The program is readonly and carry no security meaning.
Why cgroup should prevent creation of cg2/nosockets/foo directory ?
> # echo $$ >cg2/nosockets/sockets/cgroup.procs
> # ping 127.0.0.1
> PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
> 64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.029 ms
hmm. in this case the admin created a subgroup with a group that allows
ping, so? how's that a problem? In case of vrf I can imagine
a set of application auto-binding to one vrf and smaller
set of applications binding to a different vrf.
Or broader set of applications monitoring all tcp traffic
and subset of them monitoring udp instead.
Those are valid use cases.
I guess you're advocating to run a link list of programs?
That won't be useful for the above use cases, where there is no
reason to run more than one program that control plane
assigned to run for this cgroup.
At this stage we decided to allow only one program per cgroup per hook
and later can extend it if necessary.
As you're pointing out, in case of security, we probably
want to preserve original bpf program that should always be
run first and only after it returned 'ok' (we'd need to define
what 'ok' means in secruity context) run program attached to sub-hierarchy.
Another alternative is to disallow attaching programs in sub-hierarchy
if parent has something already attached, but it's not useful
for general case.
All of these are possible future extensions.
> In 4.10 with With CONFIG_CGROUP_BPF=y, a cgroup can have bpf
> programs attached that can do things if various events occur. (Right
> now, this means socket operations, but there are plans in the works
> to do this for LSM hooks too.) These bpf programs can say yes or no,
> but they can also read out various data (including socket payloads!)
> and save them away where an attacker can find them. This sounds a
> lot like seccomp with a narrower scope but a much stronger ability to
> exfiltrate private information.
that sounds like a future problem to solve when bpf+lsm+cgroup are
used for security.
> This isn't much of a problem yet because you currently need
> CAP_NET_ADMIN to create a malicious sandbox in the first place. I'm
> sure that, in the near future, someone will want to make this stuff
> work in containers with delegated cgroup hierarchies, and then there
> may be a real problem here.
I agree. Currently cgroup+bpf+hooks are for root only and have to
go long way before they can be used for security and sandboxing.
> I've included a few security people on this thread. The current API
> looks abusable, and it would be nice to find all the holes before
> 4.10 comes out.
I disagree with the urgency. I see nothing that needs immediate action.
bpf+lsm+cgroup is not in the tree yet.
^ permalink raw reply
* Re: HalfSipHash Acceptable Usage
From: Jason A. Donenfeld @ 2016-12-19 21:00 UTC (permalink / raw)
To: Jean-Philippe Aumasson
Cc: Theodore Ts'o, Hannes Frederic Sowa, LKML, Eric Biggers,
Daniel J . Bernstein, David Laight, David Miller, Andi Kleen,
George Spelvin, kernel-hardening, Andy Lutomirski,
Linux Crypto Mailing List, Tom Herbert, Vegard Nossum, Netdev,
Linus Torvalds
In-Reply-To: <CAGiyFdduUNSGq24zfsk0ZU=hnOCmewAw8vw6XvDoS-3f+3UPKQ@mail.gmail.com>
Hi JP,
On Mon, Dec 19, 2016 at 9:49 PM, Jean-Philippe Aumasson
<jeanphilippe.aumasson@gmail.com> wrote:
>
> On Mon, Dec 19, 2016 at 6:32 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>
>> Hi JP,
>>
>> With the threads getting confusing, I've been urged to try and keep
>> the topics and threads more closely constrained. Here's where we're
>> at, and here's the current pressing security concern. It'd be helpful
>> to have a definitive statement on what you think is best, so we can
>> just build on top of that, instead of getting lost in the chorus of
>> opinions.
>>
>> 1) Anything that requires actual long-term security will use
>> SipHash2-4, with the 64-bit output and the 128-bit key. This includes
>> things like TCP sequence numbers. This seems pretty uncontroversial to
>> me. Seem okay to you?
>
>
>
> Right, since 2012 when we published SipHash many cryptanalysts attempted to
> break SipHash-2-4 with a 128-bit key, for various notions of "break", and
> nothing worth worrying was ever found. I'm totally confident that
> SipHash-2-4 will live up to its security promises.
>
> Don't use something weaker for things like TCP sequence numbers or RNGs. Use
> SipHash2-4 for those. That is the correct choice.
>
>>
>>
>> 2) People seem to want something competitive, performance-wise, with
>> jhash if it's going to replace jhash. The kernel community
>> instinctively pushes back on anything that could harm performance,
>> especially in networking and in critical data structures, so there
>> have been some calls for something faster than SipHash. So, questions
>> regarding this:
>>
>
> No free lunch I guess: either go with a cryptographically secure,
> time-proved keyed hash such as SipHash, or go with some simpler hash deemed
> secure cos its designer can't break it :) #DontRollYourOwnCrypto
>
>> 2a) George thinks that HalfSipHash on 32-bit systems will have roughly
>> comparable speed as SipHash on 64-bit systems, so the idea would be to
>> use HalfSipHash on 32-bit systems' hash tables and SipHash on 64-bit
>> systems' hash tables. The big obvious question is: does HalfSipHash
>> have a sufficient security margin for hashtable usage and hashtable
>> attacks? I'm not wondering about the security margin for other usages,
>> but just of the hashtable usage. In your opinion, does HalfSipHash cut
>> it?
>
>
> HalfSipHash takes its core function from Chaskey and uses the same
> construction as SipHash, so it *should* be secure. Nonetheless it hasn't
> received the same amount of attention as 64-bit SipHash did. So I'm less
> confident about its security than about SipHash's, but it obviously inspires
> a lot more confidence than non-crypto hashes.
>
> Too, HalfSipHash only has a 64-bit key, not a 128-bit key like SipHash, so
> only use this as a mitigation for hash-flooding attacks, where the output of
> the hash function is never directly shown to the caller. Do not use
> HalfSipHash for TCP sequence numbers or RNGs.
>
>
>>
>>
>> 2b) While I certainly wouldn't consider making the use case in
>> question (1) employ a weaker function, for this question (2), there
>> has been some discussion about using HalfSipHash1-3 (or SipHash1-3 on
>> 64-bit) instead of 2-4. So, the same question is therefore posed:
>> would using HalfSipHash1-3 give a sufficient security margin for
>> hashtable usage and hashtable attacks?
>
>
> My educated guess is that yes, it will, but that it may not withhold
> cryptanalysis as a pseudorandom function (PRF). For example I wouldn't be
> surprised if there were a "distinguishing attack" that detects non-random
> patterns in HalfSipHash-1-3's output. But most of the non-crypto hashes I've
> seen have obvious distinguishing attacks. So the upshot is that HSH will get
> you better security that AnyWeakHash even with 1 & 3 rounds.
>
> So, if you're willing to compromise on security, but still want something
> not completely unreasonable, you might be able to get away with using
> HalfSipHash1-3 as a replacement for jhash—in circumstances where the output
> of the hash function is kept secret—in order to mitigate hash-flooding
> attacks.
>
Thanks for the detailed response. I will continue exactly how you've specified.
1. SipHash2-4 for TCP sequence numbers, syncookies, and RNG. IOW, the
things that MD5 is used for now.
2. HalfSipHash1-3 for hash tables where the output is not revealed,
for jhash replacements. On 64-bit this will alias to SipHash1-3.
3. I will write Documentation/siphash.txt detailing this.
4. I'll continue to discourage other kernel developers from rolling
their own crypto or departing from the tried&true in substantial ways.
Thanks again,
Jason
^ permalink raw reply
* Re: Potential issues (security and otherwise) with the current cgroup-bpf API
From: Andy Lutomirski @ 2016-12-19 21:23 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andy Lutomirski, Daniel Mack, Mickaël Salaün, Kees Cook,
Jann Horn, Tejun Heo, David Ahern, David S. Miller, Thomas Graf,
Michael Kerrisk, Peter Zijlstra, Linux API,
linux-kernel@vger.kernel.org, Network Development
In-Reply-To: <20161219205631.GA31242@ast-mbp.thefacebook.com>
On Mon, Dec 19, 2016 at 12:56 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Sat, Dec 17, 2016 at 10:18:44AM -0800, Andy Lutomirski wrote:
>> Hi all-
>>
>> I apologize for being rather late with this. I didn't realize that
>> cgroup-bpf was going to be submitted for Linux 4.10, and I didn't see
>> it on the linux-api list, so I missed the discussion.
>>
>> I think that the inet ingress, egress etc filters are a neat feature,
>> but I think the API has some issues that will bite us down the road
>> if it becomes stable in its current form.
>>
>> Most of the problems I see are summarized in this transcript:
>>
>> # mkdir cg2
>> # mount -t cgroup2 none cg2
>> # mkdir cg2/nosockets
>> # strace cgrp_socket_rule cg2/nosockets/ 0
>> ...
>> open("cg2/nosockets/", O_RDONLY|O_DIRECTORY) = 3
>>
>> ^^^^ You can modify a cgroup after opening it O_RDONLY?
>>
>> bpf(BPF_PROG_LOAD, {prog_type=0x9 /* BPF_PROG_TYPE_??? */, insn_cnt=2,
>> insns=0x7fffe3568c10, license="GPL", log_level=1, log_size=262144,
>> log_buf=0x6020c0, kern_version=0}, 48) = 4
>>
>> ^^^^ This is fine. The bpf() syscall manipulates bpf objects.
>>
>> bpf(0x8 /* BPF_??? */, 0x7fffe3568bf0, 48) = 0
>>
>> ^^^^ This is not so good:
>> ^^^^
>> ^^^^ a) The bpf() syscall is supposed to manipulate bpf objects. This
>> ^^^^ is manipulating a cgroup. There's no reason that a socket creation
>> ^^^^ filter couldn't be written in a different language (new iptables
>> ^^^^ table? Simple list of address families?), but if that happened,
>> ^^^^ then using bpf() to install it would be entirely nonsensical.
>
> I don't see why it's _modifing_ the cgroup. I'm looking at it as
> network stack is using cgroup as an application group that should
> invoke bpf program at the certain point in the stack.
> imo cgroup management is orthogonal.
It is literally modifying the struct cgroup, and, as a practical
matter, it's causing membership in the cgroup to have a certain
effect. But rather than pointless arguing, let me propose an
alternative API that I think solves most of the problems here.
In my model, BPF_PROG_ATTACH and BPF_PROG_DETACH go away completely.
Instead, the cgroup gets three new control files:
"net.ingress_filter", "net.egress_filter", and
"net.socket_create_filter". Initially, if you read these files, you
see "none\n".
To attach a bpf filter, you open the file for write and do an ioctl on
it. After doing the ioctl, if you read the file, you'll see
"bpf:[hash]\n" where "[hash]" is exactly what you'd see in fdinfo for
the bpf program.
To detach any type of filter, bpf or otherwise, you open the file for
write and write "none\n" (or just "none").
If you write anything else to the file, you get -EINVAL. But, if
someone writes a new type of filter (perhaps a simple list of address
families), maybe you can enable the filter by writing something
appropriate to the file.
Now the API matches the effect. A cgroup with something other than
"none" in one of its net.*_filter files is a cgroup that filters
network activity. And you get CRIU compatibility for free: CRIU can
read the control file and use whatever mechanism is uses for BPF in
general to restore the cgroup filter state. As an added bonus, you
get ACLs for free and the ugly multiplexer goes away.
>> # mkdir cg2/nosockets/sockets
>> # /home/luto/apps/linux/samples/bpf/cgrp_socket_rule cg2/nosockets/sockets/ 1
>>
>> ^^^^ This succeeded, which means that, if this feature is enabled in 4.10,
>> ^^^^ then we're stuck with its semantics. If it returned -EINVAL instead,
>> ^^^^ there would be a chance to refine it.
>
> i don't see the problem with this behavior. bpf and cgroup are indepedent.
> Imange that socket accounting program is attached to cg2/nosockets.
> The program is readonly and carry no security meaning.
> Why cgroup should prevent creation of cg2/nosockets/foo directory ?
I think you're misunderstanding me. What I'm saying is that, if you
allow a cgroup and one of its descendents to both enable the same type
of filter, you have just committed to some particular semantics for
what happens. And I think that the current semantics are the *wrong*
semantics for default long-term use, so you should either fix the
semantics or disable the problematic case.
>
>> # echo $$ >cg2/nosockets/sockets/cgroup.procs
>> # ping 127.0.0.1
>> PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
>> 64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.029 ms
>
> hmm. in this case the admin created a subgroup with a group that allows
> ping, so? how's that a problem?
I think you're forgetting about namespaces. There are two different
admins here. There's the admin who created the outer container and
said "no sockets here". Then there's the admin inside the container
who said "muahaha, I can create a sub-container and allow sockets
*there*".
> In case of vrf I can imagine
> a set of application auto-binding to one vrf and smaller
> set of applications binding to a different vrf.
> Or broader set of applications monitoring all tcp traffic
> and subset of them monitoring udp instead.
> Those are valid use cases.
> I guess you're advocating to run a link list of programs?
> That won't be useful for the above use cases, where there is no
> reason to run more than one program that control plane
> assigned to run for this cgroup.
Yes there is, for both monitoring and policy. If I want to monitor
all activity in a cgroup, I probably want to monitor descendents as
well. If I want to restrict a cgroup, I want to restrict its
descendents.
In the case where I actually don't want to hook the descendents, I'd
be find with having an option to turn off recursion. Maybe
net.egress_filter could also say "bpf(overridable):[hash]" or
"bpf(nonrecursive):[hash]". But you should have to opt in to allowing
your filter to be overridden.
> At this stage we decided to allow only one program per cgroup per hook
> and later can extend it if necessary.
No you can't. Since you allow the problematic case and you gave it
the surprising "one program" semantics, you can't change it later.
> As you're pointing out, in case of security, we probably
> want to preserve original bpf program that should always be
> run first and only after it returned 'ok' (we'd need to define
> what 'ok' means in secruity context) run program attached to sub-hierarchy.
It's already defined AFAICT. 1 means okay. 0 means not okay.
> Another alternative is to disallow attaching programs in sub-hierarchy
> if parent has something already attached, but it's not useful
> for general case.
> All of these are possible future extensions.
I like this last one, but IT'S NOT A POSSIBLE FUTURE EXTENSION. You
have to do it now (or disable the feature for 4.10). This is why I'm
bringing this whole thing up now.
>
>> In 4.10 with With CONFIG_CGROUP_BPF=y, a cgroup can have bpf
>> programs attached that can do things if various events occur. (Right
>> now, this means socket operations, but there are plans in the works
>> to do this for LSM hooks too.) These bpf programs can say yes or no,
>> but they can also read out various data (including socket payloads!)
>> and save them away where an attacker can find them. This sounds a
>> lot like seccomp with a narrower scope but a much stronger ability to
>> exfiltrate private information.
>
> that sounds like a future problem to solve when bpf+lsm+cgroup are
> used for security.
[...]
>
> I disagree with the urgency. I see nothing that needs immediate action.
> bpf+lsm+cgroup is not in the tree yet.
>
I disagree very strongly here. The API in 4.10 is IMO quite ugly, but
the result if bpf+lsm+cgroup works *differently* will be far, far
uglier.
I think the right solution here is to clean up the API so that it'll
work for future extensions that people are already imagining. If this
can happen for 4.10, great. If not, then postpone this stuff
entirely. I've had code I've written for Linux postponed extensively
until I've gotten the API right, and it's not so bad. (Actually, I've
even had API changes I've made reverted in -stable, IIRC. This is
much worse than postponing before a release.)
--Andy
^ permalink raw reply
* Re: [v3] net: ethernet: cavium: octeon: octeon_mgmt: Handle return NULL error from devm_ioremap
From: David Daney @ 2016-12-19 21:37 UTC (permalink / raw)
To: David Miller, arvind.yadav.cs
Cc: peter.chen, fw, david.daney, f.fainelli, netdev, linux-kernel
In-Reply-To: <20161219.110420.1570801461162159172.davem@davemloft.net>
On 12/19/2016 08:04 AM, David Miller wrote:
> From: Arvind Yadav <arvind.yadav.cs@gmail.com>
> Date: Thu, 15 Dec 2016 00:33:30 +0530
>
>> Here, If devm_ioremap will fail. It will return NULL.
>> Kernel can run into a NULL-pointer dereference.
>> This error check will avoid NULL pointer dereference.
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>
> Since ioremap() is in fact designed to possibly fail, we do
> need to always check it's return value. So this change is
> correct and I have applied it to the 'net' tree.
Yes, I think that is fine, although I have not tested the patch.
In general I like to know if a patch fixes a problem that has occurred
on a platform used by the patch author, or if the author just noticed an
issue through code inspection or automated tool for a platform that they
cannot test on.
This patch appears to fall into the second category, but attempts to
determine this for sure were for the most part unsuccessful.
With respect to ioremap(), in general I agree that it is designed to
possibly fail. For mips64 however, I think the implementation can never
fail. Certainly testing for failure fits better with what we expect to
see in Linux kernel code.
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: btusb: Configure Marvel to use one of the pins for oob wakeup
From: Rob Herring @ 2016-12-19 22:04 UTC (permalink / raw)
To: Rajat Jain
Cc: Mark Rutland, Marcel Holtmann, Gustavo Padovan, Johan Hedberg,
Amitkumar Karwar, Wei-Ning Huang, Xinming Hu, netdev, devicetree,
linux-bluetooth, Brian Norris, rajatxjain
In-Reply-To: <1481742779-15105-3-git-send-email-rajatja@google.com>
On Wed, Dec 14, 2016 at 11:12:59AM -0800, Rajat Jain wrote:
> The Marvell devices may have many gpio pins, and hence for wakeup
> on these out-of-band pins, the chip needs to be told which pin is
> to be used for wakeup, using an hci command.
>
> Thus, we read the pin number etc from the device tree node and send
> a command to the chip.
>
> Signed-off-by: Rajat Jain <rajatja@google.com>
> ---
> Note that while I would have liked to name the compatible string as more
> like "marvell, usb8997-bt", the devicetrees/bindings/usb/usb-device.txt
> requires the compatible property to be of the form "usbVID,PID".
>
> .../{marvell-bt-sd8xxx.txt => marvell-bt-8xxx.txt} | 25 ++++++++-
Acked-by: Rob Herring <robh@kernel.org>
> drivers/bluetooth/btusb.c | 59 ++++++++++++++++++++++
> 2 files changed, 82 insertions(+), 2 deletions(-)
> rename Documentation/devicetree/bindings/net/{marvell-bt-sd8xxx.txt => marvell-bt-8xxx.txt} (76%)
^ permalink raw reply
* Re: [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock
From: Rob Herring @ 2016-12-19 22:31 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo,
mark.rutland, netdev, devicetree, linux-kernel, mgreer, justin
In-Reply-To: <1481841044-4314-1-git-send-email-glansberry@gmail.com>
On Thu, Dec 15, 2016 at 05:30:42PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <geoff@kuvee.com>
>
> ---
> .../devicetree/bindings/net/nfc/trf7970a.txt | 3 ++
> drivers/nfc/trf7970a.c | 42 ++++++++++++++++------
> 2 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> index 32b35a0..9dda879 100644
> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> @@ -21,6 +21,8 @@ Optional SoC Specific Properties:
> - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
> where an extra byte is returned by Read Multiple Block commands issued
> to Type 5 tags.
> +- crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
> +
Can't you use 'clock-frequency = "27000000";'?
>
> Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>
> @@ -43,6 +45,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
> irq-status-read-quirk;
> en2-rf-quirk;
> t5t-rmb-extra-byte-quirk;
> + crystal_27mhz;
> status = "okay";
> };
> };
^ permalink raw reply
* Re: [PATCH 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage
From: Rob Herring @ 2016-12-19 22:35 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo,
mark.rutland, netdev, devicetree, linux-kernel, mgreer, justin
In-Reply-To: <1481841044-4314-2-git-send-email-glansberry@gmail.com>
On Thu, Dec 15, 2016 at 05:30:43PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <geoff@kuvee.com>
>
> ---
> Documentation/devicetree/bindings/net/nfc/trf7970a.txt | 2 ++
> drivers/nfc/trf7970a.c | 13 ++++++++++++-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> index 9dda879..208f045 100644
> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> @@ -21,6 +21,7 @@ Optional SoC Specific Properties:
> - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
> where an extra byte is returned by Read Multiple Block commands issued
> to Type 5 tags.
> +- vdd_io_1v8: Set to specify that the trf7970a io voltage should be set to 1.8V
Use the regulator binding and provide a fixed 1.8V supply.
> - crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
>
>
> @@ -45,6 +46,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
> irq-status-read-quirk;
> en2-rf-quirk;
> t5t-rmb-extra-byte-quirk;
> + vdd_io_1v8;
> crystal_27mhz;
> status = "okay";
> };
^ permalink raw reply
* Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock
From: Lino Sanfilippo @ 2016-12-19 22:49 UTC (permalink / raw)
To: Pavel Machek
Cc: Francois Romieu, bh74.an, ks.giri, vipul.pandya, peppe.cavallaro,
alexandre.torgue, davem, linux-kernel, netdev
In-Reply-To: <20161218183010.GA7886@amd>
Hi,
On 18.12.2016 19:30, Pavel Machek wrote:
> Hi!
>
>> > - e1efa87241272104d6a12c8b9fcdc4f62634d447
>>
>> Yep, a sync of the dma descriptors before the hardware gets ownership of the tx tail
>> idx is missing in the stmmac, too.
>
> I can reproduce failure with 4.4 fairly easily. I tried with dma_
> variant of barriers, and it failed, too
>
> [ 1018.410012] stmmac: early irq
> [ 1023.939702] fpga_cmd_read:wait_event timed out!
> [ 1033.128692] ------------[ cut here ]------------
> [ 1033.133329] WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:303
> dev_watchdog+0x264/0x284()
> [ 1033.141748] NETDEV WATCHDOG: eth0 (socfpga-dwmac): transmit queue 0
> timed out
> [ 1033.148861] Modules linked in:
This watchdog warning clearly says that for some reason the tx queue was stopped
but never woken up in a certain timespan (5 sec in our case, which is a lot).
Does this occur after the queue has been stopped and woken up again a few times or
is it already the first time the queue is stopped (and never woken up)?
Regards,
Lino
^ permalink raw reply
* [PATCH net 1/2] net: netcp: ethss: fix errors in ethtool ops
From: Murali Karicheri @ 2016-12-19 22:55 UTC (permalink / raw)
To: netdev, davem, linux-kernel
From: WingMan Kwok <w-kwok2@ti.com>
In ethtool ops, it needs to retrieve the corresponding
ethss module (gbe or xgbe) from the net_device structure.
Prior to this patch, the retrieving procedure only
checks for the gbe module. This patch fixes the issue
by checking the xgbe module if the net_device structure
does not correspond to the gbe module.
Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
drivers/net/ethernet/ti/netcp_ethss.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index c7e547e..a31931c 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -1746,6 +1746,17 @@ static void keystone_set_msglevel(struct net_device *ndev, u32 value)
netcp->msg_enable = value;
}
+static struct gbe_intf *keystone_get_intf_data(struct netcp_intf *netcp)
+{
+ struct gbe_intf *gbe_intf;
+
+ gbe_intf = netcp_module_get_intf_data(&gbe_module, netcp);
+ if (!gbe_intf)
+ gbe_intf = netcp_module_get_intf_data(&xgbe_module, netcp);
+
+ return gbe_intf;
+}
+
static void keystone_get_stat_strings(struct net_device *ndev,
uint32_t stringset, uint8_t *data)
{
@@ -1754,7 +1765,7 @@ static void keystone_get_stat_strings(struct net_device *ndev,
struct gbe_priv *gbe_dev;
int i;
- gbe_intf = netcp_module_get_intf_data(&gbe_module, netcp);
+ gbe_intf = keystone_get_intf_data(netcp);
if (!gbe_intf)
return;
gbe_dev = gbe_intf->gbe_dev;
@@ -1778,7 +1789,7 @@ static int keystone_get_sset_count(struct net_device *ndev, int stringset)
struct gbe_intf *gbe_intf;
struct gbe_priv *gbe_dev;
- gbe_intf = netcp_module_get_intf_data(&gbe_module, netcp);
+ gbe_intf = keystone_get_intf_data(netcp);
if (!gbe_intf)
return -EINVAL;
gbe_dev = gbe_intf->gbe_dev;
@@ -1896,7 +1907,7 @@ static void keystone_get_ethtool_stats(struct net_device *ndev,
struct gbe_intf *gbe_intf;
struct gbe_priv *gbe_dev;
- gbe_intf = netcp_module_get_intf_data(&gbe_module, netcp);
+ gbe_intf = keystone_get_intf_data(netcp);
if (!gbe_intf)
return;
@@ -1920,7 +1931,7 @@ static int keystone_get_link_ksettings(struct net_device *ndev,
if (!phy)
return -EINVAL;
- gbe_intf = netcp_module_get_intf_data(&gbe_module, netcp);
+ gbe_intf = keystone_get_intf_data(netcp);
if (!gbe_intf)
return -EINVAL;
@@ -1953,7 +1964,7 @@ static int keystone_set_link_ksettings(struct net_device *ndev,
if (!phy)
return -EINVAL;
- gbe_intf = netcp_module_get_intf_data(&gbe_module, netcp);
+ gbe_intf = keystone_get_intf_data(netcp);
if (!gbe_intf)
return -EINVAL;
--
1.9.1
^ permalink raw reply related
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