* Re: [PATCH net 2/2] conntrack: enable to tune gc parameters
From: Florian Westphal @ 2016-10-14 10:37 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: Florian Westphal, davem, pablo, netdev, netfilter-devel
In-Reply-To: <f1df137d-7278-f6fb-3237-1bd20afadb48@6wind.com>
Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> Le 13/10/2016 à 22:43, Florian Westphal a écrit :
> > Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> >> Le 10/10/2016 à 16:04, Florian Westphal a écrit :
> >>> Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> >>>> After commit b87a2f9199ea ("netfilter: conntrack: add gc worker to remove
> >>>> timed-out entries"), netlink conntrack deletion events may be sent with a
> >>>> huge delay. It could be interesting to let the user tweak gc parameters
> >>>> depending on its use case.
> >>>
> >>> Hmm, care to elaborate?
> >>>
> >>> I am not against doing this but I'd like to hear/read your use case.
> >>>
> >>> The expectation is that in almot all cases eviction will happen from
> >>> packet path. The gc worker is jusdt there for case where a busy system
> >>> goes idle.
> >> It was precisely that case. After a period of activity, the event is sent a long
> >> time after the timeout. If the router does not manage a lot of flows, why not
> >> trying to parse more entries instead of the default 1/64 of the table?
> >> In fact, I don't understand why using GC_MAX_BUCKETS_DIV instead of using always
> >> GC_MAX_BUCKETS whatever the size of the table is.
> >
> > I wanted to make sure that we have a known upper bound on the number of
> > buckets we process so that we do not block other pending kworker items
> > for too long.
> I don't understand. GC_MAX_BUCKETS is the upper bound and I agree that it is
> needed. But why GC_MAX_BUCKETS_DIV (ie 1/64)?
> In other words, why this line:
> goal = min(nf_conntrack_htable_size / GC_MAX_BUCKETS_DIV, GC_MAX_BUCKETS);
> instead of:
> goal = GC_MAX_BUCKETS;
Sure, we can do that. But why is a fixed size better than a fraction?
E.g. with 8k buckets and simple goal = GC_MAX_BUCKETS we scan entire
table on every run, currently we only scan 128.
I wanted to keep too many destroy notifications from firing at once
but maybe i was too paranoid...
> > (Or cause too many useless scans)
> >
> > Another idea worth trying might be to get rid of the max cap and
> > instead break early in case too many jiffies expired.
> >
> > I don't want to add sysctl knobs for this unless absolutely needed; its already
> > possible to 'force' eviction cycle by running 'conntrack -L'.
> >
> Sure, but this is not a "real" solution, just a workaround.
> We need to find a way to deliver conntrack deletion events in a reasonable
> delay, whatever the traffic on the machine is.
Agree, but that depends on what 'reasonable' means and what kind of
uneeded cpu churn we're willing to add.
We can add a sysctl for this but we should use a low default to not do
too much unneeded work.
So what about your original patch, but only add
nf_conntrack_gc_interval
(and also add instant-resched in case entire budget was consumed)?
^ permalink raw reply
* Re: [PATCH net 3/5] net/ncsi: Fix stale link state of inactive channels on failover
From: Gavin Shan @ 2016-10-14 10:37 UTC (permalink / raw)
To: Joel Stanley; +Cc: Gavin Shan, netdev, davem
In-Reply-To: <CACPK8Xf1A2pvE0WCe2araRxZyMu9wkw=L=2-WuXrkBthehdfgw@mail.gmail.com>
On Fri, Oct 14, 2016 at 04:32:28PM +1030, Joel Stanley wrote:
>On Fri, Oct 14, 2016 at 1:23 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>> The issue was found on BCM5718 which has two NCSI channels in one
>> package: C0 and C1. Both of them are connected to different LANs,
>> means they are in link-up state and C0 is chosen as the active
>> one until resetting BCM5718 happens as below.
>>
>> Resetting BCM5718 results in LSC (Link State Change) AEN packet
>> received on C0, meaning LSC AEN is missed on C1. When LSC AEN packet
>> received on C0 to report link-down, it fails over to C1 because C1
>> is in link-up state as software can see. However, C1 is in link-down
>> state in hardware. It means the link state is out of synchronization
>> between hardware and software, resulting in inappropriate channel (C1)
>> selected as active one.
>>
>> This resolves the issue by sending separate GLS (Get Link Status)
>> commands to all channels in the package before trying to do failover.
>> The last link state on all channels in the package is retrieved. With
>> it, C0 is selected as active one as expected.
>
>I follow this, and can see that happening in the
>ncsi_dev_state_suspend_gls state. However, what is
>
>> - nd->state = ncsi_dev_state_suspend_dcnt;
>> + if (ndp->flags & NCSI_DEV_RESHUFFLE)
>> + nd->state = ncsi_dev_state_suspend_gls;
>> + else
>> + nd->state = ncsi_dev_state_suspend_dcnt;
>
>However, what is this doing? I'm not quite sure what
>NCSI_DEV_RESHUFFLE is and why we enable it?
>
NCSI_DEV_RESHUFFLE is set when we need failover, which happens on
resetting NIC or unplugging the cable connected to the active channel
(port) or other events. The first step for failover is to suspend
currently active channel and then choose the best one (channel's link
state is important factor) to be active. ncsi_dev_state_suspend_gls
ensures we will get updated link state of available channels before
choosing and enabling next active channel. If there are no failover
happening, we needn't get the update link state on the available
channels and the state ncsi_dev_state_suspend_gls will be skipped.
I think I need put comments here to explain the change in next revision.
Thanks,
Gavin
>>
>> ret = ncsi_xmit_cmd(&nca);
>> if (ret)
>> goto error;
>>
>> break;
>> + case ncsi_dev_state_suspend_gls:
>> + ndp->pending_req_num = np->channel_num;
>> +
>> + nca.type = NCSI_PKT_CMD_GLS;
>> + nca.package = np->id;
>> + nd->state = ncsi_dev_state_suspend_dcnt;
>> +
>> + NCSI_FOR_EACH_CHANNEL(np, nc) {
>> + nca.channel = nc->id;
>> + ret = ncsi_xmit_cmd(&nca);
>> + if (ret)
>> + goto error;
>> + }
>> +
>> + break;
>> case ncsi_dev_state_suspend_dcnt:
>> case ncsi_dev_state_suspend_dc:
>> case ncsi_dev_state_suspend_deselect:
>> --
>> 2.1.0
>>
>
^ permalink raw reply
* Re: [PATCH net 2/5] net/ncsi: Split out logic for ncsi_dev_state_suspend_select
From: Gavin Shan @ 2016-10-14 10:28 UTC (permalink / raw)
To: Joel Stanley; +Cc: Gavin Shan, netdev, davem
In-Reply-To: <CACPK8Xe58OE=WKZWL16K33FsY0vXb+i47PNG+nLSdhwmCeWNGw@mail.gmail.com>
On Fri, Oct 14, 2016 at 04:32:22PM +1030, Joel Stanley wrote:
>Hi Gavin,
>
>On Fri, Oct 14, 2016 at 1:23 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>> This splits out the code that handles ncsi_dev_state_suspend_select
>> so that we can add more code to the handler in subsequent patch.
>> Apart from adding a error tag to reuse the code in error path,
>> no logical changes introduced.
>>
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>> net/ncsi/ncsi-manage.c | 38 +++++++++++++++++++++++++-------------
>> 1 file changed, 25 insertions(+), 13 deletions(-)
>>
>> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
>> index 1bc96dc..5758a26 100644
>> --- a/net/ncsi/ncsi-manage.c
>> +++ b/net/ncsi/ncsi-manage.c
>> @@ -540,21 +540,30 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
>> nd->state = ncsi_dev_state_suspend_select;
>> /* Fall through */
>> case ncsi_dev_state_suspend_select:
>> + ndp->pending_req_num = 1;
>> +
>> + nca.type = NCSI_PKT_CMD_SP;
>> + nca.package = np->id;
>> + nca.channel = NCSI_RESERVED_CHANNEL;
>> + if (ndp->flags & NCSI_DEV_HWA)
>> + nca.bytes[0] = 0;
>> + else
>> + nca.bytes[0] = 1;
>> +
>> + nd->state = ncsi_dev_state_suspend_dcnt;
>> +
>> + ret = ncsi_xmit_cmd(&nca);
>> + if (ret)
>> + goto error;
>> +
>> + break;
>> case ncsi_dev_state_suspend_dcnt:
>> case ncsi_dev_state_suspend_dc:
>> case ncsi_dev_state_suspend_deselect:
>> ndp->pending_req_num = 1;
>>
>> nca.package = np->id;
>> - if (nd->state == ncsi_dev_state_suspend_select) {
>> - nca.type = NCSI_PKT_CMD_SP;
>> - nca.channel = NCSI_RESERVED_CHANNEL;
>> - if (ndp->flags & NCSI_DEV_HWA)
>> - nca.bytes[0] = 0;
>> - else
>> - nca.bytes[0] = 1;
>> - nd->state = ncsi_dev_state_suspend_dcnt;
>> - } else if (nd->state == ncsi_dev_state_suspend_dcnt) {
>> + if (nd->state == ncsi_dev_state_suspend_dcnt) {
>> nca.type = NCSI_PKT_CMD_DCNT;
>> nca.channel = nc->id;
>> nd->state = ncsi_dev_state_suspend_dc;
>
>This is a messy switch statement. How about break out out all of the
>states as you've done with suspend_select, instead of grouping them
>and then doing if ... else if .. else if. I realise there might be one
>or two lines duplicated for each state, but I think that's okay at the
>expense of readability.
>
>Also, patch 1 could also be merged into this when making this cleanup.
>
>What do you think?
>
Thanks, Joel. I agree with you that code readability is important than
duplicated code. I will do in next revision.
Thanks,
Gavin
>> @@ -570,10 +579,8 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
>> }
>>
>> ret = ncsi_xmit_cmd(&nca);
>> - if (ret) {
>> - nd->state = ncsi_dev_state_functional;
>> - return;
>> - }
>> + if (ret)
>> + goto error;
>>
>> break;
>> case ncsi_dev_state_suspend_done:
>> @@ -587,6 +594,11 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
>> netdev_warn(nd->dev, "Wrong NCSI state 0x%x in suspend\n",
>> nd->state);
>> }
>> +
>> + return;
>> +
>> +error:
>> + nd->state = ncsi_dev_state_functional;
>> }
>>
>> static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
>> --
>> 2.1.0
>>
>
^ permalink raw reply
* Re: [PATCH v3] IB/ipoib: move back IB LL address into the hard header
From: Or Gerlitz @ 2016-10-14 10:23 UTC (permalink / raw)
To: Paolo Abeni, David Miller, Doug Ledford, Erez Shitrit,
Alex Vesker
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sean Hefty,
Hal Rosenstock, Jason Gunthorpe, Linux Netdev List,
talal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
In-Reply-To: <9fce9b87ecc4c5c4f03401359806f40b1dcd0eb3.1476374992.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Thu, Oct 13, 2016 at 7:26 PM, Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> After the commit 9207f9d45b0a ("net: preserve IP control block
> during GSO segmentation"), the GSO CB and the IPoIB CB conflict.
the commit --> commit (remove the word "the" to make the sentence a
bit more clear)
> That destroy the IPoIB address information cached there,
> causing a severe performance regression, as better described here:
> http://marc.info/?l=linux-kernel&m=146787279825501&w=2
I don't think that links into this archive last for long.. try to find
something better
best if you can provide quick wording telling what is broken (e.g HW LSO)
> This change moves the data cached by the IPoIB driver from the
> skb control lock into the IPoIB hard header, as done before
lock --> block ?
> the commit 936d7de3d736 ("IPoIB: Stop lying about hard_header_len
the commit --> commit
> and use skb->cb to stash LL addresses").
> In order to avoid GRO issue, on packet reception, the IPoIB driver
> stash into the skb a dummy pseudo header, so that the received
> packets have actually a hard header matching the declared length.
> To avoid changing the connected mode maximum mtu, the allocated
> head buffer size is increased by the pseudo header length.
> After this commit, IPoIB performances are back to pre-regression
> value.
>
> v2 -> v3: rebased
> v1 -> v2: avoid changing the max mtu, increasing the head buf size
> Fixes: 9207f9d45b0a ("net: preserve IP control block during GSO segmentation")
> Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Paolo,
Is this fix backportable to any kernel since the breakage? AFAIR,
Roland mentioned
that a 2nd change introduced in 4.7-rc1 changed things a bit more such
that the fix
he had in his head didn't apply any more.
Dave, Doug
I am still travelling after netdev and would like to put an eye on the
patch and also see that @mellanox.com someone
provides a tested-by ack. Considering the fact that the bug is soon
to (de-)celebrate it's 1st anniversary and as we're still not in rc1,
the patch has enough time to get into 4.9... can you let it be here
for another week or so?
Or
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net 2/2] conntrack: enable to tune gc parameters
From: Nicolas Dichtel @ 2016-10-14 10:12 UTC (permalink / raw)
To: Florian Westphal; +Cc: davem, pablo, netdev, netfilter-devel
In-Reply-To: <20161013204338.GA32449@breakpoint.cc>
Le 13/10/2016 à 22:43, Florian Westphal a écrit :
> Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>> Le 10/10/2016 à 16:04, Florian Westphal a écrit :
>>> Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>>>> After commit b87a2f9199ea ("netfilter: conntrack: add gc worker to remove
>>>> timed-out entries"), netlink conntrack deletion events may be sent with a
>>>> huge delay. It could be interesting to let the user tweak gc parameters
>>>> depending on its use case.
>>>
>>> Hmm, care to elaborate?
>>>
>>> I am not against doing this but I'd like to hear/read your use case.
>>>
>>> The expectation is that in almot all cases eviction will happen from
>>> packet path. The gc worker is jusdt there for case where a busy system
>>> goes idle.
>> It was precisely that case. After a period of activity, the event is sent a long
>> time after the timeout. If the router does not manage a lot of flows, why not
>> trying to parse more entries instead of the default 1/64 of the table?
>> In fact, I don't understand why using GC_MAX_BUCKETS_DIV instead of using always
>> GC_MAX_BUCKETS whatever the size of the table is.
>
> I wanted to make sure that we have a known upper bound on the number of
> buckets we process so that we do not block other pending kworker items
> for too long.
I don't understand. GC_MAX_BUCKETS is the upper bound and I agree that it is
needed. But why GC_MAX_BUCKETS_DIV (ie 1/64)?
In other words, why this line:
goal = min(nf_conntrack_htable_size / GC_MAX_BUCKETS_DIV, GC_MAX_BUCKETS);
instead of:
goal = GC_MAX_BUCKETS;
?
>
> (Or cause too many useless scans)
>
> Another idea worth trying might be to get rid of the max cap and
> instead break early in case too many jiffies expired.
>
> I don't want to add sysctl knobs for this unless absolutely needed; its already
> possible to 'force' eviction cycle by running 'conntrack -L'.
>
Sure, but this is not a "real" solution, just a workaround.
We need to find a way to deliver conntrack deletion events in a reasonable
delay, whatever the traffic on the machine is.
^ permalink raw reply
* [PATCH v3 net-next 7/7] qed: Fix possible race when reading firmware return code.
From: Manish Chopra @ 2016-10-14 9:19 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Manish Chopra
In-Reply-To: <1476436763-5274-1-git-send-email-manish.chopra@qlogic.com>
From: Manish Chopra <manish.chopra@caviumnetworks.com>
While handling SPQ ramrod completion, there is a possible race
where driver might not read updated fw return code based on
ramrod completion done. This patch ensures that fw return code
is written first and then completion done flag is updated
using appropriate memory barriers.
Signed-off-by: Manish Chopra <manish.chopra@caviumnetworks.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_sp.h | 4 ++--
drivers/net/ethernet/qlogic/qed/qed_spq.c | 12 ++++++------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sp.h b/drivers/net/ethernet/qlogic/qed/qed_sp.h
index 652c908..27c450f 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_sp.h
@@ -111,8 +111,8 @@ union qed_spq_req_comp {
};
struct qed_spq_comp_done {
- u64 done;
- u8 fw_return_code;
+ unsigned int done;
+ u8 fw_return_code;
};
struct qed_spq_entry {
diff --git a/drivers/net/ethernet/qlogic/qed/qed_spq.c b/drivers/net/ethernet/qlogic/qed/qed_spq.c
index 259a615..6c05402 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_spq.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_spq.c
@@ -54,11 +54,10 @@ static void qed_spq_blocking_cb(struct qed_hwfn *p_hwfn,
comp_done = (struct qed_spq_comp_done *)cookie;
- comp_done->done = 0x1;
- comp_done->fw_return_code = fw_return_code;
+ comp_done->fw_return_code = fw_return_code;
- /* make update visible to waiting thread */
- smp_wmb();
+ /* Make sure completion done is visible on waiting thread */
+ smp_store_release(&comp_done->done, 0x1);
}
static int __qed_spq_block(struct qed_hwfn *p_hwfn,
@@ -74,8 +73,9 @@ static int __qed_spq_block(struct qed_hwfn *p_hwfn,
while (iter_cnt--) {
/* Validate we receive completion update */
- smp_rmb();
- if (comp_done->done == 1) {
+ if (READ_ONCE(comp_done->done) == 1) {
+ /* Read updated FW return value */
+ smp_read_barrier_depends();
if (p_fw_ret)
*p_fw_ret = comp_done->fw_return_code;
return 0;
--
2.7.2
^ permalink raw reply related
* [PATCH v3 net-next 3/7] qede: Prevent GSO on long Geneve headers
From: Manish Chopra @ 2016-10-14 9:19 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Manish Chopra
In-Reply-To: <1476436763-5274-1-git-send-email-manish.chopra@qlogic.com>
From: Manish Chopra <manish.chopra@caviumnetworks.com>
Due to hardware limitation, when transmitting a geneve-encapsulated
packet with more than 32 bytes worth of geneve options the hardware
would not be able to crack the packet and consider it a regular UDP
packet.
This implements the ndo_features_check() in qede in order to prevent
GSO on said transmitted packets.
Signed-off-by: Manish Chopra <manish.chopra@caviumnetworks.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qede/qede_main.c | 35 ++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 7d5dc1e..6c2b09c 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -2240,6 +2240,40 @@ static void qede_udp_tunnel_del(struct net_device *dev,
schedule_delayed_work(&edev->sp_task, 0);
}
+/* 8B udp header + 8B base tunnel header + 32B option length */
+#define QEDE_MAX_TUN_HDR_LEN 48
+
+static netdev_features_t qede_features_check(struct sk_buff *skb,
+ struct net_device *dev,
+ netdev_features_t features)
+{
+ if (skb->encapsulation) {
+ u8 l4_proto = 0;
+
+ switch (vlan_get_protocol(skb)) {
+ case htons(ETH_P_IP):
+ l4_proto = ip_hdr(skb)->protocol;
+ break;
+ case htons(ETH_P_IPV6):
+ l4_proto = ipv6_hdr(skb)->nexthdr;
+ break;
+ default:
+ return features;
+ }
+
+ /* Disable offloads for geneve tunnels, as HW can't parse
+ * the geneve header which has option length greater than 32B.
+ */
+ if ((l4_proto == IPPROTO_UDP) &&
+ ((skb_inner_mac_header(skb) -
+ skb_transport_header(skb)) > QEDE_MAX_TUN_HDR_LEN))
+ return features & ~(NETIF_F_CSUM_MASK |
+ NETIF_F_GSO_MASK);
+ }
+
+ return features;
+}
+
static const struct net_device_ops qede_netdev_ops = {
.ndo_open = qede_open,
.ndo_stop = qede_close,
@@ -2264,6 +2298,7 @@ static const struct net_device_ops qede_netdev_ops = {
#endif
.ndo_udp_tunnel_add = qede_udp_tunnel_add,
.ndo_udp_tunnel_del = qede_udp_tunnel_del,
+ .ndo_features_check = qede_features_check,
};
/* -------------------------------------------------------------------------
--
2.7.2
^ permalink raw reply related
* [PATCH v3 net-next 2/7] qede: GSO support for tunnels with outer csum
From: Manish Chopra @ 2016-10-14 9:19 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Manish Chopra
In-Reply-To: <1476436763-5274-1-git-send-email-manish.chopra@qlogic.com>
From: Manish Chopra <manish.chopra@caviumnetworks.com>
This patch adds GSO support for GRE and UDP tunnels
where outer checksums are enabled.
Signed-off-by: Manish Chopra <manish.chopra@caviumnetworks.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qede/qede.h | 1 +
drivers/net/ethernet/qlogic/qede/qede_main.c | 26 +++++++++++++++++++++++---
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index 28c0e9f..f50e527 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -320,6 +320,7 @@ struct qede_fastpath {
#define XMIT_L4_CSUM BIT(0)
#define XMIT_LSO BIT(1)
#define XMIT_ENC BIT(2)
+#define XMIT_ENC_GSO_L4_CSUM BIT(3)
#define QEDE_CSUM_ERROR BIT(0)
#define QEDE_CSUM_UNNECESSARY BIT(1)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 9866d95..7d5dc1e 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -400,8 +400,19 @@ static u32 qede_xmit_type(struct qede_dev *edev,
(ipv6_hdr(skb)->nexthdr == NEXTHDR_IPV6))
*ipv6_ext = 1;
- if (skb->encapsulation)
+ if (skb->encapsulation) {
rc |= XMIT_ENC;
+ if (skb_is_gso(skb)) {
+ unsigned short gso_type = skb_shinfo(skb)->gso_type;
+
+ if ((gso_type & SKB_GSO_UDP_TUNNEL_CSUM) ||
+ (gso_type & SKB_GSO_GRE_CSUM))
+ rc |= XMIT_ENC_GSO_L4_CSUM;
+
+ rc |= XMIT_LSO;
+ return rc;
+ }
+ }
if (skb_is_gso(skb))
rc |= XMIT_LSO;
@@ -637,6 +648,12 @@ static netdev_tx_t qede_start_xmit(struct sk_buff *skb,
if (unlikely(xmit_type & XMIT_ENC)) {
first_bd->data.bd_flags.bitfields |=
1 << ETH_TX_1ST_BD_FLAGS_TUNN_IP_CSUM_SHIFT;
+
+ if (xmit_type & XMIT_ENC_GSO_L4_CSUM) {
+ u8 tmp = ETH_TX_1ST_BD_FLAGS_TUNN_L4_CSUM_SHIFT;
+
+ first_bd->data.bd_flags.bitfields |= 1 << tmp;
+ }
hlen = qede_get_skb_hlen(skb, true);
} else {
first_bd->data.bd_flags.bitfields |=
@@ -2320,11 +2337,14 @@ static void qede_init_ndev(struct qede_dev *edev)
/* Encap features*/
hw_features |= NETIF_F_GSO_GRE | NETIF_F_GSO_UDP_TUNNEL |
- NETIF_F_TSO_ECN;
+ NETIF_F_TSO_ECN | NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_GRE_CSUM;
ndev->hw_enc_features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
NETIF_F_SG | NETIF_F_TSO | NETIF_F_TSO_ECN |
NETIF_F_TSO6 | NETIF_F_GSO_GRE |
- NETIF_F_GSO_UDP_TUNNEL | NETIF_F_RXCSUM;
+ NETIF_F_GSO_UDP_TUNNEL | NETIF_F_RXCSUM |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_GRE_CSUM;
ndev->vlan_features = hw_features | NETIF_F_RXHASH | NETIF_F_RXCSUM |
NETIF_F_HIGHDMA;
--
2.7.2
^ permalink raw reply related
* [PATCH v3 net-next 6/7] qed: Handle malicious VFs events
From: Manish Chopra @ 2016-10-14 9:19 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Yuval Mintz
In-Reply-To: <1476436763-5274-1-git-send-email-manish.chopra@qlogic.com>
From: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
Malicious VFs might be caught in several different methods:
- Misusing their bar permission and being blocked by hardware.
- Misusing their fastpath logic and being blocked by firmware.
- Misusing their interaction with their PF via hw-channel,
and being blocked by PF driver.
On the first two items, firmware would indicate to driver that
the VF is to be considered malicious, but would sometime still
allow the VF to communicate with the PF [depending on the exact
nature of the malicious activity done by the VF].
The current existing logic on the PF side lacks handling of such events,
and might allow the PF to perform some incorrect configuration on behalf
of a VF that was previously indicated as malicious.
The new scheme is simple -
Once the PF determines a VF is malicious it would:
a. Ignore any further requests on behalf of the VF-driver.
b. Prevent any configurations initiated by the hyperuser for
the malicious VF, as firmware isn't willing to serve such.
The malicious indication would be cleared upon the VF flr,
after which it would become usable once again.
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_sriov.c | 114 +++++++++++++++++++++++-----
drivers/net/ethernet/qlogic/qed/qed_sriov.h | 1 +
drivers/net/ethernet/qlogic/qed/qed_vf.h | 1 +
3 files changed, 96 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.c b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
index d2d6621..6f029f9 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sriov.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
@@ -109,7 +109,8 @@ static int qed_sp_vf_stop(struct qed_hwfn *p_hwfn,
}
static bool qed_iov_is_valid_vfid(struct qed_hwfn *p_hwfn,
- int rel_vf_id, bool b_enabled_only)
+ int rel_vf_id,
+ bool b_enabled_only, bool b_non_malicious)
{
if (!p_hwfn->pf_iov_info) {
DP_NOTICE(p_hwfn->cdev, "No iov info\n");
@@ -124,6 +125,10 @@ static bool qed_iov_is_valid_vfid(struct qed_hwfn *p_hwfn,
b_enabled_only)
return false;
+ if ((p_hwfn->pf_iov_info->vfs_array[rel_vf_id].b_malicious) &&
+ b_non_malicious)
+ return false;
+
return true;
}
@@ -138,7 +143,8 @@ static struct qed_vf_info *qed_iov_get_vf_info(struct qed_hwfn *p_hwfn,
return NULL;
}
- if (qed_iov_is_valid_vfid(p_hwfn, relative_vf_id, b_enabled_only))
+ if (qed_iov_is_valid_vfid(p_hwfn, relative_vf_id,
+ b_enabled_only, false))
vf = &p_hwfn->pf_iov_info->vfs_array[relative_vf_id];
else
DP_ERR(p_hwfn, "qed_iov_get_vf_info: VF[%d] is not enabled\n",
@@ -542,7 +548,8 @@ int qed_iov_hw_info(struct qed_hwfn *p_hwfn)
return 0;
}
-static bool qed_iov_pf_sanity_check(struct qed_hwfn *p_hwfn, int vfid)
+bool _qed_iov_pf_sanity_check(struct qed_hwfn *p_hwfn,
+ int vfid, bool b_fail_malicious)
{
/* Check PF supports sriov */
if (IS_VF(p_hwfn->cdev) || !IS_QED_SRIOV(p_hwfn->cdev) ||
@@ -550,12 +557,17 @@ static bool qed_iov_pf_sanity_check(struct qed_hwfn *p_hwfn, int vfid)
return false;
/* Check VF validity */
- if (!qed_iov_is_valid_vfid(p_hwfn, vfid, true))
+ if (!qed_iov_is_valid_vfid(p_hwfn, vfid, true, b_fail_malicious))
return false;
return true;
}
+bool qed_iov_pf_sanity_check(struct qed_hwfn *p_hwfn, int vfid)
+{
+ return _qed_iov_pf_sanity_check(p_hwfn, vfid, true);
+}
+
static void qed_iov_set_vf_to_disable(struct qed_dev *cdev,
u16 rel_vf_id, u8 to_disable)
{
@@ -652,6 +664,9 @@ static int qed_iov_enable_vf_access(struct qed_hwfn *p_hwfn,
qed_iov_vf_igu_reset(p_hwfn, p_ptt, vf);
+ /* It's possible VF was previously considered malicious */
+ vf->b_malicious = false;
+
rc = qed_mcp_config_vf_msix(p_hwfn, p_ptt, vf->abs_vf_id, vf->num_sbs);
if (rc)
return rc;
@@ -2804,6 +2819,13 @@ qed_iov_execute_vf_flr_cleanup(struct qed_hwfn *p_hwfn,
return rc;
}
+ /* Workaround to make VF-PF channel ready, as FW
+ * doesn't do that as a part of FLR.
+ */
+ REG_WR(p_hwfn,
+ GTT_BAR0_MAP_REG_USDM_RAM +
+ USTORM_VF_PF_CHANNEL_READY_OFFSET(vfid), 1);
+
/* VF_STOPPED has to be set only after final cleanup
* but prior to re-enabling the VF.
*/
@@ -2942,7 +2964,8 @@ static void qed_iov_process_mbx_req(struct qed_hwfn *p_hwfn,
mbx->first_tlv = mbx->req_virt->first_tlv;
/* check if tlv type is known */
- if (qed_iov_tlv_supported(mbx->first_tlv.tl.type)) {
+ if (qed_iov_tlv_supported(mbx->first_tlv.tl.type) &&
+ !p_vf->b_malicious) {
switch (mbx->first_tlv.tl.type) {
case CHANNEL_TLV_ACQUIRE:
qed_iov_vf_mbx_acquire(p_hwfn, p_ptt, p_vf);
@@ -2984,6 +3007,15 @@ static void qed_iov_process_mbx_req(struct qed_hwfn *p_hwfn,
qed_iov_vf_mbx_release(p_hwfn, p_ptt, p_vf);
break;
}
+ } else if (qed_iov_tlv_supported(mbx->first_tlv.tl.type)) {
+ DP_VERBOSE(p_hwfn, QED_MSG_IOV,
+ "VF [%02x] - considered malicious; Ignoring TLV [%04x]\n",
+ p_vf->abs_vf_id, mbx->first_tlv.tl.type);
+
+ qed_iov_prepare_resp(p_hwfn, p_ptt, p_vf,
+ mbx->first_tlv.tl.type,
+ sizeof(struct pfvf_def_resp_tlv),
+ PFVF_STATUS_MALICIOUS);
} else {
/* unknown TLV - this may belong to a VF driver from the future
* - a version written after this PF driver was written, which
@@ -3033,20 +3065,30 @@ static void qed_iov_pf_get_and_clear_pending_events(struct qed_hwfn *p_hwfn,
memset(p_pending_events, 0, sizeof(u64) * QED_VF_ARRAY_LENGTH);
}
-static int qed_sriov_vfpf_msg(struct qed_hwfn *p_hwfn,
- u16 abs_vfid, struct regpair *vf_msg)
+static struct qed_vf_info *qed_sriov_get_vf_from_absid(struct qed_hwfn *p_hwfn,
+ u16 abs_vfid)
{
- u8 min = (u8)p_hwfn->cdev->p_iov_info->first_vf_in_pf;
- struct qed_vf_info *p_vf;
+ u8 min = (u8) p_hwfn->cdev->p_iov_info->first_vf_in_pf;
- if (!qed_iov_pf_sanity_check(p_hwfn, (int)abs_vfid - min)) {
+ if (!_qed_iov_pf_sanity_check(p_hwfn, (int)abs_vfid - min, false)) {
DP_VERBOSE(p_hwfn,
QED_MSG_IOV,
- "Got a message from VF [abs 0x%08x] that cannot be handled by PF\n",
+ "Got indication for VF [abs 0x%08x] that cannot be handled by PF\n",
abs_vfid);
- return 0;
+ return NULL;
}
- p_vf = &p_hwfn->pf_iov_info->vfs_array[(u8)abs_vfid - min];
+
+ return &p_hwfn->pf_iov_info->vfs_array[(u8) abs_vfid - min];
+}
+
+static int qed_sriov_vfpf_msg(struct qed_hwfn *p_hwfn,
+ u16 abs_vfid, struct regpair *vf_msg)
+{
+ struct qed_vf_info *p_vf = qed_sriov_get_vf_from_absid(p_hwfn,
+ abs_vfid);
+
+ if (!p_vf)
+ return 0;
/* List the physical address of the request so that handler
* could later on copy the message from it.
@@ -3060,6 +3102,23 @@ static int qed_sriov_vfpf_msg(struct qed_hwfn *p_hwfn,
return 0;
}
+static void qed_sriov_vfpf_malicious(struct qed_hwfn *p_hwfn,
+ struct malicious_vf_eqe_data *p_data)
+{
+ struct qed_vf_info *p_vf;
+
+ p_vf = qed_sriov_get_vf_from_absid(p_hwfn, p_data->vf_id);
+
+ if (!p_vf)
+ return;
+
+ DP_INFO(p_hwfn,
+ "VF [%d] - Malicious behavior [%02x]\n",
+ p_vf->abs_vf_id, p_data->err_id);
+
+ p_vf->b_malicious = true;
+}
+
int qed_sriov_eqe_event(struct qed_hwfn *p_hwfn,
u8 opcode, __le16 echo, union event_ring_data *data)
{
@@ -3067,6 +3126,9 @@ int qed_sriov_eqe_event(struct qed_hwfn *p_hwfn,
case COMMON_EVENT_VF_PF_CHANNEL:
return qed_sriov_vfpf_msg(p_hwfn, le16_to_cpu(echo),
&data->vf_pf_channel.msg_addr);
+ case COMMON_EVENT_MALICIOUS_VF:
+ qed_sriov_vfpf_malicious(p_hwfn, &data->malicious_vf);
+ return 0;
default:
DP_INFO(p_hwfn->cdev, "Unknown sriov eqe event 0x%02x\n",
opcode);
@@ -3083,7 +3145,7 @@ u16 qed_iov_get_next_active_vf(struct qed_hwfn *p_hwfn, u16 rel_vf_id)
goto out;
for (i = rel_vf_id; i < p_iov->total_vfs; i++)
- if (qed_iov_is_valid_vfid(p_hwfn, rel_vf_id, true))
+ if (qed_iov_is_valid_vfid(p_hwfn, rel_vf_id, true, false))
return i;
out:
@@ -3130,6 +3192,12 @@ static void qed_iov_bulletin_set_forced_mac(struct qed_hwfn *p_hwfn,
return;
}
+ if (vf_info->b_malicious) {
+ DP_NOTICE(p_hwfn->cdev,
+ "Can't set forced MAC to malicious VF [%d]\n", vfid);
+ return;
+ }
+
feature = 1 << MAC_ADDR_FORCED;
memcpy(vf_info->bulletin.p_virt->mac, mac, ETH_ALEN);
@@ -3153,6 +3221,12 @@ static void qed_iov_bulletin_set_forced_vlan(struct qed_hwfn *p_hwfn,
return;
}
+ if (vf_info->b_malicious) {
+ DP_NOTICE(p_hwfn->cdev,
+ "Can't set forced vlan to malicious VF [%d]\n", vfid);
+ return;
+ }
+
feature = 1 << VLAN_ADDR_FORCED;
vf_info->bulletin.p_virt->pvid = pvid;
if (pvid)
@@ -3367,7 +3441,7 @@ int qed_sriov_disable(struct qed_dev *cdev, bool pci_enabled)
qed_for_each_vf(hwfn, j) {
int k;
- if (!qed_iov_is_valid_vfid(hwfn, j, true))
+ if (!qed_iov_is_valid_vfid(hwfn, j, true, false))
continue;
/* Wait until VF is disabled before releasing */
@@ -3425,7 +3499,7 @@ static int qed_sriov_enable(struct qed_dev *cdev, int num)
num_sbs = min_t(int, sb_cnt_info.sb_free_blk, limit);
for (i = 0; i < num; i++) {
- if (!qed_iov_is_valid_vfid(hwfn, i, false))
+ if (!qed_iov_is_valid_vfid(hwfn, i, false, true))
continue;
rc = qed_iov_init_hw_for_vf(hwfn,
@@ -3477,7 +3551,7 @@ static int qed_sriov_pf_set_mac(struct qed_dev *cdev, u8 *mac, int vfid)
return -EINVAL;
}
- if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vfid, true)) {
+ if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vfid, true, true)) {
DP_VERBOSE(cdev, QED_MSG_IOV,
"Cannot set VF[%d] MAC (VF is not active)\n", vfid);
return -EINVAL;
@@ -3509,7 +3583,7 @@ static int qed_sriov_pf_set_vlan(struct qed_dev *cdev, u16 vid, int vfid)
return -EINVAL;
}
- if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vfid, true)) {
+ if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vfid, true, true)) {
DP_VERBOSE(cdev, QED_MSG_IOV,
"Cannot set VF[%d] MAC (VF is not active)\n", vfid);
return -EINVAL;
@@ -3543,7 +3617,7 @@ static int qed_get_vf_config(struct qed_dev *cdev,
if (IS_VF(cdev))
return -EINVAL;
- if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vf_id, true)) {
+ if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vf_id, true, false)) {
DP_VERBOSE(cdev, QED_MSG_IOV,
"VF index [%d] isn't active\n", vf_id);
return -EINVAL;
@@ -3647,7 +3721,7 @@ static int qed_set_vf_link_state(struct qed_dev *cdev,
if (IS_VF(cdev))
return -EINVAL;
- if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vf_id, true)) {
+ if (!qed_iov_is_valid_vfid(&cdev->hwfns[0], vf_id, true, true)) {
DP_VERBOSE(cdev, QED_MSG_IOV,
"VF index [%d] isn't active\n", vf_id);
return -EINVAL;
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.h b/drivers/net/ethernet/qlogic/qed/qed_sriov.h
index 0dd23e4..3cf515b 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sriov.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.h
@@ -132,6 +132,7 @@ struct qed_vf_info {
struct qed_iov_vf_mbx vf_mbx;
enum vf_state state;
bool b_init;
+ bool b_malicious;
u8 to_disable;
struct qed_bulletin bulletin;
diff --git a/drivers/net/ethernet/qlogic/qed/qed_vf.h b/drivers/net/ethernet/qlogic/qed/qed_vf.h
index 35db7a28..944745b 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_vf.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_vf.h
@@ -40,6 +40,7 @@ enum {
PFVF_STATUS_NOT_SUPPORTED,
PFVF_STATUS_NO_RESOURCE,
PFVF_STATUS_FORCED,
+ PFVF_STATUS_MALICIOUS,
};
/* vf pf channel tlvs */
--
2.7.2
^ permalink raw reply related
* [PATCH v3 net-next 1/7] qed: Pass MAC hints to VFs
From: Manish Chopra @ 2016-10-14 9:19 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Yuval Mintz
In-Reply-To: <1476436763-5274-1-git-send-email-manish.chopra@qlogic.com>
From: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
Some hypervisors can support MAC hints to their VFs.
Even though we don't have such a hypervisor API in linux, we add
sufficient logic for the VF to be able to receive such hints and
set the mac accordingly - as long as the VF has not been set with
a MAC already.
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_vf.c | 4 ++--
drivers/net/ethernet/qlogic/qede/qede_main.c | 6 +++++-
include/linux/qed/qed_eth_if.h | 2 +-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_vf.c b/drivers/net/ethernet/qlogic/qed/qed_vf.c
index abf5bf1..f580bf4 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_vf.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_vf.c
@@ -1230,8 +1230,8 @@ static void qed_handle_bulletin_change(struct qed_hwfn *hwfn)
is_mac_exist = qed_vf_bulletin_get_forced_mac(hwfn, mac,
&is_mac_forced);
- if (is_mac_exist && is_mac_forced && cookie)
- ops->force_mac(cookie, mac);
+ if (is_mac_exist && cookie)
+ ops->force_mac(cookie, mac, !!is_mac_forced);
/* Always update link configuration according to bulletin */
qed_link_update(hwfn);
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 343038c..9866d95 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -171,10 +171,14 @@ static struct pci_driver qede_pci_driver = {
#endif
};
-static void qede_force_mac(void *dev, u8 *mac)
+static void qede_force_mac(void *dev, u8 *mac, bool forced)
{
struct qede_dev *edev = dev;
+ /* MAC hints take effect only if we haven't set one already */
+ if (is_valid_ether_addr(edev->ndev->dev_addr) && !forced)
+ return;
+
ether_addr_copy(edev->ndev->dev_addr, mac);
ether_addr_copy(edev->primary_mac, mac);
}
diff --git a/include/linux/qed/qed_eth_if.h b/include/linux/qed/qed_eth_if.h
index 33c24eb..1c77948 100644
--- a/include/linux/qed/qed_eth_if.h
+++ b/include/linux/qed/qed_eth_if.h
@@ -129,7 +129,7 @@ struct qed_tunn_params {
struct qed_eth_cb_ops {
struct qed_common_cb_ops common;
- void (*force_mac) (void *dev, u8 *mac);
+ void (*force_mac) (void *dev, u8 *mac, bool forced);
};
#ifdef CONFIG_DCB
--
2.7.2
^ permalink raw reply related
* [PATCH v3 net-next 4/7] qed*: Allow unicast filtering
From: Manish Chopra @ 2016-10-14 9:19 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Yuval Mintz
In-Reply-To: <1476436763-5274-1-git-send-email-manish.chopra@qlogic.com>
From: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
Apparently qede fails to set IFF_UNICAST_FLT, and as a result is not
actually performing unicast MAC filtering.
While we're at it - relax a hard-coded limitation that limits each
interface into using at most 15 unicast MAC addresses before turning
promiscuous. Instead utilize the HW resources to their limit.
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_l2.c | 12 ++++++++++--
drivers/net/ethernet/qlogic/qede/qede_main.c | 4 +++-
include/linux/qed/qed_eth_if.h | 1 +
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_l2.c b/drivers/net/ethernet/qlogic/qed/qed_l2.c
index ddd410a..6b0e22d 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_l2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_l2.c
@@ -1652,6 +1652,7 @@ static int qed_fill_eth_dev_info(struct qed_dev *cdev,
if (IS_PF(cdev)) {
int max_vf_vlan_filters = 0;
+ int max_vf_mac_filters = 0;
if (cdev->int_params.out.int_mode == QED_INT_MODE_MSIX) {
for_each_hwfn(cdev, i)
@@ -1665,11 +1666,18 @@ static int qed_fill_eth_dev_info(struct qed_dev *cdev,
info->num_queues = cdev->num_hwfns;
}
- if (IS_QED_SRIOV(cdev))
+ if (IS_QED_SRIOV(cdev)) {
max_vf_vlan_filters = cdev->p_iov_info->total_vfs *
QED_ETH_VF_NUM_VLAN_FILTERS;
- info->num_vlan_filters = RESC_NUM(&cdev->hwfns[0], QED_VLAN) -
+ max_vf_mac_filters = cdev->p_iov_info->total_vfs *
+ QED_ETH_VF_NUM_MAC_FILTERS;
+ }
+ info->num_vlan_filters = RESC_NUM(QED_LEADING_HWFN(cdev),
+ QED_VLAN) -
max_vf_vlan_filters;
+ info->num_mac_filters = RESC_NUM(QED_LEADING_HWFN(cdev),
+ QED_MAC) -
+ max_vf_mac_filters;
ether_addr_copy(info->port_mac,
cdev->hwfns[0].hw_info.hw_mac_addr);
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 6c2b09c..0e483af 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -2365,6 +2365,8 @@ static void qede_init_ndev(struct qede_dev *edev)
qede_set_ethtool_ops(ndev);
+ ndev->priv_flags = IFF_UNICAST_FLT;
+
/* user-changeble features */
hw_features = NETIF_F_GRO | NETIF_F_SG |
NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
@@ -3937,7 +3939,7 @@ static void qede_config_rx_mode(struct net_device *ndev)
/* Check for promiscuous */
if ((ndev->flags & IFF_PROMISC) ||
- (uc_count > 15)) { /* @@@TBD resource allocation - 1 */
+ (uc_count > edev->dev_info.num_mac_filters - 1)) {
accept_flags = QED_FILTER_RX_MODE_TYPE_PROMISC;
} else {
/* Add MAC filters according to the unicast secondary macs */
diff --git a/include/linux/qed/qed_eth_if.h b/include/linux/qed/qed_eth_if.h
index 1c77948..1513080 100644
--- a/include/linux/qed/qed_eth_if.h
+++ b/include/linux/qed/qed_eth_if.h
@@ -23,6 +23,7 @@ struct qed_dev_eth_info {
u8 port_mac[ETH_ALEN];
u8 num_vlan_filters;
+ u16 num_mac_filters;
/* Legacy VF - this affects the datapath, so qede has to know */
bool is_legacy;
--
2.7.2
^ permalink raw reply related
* [PATCH v3 net-next 5/7] qed: Allow chance for fast ramrod completions
From: Manish Chopra @ 2016-10-14 9:19 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Yuval Mintz
In-Reply-To: <1476436763-5274-1-git-send-email-manish.chopra@qlogic.com>
From: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
Whenever a ramrod is being sent for some device configuration,
the driver is going to sleep at least 5ms between each iteration
of polling on the completion of the ramrod.
However, in almost every configuration scenario the firmware
would be able to comply and complete the ramrod in a manner of
several usecs. This is especially important in cases where there
might be a lot of sequential configurations applying to the hardware
[e.g., RoCE], in which case the existing scheme might cause some
visible user delays.
This patch changes the completion scheme - instead of immediately
starting to sleep for a 'long' period, allow the device to quickly
poll on the first iteration after a couple of usecs.
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_spq.c | 85 +++++++++++++++++++++----------
1 file changed, 59 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_spq.c b/drivers/net/ethernet/qlogic/qed/qed_spq.c
index caff415..259a615 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_spq.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_spq.c
@@ -37,7 +37,11 @@
***************************************************************************/
#define SPQ_HIGH_PRI_RESERVE_DEFAULT (1)
-#define SPQ_BLOCK_SLEEP_LENGTH (1000)
+
+#define SPQ_BLOCK_DELAY_MAX_ITER (10)
+#define SPQ_BLOCK_DELAY_US (10)
+#define SPQ_BLOCK_SLEEP_MAX_ITER (1000)
+#define SPQ_BLOCK_SLEEP_MS (5)
/***************************************************************************
* Blocking Imp. (BLOCK/EBLOCK mode)
@@ -57,53 +61,81 @@ static void qed_spq_blocking_cb(struct qed_hwfn *p_hwfn,
smp_wmb();
}
-static int qed_spq_block(struct qed_hwfn *p_hwfn,
- struct qed_spq_entry *p_ent,
- u8 *p_fw_ret)
+static int __qed_spq_block(struct qed_hwfn *p_hwfn,
+ struct qed_spq_entry *p_ent,
+ u8 *p_fw_ret, bool sleep_between_iter)
{
- int sleep_count = SPQ_BLOCK_SLEEP_LENGTH;
struct qed_spq_comp_done *comp_done;
- int rc;
+ u32 iter_cnt;
comp_done = (struct qed_spq_comp_done *)p_ent->comp_cb.cookie;
- while (sleep_count) {
- /* validate we receive completion update */
+ iter_cnt = sleep_between_iter ? SPQ_BLOCK_SLEEP_MAX_ITER
+ : SPQ_BLOCK_DELAY_MAX_ITER;
+
+ while (iter_cnt--) {
+ /* Validate we receive completion update */
smp_rmb();
if (comp_done->done == 1) {
if (p_fw_ret)
*p_fw_ret = comp_done->fw_return_code;
return 0;
}
- usleep_range(5000, 10000);
- sleep_count--;
+
+ if (sleep_between_iter)
+ msleep(SPQ_BLOCK_SLEEP_MS);
+ else
+ udelay(SPQ_BLOCK_DELAY_US);
}
+ return -EBUSY;
+}
+
+static int qed_spq_block(struct qed_hwfn *p_hwfn,
+ struct qed_spq_entry *p_ent,
+ u8 *p_fw_ret, bool skip_quick_poll)
+{
+ struct qed_spq_comp_done *comp_done;
+ int rc;
+
+ /* A relatively short polling period w/o sleeping, to allow the FW to
+ * complete the ramrod and thus possibly to avoid the following sleeps.
+ */
+ if (!skip_quick_poll) {
+ rc = __qed_spq_block(p_hwfn, p_ent, p_fw_ret, false);
+ if (!rc)
+ return 0;
+ }
+
+ /* Move to polling with a sleeping period between iterations */
+ rc = __qed_spq_block(p_hwfn, p_ent, p_fw_ret, true);
+ if (!rc)
+ return 0;
+
DP_INFO(p_hwfn, "Ramrod is stuck, requesting MCP drain\n");
rc = qed_mcp_drain(p_hwfn, p_hwfn->p_main_ptt);
- if (rc != 0)
+ if (rc) {
DP_NOTICE(p_hwfn, "MCP drain failed\n");
+ goto err;
+ }
/* Retry after drain */
- sleep_count = SPQ_BLOCK_SLEEP_LENGTH;
- while (sleep_count) {
- /* validate we receive completion update */
- smp_rmb();
- if (comp_done->done == 1) {
- if (p_fw_ret)
- *p_fw_ret = comp_done->fw_return_code;
- return 0;
- }
- usleep_range(5000, 10000);
- sleep_count--;
- }
+ rc = __qed_spq_block(p_hwfn, p_ent, p_fw_ret, true);
+ if (!rc)
+ return 0;
+ comp_done = (struct qed_spq_comp_done *)p_ent->comp_cb.cookie;
if (comp_done->done == 1) {
if (p_fw_ret)
*p_fw_ret = comp_done->fw_return_code;
return 0;
}
-
- DP_NOTICE(p_hwfn, "Ramrod is stuck, MCP drain failed\n");
+err:
+ DP_NOTICE(p_hwfn,
+ "Ramrod is stuck [CID %08x cmd %02x protocol %02x echo %04x]\n",
+ le32_to_cpu(p_ent->elem.hdr.cid),
+ p_ent->elem.hdr.cmd_id,
+ p_ent->elem.hdr.protocol_id,
+ le16_to_cpu(p_ent->elem.hdr.echo));
return -EBUSY;
}
@@ -729,7 +761,8 @@ int qed_spq_post(struct qed_hwfn *p_hwfn,
* access p_ent here to see whether it's successful or not.
* Thus, after gaining the answer perform the cleanup here.
*/
- rc = qed_spq_block(p_hwfn, p_ent, fw_return_code);
+ rc = qed_spq_block(p_hwfn, p_ent, fw_return_code,
+ p_ent->queue == &p_spq->unlimited_pending);
if (p_ent->queue == &p_spq->unlimited_pending) {
/* This is an allocated p_ent which does not need to
--
2.7.2
^ permalink raw reply related
* [PATCH v3 net-next 0/7] qed*: driver updates
From: Manish Chopra @ 2016-10-14 9:19 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuval.Mintz, Manish Chopra
From: Manish Chopra <manish.chopra@caviumnetworks.com>
Hi David,
There are several new additions in this series;
Most are connected to either Tx offloading or Rx classifications
[either fastpath changes or supporting configuration].
In addition, there's a single IOV enhancement.
Please consider applying this series to `net-next'.
V2->V3:
Fixes below kbuild warning
call to '__compiletime_assert_60' declared with
attribute error: Need native word sized stores/loads for atomicity.
V1->V2:
Added a fix for the race in ramrod handling
pointed by Eric Dumazet [patch 7].
Thanks,
Manish
Manish Chopra (3):
qede: GSO support for tunnels with outer csum
qede: Prevent GSO on long Geneve headers
qed: Fix possible race when reading firmware return code.
Yuval Mintz (4):
qed: Pass MAC hints to VFs
qed*: Allow unicast filtering
qed: Allow chance for fast ramrod completions
qed: Handle malicious VFs events
drivers/net/ethernet/qlogic/qed/qed_l2.c | 12 ++-
drivers/net/ethernet/qlogic/qed/qed_sp.h | 4 +-
drivers/net/ethernet/qlogic/qed/qed_spq.c | 97 +++++++++++++++--------
drivers/net/ethernet/qlogic/qed/qed_sriov.c | 114 ++++++++++++++++++++++-----
drivers/net/ethernet/qlogic/qed/qed_sriov.h | 1 +
drivers/net/ethernet/qlogic/qed/qed_vf.c | 4 +-
drivers/net/ethernet/qlogic/qed/qed_vf.h | 1 +
drivers/net/ethernet/qlogic/qede/qede.h | 1 +
drivers/net/ethernet/qlogic/qede/qede_main.c | 71 +++++++++++++++--
include/linux/qed/qed_eth_if.h | 3 +-
10 files changed, 244 insertions(+), 64 deletions(-)
--
2.7.2
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 10:00 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu_p7neD-ttCBBA6p7VSkQt8mvfhS9cdcLftdkXNtAAFnQ@mail.gmail.com>
> So why is the performance hit acceptable for ESP but not for WPA? We
> could easily implement the same thing, i.e.,
> kmalloc(GFP_ATOMIC)/kfree the aead_req struct rather than allocate it
> on the stack
Yeah, maybe we should. It's likely a much bigger allocation, but I
don't actually know if that affects speed.
In most cases where you want high performance we never hit this anyway
since we'll have hardware crypto. I know for our (Intel's) devices we
normally never hit these code paths.
But on the other hand, you also did your changes for a reason, and the
only reason I can see of that is performance. So you'd be the one with
most "skin in the game", I guess?
johannes
^ permalink raw reply
* [PATCH] ethtool: Zero memory allocated for statistics
From: Vlad Tsyrklevich @ 2016-10-14 9:59 UTC (permalink / raw)
To: netdev; +Cc: Vlad Tsyrklevich
Zero allocations before they're passed to drivers to be filled out with
statistics. While many drivers always correctly fill out the entire
allocated space, under some failure conditions some drivers will not
clear the allocated space appropriately. Unprivileged users could
induce some of these failure conditions to leak kernel memory. Instead
of fixing drivers one by one, the best solution is to eliminate the
possibility of driver errors leaking kernel memory entirely.
Given that ethtool_get_stats(), ethtool_get_phy_stats(), and
ethtool_get_tunable() are accessible without CAP_NET_ADMIN they are the
most important to clear to avoid memory leaks. ethtool_self_test() and
ethtool_get_any_eeprom() require CAP_NET_ADMIN but were also included
for completeness.
Some examples of driver methods that could fail to fill out memory:
enic_get_ethtool_stats(), cp_get_ethtool_stats(),
mv88e6xxx_get_ethtool_stats(), bnx2x_self_test(), be_self_test(), etc.
Signed-off-by: Vlad Tsyrklevich <vlad@tsyrklevich.net>
---
net/core/ethtool.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 9774898..7202915 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1538,7 +1538,7 @@ static int ethtool_get_any_eeprom(struct net_device *dev, void __user *useraddr,
if (eeprom.offset + eeprom.len > total_len)
return -EINVAL;
- data = kmalloc(PAGE_SIZE, GFP_USER);
+ data = kzalloc(PAGE_SIZE, GFP_USER);
if (!data)
return -ENOMEM;
@@ -1775,7 +1775,7 @@ static int ethtool_self_test(struct net_device *dev, char __user *useraddr)
return -EFAULT;
test.len = test_len;
- data = kmalloc(test_len * sizeof(u64), GFP_USER);
+ data = kcalloc(test_len, sizeof(u64), GFP_USER);
if (!data)
return -ENOMEM;
@@ -1907,7 +1907,7 @@ static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
return -EFAULT;
stats.n_stats = n_stats;
- data = kmalloc(n_stats * sizeof(u64), GFP_USER);
+ data = kcalloc(n_stats, sizeof(u64), GFP_USER);
if (!data)
return -ENOMEM;
@@ -1946,7 +1946,7 @@ static int ethtool_get_phy_stats(struct net_device *dev, void __user *useraddr)
return -EFAULT;
stats.n_stats = n_stats;
- data = kmalloc_array(n_stats, sizeof(u64), GFP_USER);
+ data = kcalloc(n_stats, sizeof(u64), GFP_USER);
if (!data)
return -ENOMEM;
@@ -2269,7 +2269,7 @@ static int ethtool_get_tunable(struct net_device *dev, void __user *useraddr)
ret = ethtool_tunable_valid(&tuna);
if (ret)
return ret;
- data = kmalloc(tuna.len, GFP_USER);
+ data = kzalloc(tuna.len, GFP_USER);
if (!data)
return -ENOMEM;
ret = ops->get_tunable(dev, &tuna, data);
--
2.7.0
^ permalink raw reply related
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 9:35 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <1476437149.31114.27.camel@sipsolutions.net>
On 14 October 2016 at 10:25, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2016-10-14 at 10:21 +0100, Ard Biesheuvel wrote:
>
>> It is annotated with a TODO, though :-)
>>
>> 38320c70d282b (Herbert Xu 2008-01-28 19:35:05
>> -0800 41)
>> * TODO: Use spare space in skb for this where possible.
>
> I saw that, but I don't think generally there will be spare space for
> it - the stuff there is likely far too big. Anyway ... same problem
> that we have.
>
> I'm not inclined to allocate ~500 bytes temporarily for every frame
> either though.
>
> Maybe we could try to manage it in mac80211, we'd "only" need 5 AEAD
> structs (which are today on the stack) in parallel for each key (4 TX,
> 1 RX), but in a typical case of having 3 keys that's already 7.5K worth
> of memory that we almost never use. Again, with more complexity, we
> could know that the TX will not be used if the driver does the TX, but
> the single RX one we'd need unconditionally... decisions decisions...
>
So why is the performance hit acceptable for ESP but not for WPA? We
could easily implement the same thing, i.e., kmalloc(GFP_ATOMIC)/kfree
the aead_req struct rather than allocate it on the stack
^ permalink raw reply
* RE: [PATCH NET] ethtool: silence warning on bit loss
From: David Laight @ 2016-10-14 9:26 UTC (permalink / raw)
To: 'Jesse Brandeburg', netdev@vger.kernel.org
In-Reply-To: <1476400435-18400-1-git-send-email-jesse.brandeburg@intel.com>
From: Jesse Brandeburg
> Sent: 14 October 2016 00:14
> Sparse was complaining when we went to prototype some code
> using ethtool_cmd_speed_set and SPEED_100000, which uses
> the upper 16 bits of __u32 speed for the first time.
...
> Reported-by: Preethi Banala <preethi.banala@intel.com>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
> include/uapi/linux/ethtool.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 099a420..8e54723 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -119,8 +119,7 @@ struct ethtool_cmd {
> static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
> __u32 speed)
> {
> -
> - ep->speed = (__u16)speed;
> + ep->speed = (__u16)(speed & 0xFFFF);
> ep->speed_hi = (__u16)(speed >> 16);
I suspect that deleting both (__u16) casts also fixes it?
David
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 9:25 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu-Y6wuMGuRgrHRGCvAM9qZjvufs=7VWL8hAUKFSF=tZWg@mail.gmail.com>
On Fri, 2016-10-14 at 10:21 +0100, Ard Biesheuvel wrote:
> It is annotated with a TODO, though :-)
>
> 38320c70d282b (Herbert Xu 2008-01-28 19:35:05
> -0800 41)
> * TODO: Use spare space in skb for this where possible.
I saw that, but I don't think generally there will be spare space for
it - the stuff there is likely far too big. Anyway ... same problem
that we have.
I'm not inclined to allocate ~500 bytes temporarily for every frame
either though.
Maybe we could try to manage it in mac80211, we'd "only" need 5 AEAD
structs (which are today on the stack) in parallel for each key (4 TX,
1 RX), but in a typical case of having 3 keys that's already 7.5K worth
of memory that we almost never use. Again, with more complexity, we
could know that the TX will not be used if the driver does the TX, but
the single RX one we'd need unconditionally... decisions decisions...
johannes
^ permalink raw reply
* [PATCH] p54: memset(0) whole array
From: Jiri Slaby @ 2016-10-14 9:23 UTC (permalink / raw)
To: chunkeey; +Cc: linux-kernel, Jiri Slaby, Kalle Valo, linux-wireless, netdev
gcc 7 complains:
drivers/net/wireless/intersil/p54/fwio.c: In function 'p54_scan':
drivers/net/wireless/intersil/p54/fwio.c:491:4: warning: 'memset' used with length equal to number of elements without multiplication by element size [-Wmemset-elt-size]
Fix that by passing the correct size to memset.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Christian Lamparter <chunkeey@googlemail.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
drivers/net/wireless/intersil/p54/fwio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intersil/p54/fwio.c b/drivers/net/wireless/intersil/p54/fwio.c
index 257a9eadd595..4ac6764f4897 100644
--- a/drivers/net/wireless/intersil/p54/fwio.c
+++ b/drivers/net/wireless/intersil/p54/fwio.c
@@ -488,7 +488,7 @@ int p54_scan(struct p54_common *priv, u16 mode, u16 dwell)
entry += sizeof(__le16);
chan->pa_points_per_curve = 8;
- memset(chan->curve_data, 0, sizeof(*chan->curve_data));
+ memset(chan->curve_data, 0, sizeof(chan->curve_data));
memcpy(chan->curve_data, entry,
sizeof(struct p54_pa_curve_data_sample) *
min((u8)8, curve_data->points_per_channel));
--
2.10.1
^ permalink raw reply related
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 9:21 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <1476436248.31114.21.camel@sipsolutions.net>
On 14 October 2016 at 10:10, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2016-10-14 at 10:05 +0100, Ard Biesheuvel wrote:
>>
>> Indeed. And the decrypt path does the same for auth_tag[].
>
> Hadn't gotten that far, due to the BUG_ON() in CONFIG_DEBUG_SG in the
> encrypt path :)
>
>> But that still means there are two separate problems here, one which
>> affects the WPA code, and one that only affects the generic CCM
>> chaining mode (but not the accelerated arm64 implementation)
>
> Yes. The generic CCM chaining still doesn't typically have a request on
> the stack though. In fact, ESP (net/ipv4/esp4.c) for example will do
> temporary allocations with kmalloc for every frame, it seems.
>
It is annotated with a TODO, though :-)
38320c70d282b (Herbert Xu 2008-01-28 19:35:05 -0800 41)
* TODO: Use spare space in skb for this where possible.
>> Unsurprisingly, I would strongly prefer those to be fixed properly
>> rather than backing out my patch, but I'm happy to help out whichever
>> solution we reach consensus on.
>
> Yeah, obviously, it would be good to use the accelerated versions after
> all.
>
>> I will check whether this removes the issue when not using
>> crypto/ccm.ko
>
> Ok. I think we can probably live with having those 48 bytes in per-CPU
> buffers, but I suppose we don't really want to have ~500.
>
Agreed.
^ permalink raw reply
* hello
From: maowenan @ 2016-10-14 9:18 UTC (permalink / raw)
To: netdev
i want to subscribe this mail, thank you very much.
^ permalink raw reply
* Hello
From: yuehaibing @ 2016-10-14 9:17 UTC (permalink / raw)
To: netdev
subscribe linux-kernel
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 9:10 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu9KObA61fEYxr1FFj_3SKNS2RUbqZo01yfnm9Bf0AncwA@mail.gmail.com>
On Fri, 2016-10-14 at 10:05 +0100, Ard Biesheuvel wrote:
>
> Indeed. And the decrypt path does the same for auth_tag[].
Hadn't gotten that far, due to the BUG_ON() in CONFIG_DEBUG_SG in the
encrypt path :)
> But that still means there are two separate problems here, one which
> affects the WPA code, and one that only affects the generic CCM
> chaining mode (but not the accelerated arm64 implementation)
Yes. The generic CCM chaining still doesn't typically have a request on
the stack though. In fact, ESP (net/ipv4/esp4.c) for example will do
temporary allocations with kmalloc for every frame, it seems.
> Unsurprisingly, I would strongly prefer those to be fixed properly
> rather than backing out my patch, but I'm happy to help out whichever
> solution we reach consensus on.
Yeah, obviously, it would be good to use the accelerated versions after
all.
> I will check whether this removes the issue when not using
> crypto/ccm.ko
Ok. I think we can probably live with having those 48 bytes in per-CPU
buffers, but I suppose we don't really want to have ~500.
johannes
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 9:05 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <1476435303.31114.14.camel@sipsolutions.net>
On 14 October 2016 at 09:55, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2016-10-14 at 09:47 +0100, Ard Biesheuvel wrote:
>>
>> Do you have a reference for the sg_set_buf() call on odata?
>> crypto/ccm.c does not seem to have it (afaict),
>
> It's indirect - crypto_ccm_encrypt() calls crypto_ccm_init_crypt()
> which does it.
>
Indeed. And the decrypt path does the same for auth_tag[].
But that still means there are two separate problems here, one which
affects the WPA code, and one that only affects the generic CCM
chaining mode (but not the accelerated arm64 implementation)
Unsurprisingly, I would strongly prefer those to be fixed properly
rather than backing out my patch, but I'm happy to help out whichever
solution we reach consensus on.
>> and the same problem
>> does not exist in the accelerated arm64 implementation. In the mean
>> time, I will try and see if we can move aad[] off the stack in the
>> WPA code.
>
> I had that with per-CPU buffers, just sent the patch upthread.
>
I will check whether this removes the issue when not using crypto/ccm.ko
^ permalink raw reply
* Re: [PATCH net-next 2/2] ila: Cache a route to translated address
From: Jiri Benc @ 2016-10-14 9:04 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: Tom Herbert, davem, netdev, kernel-team
In-Reply-To: <58007996.2070500@cumulusnetworks.com>
On Thu, 13 Oct 2016 23:22:14 -0700, Roopa Prabhu wrote:
> This removes the last and only user of lwt orig_output. we can drop it
> subsequently. But since orig_input is still in use, probably better to keep it
> around for symmetry and for other uses in the future.
If it's no longer used, let's remove it. It can be always added later
again if needed. We don't keep things just because they maybe can be
used for something in the future.
Thanks,
Jiri
^ 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