* Re: [virtio-dev] Re: [Qemu-devel] [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net
From: Cornelia Huck @ 2018-06-20 16:06 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, konrad.wilk,
Jakub Kicinski, Samudrala, Sridhar, qemu-devel, virtualization,
Siwei Liu, Venu Busireddy, Netdev, boris.ostrovsky, aaron.f.brown,
Joao Martins
In-Reply-To: <20180620170904-mutt-send-email-mst@kernel.org>
On Wed, 20 Jun 2018 17:11:59 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Jun 20, 2018 at 11:53:59AM +0200, Cornelia Huck wrote:
> > On Tue, 19 Jun 2018 23:32:06 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > On Tue, Jun 19, 2018 at 12:54:53PM +0200, Cornelia Huck wrote:
> > > > Sorry about dragging mainframes into this, but this will only work for
> > > > homogenous device coupling, not for heterogenous. Consider my vfio-pci
> > > > + virtio-net-ccw example again: The guest cannot find out that the two
> > > > belong together by checking some group ID, it has to either use the MAC
> > > > or some needs-to-be-architectured property.
> > > >
> > > > Alternatively, we could propose that mechanism as pci-only, which means
> > > > we can rely on mechanisms that won't necessarily work on non-pci
> > > > transports. (FWIW, I don't see a use case for using vfio-ccw to pass
> > > > through a network card anytime in the near future, due to the nature of
> > > > network cards currently in use on s390.)
> > >
> > > That's what it boils down to, yes. If there's need to have this for
> > > non-pci devices, then we should put it in config space.
> > > Cornelia, what do you think?
> > >
> >
> > I think the only really useful config on s390 is the vfio-pci network
> > card coupled with a virtio-net-ccw device: Using an s390 network card
> > via vfio-ccw is out due to the nature of the s390 network cards, and
> > virtio-ccw is the default transport (virtio-pci is not supported on any
> > enterprise distro AFAIK).
> >
> > For this, having a uuid in the config space could work (vfio-pci
> > devices have a config space by virtue of being pci devices, and
> > virtio-net-ccw devices have a config space by virtue of being virtio
> > devices -- ccw devices usually don't have that concept).
>
> OK so this calls for adding such a field generally (it's
> device agnostic right now).
>
> How would you suggest doing that?
I hope that I'm not thoroughly confused at this point in time, so I'll
summarize my current understanding (also keep in mind that I haven't
looked at Venu's patches yet):
- The Linux guest initiates coupling from the virtio-net driver.
Matching the other device is done via the MAC, and only pci devices
are allowed for the failover device. (There does not seem to be any
restriction on the transport of the virtio-net device.)
- The Linux guest virtio-net driver does not allow changing the MAC if
standby has been negotiated (implying that the hypervisor needs to
configure the correct MAC).
- In QEMU, we need to know which two devices (vfio-pci and virtio-net)
go together, so that the virtio-net device gets the correct MAC. We
also need the pairing so that we can make the vfio-pci device
available once the guest has negotiated the standby feature.
We can tack the two devices together in QEMU by introducing new,
optional properties pointing from the virtio-net device to the vfio-pci
device (only offer standby if this is set) and the other way around
(don't make the device visible at the start if this is set). Problems:
- The admin needs to figure out the MAC by themselves and set it
correctly. If this is incorrect, the vfio-pci device cannot be found
in the guest. (Not sure how much of a problem this is in practice --
and QEMU cannot figure out the MAC without poking at the vfio-pci
device, and we probably want to avoid that.)
- This two-way pointing makes for interesting handing of the command
line and when both devices are plugged later.
In any case, I'm not sure anymore why we'd want the extra uuid. Is
there any way QEMU (or libvirt) can figure it out without actually
looking at the vfio-pci device?
^ permalink raw reply
* Re: [PATCH v4 net-next] net:sched: add action inheritdsfield to skbedit
From: Davide Caratti @ 2018-06-20 16:08 UTC (permalink / raw)
To: Fu, Qiaobin, davem@davemloft.net
Cc: netdev@vger.kernel.org, jhs@mojatatu.com, Michel Machado,
Marcelo Ricardo Leitner, xiyou.wangcong@gmail.com
In-Reply-To: <E5616B71-959E-43FE-9096-BF523948ABB2@bu.edu>
On Tue, 2018-06-12 at 15:42 +0000, Fu, Qiaobin wrote:
> The new action inheritdsfield copies the field DS of
> IPv4 and IPv6 packets into skb->priority. This enables
> later classification of packets based on the DS field.
>
> v4:
> *Not allow setting flags other than the expected ones.
>
> *Allow dumping the pure flags.
>
> Original idea by Jamal Hadi Salim <jhs@mojatatu.com>
>
> Signed-off-by: Qiaobin Fu <qiaobinf@bu.edu>
> Reviewed-by: Michel Machado <michel@digirati.com.br>
> ---
>
[...]
> @@ -41,6 +44,25 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
>
> if (d->flags & SKBEDIT_F_PRIORITY)
> skb->priority = d->priority;
> + if (d->flags & SKBEDIT_F_INHERITDSFIELD) {
> + int wlen = skb_network_offset(skb);
> +
> + switch (tc_skb_protocol(skb)) {
> + case htons(ETH_P_IP):
> + wlen += sizeof(struct iphdr);
> + if (!pskb_may_pull(skb, wlen))
> + goto err;
> + skb->priority = ipv4_get_dsfield(ip_hdr(skb)) >> 2;
> + break;
> +
> + case htons(ETH_P_IPV6):
> + wlen += sizeof(struct ipv6hdr);
> + if (!pskb_may_pull(skb, wlen))
> + goto err;
> + skb->priority = ipv6_get_dsfield(ipv6_hdr(skb)) >> 2;
> + break;
> + }
> + }
> if (d->flags & SKBEDIT_F_QUEUE_MAPPING &&
> skb->dev->real_num_tx_queues > d->queue_mapping)
> skb_set_queue_mapping(skb, d->queue_mapping);
> @@ -53,6 +75,10 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
>
> spin_unlock(&d->tcf_lock);
> return d->tcf_action;
> +
> +err:
> + spin_unlock(&d->tcf_lock);
> + return TC_ACT_SHOT;
> }
>
sorry for asking this when the patch is a v4...
I spotted this, as I'm rebasing a small series that removes the tcf_lock
from the data plane of skbedit to gain some speed, and it converts the
stats to be per-cpu counters.
in the code above, you are catching failures of pskb_may_pull(skb) and
then you return TC_ACT_SHOT. That's OK, but I think you should update the
drop counter, like other TC actions do.
If you (author / reviewers) think this is a minor issue, like I do think,
then I can add the missing update in my series and post it when net-next
reopens.
WDYT?
thank you in advance!
regards,
--
davide
^ permalink raw reply
* [PATCH v2] ucc_geth: Add BQL support
From: Joakim Tjernlund @ 2018-06-20 16:29 UTC (permalink / raw)
To: David Miller, leoyang.li, netdev; +Cc: Joakim Tjernlund
In-Reply-To: <20180619163036.20578-1-joakim.tjernlund@infinera.com>
Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
---
v2 - Reoder varibles according to Dave
Add call to netdev_reset_queue(dev) open/close
drivers/net/ethernet/freescale/ucc_geth.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index f77ba9fa257b..e8debbde0a34 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3096,6 +3096,7 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev)
ugeth_vdbg("%s: IN", __func__);
+ netdev_sent_queue(dev, skb->len);
spin_lock_irqsave(&ugeth->lock, flags);
dev->stats.tx_bytes += skb->len;
@@ -3240,6 +3241,8 @@ static int ucc_geth_tx(struct net_device *dev, u8 txQ)
{
/* Start from the next BD that should be filled */
struct ucc_geth_private *ugeth = netdev_priv(dev);
+ unsigned int bytes_sent = 0;
+ int howmany = 0;
u8 __iomem *bd; /* BD pointer */
u32 bd_status;
@@ -3257,7 +3260,8 @@ static int ucc_geth_tx(struct net_device *dev, u8 txQ)
skb = ugeth->tx_skbuff[txQ][ugeth->skb_dirtytx[txQ]];
if (!skb)
break;
-
+ howmany++;
+ bytes_sent += skb->len;
dev->stats.tx_packets++;
dev_consume_skb_any(skb);
@@ -3279,6 +3283,7 @@ static int ucc_geth_tx(struct net_device *dev, u8 txQ)
bd_status = in_be32((u32 __iomem *)bd);
}
ugeth->confBd[txQ] = bd;
+ netdev_completed_queue(dev, howmany, bytes_sent);
return 0;
}
@@ -3479,6 +3484,7 @@ static int ucc_geth_open(struct net_device *dev)
phy_start(ugeth->phydev);
napi_enable(&ugeth->napi);
+ netdev_reset_queue(dev);
netif_start_queue(dev);
device_set_wakeup_capable(&dev->dev,
@@ -3509,6 +3515,7 @@ static int ucc_geth_close(struct net_device *dev)
free_irq(ugeth->ug_info->uf_info.irq, ugeth->ndev);
netif_stop_queue(dev);
+ netdev_reset_queue(dev);
return 0;
}
--
2.13.6
^ permalink raw reply related
* Re: [PATCH] tc: fix batch force option
From: Stephen Hemminger @ 2018-06-20 16:33 UTC (permalink / raw)
To: Vlad Buslov; +Cc: netdev, chrism
In-Reply-To: <1529479461-32048-1-git-send-email-vladbu@mellanox.com>
On Wed, 20 Jun 2018 10:24:21 +0300
Vlad Buslov <vladbu@mellanox.com> wrote:
> When sending accumulated compound command results an error, check 'force'
> option before exiting. Move return code check after putting batch bufs and
> freeing iovs to prevent memory leak. Break from loop, instead of returning
> error code to allow cleanup at the end of batch function. Don't reset ret
> code on each iteration.
>
> Fixes: 485d0c6001c4 ("tc: Add batchsize feature for filter and actions")
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> Reviewed-by: Chris Mi <chrism@mellanox.com>
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Applied
^ permalink raw reply
* [PATCH net] netfilter: nf_log: fix uninit read in nf_log_proc_dostring
From: Jann Horn @ 2018-06-20 16:33 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
netfilter-devel, coreteam, jannh
Cc: David S. Miller, netdev, linux-kernel
When proc_dostring() is called with a non-zero offset in strict mode, it
doesn't just write to the ->data buffer, it also reads. Make sure it
doesn't read uninitialized data.
Fixes: c6ac37d8d884 ("netfilter: nf_log: fix error on write NONE to [...]")
Signed-off-by: Jann Horn <jannh@google.com>
---
net/netfilter/nf_log.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
index 426457047578..2c47f9ec3511 100644
--- a/net/netfilter/nf_log.c
+++ b/net/netfilter/nf_log.c
@@ -424,6 +424,10 @@ static int nf_log_proc_dostring(struct ctl_table *table, int write,
if (write) {
struct ctl_table tmp = *table;
+ /* proc_dostring() can append to existing strings, so we need to
+ * initialize it as an empty string.
+ */
+ buf[0] = '\0';
tmp.data = buf;
r = proc_dostring(&tmp, write, buffer, lenp, ppos);
if (r)
--
2.18.0.rc1.244.gcf134e6275-goog
^ permalink raw reply related
* Re: [PATCH] [Bug 16494] NFS client over TCP hangs due to packet loss
From: Andy C @ 2018-06-20 16:40 UTC (permalink / raw)
To: Joe Perches, 'Trond Myklebust'
Cc: 'Andrew Morton', 'David Miller', kuznet, pekkas,
jmorris, yoshfuji, kaber, eric.dumazet, William.Allen.Simpson,
gilad, ilpo.jarvinen, netdev, linux-kernel, linux-nfs,
'J. Bruce Fields', 'Neil Brown',
'Chuck Lever', 'Benny Halevy',
'Alexandros Batsakis', Andy Chittenden
In-Reply-To: <0dead6e0deabafe0abdd4b30f97bcdf5c12cd824.camel@perches.com>
On 2018-06-19 22:56, Joe Perches wrote:
> On Mon, 2010-08-09 at 10:27 +0100, Andy Chittenden wrote:
>
> You really need to check your clock.
> Mail sent in the year 2010?
>
I didn't send it just now. I sent it on 2010-08-09 at 10:27. And I did
receive a copy at that time. If I'm reading the headers correctly,
something happened here:
Received: from mchehab by bombadil.infradead.org with local (Exim 4.90_1 #2 (Red Hat Linux))
id 1fVNCq-0005da-3g; Tue, 19 Jun 2018 20:26:24 +0000
Received: from vger.kernel.org ([209.132.180.67])
by bombadil.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux))
id 1OiOdm-0004za-Ks; Mon, 09 Aug 2010 09:27:30 +0000
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S1755981Ab0HIJ1U (ORCPT <rfc822;tgraf@infradead.org> + 8 others);
Mon, 9 Aug 2010 05:27:20 -0400
--
Andy
^ permalink raw reply
* Re: [PATCH] tc, bpf: add option to dump bpf verifier as C program fragment
From: Stephen Hemminger @ 2018-06-20 16:40 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David Ahern, Jakub Kicinski, Ophir Munk, netdev, Thomas Monjalon,
Olga Shern, ast
In-Reply-To: <e454ef12-0ae7-7122-3801-321a62c5d0ce@iogearbox.net>
On Wed, 20 Jun 2018 00:13:52 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 06/18/2018 11:44 PM, David Ahern wrote:
> > On 6/18/18 2:18 PM, Jakub Kicinski wrote:
> >> On Sun, 17 Jun 2018 08:48:41 +0000, Ophir Munk wrote:
> >>> Similar to cbpf used within tcpdump utility with a "-d" option to dump
> >>> the compiled packet-matching code in a human readable form - tc has the
> >>> "verbose" option to dump ebpf verifier output.
> >>> Another useful option of cbpf using tcpdump "-dd" option is to dump
> >>> packet-matching code a C program fragment. Similar to this - this commit
> >>> adds a new tc ebpf option named "code" to dump ebpf verifier as C program
> >>> fragment.
> >>>
> >>> Existing "verbose" option sample output:
> >>>
> >>> Verifier analysis:
> >>> 0: (61) r2 = *(u32 *)(r1 +52)
> >>> 1: (18) r3 = 0xdeadbeef
> >>> 3: (63) *(u32 *)(r10 -4) = r3
> >>> .
> >>> .
> >>> 11: (63) *(u32 *)(r1 +52) = r2
> >>> 12: (18) r0 = 0xffffffff
> >>> 14: (95) exit
> >>>
> >>> New "code" option sample output:
> >>>
> >>> /* struct bpf_insn cls_q_code[] = { */
> >>> {0x61, 2, 1, 52, 0x00000000},
> >>> {0x18, 3, 0, 0, 0xdeadbeef},
> >>> {0x00, 0, 0, 0, 0x00000000},
> >>> .
> >>> .
> >>> {0x63, 1, 2, 52, 0x00000000},
> >>> {0x18, 0, 0, 0, 0xffffffff},
> >>> {0x00, 0, 0, 0, 0x00000000},
> >>> {0x95, 0, 0, 0, 0x00000000},
> >>>
> >>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> >>
> >> Hmm... printing C arrays looks like hacky integration with some C
> >> code... Would you not be better served by simply using libbpf in
> >> whatever is consuming this output?
> >
> > I was thinking the same. bpftool would provide options too -- print the
> > above, print in macro encodings and verifier. I gave an example of this
> > side by side dump at netconf 2.1. Does not look like the slides made it
> > online; see attached.
>
> +1, I would also doubt that this adds a lot in terms of debuggability
> when you're trying to load an object file with thousands of insns. Better
> way would be to use llvm-objdump on the obj file to get to the annotated
> disassembly, see also example in [0]. A .o to .c converter is wip for
> libbpf/bpftool as presented in [1], which should provide the flexibility
> to embedd an obj file.
>
> Cheers,
> Daniel
>
> [0] http://cilium.readthedocs.io/en/latest/bpf/#llvm
> [1] http://vger.kernel.org/netconf2018_files/AlexeiStarovoitov_netconf2018.pdf page 22
I am going to not accept this for now. Please respin for iproute2 next if
you think bpftool won't be able to handle this.
^ permalink raw reply
* Re: [PATCH v4 net-next] net:sched: add action inheritdsfield to skbedit
From: Michel Machado @ 2018-06-20 16:47 UTC (permalink / raw)
To: Davide Caratti, Fu, Qiaobin, davem@davemloft.net
Cc: netdev@vger.kernel.org, jhs@mojatatu.com, Marcelo Ricardo Leitner,
xiyou.wangcong@gmail.com
In-Reply-To: <264a03455daf28d33809ea77e4badc6a5ec5c9e6.camel@redhat.com>
On 06/20/2018 12:08 PM, Davide Caratti wrote:
> On Tue, 2018-06-12 at 15:42 +0000, Fu, Qiaobin wrote:
>> The new action inheritdsfield copies the field DS of
>> IPv4 and IPv6 packets into skb->priority. This enables
>> later classification of packets based on the DS field.
>>
>> v4:
>> *Not allow setting flags other than the expected ones.
>>
>> *Allow dumping the pure flags.
>>
>> Original idea by Jamal Hadi Salim <jhs@mojatatu.com>
>>
>> Signed-off-by: Qiaobin Fu <qiaobinf@bu.edu>
>> Reviewed-by: Michel Machado <michel@digirati.com.br>
>> ---
>>
>
> [...]
>
>> @@ -41,6 +44,25 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
>>
>> if (d->flags & SKBEDIT_F_PRIORITY)
>> skb->priority = d->priority;
>> + if (d->flags & SKBEDIT_F_INHERITDSFIELD) {
>> + int wlen = skb_network_offset(skb);
>> +
>> + switch (tc_skb_protocol(skb)) {
>> + case htons(ETH_P_IP):
>> + wlen += sizeof(struct iphdr);
>> + if (!pskb_may_pull(skb, wlen))
>> + goto err;
>> + skb->priority = ipv4_get_dsfield(ip_hdr(skb)) >> 2;
>> + break;
>> +
>> + case htons(ETH_P_IPV6):
>> + wlen += sizeof(struct ipv6hdr);
>> + if (!pskb_may_pull(skb, wlen))
>> + goto err;
>> + skb->priority = ipv6_get_dsfield(ipv6_hdr(skb)) >> 2;
>> + break;
>> + }
>> + }
>> if (d->flags & SKBEDIT_F_QUEUE_MAPPING &&
>> skb->dev->real_num_tx_queues > d->queue_mapping)
>> skb_set_queue_mapping(skb, d->queue_mapping);
>> @@ -53,6 +75,10 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
>>
>> spin_unlock(&d->tcf_lock);
>> return d->tcf_action;
>> +
>> +err:
>> + spin_unlock(&d->tcf_lock);
>> + return TC_ACT_SHOT;
>> }
>>
>
> sorry for asking this when the patch is a v4...
>
> I spotted this, as I'm rebasing a small series that removes the tcf_lock
> from the data plane of skbedit to gain some speed, and it converts the
> stats to be per-cpu counters.
>
> in the code above, you are catching failures of pskb_may_pull(skb) and
> then you return TC_ACT_SHOT. That's OK, but I think you should update the
> drop counter, like other TC actions do.
>
> If you (author / reviewers) think this is a minor issue, like I do think,
> then I can add the missing update in my series and post it when net-next
> reopens.
>
> WDYT?
>
> thank you in advance!
> regards,
>
Hi Davide,
I agree that we should update the drop counter, but given that
you're already converting the stats to be per-cpu counters, whatever we
add now will be just symbolic since you're going to change it anyway. If
reviewers think that Qiaobin's patch must add the update line, could you
provide the exact line and location so we avoid going to v6 of this patch?
[ ]'s
Michel Machado
^ permalink raw reply
* Re: [PATCH v4 net-next] net:sched: add action inheritdsfield to skbedit
From: Davide Caratti @ 2018-06-20 17:02 UTC (permalink / raw)
To: Michel Machado, Fu, Qiaobin, davem@davemloft.net
Cc: netdev@vger.kernel.org, jhs@mojatatu.com, Marcelo Ricardo Leitner,
xiyou.wangcong@gmail.com
In-Reply-To: <9e0d0a90-4ad5-722f-1033-40b72c3841ed@digirati.com.br>
On Wed, 2018-06-20 at 12:47 -0400, Michel Machado wrote:
> On 06/20/2018 12:08 PM, Davide Caratti wrote:
> > On Tue, 2018-06-12 at 15:42 +0000, Fu, Qiaobin wrote:
> > > The new action inheritdsfield copies the field DS of
> > > IPv4 and IPv6 packets into skb->priority. This enables
> > > later classification of packets based on the DS field.
> > >
> > > v4:
> > > *Not allow setting flags other than the expected ones.
> > >
> > > *Allow dumping the pure flags.
> > >
> > > Original idea by Jamal Hadi Salim <jhs@mojatatu.com>
> > >
> > > Signed-off-by: Qiaobin Fu <qiaobinf@bu.edu>
> > > Reviewed-by: Michel Machado <michel@digirati.com.br>
> > > ---
> > >
> >
> > [...]
> >
> > > @@ -41,6 +44,25 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
> > >
> > > if (d->flags & SKBEDIT_F_PRIORITY)
> > > skb->priority = d->priority;
> > > + if (d->flags & SKBEDIT_F_INHERITDSFIELD) {
> > > + int wlen = skb_network_offset(skb);
> > > +
> > > + switch (tc_skb_protocol(skb)) {
> > > + case htons(ETH_P_IP):
> > > + wlen += sizeof(struct iphdr);
> > > + if (!pskb_may_pull(skb, wlen))
> > > + goto err;
> > > + skb->priority = ipv4_get_dsfield(ip_hdr(skb)) >> 2;
> > > + break;
> > > +
> > > + case htons(ETH_P_IPV6):
> > > + wlen += sizeof(struct ipv6hdr);
> > > + if (!pskb_may_pull(skb, wlen))
> > > + goto err;
> > > + skb->priority = ipv6_get_dsfield(ipv6_hdr(skb)) >> 2;
> > > + break;
> > > + }
> > > + }
> > > if (d->flags & SKBEDIT_F_QUEUE_MAPPING &&
> > > skb->dev->real_num_tx_queues > d->queue_mapping)
> > > skb_set_queue_mapping(skb, d->queue_mapping);
> > > @@ -53,6 +75,10 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
> > >
> > > spin_unlock(&d->tcf_lock);
> > > return d->tcf_action;
> > > +
> > > +err:
> > > + spin_unlock(&d->tcf_lock);
> > > + return TC_ACT_SHOT;
> > > }
> > >
> >
> > sorry for asking this when the patch is a v4...
> >
> > I spotted this, as I'm rebasing a small series that removes the tcf_lock
> > from the data plane of skbedit to gain some speed, and it converts the
> > stats to be per-cpu counters.
> >
> > in the code above, you are catching failures of pskb_may_pull(skb) and
> > then you return TC_ACT_SHOT. That's OK, but I think you should update the
> > drop counter, like other TC actions do.
> >
> > If you (author / reviewers) think this is a minor issue, like I do think,
> > then I can add the missing update in my series and post it when net-next
> > reopens.
> >
> > WDYT?
> >
> > thank you in advance!
> > regards,
> >
>
> Hi Davide,
>
> I agree that we should update the drop counter, but given that
> you're already converting the stats to be per-cpu counters, whatever we
> add now will be just symbolic since you're going to change it anyway.
that's ok for me also, as I can use the current v4 code for the rebase
(and not wait for another respin) _ but let's hear what reviewers think.
> If
> reviewers think that Qiaobin's patch must add the update line, could you
> provide the exact line and location so we avoid going to v6 of this patch?
In case, I was thinking of something like:
https://elixir.bootlin.com/linux/v4.18-rc1/source/net/sched/act_ipt.c#L249
so, between 'err:' and 'spin_unlock(&d->tcf_lock)', insert a line like:
d->tcf_qstats.drop++;
regards,
--
davide
^ permalink raw reply
* Re: [PATCH net] sctp: fix erroneous inc of snmp SctpFragUsrMsgs
From: Neil Horman @ 2018-06-20 17:08 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: netdev, linux-sctp, Vlad Yasevich
In-Reply-To: <d89c1e422158d21710ce938aa093a20960bd55e9.1529509634.git.marcelo.leitner@gmail.com>
On Wed, Jun 20, 2018 at 12:47:52PM -0300, Marcelo Ricardo Leitner wrote:
> Currently it is incrementing SctpFragUsrMsgs when the user message size
> is of the exactly same size as the maximum fragment size, which is wrong.
>
> The fix is to increment it only when user message is bigger than the
> maximum fragment size.
>
> Fixes: bfd2e4b8734d ("sctp: refactor sctp_datamsg_from_user")
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/sctp/chunk.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> index 79daa98208c391c780440144d69bc7be875c3476..bfb9f812e2ef9fa605b08dc1f534781573c3abf8 100644
> --- a/net/sctp/chunk.c
> +++ b/net/sctp/chunk.c
> @@ -237,7 +237,9 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> /* Account for a different sized first fragment */
> if (msg_len >= first_len) {
> msg->can_delay = 0;
> - SCTP_INC_STATS(sock_net(asoc->base.sk), SCTP_MIB_FRAGUSRMSGS);
> + if (msg_len > first_len)
> + SCTP_INC_STATS(sock_net(asoc->base.sk),
> + SCTP_MIB_FRAGUSRMSGS);
> } else {
> /* Which may be the only one... */
> first_len = msg_len;
> --
> 2.14.4
>
>
Acked-by: Neil Horman <nhorman@redhat.com>
^ permalink raw reply
* Re: [PATCH] net: Fix device name resolving crash in default_device_exit()
From: David Ahern @ 2018-06-20 17:15 UTC (permalink / raw)
To: Kirill Tkhai, netdev
Cc: davem, daniel, jakub.kicinski, ast, linux, john.fastabend, brouer
In-Reply-To: <88c1a635-4404-253b-6144-d7f3f9779531@virtuozzo.com>
On 6/20/18 2:57 AM, Kirill Tkhai wrote:
> From: Kirill Tkhai <ktkhai@virtuozzo.com>
>
> The following script makes kernel to crash since it can't obtain
> a name for a device, when the name is occupied by another device:
>
> #!/bin/bash
> ifconfig eth0 down
> ifconfig eth1 down
> index=`cat /sys/class/net/eth1/ifindex`
> ip link set eth1 name dev$index
> unshare -n sleep 1h &
> pid=$!
> while [[ "`readlink /proc/self/ns/net`" == "`readlink /proc/$pid/ns/net`" ]]; do continue; done
> ip link set dev$index netns $pid
> ip link set eth0 name dev$index
> kill -9 $pid
>
> Kernel messages:
>
> virtio_net virtio1 dev3: renamed from eth1
> virtio_net virtio0 dev3: renamed from eth0
> default_device_exit: failed to move dev3 to init_net: -17
> ------------[ cut here ]------------
> kernel BUG at net/core/dev.c:8978!
> invalid opcode: 0000 [#1] PREEMPT SMP
> CPU: 1 PID: 276 Comm: kworker/u8:3 Not tainted 4.17.0+ #292
> Workqueue: netns cleanup_net
> RIP: 0010:default_device_exit+0x9c/0xb0
> [stack trace snipped]
>
> This patch gives more variability during choosing new name
> of device and fixes the problem.
>
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>
> Since there is no suggestions how to fix this in another way, I'm resending the patch.
This patch does not remove the BUG, so does not really solve the
problem. ie., it is fairly trivial to write a script (32k dev%d named
devices in init_net) that triggers it again, so your commit subject and
commit log are not correct with the references to 'fixing the problem'.
The change does provide more variability in naming and reduces the
likelihood of not being able to push a device back to init_net.
^ permalink raw reply
* Re: [PATCH v3 07/14] net: smc911x: remove the dmaengine compat need
From: Robert Jarzmik @ 2018-06-20 17:17 UTC (permalink / raw)
To: David S. Miller; +Cc: Daniel Mack, linux-kernel, netdev
In-Reply-To: <20180617170217.24177-8-robert.jarzmik@free.fr>
Hi David,
I have converted all the pxa related drivers to DMA slave maps, the only 2 that
remain in my queue are touching your tree, ie. smc911x and smc91x (see [1]).
Once these 2 are done, I have my last dependency on the dma filter function
removed, and can queue the remaining cleanup patches.
Could you (or somebody from netdev) review it and either ack it (and I'll take
it through the pxa tree), or take it for v4.19 please ?
Cheers.
--
Robert
[1] Submission
Robert Jarzmik <robert.jarzmik@free.fr> writes:
> As the pxa architecture switched towards the dmaengine slave map, the
> old compatibility mechanism to acquire the dma requestor line number and
> priority are not needed anymore.
>
> This patch simplifies the dma resource acquisition, using the more
> generic function dma_request_slave_channel().
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
> Since v2: converted to NULL filter function and NULL dma parameter call
> ---
> drivers/net/ethernet/smsc/smc911x.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/smsc/smc911x.c b/drivers/net/ethernet/smsc/smc911x.c
> index 05157442a980..b1b53f6c452f 100644
> --- a/drivers/net/ethernet/smsc/smc911x.c
> +++ b/drivers/net/ethernet/smsc/smc911x.c
> @@ -74,7 +74,6 @@ static const char version[] =
> #include <linux/skbuff.h>
>
> #include <linux/dmaengine.h>
> -#include <linux/dma/pxa-dma.h>
>
> #include <asm/io.h>
>
> @@ -1795,7 +1794,6 @@ static int smc911x_probe(struct net_device *dev)
> #ifdef SMC_USE_DMA
> struct dma_slave_config config;
> dma_cap_mask_t mask;
> - struct pxad_param param;
> #endif
>
> DBG(SMC_DEBUG_FUNC, dev, "--> %s\n", __func__);
> @@ -1971,15 +1969,8 @@ static int smc911x_probe(struct net_device *dev)
>
> dma_cap_zero(mask);
> dma_cap_set(DMA_SLAVE, mask);
> - param.prio = PXAD_PRIO_LOWEST;
> - param.drcmr = -1UL;
> -
> - lp->rxdma =
> - dma_request_slave_channel_compat(mask, pxad_filter_fn,
> - ¶m, &dev->dev, "rx");
> - lp->txdma =
> - dma_request_slave_channel_compat(mask, pxad_filter_fn,
> - ¶m, &dev->dev, "tx");
> + lp->rxdma = dma_request_channel(mask, NULL, NULL);
> + lp->txdma = dma_request_channel(mask, NULL, NULL);
> lp->rxdma_active = 0;
> lp->txdma_active = 0;
--
Robert
^ permalink raw reply
* Re: [PATCH v3 08/14] net: smc91x: remove the dmaengine compat need
From: Robert Jarzmik @ 2018-06-20 17:17 UTC (permalink / raw)
To: David S. Miller; +Cc: Daniel Mack, linux-kernel, netdev
In-Reply-To: <20180617170217.24177-9-robert.jarzmik@free.fr>
Hi David,
I have converted all the pxa related drivers to DMA slave maps, the only 2 that
remain in my queue are touching your tree, ie. smc911x and smc91x (see [1]).
Once these 2 are done, I have my last dependency on the dma filter function
removed, and can queue the remaining cleanup patches.
Could you (or somebody from netdev) review it and either ack it (and I'll take
it through the pxa tree), or take it for v4.19 please ?
Cheers.
--
Robert
[1] Submission
Robert Jarzmik <robert.jarzmik@free.fr> writes:
> As the pxa architecture switched towards the dmaengine slave map, the
> old compatibility mechanism to acquire the dma requestor line number and
> priority are not needed anymore.
>
> This patch simplifies the dma resource acquisition, using the more
> generic function dma_request_slave_channel().
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
> Since v2: converted to NULL filter function and NULL dma parameter call
> ---
> drivers/net/ethernet/smsc/smc91x.c | 9 +--------
> drivers/net/ethernet/smsc/smc91x.h | 1 -
> 2 files changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/smsc/smc91x.c b/drivers/net/ethernet/smsc/smc91x.c
> index 080428762858..b944828f9ea3 100644
> --- a/drivers/net/ethernet/smsc/smc91x.c
> +++ b/drivers/net/ethernet/smsc/smc91x.c
> @@ -2019,17 +2019,10 @@ static int smc_probe(struct net_device *dev, void __iomem *ioaddr,
> # endif
> if (lp->cfg.flags & SMC91X_USE_DMA) {
> dma_cap_mask_t mask;
> - struct pxad_param param;
>
> dma_cap_zero(mask);
> dma_cap_set(DMA_SLAVE, mask);
> - param.prio = PXAD_PRIO_LOWEST;
> - param.drcmr = -1UL;
> -
> - lp->dma_chan =
> - dma_request_slave_channel_compat(mask, pxad_filter_fn,
> - ¶m, &dev->dev,
> - "data");
> + lp->dma_chan = dma_request_channel(mask, NULL, NULL);
> }
> #endif
>
> diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
> index b337ee97e0c0..a27352229fc2 100644
> --- a/drivers/net/ethernet/smsc/smc91x.h
> +++ b/drivers/net/ethernet/smsc/smc91x.h
> @@ -301,7 +301,6 @@ struct smc_local {
> * as RX which can overrun memory and lose packets.
> */
> #include <linux/dma-mapping.h>
> -#include <linux/dma/pxa-dma.h>
>
> #ifdef SMC_insl
> #undef SMC_insl
--
Robert
^ permalink raw reply
* Re: [PATCH iproute2-next v3] ip-xfrm: Add support for OUTPUT_MARK
From: David Ahern @ 2018-06-20 17:18 UTC (permalink / raw)
To: Subash Abhinov Kasiviswanathan, lorenzo, netdev, stephen,
steffen.klassert
In-Reply-To: <1529116375-7578-1-git-send-email-subashab@codeaurora.org>
On 6/15/18 8:32 PM, Subash Abhinov Kasiviswanathan wrote:
> This patch adds support for OUTPUT_MARK in xfrm state to exercise the
> functionality added by kernel commit 077fbac405bf
> ("net: xfrm: support setting an output mark.").
>
...
> v1->v2: Moved the XFRMA_OUTPUT_MARK print after XFRMA_MARK in
> xfrm_xfrma_print() as mentioned by Lorenzo
>
> v2->v3: Fix one help formatting error as mentioned by Lorenzo.
> Keep mark and output-mark on the same line and add man page info as
> mentioned by David.
>
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
> ---
> ip/ipxfrm.c | 17 ++++++++++++++++-
> ip/xfrm_state.c | 9 +++++++++
> man/man8/ip-xfrm.8 | 11 +++++++++++
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
applied to iproute2-next. Thanks
^ permalink raw reply
* Re: [PATCH iproute2-next 1/1] tc: jsonify nat action
From: David Ahern @ 2018-06-20 17:24 UTC (permalink / raw)
To: Keara Leibovitz, dsahern; +Cc: stephen, netdev, kernel
In-Reply-To: <1529348269-30039-1-git-send-email-kleib@mojatatu.com>
On 6/18/18 12:57 PM, Keara Leibovitz wrote:
> Add json output support for nat action
>
...
>
> Signed-off-by: Keara Leibovitz <kleib@mojatatu.com>
> ---
> tc/m_nat.c | 32 +++++++++++++++++++-------------
> 1 file changed, 19 insertions(+), 13 deletions(-)
>
applied to iproute2-next. Thanks
^ permalink raw reply
* Re: [PATCH net][RESEND] strparser: Don't schedule in workqueue in paused state
From: Dave Watson @ 2018-06-20 17:32 UTC (permalink / raw)
To: Vakul Garg
Cc: davem, doronrk, tom, john.fastabend, netdev, ebiggers,
linux-kernel
In-Reply-To: <20180620215949.32334-1-vakul.garg@nxp.com>
On 06/21/18 03:29 AM, Vakul Garg wrote:
> In function strp_data_ready(), it is useless to call queue_work if
> the state of strparser is already paused. The state checking should
> be done before calling queue_work. The change reduces the context
> switches and improves the ktls-rx throughput by approx 20% (measured
> on cortex-a53 based platform).
Nice find, works for me. Although current code isn't broken, just
slower, net-next would be fine.
Acked-by: Dave Watson <davejwatson@fb.com>
^ permalink raw reply
* [PATCH net] cls_flower: fix use after free in flower S/W path
From: Paolo Abeni @ 2018-06-20 17:34 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Marcelo Ricardo Leitner,
Paul Blakey
If flower filter is created without the skip_sw flag, fl_mask_put()
can race with fl_classify() and we can destroy the mask rhashtable
while a lookup operation is accessing it.
BUG: unable to handle kernel paging request at 00000000000911d1
PGD 0 P4D 0
SMP PTI
CPU: 3 PID: 5582 Comm: vhost-5541 Not tainted 4.18.0-rc1.vanilla+ #1950
Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.1.7 06/16/2016
RIP: 0010:rht_bucket_nested+0x20/0x60
Code: 31 c8 c1 c1 18 29 c8 c3 66 90 8b 4f 04 ba 01 00 00 00 8b 07 48 8b bf 80 00 00 0
RSP: 0018:ffffafc5cfbb7a48 EFLAGS: 00010206
RAX: 0000000000001978 RBX: ffff9f12dff88a00 RCX: 00000000ffff9f12
RDX: 00000000000911d1 RSI: 0000000000000148 RDI: 0000000000000001
RBP: ffff9f12dff88a00 R08: 000000005f1cc119 R09: 00000000a715fae2
R10: ffffafc5cfbb7aa8 R11: ffff9f1cb4be804e R12: ffff9f1265e13000
R13: 0000000000000000 R14: ffffafc5cfbb7b48 R15: ffff9f12dff88b68
FS: 0000000000000000(0000) GS:ffff9f1d3f0c0000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000000911d1 CR3: 0000001575a94006 CR4: 00000000001626e0
Call Trace:
fl_lookup+0x134/0x140 [cls_flower]
fl_classify+0xf3/0x180 [cls_flower]
tcf_classify+0x78/0x150
__netif_receive_skb_core+0x69e/0xa50
netif_receive_skb_internal+0x42/0xf0
tun_get_user+0xdd5/0xfd0 [tun]
tun_sendmsg+0x52/0x70 [tun]
handle_tx+0x2b3/0x5f0 [vhost_net]
vhost_worker+0xab/0x100 [vhost]
kthread+0xf8/0x130
ret_from_fork+0x35/0x40
Modules linked in: act_mirred act_gact cls_flower vhost_net vhost tap sch_ingress
CR2: 00000000000911d1
Fix the above waiting for a RCU grace period before destroying the
rhashtable.
Fixes: 05cd271fd61a ("cls_flower: Support multiple masks per priority")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/sched/cls_flower.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 2b5be42a9f1c..5e7333c6472d 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -203,6 +203,17 @@ static int fl_init(struct tcf_proto *tp)
return rhashtable_init(&head->ht, &mask_ht_params);
}
+static void fl_mask_free(struct fl_flow_mask *mask)
+{
+ rhashtable_destroy(&mask->ht);
+ kfree(mask);
+}
+
+static void fl_mask_free_rcu(struct rcu_head *entry)
+{
+ fl_mask_free(container_of(entry, struct fl_flow_mask, rcu));
+}
+
static bool fl_mask_put(struct cls_fl_head *head, struct fl_flow_mask *mask,
bool async)
{
@@ -210,12 +221,11 @@ static bool fl_mask_put(struct cls_fl_head *head, struct fl_flow_mask *mask,
return false;
rhashtable_remove_fast(&head->ht, &mask->ht_node, mask_ht_params);
- rhashtable_destroy(&mask->ht);
list_del_rcu(&mask->list);
if (async)
- kfree_rcu(mask, rcu);
+ call_rcu(&mask->rcu, fl_mask_free_rcu);
else
- kfree(mask);
+ fl_mask_free(mask);
return true;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net] ipvlan: call dev_change_flags when reset ipvlan mode
From: Cong Wang @ 2018-06-20 17:45 UTC (permalink / raw)
To: David Miller
Cc: Hangbin Liu, Linux Kernel Network Developers, Stefano Brivio,
Paolo Abeni, Mahesh Bandewar
In-Reply-To: <20180620.143147.2291173423483856091.davem@davemloft.net>
On Tue, Jun 19, 2018 at 10:31 PM, David Miller <davem@davemloft.net> wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> Date: Wed, 20 Jun 2018 11:22:54 +0800
>
>> The only case dev_change_flags() return an err is when we change IFF_UP flag.
>> Since we only set/reset IFF_NOARP, do you think we still need to check the
>> return value?
>
> It is bad to try and take shortcuts on error handling using assumptions
> like that.
>
> If dev_change_flags() is adjusted to return error codes in more
> situations, nobody is going to remember to undo your "optimziation"
> here.
>
> Please check for errors, thank you.
Yeah. Also since the notifier is triggered in this case:
if (dev->flags & IFF_UP &&
(changes & ~(IFF_UP | IFF_PROMISC | IFF_ALLMULTI | IFF_VOLATILE))) {
struct netdev_notifier_change_info change_info = {
.info = {
.dev = dev,
},
.flags_changed = changes,
};
call_netdevice_notifiers_info(NETDEV_CHANGE, &change_info.info);
}
the return value of call_netdevice_notifiers_info() isn't captured
either, but it should be.
^ permalink raw reply
* Re: [RFC v2, net-next, PATCH 4/4] net/cpsw_switchdev: add switchdev mode of operation on cpsw driver
From: Ilias Apalodimas @ 2018-06-20 17:51 UTC (permalink / raw)
To: Ivan Vecera
Cc: Andrew Lunn, netdev, grygorii.strashko, ivan.khoronzhuk, nsekhar,
jiri, f.fainelli, francois.ozog, yogeshs, spatton, Jose.Abreu
In-Reply-To: <671c83ea-6227-cc09-e604-14cfa6804726@redhat.com>
Hello Ivan,
On Wed, Jun 20, 2018 at 02:56:48PM +0200, Ivan Vecera wrote:
> On 18.6.2018 22:19, Ilias Apalodimas wrote:
> > Jiri proposed using devlink, which makes sense, but i am not sure it's
> > applicable on this patchset. This will change the driver completely and will
> > totally break backwards compatibility.
>
> Another good reason for a new driver.
>
> I.
This is actually conflicting at least to my understanding. Jiri proposed using
devlink was used as an alternative method to enable a new mode instead of
adding it on a .config option. A new driver wouldn't have a need for that right?
Thanks
Ilias
^ permalink raw reply
* Re: [RFC v2, net-next, PATCH 4/4] net/cpsw_switchdev: add switchdev mode of operation on cpsw driver
From: Andrew Lunn @ 2018-06-20 17:57 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: Ivan Vecera, netdev, grygorii.strashko, ivan.khoronzhuk, nsekhar,
jiri, f.fainelli, francois.ozog, yogeshs, spatton, Jose.Abreu
In-Reply-To: <20180620175128.GA27235@apalos>
> > Another good reason for a new driver.
> >
> > I.
> This is actually conflicting at least to my understanding. Jiri proposed using
> devlink was used as an alternative method to enable a new mode instead of
> adding it on a .config option. A new driver wouldn't have a need for that right?
A new driver would only do switchdev. So correct, there would not be
any configuration need. It would also give you a chance to have a new
device tree binding, if that helps at all.
Andrew
^ permalink raw reply
* Re: [RFC v2, net-next, PATCH 4/4] net/cpsw_switchdev: add switchdev mode of operation on cpsw driver
From: Florian Fainelli @ 2018-06-20 17:58 UTC (permalink / raw)
To: Ilias Apalodimas, Ivan Vecera
Cc: Andrew Lunn, netdev, grygorii.strashko, ivan.khoronzhuk, nsekhar,
jiri, francois.ozog, yogeshs, spatton, Jose.Abreu
In-Reply-To: <20180620175128.GA27235@apalos>
On 06/20/2018 10:51 AM, Ilias Apalodimas wrote:
> Hello Ivan,
> On Wed, Jun 20, 2018 at 02:56:48PM +0200, Ivan Vecera wrote:
>> On 18.6.2018 22:19, Ilias Apalodimas wrote:
>>> Jiri proposed using devlink, which makes sense, but i am not sure it's
>>> applicable on this patchset. This will change the driver completely and will
>>> totally break backwards compatibility.
>>
>> Another good reason for a new driver.
>>
>> I.
> This is actually conflicting at least to my understanding. Jiri proposed using
> devlink was used as an alternative method to enable a new mode instead of
> adding it on a .config option. A new driver wouldn't have a need for that right?
Correct, with a new driver would likely behave correctly upon being
probed such that you could have your switch ports act as normal network
devices from which you could run IP-config and do NFS boot.
--
Florian
^ permalink raw reply
* Re: [RFC v2, net-next, PATCH 4/4] net/cpsw_switchdev: add switchdev mode of operation on cpsw driver
From: Ilias Apalodimas @ 2018-06-20 18:03 UTC (permalink / raw)
To: Florian Fainelli
Cc: Ivan Vecera, Andrew Lunn, netdev, grygorii.strashko,
ivan.khoronzhuk, nsekhar, jiri, francois.ozog, yogeshs, spatton,
Jose.Abreu
In-Reply-To: <edad81cb-fb30-fede-1629-b32295979e95@gmail.com>
Hi Florian,
On Wed, Jun 20, 2018 at 10:58:26AM -0700, Florian Fainelli wrote:
> On 06/20/2018 10:51 AM, Ilias Apalodimas wrote:
> > Hello Ivan,
> > On Wed, Jun 20, 2018 at 02:56:48PM +0200, Ivan Vecera wrote:
> >> On 18.6.2018 22:19, Ilias Apalodimas wrote:
> >>> Jiri proposed using devlink, which makes sense, but i am not sure it's
> >>> applicable on this patchset. This will change the driver completely and will
> >>> totally break backwards compatibility.
> >>
> >> Another good reason for a new driver.
> >>
> >> I.
> > This is actually conflicting at least to my understanding. Jiri proposed using
> > devlink was used as an alternative method to enable a new mode instead of
> > adding it on a .config option. A new driver wouldn't have a need for that right?
>
> Correct, with a new driver would likely behave correctly upon being
> probed such that you could have your switch ports act as normal network
> devices from which you could run IP-config and do NFS boot.
The current driver also does NFS properly and the 2 ethernet ports act as normal
network interfaces. The NFS section in the cover letter
is to cover the cases were users running on NFS need to change the running
switch configuration(starting from adding the 2 interfaces on a bridge).
Since iproute2 is located on the NFS filesystem the moment
network connectivity is lost, you loose the ability to perform further
configuration and in certian configuration scenarios render the device
unusable.
> --
> Florian
Thanks
Ilias
^ permalink raw reply
* Re: [PATCH net] cls_flower: fix use after free in flower S/W path
From: Cong Wang @ 2018-06-20 18:06 UTC (permalink / raw)
To: Paolo Abeni
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
Marcelo Ricardo Leitner, Paul Blakey
In-Reply-To: <e3a9263c79766595ee701b3d66c885ea3d2febe6.1529515725.git.pabeni@redhat.com>
On Wed, Jun 20, 2018 at 10:34 AM, Paolo Abeni <pabeni@redhat.com> wrote:
>
> +static void fl_mask_free(struct fl_flow_mask *mask)
> +{
> + rhashtable_destroy(&mask->ht);
I don't believe you can call rhashtable_destroy() in BH
context, it acquires a mutex...
^ permalink raw reply
* Re: [PATCH v4 net-next] net:sched: add action inheritdsfield to skbedit
From: Marcelo Ricardo Leitner @ 2018-06-20 18:40 UTC (permalink / raw)
To: Davide Caratti
Cc: Michel Machado, Fu, Qiaobin, davem@davemloft.net,
netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com
In-Reply-To: <a1716a3accbbf9a72bbedb011862ac57eaddf481.camel@redhat.com>
On Wed, Jun 20, 2018 at 07:02:41PM +0200, Davide Caratti wrote:
...
> > I agree that we should update the drop counter, but given that
> > you're already converting the stats to be per-cpu counters, whatever we
> > add now will be just symbolic since you're going to change it anyway.
It wouldn't be symbolic. One thing is to convert a given increment
into something else, another is to start increasing it for some (new)
reason.
>
> that's ok for me also, as I can use the current v4 code for the rebase
> (and not wait for another respin) _ but let's hear what reviewers think.
>
> > If
> > reviewers think that Qiaobin's patch must add the update line, could you
> > provide the exact line and location so we avoid going to v6 of this patch?
>
> In case, I was thinking of something like:
>
> https://elixir.bootlin.com/linux/v4.18-rc1/source/net/sched/act_ipt.c#L249
>
> so, between 'err:' and 'spin_unlock(&d->tcf_lock)', insert a line like:
>
> d->tcf_qstats.drop++;
I prefer the more complete version. Then it will have a more complete
(git) history and help people when troubleshooting.
Thanks,
Marcelo
^ permalink raw reply
* [PATCH bpf 0/2] tools: bpftool: small fixes for error handling in prog load
From: Jakub Kicinski @ 2018-06-20 18:42 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, oss-drivers, Jakub Kicinski
Hi!
Two small fixes for error handling in bpftool prog load, first patch
removes a duplicated message, second one frees resources correctly.
Multiple error messages break JSON:
# bpftool -jp prog load tracex1_kern.o /sys/fs/bpf/a
{
"error": "can't pin the object (/sys/fs/bpf/a): File exists"
},{
"error": "failed to pin program"
}
Jakub Kicinski (2):
tools: bpftool: remove duplicated error message on prog load
tools: bpftool: remember to close the libbpf object on prog load error
tools/bpf/bpftool/prog.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
--
2.17.1
^ 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