* [PATCH] iwlwifi: dvm: excessive if in rs_bt_update_lq()
From: Denis Efremov @ 2019-09-25 20:49 UTC (permalink / raw)
To: Kalle Valo
Cc: Denis Efremov, Johannes Berg, Emmanuel Grumbach, Luca Coelho,
Intel Linux Wireless, David S. Miller, linux-wireless, netdev,
linux-kernel
There is no need to check 'priv->bt_ant_couple_ok' twice in
rs_bt_update_lq(). The second check is always true. Thus, the
expression can be simplified.
Signed-off-by: Denis Efremov <efremov@linux.com>
---
drivers/net/wireless/intel/iwlwifi/dvm/rs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/rs.c b/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
index 74229fcb63a9..226165db7dfd 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
@@ -851,7 +851,7 @@ static void rs_bt_update_lq(struct iwl_priv *priv, struct iwl_rxon_context *ctx,
* Is there a need to switch between
* full concurrency and 3-wire?
*/
- if (priv->bt_ci_compliance && priv->bt_ant_couple_ok)
+ if (priv->bt_ci_compliance)
full_concurrent = true;
else
full_concurrent = false;
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] openvswitch: change type of UPCALL_PID attribute to NLA_UNSPEC
From: Pravin Shelar @ 2019-09-25 21:02 UTC (permalink / raw)
To: Li RongQing; +Cc: Linux Kernel Network Developers
In-Reply-To: <1569323512-19195-1-git-send-email-lirongqing@baidu.com>
On Tue, Sep 24, 2019 at 4:11 AM Li RongQing <lirongqing@baidu.com> wrote:
>
> userspace openvswitch patch "(dpif-linux: Implement the API
> functions to allow multiple handler threads read upcall)"
> changes its type from U32 to UNSPEC, but leave the kernel
> unchanged
>
> and after kernel 6e237d099fac "(netlink: Relax attr validation
> for fixed length types)", this bug is exposed by the below
> warning
>
> [ 57.215841] netlink: 'ovs-vswitchd': attribute type 5 has an invalid length.
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
Acked-by: Pravin B Shelar <pshelar@ovn.org>
Thanks,
Pravin.
^ permalink raw reply
* [PATCH] rtlwifi: Remove excessive check in _rtl_ps_inactive_ps()
From: Denis Efremov @ 2019-09-25 20:58 UTC (permalink / raw)
To: Kalle Valo
Cc: Denis Efremov, Ping-Ke Shih, David S. Miller, linux-wireless,
netdev, linux-kernel
There is no need to check "rtlhal->interface == INTF_PCI" twice in
_rtl_ps_inactive_ps(). The nested check is always true. Thus, the
expression can be simplified.
Signed-off-by: Denis Efremov <efremov@linux.com>
---
drivers/net/wireless/realtek/rtlwifi/ps.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/ps.c b/drivers/net/wireless/realtek/rtlwifi/ps.c
index 70f04c2f5b17..6a8127539ea7 100644
--- a/drivers/net/wireless/realtek/rtlwifi/ps.c
+++ b/drivers/net/wireless/realtek/rtlwifi/ps.c
@@ -161,8 +161,7 @@ static void _rtl_ps_inactive_ps(struct ieee80211_hw *hw)
if (ppsc->inactive_pwrstate == ERFON &&
rtlhal->interface == INTF_PCI) {
if ((ppsc->reg_rfps_level & RT_RF_OFF_LEVL_ASPM) &&
- RT_IN_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM) &&
- rtlhal->interface == INTF_PCI) {
+ RT_IN_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM)) {
rtlpriv->intf_ops->disable_aspm(hw);
RT_CLEAR_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM);
}
--
2.21.0
^ permalink raw reply related
* [PATCH v2] hv_sock: Add the support of hibernation
From: Dexuan Cui @ 2019-09-25 21:34 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, davem@davemloft.net,
linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Michael Kelley
Cc: Dexuan Cui
Add the necessary dummy callbacks for hibernation.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Acked-by: David S. Miller <davem@davemloft.net>
---
In v2:
Added David's Acked-by.
Removed [net-next] from the Subject.
@Sasha, can you please pick this up into the hyper-v tree?
net/vmw_vsock/hyperv_transport.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index f2084e3f7aa4..4c02e38aa728 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -930,6 +930,24 @@ static int hvs_remove(struct hv_device *hdev)
return 0;
}
+/* hv_sock connections can not persist across hibernation, and all the hv_sock
+ * channels are forced to be rescinded before hibernation: see
+ * vmbus_bus_suspend(). Here the dummy hvs_suspend() and hvs_resume()
+ * are only needed because hibernation requires that every vmbus device's
+ * driver should have a .suspend and .resume callback: see vmbus_suspend().
+ */
+static int hvs_suspend(struct hv_device *hv_dev)
+{
+ /* Dummy */
+ return 0;
+}
+
+static int hvs_resume(struct hv_device *dev)
+{
+ /* Dummy */
+ return 0;
+}
+
/* This isn't really used. See vmbus_match() and vmbus_probe() */
static const struct hv_vmbus_device_id id_table[] = {
{},
@@ -941,6 +959,8 @@ static struct hv_driver hvs_drv = {
.id_table = id_table,
.probe = hvs_probe,
.remove = hvs_remove,
+ .suspend = hvs_suspend,
+ .resume = hvs_resume,
};
static int __init hvs_init(void)
--
2.19.1
^ permalink raw reply related
* Re: [PATCH] ipv6: Properly check reference count flag before taking reference
From: Eric Dumazet @ 2019-09-25 21:37 UTC (permalink / raw)
To: Petr Vorel, Jason A. Donenfeld; +Cc: netdev, linux-kernel, stable
In-Reply-To: <20190923150600.GA27191@dell5510>
On 9/23/19 8:06 AM, Petr Vorel wrote:
> Hi,
>
>> People are reporting that WireGuard experiences erratic crashes on 5.3,
>> and bisected it down to 7d30a7f6424e. Casually flipping through that
>> commit I noticed that a flag is checked using `|` instead of `&`, which in
>> this current case, means that a reference is never incremented, which
>> would result in the use-after-free users are seeing. This commit changes
>> the `|` to the proper `&` test.
>
>> Cc: stable@vger.kernel.org
>> Fixes: 7d30a7f6424e ("Merge branch 'ipv6-avoid-taking-refcnt-on-dst-during-route-lookup'")
>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> NOTE: this change was added in d64a1f574a29 ("ipv6: honor RT6_LOOKUP_F_DST_NOREF in rule lookup logic")
>
> Kind regards,
> Petr
>
This was fixed earlier I think
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=7b09c2d052db4b4ad0b27b97918b46a7746966fa
>> ---
>> net/ipv6/ip6_fib.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
>> index 87f47bc55c5e..6e2af411cd9c 100644
>> --- a/net/ipv6/ip6_fib.c
>> +++ b/net/ipv6/ip6_fib.c
>> @@ -318,7 +318,7 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
>> if (rt->dst.error == -EAGAIN) {
>> ip6_rt_put_flags(rt, flags);
>> rt = net->ipv6.ip6_null_entry;
>> - if (!(flags | RT6_LOOKUP_F_DST_NOREF))
>> + if (!(flags & RT6_LOOKUP_F_DST_NOREF))
>> dst_hold(&rt->dst);
>> }
^ permalink raw reply
* [PATCH v2][PATCH net] hv_netvsc: Add the support of hibernation
From: Dexuan Cui @ 2019-09-25 22:04 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, davem@davemloft.net,
linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Michael Kelley
Cc: Dexuan Cui
The existing netvsc_detach() and netvsc_attach() APIs make it easy to
implement the suspend/resume callbacks.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
This patch is basically a pure Hyper-V specific change. I request this
patch should go through Sasha's Hyper-V tree rather than the net tree.
Sasha's Hyper-V tree is here:
https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git
Previously there was a dependency on the commit 271b2224d42f ("Drivers:
hv: vmbus: Implement suspend/resume for VSC drivers for hibernation"),
which was only on Sasha Levin's Hyper-V tree's hyperv-next branch:
https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git/log/?h=hyperv-next
. Now the patch has been merged into Linus's master tree, but as of now,
the patch (271b2224d42f) has not appeared in the net.git tree, so IMO
it's better for this patch to go through the Hyper-V tree. The added
code in this patch is unlikely to cause a conflict.
In v2:
Removed the superfluous "cancel_work_sync(&nvdev->subchan_work)".
Changed the [PATCH net-next] to [PATCH net] in the Subject, because
IMO this is more of a bug fix rather than a new feaure.
No other change.
drivers/net/hyperv/hyperv_net.h | 3 ++
drivers/net/hyperv/netvsc_drv.c | 57 +++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index ecc9af050387..b8763ee4c0d0 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -952,6 +952,9 @@ struct net_device_context {
u32 vf_alloc;
/* Serial number of the VF to team with */
u32 vf_serial;
+
+ /* Used to temporarily save the config info across hibernation */
+ struct netvsc_device_info *saved_netvsc_dev_info;
};
/* Per channel data */
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index afdcc5664ea6..53a9451a58a7 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2392,6 +2392,61 @@ static int netvsc_remove(struct hv_device *dev)
return 0;
}
+static int netvsc_suspend(struct hv_device *dev)
+{
+ struct net_device_context *ndev_ctx;
+ struct net_device *vf_netdev, *net;
+ struct netvsc_device *nvdev;
+ int ret;
+
+ net = hv_get_drvdata(dev);
+
+ ndev_ctx = netdev_priv(net);
+ cancel_delayed_work_sync(&ndev_ctx->dwork);
+
+ rtnl_lock();
+
+ nvdev = rtnl_dereference(ndev_ctx->nvdev);
+ if (nvdev == NULL) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev);
+ if (vf_netdev)
+ netvsc_unregister_vf(vf_netdev);
+
+ /* Save the current config info */
+ ndev_ctx->saved_netvsc_dev_info = netvsc_devinfo_get(nvdev);
+
+ ret = netvsc_detach(net, nvdev);
+out:
+ rtnl_unlock();
+
+ return ret;
+}
+
+static int netvsc_resume(struct hv_device *dev)
+{
+ struct net_device *net = hv_get_drvdata(dev);
+ struct net_device_context *net_device_ctx;
+ struct netvsc_device_info *device_info;
+ int ret;
+
+ rtnl_lock();
+
+ net_device_ctx = netdev_priv(net);
+ device_info = net_device_ctx->saved_netvsc_dev_info;
+
+ ret = netvsc_attach(net, device_info);
+
+ rtnl_unlock();
+
+ kfree(device_info);
+ net_device_ctx->saved_netvsc_dev_info = NULL;
+
+ return ret;
+}
static const struct hv_vmbus_device_id id_table[] = {
/* Network guid */
{ HV_NIC_GUID, },
@@ -2406,6 +2461,8 @@ static struct hv_driver netvsc_drv = {
.id_table = id_table,
.probe = netvsc_probe,
.remove = netvsc_remove,
+ .suspend = netvsc_suspend,
+ .resume = netvsc_resume,
.driver = {
.probe_type = PROBE_FORCE_SYNCHRONOUS,
},
--
2.19.1
^ permalink raw reply related
* [PATCH] net/ncsi: prevent memory leak in ncsi_rsp_handler_gc
From: Navid Emamdoost @ 2019-09-25 21:58 UTC (permalink / raw)
Cc: emamd001, kjlu, smccaman, Navid Emamdoost, Samuel Mendoza-Jonas,
David S. Miller, netdev, linux-kernel
In ncsi_rsp_handler_gc if allocation for nc->vlan_filter.vids fails the
allocated memory for nc->mac_filter.addrs should be released.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
net/ncsi/ncsi-rsp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index d5611f04926d..f3f7c3772994 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -800,8 +800,10 @@ static int ncsi_rsp_handler_gc(struct ncsi_request *nr)
nc->vlan_filter.vids = kcalloc(rsp->vlan_cnt,
sizeof(*nc->vlan_filter.vids),
GFP_ATOMIC);
- if (!nc->vlan_filter.vids)
+ if (!nc->vlan_filter.vids) {
+ kfree(nc->mac_filter.addrs);
return -ENOMEM;
+ }
/* Set VLAN filters active so they are cleared in the first
* configuration state
*/
--
2.17.1
^ permalink raw reply related
* [PATCH V2] net: dsa: microchip: Always set regmap stride to 1
From: Marek Vasut @ 2019-09-25 22:08 UTC (permalink / raw)
To: netdev
Cc: Marek Vasut, Andrew Lunn, David S . Miller, Florian Fainelli,
George McCollister, Tristram Ha, Vivien Didelot, Woojung Huh
The regmap stride is set to 1 for regmap describing 8bit registers already.
However, for 16/32/64bit registers, the stride is 2/4/8 respectively. This
is not correct, as the switch protocol supports unaligned register reads
and writes and the KSZ87xx even uses such unaligned register accesses to
read e.g. MIB counter.
This patch fixes MIB counter access on KSZ87xx.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: David S. Miller <davem@davemloft.net>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: George McCollister <george.mccollister@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Cc: Woojung Huh <woojung.huh@microchip.com>
Fixes: 46558d601cb6 ("net: dsa: microchip: Initial SPI regmap support")
Fixes: 255b59ad0db2 ("net: dsa: microchip: Factor out regmap config generation into common header")
---
V2: Add Fixes: tags
---
drivers/net/dsa/microchip/ksz_common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index a24d8e61fbe7..dd60d0837fc6 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -303,7 +303,7 @@ static inline void ksz_pwrite32(struct ksz_device *dev, int port, int offset,
{ \
.name = #width, \
.val_bits = (width), \
- .reg_stride = (width) / 8, \
+ .reg_stride = 1, \
.reg_bits = (regbits) + (regalign), \
.pad_bits = (regpad), \
.max_register = BIT(regbits) - 1, \
--
2.23.0
^ permalink raw reply related
* Re: [PATCH] net: dsa: microchip: Always set regmap stride to 1
From: Marek Vasut @ 2019-09-25 22:09 UTC (permalink / raw)
To: David Miller
Cc: netdev, andrew, f.fainelli, george.mccollister, Tristram.Ha,
vivien.didelot, woojung.huh
In-Reply-To: <20190925.134424.1566106400449419934.davem@davemloft.net>
On 9/25/19 1:44 PM, David Miller wrote:
> From: Marek Vasut <marex@denx.de>
> Date: Sat, 21 Sep 2019 19:53:09 +0200
>
>> The regmap stride is set to 1 for regmap describing 8bit registers already.
>> However, for 16/32/64bit registers, the stride is 2/4/8 respectively. This
>> is not correct, as the switch protocol supports unaligned register reads
>> and writes and the KSZ87xx even uses such unaligned register accesses to
>> read e.g. MIB counter.
>>
>> This patch fixes MIB counter access on KSZ87xx.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>
> Please resubmit with an appropriate Fixes: tag as per Florian's feedback.
Done
^ permalink raw reply
* [PATCH] genetlink: prevent memory leak in netlbl_unlabel_defconf
From: Navid Emamdoost @ 2019-09-25 22:10 UTC (permalink / raw)
Cc: emamd001, kjlu, smccaman, Navid Emamdoost, Paul Moore,
David S. Miller, netdev, linux-security-module, linux-kernel
In netlbl_unlabel_defconf if netlbl_domhsh_add_default fails the
allocated entry should be released.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
net/netlabel/netlabel_unlabeled.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index d2e4ab8d1cb1..c63ec480ee4e 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -1541,8 +1541,10 @@ int __init netlbl_unlabel_defconf(void)
entry->family = AF_UNSPEC;
entry->def.type = NETLBL_NLTYPE_UNLABELED;
ret_val = netlbl_domhsh_add_default(entry, &audit_info);
- if (ret_val != 0)
+ if (ret_val != 0) {
+ kfree(entry);
return ret_val;
+ }
netlbl_unlabel_acceptflg_set(1, &audit_info);
--
2.17.1
^ permalink raw reply related
* RE: [PATCH v2][PATCH net] hv_netvsc: Add the support of hibernation
From: Haiyang Zhang @ 2019-09-25 22:23 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Stephen Hemminger, sashal@kernel.org,
davem@davemloft.net, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Michael Kelley
In-Reply-To: <1569449034-29924-1-git-send-email-decui@microsoft.com>
> -----Original Message-----
> From: Dexuan Cui <decui@microsoft.com>
> Sent: Wednesday, September 25, 2019 6:04 PM
> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; sashal@kernel.org; davem@davemloft.net;
> linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Michael Kelley <mikelley@microsoft.com>
> Cc: Dexuan Cui <decui@microsoft.com>
> Subject: [PATCH v2][PATCH net] hv_netvsc: Add the support of hibernation
>
> The existing netvsc_detach() and netvsc_attach() APIs make it easy to
> implement the suspend/resume callbacks.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
^ permalink raw reply
* Re: [PATCH v3 0/2] net: stmmac: Enhanced addressing mode for DWMAC 4.10
From: Thierry Reding @ 2019-09-25 22:46 UTC (permalink / raw)
To: Florian Fainelli
Cc: Jose Abreu, David Miller, peppe.cavallaro@st.com,
alexandre.torgue@st.com, jonathanh@nvidia.com, bbiswas@nvidia.com,
netdev@vger.kernel.org, linux-tegra@vger.kernel.org
In-Reply-To: <9f0e2386-c4b1-52b0-6881-e72093eb1b05@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3251 bytes --]
On Wed, Sep 25, 2019 at 10:31:13AM -0700, Florian Fainelli wrote:
> On 9/25/19 4:46 AM, Jose Abreu wrote:
> > From: Jose Abreu <joabreu@synopsys.com>
> > Date: Sep/25/2019, 12:41:04 (UTC+00:00)
> >
> >> From: David Miller <davem@davemloft.net>
> >> Date: Sep/25/2019, 12:33:53 (UTC+00:00)
> >>
> >>> From: Jose Abreu <Jose.Abreu@synopsys.com>
> >>> Date: Wed, 25 Sep 2019 10:44:53 +0000
> >>>
> >>>> From: David Miller <davem@davemloft.net>
> >>>> Date: Sep/24/2019, 20:45:08 (UTC+00:00)
> >>>>
> >>>>> From: Thierry Reding <thierry.reding@gmail.com>
> >>>>> Date: Fri, 20 Sep 2019 19:00:34 +0200
> >>>>>
> >>>>> Also, you're now writing to the high 32-bits unconditionally, even when
> >>>>> it will always be zero because of 32-bit addressing. That looks like
> >>>>> a step backwards to me.
> >>>>
> >>>> Don't agree. As per previous discussions and as per my IP knowledge, if
> >>>> EAME is not enabled / not supported the register can still be written.
> >>>> This is not fast path and will not impact any remaining operation. Can
> >>>> you please explain what exactly is the concern about this ?
> >>>>
> >>>> Anyway, this is an important feature for performance so I hope Thierry
> >>>> re-submits this once -next opens and addressing the review comments.
> >>>
> >>> Perhaps I misunderstand the context, isn't this code writing the
> >>> descriptors for every packet?
> >>
> >> No, its just setting up the base address for the descriptors which is
> >> done in open(). The one that's in the fast path is the tail address,
> >> which is always the lower 32 bits.
> >
> > Oops, sorry. Indeed it's done in refill operation in function
> > dwmac4_set_addr() for rx/tx which is fast path so you do have a point
> > that I was not seeing. Thanks for bringing this up!
> >
> > Now, the point would be:
> > a) Is it faster to have an condition check in dwmac4_set_addr(), or
> > b) Always write to descs the upper 32 bits. Which always exists in the
> > IP and is a standard write to memory.
>
> The way I would approach it (as done in bcmgenet.c) is that if the
> platform both has CONFIG_PHYS_ADDR_T_64BIT=y and supports > 32-bits
> addresses, then you write the upper 32-bits otherwise, you do not. Given
> you indicate that the registers are safe to write regardless, then maybe
> just the check on CONFIG_PHYS_ADDR_T_64BIT is enough for your case. The
> rationale in my case is that register writes to on-chip descriptors are
> fairly expensive (~200ns per operation) and get in the hot-path.
>
> The CONFIG_PHYS_ADDR_T_64BIT check addresses both native 64-bit
> platforms (e.g.: ARM64) and those that do support LPAE (ARM LPAE for
> instance).
I think we actually want CONFIG_DMA_ADDR_T_64BIT here because we're
dealing with addresses returned from the DMA API here.
I can add an additional condition for the upper 32-bit register writes,
something like:
if (IS_ENABLED(CONFIG_DMA_ADDR_T_64BIT) && priv->dma_cfg->eame)
...
The compiler should be able to eliminate that as dead code on platforms
that don't support 64-bit DMA addresses, but the code should still be
compiler regardless of the setting, thus increasing the compile
coverage.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] net: qrtr: fix memory leak in qrtr_tun_read_iter
From: Navid Emamdoost @ 2019-09-25 23:04 UTC (permalink / raw)
Cc: emamd001, smccaman, kjlu, Navid Emamdoost, David S. Miller,
netdev, linux-kernel
In qrtr_tun_read_iter we need an error handling path to appropriately
release skb in cases of error.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
net/qrtr/tun.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/qrtr/tun.c b/net/qrtr/tun.c
index e35869e81766..0f6e6d1d2901 100644
--- a/net/qrtr/tun.c
+++ b/net/qrtr/tun.c
@@ -54,19 +54,24 @@ static ssize_t qrtr_tun_read_iter(struct kiocb *iocb, struct iov_iter *to)
int count;
while (!(skb = skb_dequeue(&tun->queue))) {
- if (filp->f_flags & O_NONBLOCK)
- return -EAGAIN;
+ if (filp->f_flags & O_NONBLOCK) {
+ count = -EAGAIN;
+ goto out;
+ }
/* Wait until we get data or the endpoint goes away */
if (wait_event_interruptible(tun->readq,
- !skb_queue_empty(&tun->queue)))
- return -ERESTARTSYS;
+ !skb_queue_empty(&tun->queue))) {
+ count = -ERESTARTSYS;
+ goto out;
+ }
}
count = min_t(size_t, iov_iter_count(to), skb->len);
if (copy_to_iter(skb->data, count, to) != count)
count = -EFAULT;
+out:
kfree_skb(skb);
return count;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] net/ncsi: prevent memory leak in ncsi_rsp_handler_gc
From: Al Viro @ 2019-09-25 23:09 UTC (permalink / raw)
To: Navid Emamdoost
Cc: emamd001, kjlu, smccaman, Samuel Mendoza-Jonas, David S. Miller,
netdev, linux-kernel
In-Reply-To: <20190925215854.14284-1-navid.emamdoost@gmail.com>
On Wed, Sep 25, 2019 at 04:58:53PM -0500, Navid Emamdoost wrote:
> In ncsi_rsp_handler_gc if allocation for nc->vlan_filter.vids fails the
> allocated memory for nc->mac_filter.addrs should be released.
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
> net/ncsi/ncsi-rsp.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index d5611f04926d..f3f7c3772994 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -800,8 +800,10 @@ static int ncsi_rsp_handler_gc(struct ncsi_request *nr)
> nc->vlan_filter.vids = kcalloc(rsp->vlan_cnt,
> sizeof(*nc->vlan_filter.vids),
> GFP_ATOMIC);
> - if (!nc->vlan_filter.vids)
> + if (!nc->vlan_filter.vids) {
> + kfree(nc->mac_filter.addrs);
> return -ENOMEM;
> + }
Again, why is it not a double-free? IOW, what guarantees that we won't
be calling <greps> ncsi_remove_channel(nc) at later point?
I'm not familiar with that code, so you _might_ be correct in this case,
but you need a lot more analysis in commit message than "should be",
considering the other similar patches from the same source, with the
same level of details in them that had been provably broken.
I don't know what kind of heuristics you are using when looking for
leaks, but they demonstrably give quite a few false positives.
It might be useful (and not just for you) to discuss those heuristics.
Could you go over the patch series you've posted and follow them up
with "here I've decided that we have a leak for such and such reason".
_Including_ the ones where you've ended up with false positives.
Look at it this way: you've posted a lot of statements without any
proofs of their correctness *or* any way to guess what those missing
proofs might've been. At least some of them are false. I can try
to prove them from scratch and post such proofs where the statement
happens to be true and counterexamples where it happens to be false.
However, it would've been much more useful to go through what you've
actually done to arrive to those statements, so that mistakes
would not be repeated in new problems. And those mistakes are very
unlikely to be yours alone, so other people would benefit as well.
^ permalink raw reply
* Re: [PATCH] net: qrtr: fix memory leak in qrtr_tun_read_iter
From: Al Viro @ 2019-09-25 23:21 UTC (permalink / raw)
To: Navid Emamdoost
Cc: emamd001, smccaman, kjlu, David S. Miller, netdev, linux-kernel
In-Reply-To: <20190925230416.20126-1-navid.emamdoost@gmail.com>
On Wed, Sep 25, 2019 at 06:04:13PM -0500, Navid Emamdoost wrote:
> In qrtr_tun_read_iter we need an error handling path to appropriately
> release skb in cases of error.
Release _what_ skb?
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
> net/qrtr/tun.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/net/qrtr/tun.c b/net/qrtr/tun.c
> index e35869e81766..0f6e6d1d2901 100644
> --- a/net/qrtr/tun.c
> +++ b/net/qrtr/tun.c
> @@ -54,19 +54,24 @@ static ssize_t qrtr_tun_read_iter(struct kiocb *iocb, struct iov_iter *to)
> int count;
>
> while (!(skb = skb_dequeue(&tun->queue))) {
The body of the loop is entered only if the loop condition has
evaluated true. In this case, it means that the value of
!(skb = skb_dequeue(&tun->queue))
had been true, i.e. the value of
skb = skb_dequeue(&tun->queue)
has been NULL, i.e. that skb_dequeue() has returned NULL, which had
been copied into skb.
In other words, in the body of that loop we have skb equal to NULL.
> - if (filp->f_flags & O_NONBLOCK)
> - return -EAGAIN;
> + if (filp->f_flags & O_NONBLOCK) {
> + count = -EAGAIN;
> + goto out;
> + }
>
> /* Wait until we get data or the endpoint goes away */
> if (wait_event_interruptible(tun->readq,
> - !skb_queue_empty(&tun->queue)))
> - return -ERESTARTSYS;
> + !skb_queue_empty(&tun->queue))) {
> + count = -ERESTARTSYS;
> + goto out;
> + }
> }
The meaning of that loop is fairly clear, isn't it? Keep looking int
tun->queue until an skb shows up there. If it's not immediately there,
fail with -EAGAIN for non-blocking files and wait on tun->readq until
some skb arrives.
^ permalink raw reply
* Re: [PATCH] genetlink: prevent memory leak in netlbl_unlabel_defconf
From: Paul Moore @ 2019-09-25 23:27 UTC (permalink / raw)
To: Navid Emamdoost
Cc: emamd001, kjlu, smccaman, David S. Miller, netdev,
linux-security-module, linux-kernel
In-Reply-To: <20190925221009.15523-1-navid.emamdoost@gmail.com>
On Wed, Sep 25, 2019 at 6:10 PM Navid Emamdoost
<navid.emamdoost@gmail.com> wrote:
>
> In netlbl_unlabel_defconf if netlbl_domhsh_add_default fails the
> allocated entry should be released.
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
> net/netlabel/netlabel_unlabeled.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
It is worth noting that netlbl_unlabel_defconf() is only called during
kernel boot by netlbl_init() and if netlbl_unlabel_defconf() returns
an error code the system panics, so this isn't currently a very
practical concern.
That said, netlbl_unlabel_defconf() *should* clean up here just on
principal if nothing else.
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index d2e4ab8d1cb1..c63ec480ee4e 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -1541,8 +1541,10 @@ int __init netlbl_unlabel_defconf(void)
> entry->family = AF_UNSPEC;
> entry->def.type = NETLBL_NLTYPE_UNLABELED;
> ret_val = netlbl_domhsh_add_default(entry, &audit_info);
> - if (ret_val != 0)
> + if (ret_val != 0) {
> + kfree(entry);
> return ret_val;
> + }
>
> netlbl_unlabel_acceptflg_set(1, &audit_info);
>
> --
> 2.17.1
>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [net-next 0/2] new PTP ioctl fixes
From: Jacob Keller @ 2019-09-25 23:33 UTC (permalink / raw)
To: netdev
Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Richard Cochran,
Felipe Balbi, David S . Miller, Christopher Hall
I noticed recently that Filip added new versions of the PTP ioctls which
correctly honor the reserved fields (making it so that we can safely extend
them in the future).
Unfortunately, this breaks the old ioctls for a couple of reasons. First,
the flags for the old ioctls get cleared. This means that the external
timestamp request can never be enabled. Further, it means future new flags
will *not* be cleared, and thus old ioctl will potentially send non-zero
data and be mis-interpreted.
Additionally, new flags are not protected against in-driver, because no
current driver verifies that the flags are only one of the supported ones.
This means new flags will be mis-interpreted by the drivers.
This series provides patches to fix drivers to verify and reject unsupported
flags, as well as to fix the ioctls to clear flags on the old version of the
ioctl properly.
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Christopher Hall <christopher.s.hall@intel.com>
Jacob Keller (2):
ptp: correctly disable flags on old ioctls
net: reject ptp requests with unsupported flags
drivers/net/dsa/mv88e6xxx/ptp.c | 5 +++++
drivers/net/ethernet/broadcom/tg3.c | 4 ++++
drivers/net/ethernet/intel/igb/igb_ptp.c | 8 +++++++
.../ethernet/mellanox/mlx5/core/lib/clock.c | 10 +++++++++
drivers/net/ethernet/microchip/lan743x_ptp.c | 4 ++++
drivers/net/ethernet/renesas/ravb_ptp.c | 9 ++++++++
.../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 4 ++++
drivers/net/phy/dp83640.c | 8 +++++++
drivers/ptp/ptp_chardev.c | 4 ++--
include/uapi/linux/ptp_clock.h | 22 +++++++++++++++++++
10 files changed, 76 insertions(+), 2 deletions(-)
--
2.23.0.245.gf157bbb9169d
^ permalink raw reply
* [net-next 1/2] ptp: correctly disable flags on old ioctls
From: Jacob Keller @ 2019-09-25 23:33 UTC (permalink / raw)
To: netdev
Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Richard Cochran,
Felipe Balbi, David S . Miller, Christopher Hall
In-Reply-To: <20190925233312.13977-1-jacob.e.keller@intel.com>
Commit 415606588c61 ("PTP: introduce new versions of IOCTLs",
2019-09-13) introduced new versions of the PTP ioctls which actually
validate that the flags are acceptable values.
As part of this, it cleared the flags value using a bitwise
and+negation, in an attempt to prevent the old ioctl from accidentally
enabling new features.
This is incorrect for a couple of reasons. First, it results in
accidentally preventing previously working flags on the request ioctl.
By clearing the "valid" flags, we now no longer allow setting the
enable, rising edge, or falling edge flags.
Second, if we add new additional flags in the future, they must not be
set by the old ioctl. (Since the flag wasn't checked before, we could
potentially break userspace programs which sent garbage flag data.
The correct way to resolve this is to check for and clear all but the
originally valid flags.
Create defines indicating which flags are correctly checked and
interpreted by the original ioctls. Use these to clear any bits which
will not be correctly interpreted by the original ioctls.
In the future, new flags must be added to the VALID_FLAGS macros, but
*not* to the V1_VALID_FLAGS macros. In this way, new features may be
exposed over the v2 ioctls, but without breaking previous userspace
which happened to not clear the flags value properly. The old ioctl will
continue to behave the same way, while the new ioctl gains the benefit
of using the flags fields.
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/ptp/ptp_chardev.c | 4 ++--
include/uapi/linux/ptp_clock.h | 22 ++++++++++++++++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 9c18476d8d10..67d0199840fd 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -155,7 +155,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
err = -EINVAL;
break;
} else if (cmd == PTP_EXTTS_REQUEST) {
- req.extts.flags &= ~PTP_EXTTS_VALID_FLAGS;
+ req.extts.flags &= PTP_EXTTS_V1_VALID_FLAGS;
req.extts.rsv[0] = 0;
req.extts.rsv[1] = 0;
}
@@ -184,7 +184,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
err = -EINVAL;
break;
} else if (cmd == PTP_PEROUT_REQUEST) {
- req.perout.flags &= ~PTP_PEROUT_VALID_FLAGS;
+ req.perout.flags &= PTP_PEROUT_V1_VALID_FLAGS;
req.perout.rsv[0] = 0;
req.perout.rsv[1] = 0;
req.perout.rsv[2] = 0;
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index f16301015949..59e89a1bc3bb 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -31,15 +31,37 @@
#define PTP_ENABLE_FEATURE (1<<0)
#define PTP_RISING_EDGE (1<<1)
#define PTP_FALLING_EDGE (1<<2)
+
+/*
+ * flag fields valid for the new PTP_EXTTS_REQUEST2 ioctl.
+ */
#define PTP_EXTTS_VALID_FLAGS (PTP_ENABLE_FEATURE | \
PTP_RISING_EDGE | \
PTP_FALLING_EDGE)
+/*
+ * flag fields valid for the original PTP_EXTTS_REQUEST ioctl.
+ * DO NOT ADD NEW FLAGS HERE.
+ */
+#define PTP_EXTTS_V1_VALID_FLAGS (PTP_ENABLE_FEATURE | \
+ PTP_RISING_EDGE | \
+ PTP_FALLING_EDGE)
+
/*
* Bits of the ptp_perout_request.flags field:
*/
#define PTP_PEROUT_ONE_SHOT (1<<0)
+
+/*
+ * flag fields valid for the new PTP_PEROUT_REQUEST2 ioctl.
+ */
#define PTP_PEROUT_VALID_FLAGS (PTP_PEROUT_ONE_SHOT)
+
+/*
+ * No flags are valid for the original PTP_PEROUT_REQUEST ioctl
+ */
+#define PTP_PEROUT_V1_VALID_FLAGS (0)
+
/*
* struct ptp_clock_time - represents a time value
*
--
2.23.0.245.gf157bbb9169d
^ permalink raw reply related
* [net-next 2/2] net: reject ptp requests with unsupported flags
From: Jacob Keller @ 2019-09-25 23:33 UTC (permalink / raw)
To: netdev
Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Richard Cochran,
Felipe Balbi, David S . Miller, Christopher Hall
In-Reply-To: <20190925233312.13977-1-jacob.e.keller@intel.com>
Fix all of the drivers which implement support for the periodic output
or external timestamp requests to reject unsupported flags.
This is important for forward compatibility: if a new flag is
introduced, the driver should reject requests to enable the flag until
it has been modified to actually support the flag in question.
This patch may not be correct for individual drivers, especially
regarding the rising vs falling edge flags. I interpreted the default
behavior to be to timestamp the rising edge of a pin transition.
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/dsa/mv88e6xxx/ptp.c | 5 +++++
drivers/net/ethernet/broadcom/tg3.c | 4 ++++
drivers/net/ethernet/intel/igb/igb_ptp.c | 8 ++++++++
drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c | 10 ++++++++++
drivers/net/ethernet/microchip/lan743x_ptp.c | 4 ++++
drivers/net/ethernet/renesas/ravb_ptp.c | 9 +++++++++
drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c | 4 ++++
drivers/net/phy/dp83640.c | 8 ++++++++
8 files changed, 52 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
index 073cbd0bb91b..2364b6b67a7b 100644
--- a/drivers/net/dsa/mv88e6xxx/ptp.c
+++ b/drivers/net/dsa/mv88e6xxx/ptp.c
@@ -273,6 +273,11 @@ static int mv88e6352_ptp_enable_extts(struct mv88e6xxx_chip *chip,
int pin;
int err;
+ /* Reject requests with unsupported flags */
+ if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
+ PTP_RISING_EDGE))
+ return -EOPNOTSUPP;
+
pin = ptp_find_pin(chip->ptp_clock, PTP_PF_EXTTS, rq->extts.index);
if (pin < 0)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 77f3511b97de..ca3aa1250dd1 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -6280,6 +6280,10 @@ static int tg3_ptp_enable(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_PEROUT:
+ /* Reject requests with unsupported flags */
+ if (rq->perout.flags)
+ return -EOPNOTSUPP;
+
if (rq->perout.index != 0)
return -EINVAL;
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index fd3071f55bd3..2867a2581a36 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -521,6 +521,10 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_EXTTS:
+ /* Reject requests with unsupported flags */
+ if (rq->extts.flags & ~(PTP_ENABLE_FEATURE | PTP_RISING_EDGE))
+ return -EOPNOTSUPP;
+
if (on) {
pin = ptp_find_pin(igb->ptp_clock, PTP_PF_EXTTS,
rq->extts.index);
@@ -551,6 +555,10 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
return 0;
case PTP_CLK_REQ_PEROUT:
+ /* Reject requests with unsupported flags */
+ if (rq->perout.flags)
+ return -EOPNOTSUPP;
+
if (on) {
pin = ptp_find_pin(igb->ptp_clock, PTP_PF_PEROUT,
rq->perout.index);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
index 0059b290e095..9a40f24e3193 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
@@ -236,6 +236,12 @@ static int mlx5_extts_configure(struct ptp_clock_info *ptp,
if (!MLX5_PPS_CAP(mdev))
return -EOPNOTSUPP;
+ /* Reject requests with unsupported flags */
+ if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
+ PTP_RISING_EDGE |
+ PTP_FALLING_EDGE))
+ return -EOPNOTSUPP;
+
if (rq->extts.index >= clock->ptp_info.n_pins)
return -EINVAL;
@@ -290,6 +296,10 @@ static int mlx5_perout_configure(struct ptp_clock_info *ptp,
if (!MLX5_PPS_CAP(mdev))
return -EOPNOTSUPP;
+ /* Reject requests with unsupported flags */
+ if (rq->perout.flags)
+ return -EOPNOTSUPP;
+
if (rq->perout.index >= clock->ptp_info.n_pins)
return -EINVAL;
diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c
index 57b26c2acf87..e8fe9a90fe4f 100644
--- a/drivers/net/ethernet/microchip/lan743x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
@@ -429,6 +429,10 @@ static int lan743x_ptp_perout(struct lan743x_adapter *adapter, int on,
int pulse_width = 0;
int perout_bit = 0;
+ /* Reject requests with unsupported flags */
+ if (perout->flags)
+ return -EOPNOTSUPP;
+
if (!on) {
lan743x_ptp_perout_off(adapter);
return 0;
diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
index 9a42580693cb..fe66697aafec 100644
--- a/drivers/net/ethernet/renesas/ravb_ptp.c
+++ b/drivers/net/ethernet/renesas/ravb_ptp.c
@@ -182,6 +182,11 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp,
struct net_device *ndev = priv->ndev;
unsigned long flags;
+ /* Reject requests with unsupported flags */
+ if (req->flags & ~(PTP_ENABLE_FEATURE |
+ PTP_RISING_EDGE))
+ return -EOPNOTSUPP;
+
if (req->index)
return -EINVAL;
@@ -211,6 +216,10 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp,
unsigned long flags;
int error = 0;
+ /* Reject requests with unsupported flags */
+ if (req->flags)
+ return -EOPNOTSUPP;
+
if (req->index)
return -EINVAL;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 173493db038c..352dc4c68625 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -140,6 +140,10 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_PEROUT:
+ /* Reject requests with unsupported flags */
+ if (rq->perout.flags)
+ return -EOPNOTSUPP;
+
cfg = &priv->pps[rq->perout.index];
cfg->start.tv_sec = rq->perout.start.sec;
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 6580094161a9..eba0569b1928 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -469,6 +469,11 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_EXTTS:
+ /* Reject requests with unsupported flags */
+ if (rq->extts.flags & ~(PTP_FEATURE_ENABLE |
+ PTP_RISING_EDGE |
+ PTP_FALLING_EDGE))
+ return -EOPNOTSUPP;
index = rq->extts.index;
if (index >= N_EXT_TS)
return -EINVAL;
@@ -491,6 +496,9 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
return 0;
case PTP_CLK_REQ_PEROUT:
+ /* Reject requests with unsupported flags */
+ if (rq->perout.flags)
+ return -EOPNOTSUPP;
if (rq->perout.index >= N_PER_OUT)
return -EINVAL;
return periodic_output(clock, rq, on, rq->perout.index);
--
2.23.0.245.gf157bbb9169d
^ permalink raw reply related
* [PATCH 0/1] bpf: Fix bpf_event_output re-entry issue
From: Allan Zhang @ 2019-09-25 23:43 UTC (permalink / raw)
To: daniel, songliubraving, netdev, bpf; +Cc: linux-kernel, Allan Zhang
BPF_PROG_TYPE_SOCK_OPS program can reenter bpf_event_output because it can
be called from atomic and non-atomic contexts since we don't have
bpf_prog_active to prevent it happen.
This patch enables 3 level of nesting to support normal, irq and nmi
context.
We can easily reproduce the issue by running neper crr mode with 100 flows
and 10 threads from neper client side.
Allan Zhang (1):
bpf: Fix bpf_event_output re-entry issue
kernel/trace/bpf_trace.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
--
2.23.0.351.gc4317032e6-goog
^ permalink raw reply
* [PATCH 1/1] bpf: Fix bpf_event_output re-entry issue
From: Allan Zhang @ 2019-09-25 23:43 UTC (permalink / raw)
To: daniel, songliubraving, netdev, bpf
Cc: linux-kernel, Allan Zhang, Stanislav Fomichev, Eric Dumazet
In-Reply-To: <20190925234312.94063-1-allanzhang@google.com>
BPF_PROG_TYPE_SOCK_OPS program can reenter bpf_event_output because it can
be called from atomic and non-atomic contexts since we don't have
bpf_prog_active to prevent it happen.
This patch enables 3 level of nesting to support normal, irq and nmi
context.
We can easily reproduce the issue by running neper crr mode with 100 flows
and 10 threads from neper client side.
Here is the whole stack dump:
[ 515.228898] WARNING: CPU: 20 PID: 14686 at kernel/trace/bpf_trace.c:549 bpf_event_output+0x1f9/0x220
[ 515.228903] CPU: 20 PID: 14686 Comm: tcp_crr Tainted: G W 4.15.0-smp-fixpanic #44
[ 515.228904] Hardware name: Intel TBG,ICH10/Ikaria_QC_1b, BIOS 1.22.0 06/04/2018
[ 515.228905] RIP: 0010:bpf_event_output+0x1f9/0x220
[ 515.228906] RSP: 0018:ffff9a57ffc03938 EFLAGS: 00010246
[ 515.228907] RAX: 0000000000000012 RBX: 0000000000000001 RCX: 0000000000000000
[ 515.228907] RDX: 0000000000000000 RSI: 0000000000000096 RDI: ffffffff836b0f80
[ 515.228908] RBP: ffff9a57ffc039c8 R08: 0000000000000004 R09: 0000000000000012
[ 515.228908] R10: ffff9a57ffc1de40 R11: 0000000000000000 R12: 0000000000000002
[ 515.228909] R13: ffff9a57e13bae00 R14: 00000000ffffffff R15: ffff9a57ffc1e2c0
[ 515.228910] FS: 00007f5a3e6ec700(0000) GS:ffff9a57ffc00000(0000) knlGS:0000000000000000
[ 515.228910] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 515.228911] CR2: 0000537082664fff CR3: 000000061fed6002 CR4: 00000000000226f0
[ 515.228911] Call Trace:
[ 515.228913] <IRQ>
[ 515.228919] [<ffffffff82c6c6cb>] bpf_sockopt_event_output+0x3b/0x50
[ 515.228923] [<ffffffff8265daee>] ? bpf_ktime_get_ns+0xe/0x10
[ 515.228927] [<ffffffff8266fda5>] ? __cgroup_bpf_run_filter_sock_ops+0x85/0x100
[ 515.228930] [<ffffffff82cf90a5>] ? tcp_init_transfer+0x125/0x150
[ 515.228933] [<ffffffff82cf9159>] ? tcp_finish_connect+0x89/0x110
[ 515.228936] [<ffffffff82cf98e4>] ? tcp_rcv_state_process+0x704/0x1010
[ 515.228939] [<ffffffff82c6e263>] ? sk_filter_trim_cap+0x53/0x2a0
[ 515.228942] [<ffffffff82d90d1f>] ? tcp_v6_inbound_md5_hash+0x6f/0x1d0
[ 515.228945] [<ffffffff82d92160>] ? tcp_v6_do_rcv+0x1c0/0x460
[ 515.228947] [<ffffffff82d93558>] ? tcp_v6_rcv+0x9f8/0xb30
[ 515.228951] [<ffffffff82d737c0>] ? ip6_route_input+0x190/0x220
[ 515.228955] [<ffffffff82d5f7ad>] ? ip6_protocol_deliver_rcu+0x6d/0x450
[ 515.228958] [<ffffffff82d60246>] ? ip6_rcv_finish+0xb6/0x170
[ 515.228961] [<ffffffff82d5fb90>] ? ip6_protocol_deliver_rcu+0x450/0x450
[ 515.228963] [<ffffffff82d60361>] ? ipv6_rcv+0x61/0xe0
[ 515.228966] [<ffffffff82d60190>] ? ipv6_list_rcv+0x330/0x330
[ 515.228969] [<ffffffff82c4976b>] ? __netif_receive_skb_one_core+0x5b/0xa0
[ 515.228972] [<ffffffff82c497d1>] ? __netif_receive_skb+0x21/0x70
[ 515.228975] [<ffffffff82c4a8d2>] ? process_backlog+0xb2/0x150
[ 515.228978] [<ffffffff82c4aadf>] ? net_rx_action+0x16f/0x410
[ 515.228982] [<ffffffff830000dd>] ? __do_softirq+0xdd/0x305
[ 515.228986] [<ffffffff8252cfdc>] ? irq_exit+0x9c/0xb0
[ 515.228989] [<ffffffff82e02de5>] ? smp_call_function_single_interrupt+0x65/0x120
[ 515.228991] [<ffffffff82e020e1>] ? call_function_single_interrupt+0x81/0x90
[ 515.228992] </IRQ>
[ 515.228996] [<ffffffff82a11ff0>] ? io_serial_in+0x20/0x20
[ 515.229000] [<ffffffff8259c040>] ? console_unlock+0x230/0x490
[ 515.229003] [<ffffffff8259cbaa>] ? vprintk_emit+0x26a/0x2a0
[ 515.229006] [<ffffffff8259cbff>] ? vprintk_default+0x1f/0x30
[ 515.229008] [<ffffffff8259d9f5>] ? vprintk_func+0x35/0x70
[ 515.229011] [<ffffffff8259d4bb>] ? printk+0x50/0x66
[ 515.229013] [<ffffffff82637637>] ? bpf_event_output+0xb7/0x220
[ 515.229016] [<ffffffff82c6c6cb>] ? bpf_sockopt_event_output+0x3b/0x50
[ 515.229019] [<ffffffff8265daee>] ? bpf_ktime_get_ns+0xe/0x10
[ 515.229023] [<ffffffff82c29e87>] ? release_sock+0x97/0xb0
[ 515.229026] [<ffffffff82ce9d6a>] ? tcp_recvmsg+0x31a/0xda0
[ 515.229029] [<ffffffff8266fda5>] ? __cgroup_bpf_run_filter_sock_ops+0x85/0x100
[ 515.229032] [<ffffffff82ce77c1>] ? tcp_set_state+0x191/0x1b0
[ 515.229035] [<ffffffff82ced10e>] ? tcp_disconnect+0x2e/0x600
[ 515.229038] [<ffffffff82cecbbb>] ? tcp_close+0x3eb/0x460
[ 515.229040] [<ffffffff82d21082>] ? inet_release+0x42/0x70
[ 515.229043] [<ffffffff82d58809>] ? inet6_release+0x39/0x50
[ 515.229046] [<ffffffff82c1f32d>] ? __sock_release+0x4d/0xd0
[ 515.229049] [<ffffffff82c1f3e5>] ? sock_close+0x15/0x20
[ 515.229052] [<ffffffff8273b517>] ? __fput+0xe7/0x1f0
[ 515.229055] [<ffffffff8273b66e>] ? ____fput+0xe/0x10
[ 515.229058] [<ffffffff82547bf2>] ? task_work_run+0x82/0xb0
[ 515.229061] [<ffffffff824086df>] ? exit_to_usermode_loop+0x7e/0x11f
[ 515.229064] [<ffffffff82408171>] ? do_syscall_64+0x111/0x130
[ 515.229067] [<ffffffff82e0007c>] ? entry_SYSCALL_64_after_hwframe+0x3d/0xa2
Fixes: a5a3a828cd00 ("bpf: add perf event notificaton support for sock_ops")
Effort: BPF
Signed-off-by: Allan Zhang <allanzhang@google.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
kernel/trace/bpf_trace.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index ca1255d14576..3e38a010003c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -500,14 +500,17 @@ static const struct bpf_func_proto bpf_perf_event_output_proto = {
.arg5_type = ARG_CONST_SIZE_OR_ZERO,
};
-static DEFINE_PER_CPU(struct pt_regs, bpf_pt_regs);
-static DEFINE_PER_CPU(struct perf_sample_data, bpf_misc_sd);
+static DEFINE_PER_CPU(int, bpf_event_output_nest_level);
+struct bpf_nested_pt_regs {
+ struct pt_regs regs[3];
+};
+static DEFINE_PER_CPU(struct bpf_nested_pt_regs, bpf_pt_regs);
+static DEFINE_PER_CPU(struct bpf_trace_sample_data, bpf_misc_sds);
u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
void *ctx, u64 ctx_size, bpf_ctx_copy_t ctx_copy)
{
- struct perf_sample_data *sd = this_cpu_ptr(&bpf_misc_sd);
- struct pt_regs *regs = this_cpu_ptr(&bpf_pt_regs);
+ int nest_level = this_cpu_inc_return(bpf_event_output_nest_level);
struct perf_raw_frag frag = {
.copy = ctx_copy,
.size = ctx_size,
@@ -522,12 +525,25 @@ u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
.data = meta,
},
};
+ struct perf_sample_data *sd;
+ struct pt_regs *regs;
+ u64 ret;
+
+ if (WARN_ON_ONCE(nest_level > ARRAY_SIZE(bpf_misc_sds.sds))) {
+ ret = -EBUSY;
+ goto out;
+ }
+ sd = this_cpu_ptr(&bpf_misc_sds.sds[nest_level - 1]);
+ regs = this_cpu_ptr(&bpf_pt_regs.regs[nest_level - 1]);
perf_fetch_caller_regs(regs);
perf_sample_data_init(sd, 0, 0);
sd->raw = &raw;
- return __bpf_perf_event_output(regs, map, flags, sd);
+ ret = __bpf_perf_event_output(regs, map, flags, sd);
+out:
+ this_cpu_dec(bpf_event_output_nest_level);
+ return ret;
}
BPF_CALL_0(bpf_get_current_task)
--
2.23.0.351.gc4317032e6-goog
^ permalink raw reply related
* Re: [PATCH] rtlwifi: Remove excessive check in _rtl_ps_inactive_ps()
From: Larry Finger @ 2019-09-26 0:06 UTC (permalink / raw)
To: Denis Efremov, Kalle Valo
Cc: Ping-Ke Shih, David S. Miller, linux-wireless, netdev,
linux-kernel
In-Reply-To: <20190925205858.30216-1-efremov@linux.com>
On 9/25/19 3:58 PM, Denis Efremov wrote:
> There is no need to check "rtlhal->interface == INTF_PCI" twice in
> _rtl_ps_inactive_ps(). The nested check is always true. Thus, the
> expression can be simplified.
>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
> drivers/net/wireless/realtek/rtlwifi/ps.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/ps.c b/drivers/net/wireless/realtek/rtlwifi/ps.c
> index 70f04c2f5b17..6a8127539ea7 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/ps.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/ps.c
> @@ -161,8 +161,7 @@ static void _rtl_ps_inactive_ps(struct ieee80211_hw *hw)
> if (ppsc->inactive_pwrstate == ERFON &&
> rtlhal->interface == INTF_PCI) {
> if ((ppsc->reg_rfps_level & RT_RF_OFF_LEVL_ASPM) &&
> - RT_IN_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM) &&
> - rtlhal->interface == INTF_PCI) {
> + RT_IN_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM)) {
> rtlpriv->intf_ops->disable_aspm(hw);
> RT_CLEAR_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM);
> }
>
Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
Thanks,
Larry
^ permalink raw reply
* Re: [PATCH v2] dimlib: make DIMLIB a hidden symbol
From: Randy Dunlap @ 2019-09-26 0:18 UTC (permalink / raw)
To: Uwe Kleine-König, Tal Gilboa, Saeed Mahameed, David Miller; +Cc: netdev
In-Reply-To: <20190924160259.10987-1-uwe@kleine-koenig.org>
On 9/24/19 9:02 AM, Uwe Kleine-König wrote:
> According to Tal Gilboa the only benefit from DIM comes from a driver
> that uses it. So it doesn't make sense to make this symbol user visible,
> instead all drivers that use it should select it (as is already the case
> AFAICT).
>
> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Thanks.
> ---
> Hello David,
>
> On Tue, Sep 24, 2019 at 04:45:28PM +0200, David Miller wrote:
>> Since this doesn't apply due to the moderation typo being elsewhere, I'd
>> really like you to fix up this submission to properly be against 'net'.
>
> I thought it would be possible to git-apply my patch with the -3 option.
> I even tested that, but obviously it only applies to my tree that has
> the git object with the typo fixed. Sorry for the extra effort I'm
> forcing on you. This patch applies to your public tree from just now.
>
> Best regads
> Uwe
>
> lib/Kconfig | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 4e6b1c3e4c98..d7fc9eb33b9b 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -555,8 +555,7 @@ config SIGNATURE
> Implementation is done using GnuPG MPI library
>
> config DIMLIB
> - bool "DIM library"
> - default y
> + bool
> help
> Dynamic Interrupt Moderation library.
> Implements an algorithm for dynamically change CQ modertion values
>
--
~Randy
^ permalink raw reply
* [PATCH] lib: dimlib: fix help text typos
From: Randy Dunlap @ 2019-09-26 0:20 UTC (permalink / raw)
To: netdev@vger.kernel.org, David Miller
Cc: Uwe Kleine-König, Tal Gilboa, Saeed Mahameed
From: Randy Dunlap <rdunlap@infradead.org>
Fix help text typos for DIMLIB.
Fixes: 4f75da3666c0 ("linux/dim: Move implementation to .c files")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
Cc: Tal Gilboa <talgi@mellanox.com>
Cc: Saeed Mahameed <saeedm@mellanox.com>
---
Applies after Uwe's patch v2: dimlib: make DIMLIB a hidden symbol
lib/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- lnx-53.orig/lib/Kconfig
+++ lnx-53/lib/Kconfig
@@ -558,7 +558,7 @@ config DIMLIB
bool
help
Dynamic Interrupt Moderation library.
- Implements an algorithm for dynamically change CQ modertion values
+ Implements an algorithm for dynamically changing CQ moderation values
according to run time performance.
#
^ permalink raw reply
* RE: [PATCH V2 6/8] mdev: introduce virtio device and its device ops
From: Tian, Kevin @ 2019-09-26 0:48 UTC (permalink / raw)
To: Jason Wang, kvm@vger.kernel.org, linux-s390@vger.kernel.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
intel-gfx@lists.freedesktop.org,
intel-gvt-dev@lists.freedesktop.org, kwankhede@nvidia.com,
alex.williamson@redhat.com, mst@redhat.com, Bie, Tiwei
Cc: christophe.de.dinechin@gmail.com, sebott@linux.ibm.com,
airlied@linux.ie, joonas.lahtinen@linux.intel.com,
heiko.carstens@de.ibm.com,
virtualization@lists.linux-foundation.org,
rob.miller@broadcom.com, lulu@redhat.com, eperezma@redhat.com,
pasic@linux.ibm.com, borntraeger@de.ibm.com,
haotian.wang@sifive.com, Wang, Zhi A, farman@linux.ibm.com,
idos@mellanox.com, gor@linux.ibm.com, Liang, Cunming,
zhenyuw@linux.intel.com, Vivi, Rodrigo, Wang, Xiao W,
freude@linux.ibm.com, jani.nikula@linux.intel.com,
parav@mellanox.com, Wang, Zhihong, akrowiak@linux.ibm.com,
netdev@vger.kernel.org, cohuck@redhat.com, oberpar@linux.ibm.com,
maxime.coquelin@redhat.com, daniel@ffwll.ch, Zhu, Lingshan
In-Reply-To: <2210d23d-38e4-e654-e53d-7867348de86a@redhat.com>
> From: Jason Wang
> Sent: Wednesday, September 25, 2019 8:45 PM
>
>
> On 2019/9/25 下午5:09, Tian, Kevin wrote:
> >> From: Jason Wang [mailto:jasowang@redhat.com]
> >> Sent: Tuesday, September 24, 2019 9:54 PM
> >>
> >> This patch implements basic support for mdev driver that supports
> >> virtio transport for kernel virtio driver.
> >>
> >> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >> ---
> >> include/linux/mdev.h | 2 +
> >> include/linux/virtio_mdev.h | 145
> >> ++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 147 insertions(+)
> >> create mode 100644 include/linux/virtio_mdev.h
> >>
> >> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> >> index 3414307311f1..73ac27b3b868 100644
> >> --- a/include/linux/mdev.h
> >> +++ b/include/linux/mdev.h
> >> @@ -126,6 +126,8 @@ struct mdev_device *mdev_from_dev(struct
> device
> >> *dev);
> >>
> >> enum {
> >> MDEV_ID_VFIO = 1,
> >> + MDEV_ID_VIRTIO = 2,
> >> + MDEV_ID_VHOST = 3,
> >> /* New entries must be added here */
> >> };
> >>
> >> diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h
> >> new file mode 100644
> >> index 000000000000..d1a40a739266
> >> --- /dev/null
> >> +++ b/include/linux/virtio_mdev.h
> >> @@ -0,0 +1,145 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-only */
> >> +/*
> >> + * Virtio mediated device driver
> >> + *
> >> + * Copyright 2019, Red Hat Corp.
> >> + * Author: Jason Wang <jasowang@redhat.com>
> >> + */
> >> +#ifndef _LINUX_VIRTIO_MDEV_H
> >> +#define _LINUX_VIRTIO_MDEV_H
> >> +
> >> +#include <linux/interrupt.h>
> >> +#include <linux/mdev.h>
> >> +#include <uapi/linux/vhost.h>
> >> +
> >> +#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-
> mdev"
> >> +#define VIRTIO_MDEV_VERSION 0x1
> > Just be curious. is this version identical to virtio spec version that below
> > callbacks are created for, or just irrelevant?
>
>
> It could be a hint but basically it's a way for userspace driver
> compatibility. For kernel we don't need this.
>
>
> >
> >> +
> >> +struct virtio_mdev_callback {
> >> + irqreturn_t (*callback)(void *data);
> >> + void *private;
> >> +};
> >> +
> >> +/**
> >> + * struct vfio_mdev_device_ops - Structure to be registered for each
> >> + * mdev device to register the device to virtio-mdev module.
> >> + *
> >> + * @set_vq_address: Set the address of virtqueue
> >> + * @mdev: mediated device
> >> + * @idx: virtqueue index
> >> + * @desc_area: address of desc area
> >> + * @driver_area: address of driver area
> >> + * @device_area: address of device area
> >> + * Returns integer: success (0) or error (< 0)
> >> + * @set_vq_num: Set the size of virtqueue
> >> + * @mdev: mediated device
> >> + * @idx: virtqueue index
> >> + * @num: the size of virtqueue
> >> + * @kick_vq: Kick the virtqueue
> >> + * @mdev: mediated device
> >> + * @idx: virtqueue index
> >> + * @set_vq_cb: Set the interrut calback function for
> >> + * a virtqueue
> >> + * @mdev: mediated device
> >> + * @idx: virtqueue index
> >> + * @cb: virtio-mdev interrupt callback
> >> structure
> >> + * @set_vq_ready: Set ready status for a virtqueue
> >> + * @mdev: mediated device
> >> + * @idx: virtqueue index
> >> + * @ready: ready (true) not ready(false)
> >> + * @get_vq_ready: Get ready status for a virtqueue
> >> + * @mdev: mediated device
> >> + * @idx: virtqueue index
> >> + * Returns boolean: ready (true) or not (false)
> >> + * @set_vq_state: Set the state for a virtqueue
> >> + * @mdev: mediated device
> >> + * @idx: virtqueue index
> >> + * @state: virtqueue state (last_avail_idx)
> >> + * Returns integer: success (0) or error (< 0)
> >> + * @get_vq_state: Get the state for a virtqueue
> >> + * @mdev: mediated device
> >> + * @idx: virtqueue index
> >> + * Returns virtqueue state (last_avail_idx)
> >> + * @get_vq_align: Get the virtqueue align requirement
> >> + * for the device
> >> + * @mdev: mediated device
> >> + * Returns virtqueue algin requirement
> >> + * @get_features: Get virtio features supported by the device
> >> + * @mdev: mediated device
> >> + * Returns the features support by the
> >> + * device
> >> + * @get_features: Set virtio features supported by the driver
> >> + * @mdev: mediated device
> >> + * @features: feature support by the driver
> >> + * Returns integer: success (0) or error (< 0)
> >> + * @set_config_cb: Set the config interrupt callback
> >> + * @mdev: mediated device
> >> + * @cb: virtio-mdev interrupt callback
> >> structure
> >> + * @get_device_id: Get virtio device id
> >> + * @mdev: mediated device
> >> + * Returns u32: virtio device id
> >> + * @get_vendor_id: Get virtio vendor id
> >> + * @mdev: mediated device
> >> + * Returns u32: virtio vendor id
> >> + * @get_status: Get the device status
> >> + * @mdev: mediated device
> >> + * Returns u8: virtio device status
> >> + * @set_status: Set the device status
> >> + * @mdev: mediated device
> >> + * @status: virtio device status
> >> + * @get_config: Read from device specific confiugration
> >> space
> > configuration (and similar typos downward)
>
>
> Let me fix.
>
>
> >
> >> + * @mdev: mediated device
> >> + * @offset: offset from the beginning of
> >> + * configuration space
> >> + * @buf: buffer used to read to
> >> + * @len: the length to read from
> >> + * configration space
> >> + * @set_config: Write to device specific confiugration space
> >> + * @mdev: mediated device
> >> + * @offset: offset from the beginning of
> >> + * configuration space
> >> + * @buf: buffer used to write from
> >> + * @len: the length to write to
> >> + * configration space
> >> + * @get_version: Get the version of virtio mdev device
> >> + * @mdev: mediated device
> >> + * Returns integer: version of the device
> >> + * @get_generation: Get device generaton
> >> + * @mdev: mediated device
> >> + * Returns u32: device generation
> >> + */
> >> +struct virtio_mdev_device_ops {
> >> + /* Virtqueue ops */
> >> + int (*set_vq_address)(struct mdev_device *mdev,
> >> + u16 idx, u64 desc_area, u64 driver_area,
> >> + u64 device_area);
> >> + void (*set_vq_num)(struct mdev_device *mdev, u16 idx, u32 num);
> >> + void (*kick_vq)(struct mdev_device *mdev, u16 idx);
> >> + void (*set_vq_cb)(struct mdev_device *mdev, u16 idx,
> >> + struct virtio_mdev_callback *cb);
> >> + void (*set_vq_ready)(struct mdev_device *mdev, u16 idx, bool
> >> ready);
> >> + bool (*get_vq_ready)(struct mdev_device *mdev, u16 idx);
> >> + int (*set_vq_state)(struct mdev_device *mdev, u16 idx, u64 state);
> >> + u64 (*get_vq_state)(struct mdev_device *mdev, u16 idx);
> >> +
> >> + /* Device ops */
> >> + u16 (*get_vq_align)(struct mdev_device *mdev);
> >> + u64 (*get_features)(struct mdev_device *mdev);
> >> + int (*set_features)(struct mdev_device *mdev, u64 features);
> >> + void (*set_config_cb)(struct mdev_device *mdev,
> >> + struct virtio_mdev_callback *cb);
> >> + u16 (*get_queue_max)(struct mdev_device *mdev);
> >> + u32 (*get_device_id)(struct mdev_device *mdev);
> >> + u32 (*get_vendor_id)(struct mdev_device *mdev);
> >> + u8 (*get_status)(struct mdev_device *mdev);
> >> + void (*set_status)(struct mdev_device *mdev, u8 status);
> >> + void (*get_config)(struct mdev_device *mdev, unsigned int offset,
> >> + void *buf, unsigned int len);
> >> + void (*set_config)(struct mdev_device *mdev, unsigned int offset,
> >> + const void *buf, unsigned int len);
> >> + int (*get_version)(struct mdev_device *mdev);
> >> + u32 (*get_generation)(struct mdev_device *mdev);
> >> +};
> > I'm not sure how stable above ops are.
>
>
> It's the kernel internal API, so there's no strict requirement for this.
> We will export a version value for userspace for compatibility.
>
>
> > Does it make sense if defining
> > just two callbacks here, e.g. vq_ctrl and device_ctrl, and then let the
> > vendor driver to handle specific ops in each category (similar to how
> > ioctl works)?
>
>
> My understanding is that it introduce another indirection, you still
> need to differ from different command, and it's less flexible than
> direct callback.
>
> What's the value of doing this?
>
I just thought doing so may provide better compatibility to the
parent driver. Even when new op is introduced, a parent driver
that was developed against the old set can still be loaded in the
new kernel. It just returns error when unrecognized ops are
routed through vq_ctrl and device_ctrl, if the userspace doesn't
favor the exposed version value. But if above ops set is pretty
stable, then this comment can be ignored.
Thanks
Kevin
^ 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