Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net/mlx4_en: Verify coalescing parameters are in range
From: Tariq Toukan @ 2018-05-09 15:35 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Moshe Shemesh, Tariq Toukan

From: Moshe Shemesh <moshe@mellanox.com>

Add check of coalescing parameters received through ethtool are within
range of values supported by the HW.
Driver gets the coalescing rx/tx-usecs and rx/tx-frames as set by the
users through ethtool. The ethtool support up to 32 bit value for each.
However, mlx4 modify cq limits the coalescing time parameter and
coalescing frames parameters to 16 bits.
Return out of range error if user tries to set these parameters to
higher values.
Change type of sample-interval and adaptive_rx_coal parameters in mlx4
driver to u32 as the ethtool holds them as u32 and these parameters are
not limited due to mlx4 HW.

Fixes: c27a02cd94d6 ('mlx4_en: Add driver for Mellanox ConnectX 10GbE NIC')
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 16 ++++++++++++++++
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h    |  7 +++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index a30a2e95d13f..f11b45001cad 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -1027,6 +1027,22 @@ static int mlx4_en_set_coalesce(struct net_device *dev,
 	if (!coal->tx_max_coalesced_frames_irq)
 		return -EINVAL;
 
+	if (coal->tx_coalesce_usecs > MLX4_EN_MAX_COAL_TIME ||
+	    coal->rx_coalesce_usecs > MLX4_EN_MAX_COAL_TIME ||
+	    coal->rx_coalesce_usecs_low > MLX4_EN_MAX_COAL_TIME ||
+	    coal->rx_coalesce_usecs_high > MLX4_EN_MAX_COAL_TIME) {
+		netdev_info(dev, "%s: maximum coalesce time supported is %d usecs\n",
+			    __func__, MLX4_EN_MAX_COAL_TIME);
+		return -ERANGE;
+	}
+
+	if (coal->tx_max_coalesced_frames > MLX4_EN_MAX_COAL_PKTS ||
+	    coal->rx_max_coalesced_frames > MLX4_EN_MAX_COAL_PKTS) {
+		netdev_info(dev, "%s: maximum coalesced frames supported is %d\n",
+			    __func__, MLX4_EN_MAX_COAL_PKTS);
+		return -ERANGE;
+	}
+
 	priv->rx_frames = (coal->rx_max_coalesced_frames ==
 			   MLX4_EN_AUTO_CONF) ?
 				MLX4_EN_RX_COAL_TARGET :
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index f7c81133594f..ace6545f82e6 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -132,6 +132,9 @@
 #define MLX4_EN_TX_COAL_PKTS	16
 #define MLX4_EN_TX_COAL_TIME	0x10
 
+#define MLX4_EN_MAX_COAL_PKTS	U16_MAX
+#define MLX4_EN_MAX_COAL_TIME	U16_MAX
+
 #define MLX4_EN_RX_RATE_LOW		400000
 #define MLX4_EN_RX_COAL_TIME_LOW	0
 #define MLX4_EN_RX_RATE_HIGH		450000
@@ -552,8 +555,8 @@ struct mlx4_en_priv {
 	u16 rx_usecs_low;
 	u32 pkt_rate_high;
 	u16 rx_usecs_high;
-	u16 sample_interval;
-	u16 adaptive_rx_coal;
+	u32 sample_interval;
+	u32 adaptive_rx_coal;
 	u32 msg_enable;
 	u32 loopback_ok;
 	u32 validate_loopback;
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH] selinux: add AF_UNSPEC and INADDR_ANY checks to selinux_socket_bind()
From: Paul Moore @ 2018-05-09 15:34 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Alexey Kodanev, Richard Haines, selinux, Eric Paris,
	linux-security-module, netdev
In-Reply-To: <d637da77-2789-e08b-c78b-b48821cbf0fe@tycho.nsa.gov>

On Wed, May 9, 2018 at 11:11 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 05/09/2018 11:01 AM, Paul Moore wrote:
>> On Wed, May 9, 2018 at 8:37 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>> On 05/08/2018 08:25 PM, Paul Moore wrote:
>>>> On Tue, May 8, 2018 at 2:40 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>>>> On 05/08/2018 01:05 PM, Paul Moore wrote:
>>>>>> On Tue, May 8, 2018 at 10:05 AM, Alexey Kodanev
>>>>>> <alexey.kodanev@oracle.com> wrote:
>>>>>>> Commit d452930fd3b9 ("selinux: Add SCTP support") breaks compatibility
>>>>>>> with the old programs that can pass sockaddr_in with AF_UNSPEC and
>>>>>>> INADDR_ANY to bind(). As a result, bind() returns EAFNOSUPPORT error.
>>>>>>> It was found with LTP/asapi_01 test.
>>>>>>>
>>>>>>> Similar to commit 29c486df6a20 ("net: ipv4: relax AF_INET check in
>>>>>>> bind()"), which relaxed AF_INET check for compatibility, add AF_UNSPEC
>>>>>>> case to AF_INET and make sure that the address is INADDR_ANY.
>>>>>>>
>>>>>>> Also, in the end of selinux_socket_bind(), instead of adding AF_UNSPEC
>>>>>>> to 'address->sa_family == AF_INET', verify AF_INET6 first.
>>>>>>>
>>>>>>> Fixes: d452930fd3b9 ("selinux: Add SCTP support")
>>>>>>> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
>>>>>>> ---
>>>>>>>  security/selinux/hooks.c | 12 +++++++++---
>>>>>>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> Thanks for finding and reporting this regression.
>>>>>>
>>>>>> I think I would prefer to avoid having to duplicate the
>>>>>> AF_UNSPEC/INADDR_ANY checking logic in the SELinux hook, even though
>>>>>> it is a small bit of code and unlikely to change.  I'm wondering if it
>>>>>> would be better to check both the socket and sockaddr address family
>>>>>> in the main if conditional inside selinux_socket_bind(), what do you
>>>>>> think?  Another option would be to go back to just checking the
>>>>>> soackaddr address family; we moved away from that for a reason which
>>>>>> escapes at the moment (code cleanliness?), but perhaps that was a
>>>>>> mistake.
>>>>>
>>>>> We've always used the sk->sk_family there; it was only the recent code from Richard that started
>>>>> using the socket address family.
>>>>
>>>> Yes I know, I thought I was the one that suggested it at some point
>>>> (I'll take the blame) ... although now that I'm looking at the git
>>>> log, maybe I'm confusing it with something else.
>>>>
>>>>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>>>>> index 4cafe6a19167..a3789b167667 100644
>>>>>> --- a/security/selinux/hooks.c
>>>>>> +++ b/security/selinux/hooks.c
>>>>>> @@ -4577,6 +4577,7 @@ static int selinux_socket_bind(struct socket *sock, struc>
>>>>>> {
>>>>>>        struct sock *sk = sock->sk;
>>>>>>        u16 family;
>>>>>> +       u16 family_sa;
>>>>>>        int err;
>>>>>>
>>>>>>        err = sock_has_perm(sk, SOCKET__BIND);
>>>>>> @@ -4585,7 +4586,9 @@ static int selinux_socket_bind(struct socket *sock, struc>
>>>>>>
>>>>>>        /* If PF_INET or PF_INET6, check name_bind permission for the port. */
>>>>>>        family = sk->sk_family;
>>>>>> -       if (family == PF_INET || family == PF_INET6) {
>>>>>> +       family_sa = address->sa_family;
>>>>>> +       if ((family == PF_INET || family == PF_INET6) &&
>>>>>> +           (family_sa == PF_INET || family_sa == PF_INET6)) {
>>>>>
>>>>> Wouldn't this allow bypassing the name_bind permission check by passing in AF_UNSPEC?
>>>>
>>>> I believe these name_bind permission checkis skipped for AF_UNSPEC
>>>> already, isn't it?  The only way the name_bind check would be
>>>> triggered is if the source port, snum, was non-zero and I didn't think
>>>> that was really legal for AF_UNSPEC/INADDR_ANY, is it?
>>>
>>> 1) What in inet_bind() prevents that from occurring?
>>> 2) Regardless, what about the node_bind check?
>>
>> Fair enough.  As mentioned above, perhaps the right fix is to move the
>> address family checking back to how it was pre-SCTP.
>
> It isn't clear to me how to do that without breaking SCTP multiple address binding, which is why
> Richard had to switch to checking address->sa_family instead of just using the sk->sk_family.
> Alexey's original fix might be the simplest solution.

I'm going to have to apologize, I'm traveling at the moment and more
distracted than usual as a result.  Let me take a closer look later
today.  It may be that Alexey's original fix the only practical
solution, but I really would like to avoid having to duplicate checks
like that in the SELinux code.

>> Alexey, is this something you want to do, or should we take care of that?
>>
>>>>>>                char *addrp;
>>>>>>                struct sk_security_struct *sksec = sk->sk_security;
>>>>>>                struct common_audit_data ad;
>>>>>> @@ -4601,7 +4604,7 @@ static int selinux_socket_bind(struct socket *sock, struc>
>>>>>>                 * need to check address->sa_family as it is possible to have
>>>>>>                 * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
>>>>>>                 */
>>>>>> -               switch (address->sa_family) {
>>>>>> +               switch (family_sa) {
>>>>>>                case AF_INET:
>>>>>>                        if (addrlen < sizeof(struct sockaddr_in))
>>>>>>                                return -EINVAL;
>>>>>>
>>>>>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>>>>>> index 4cafe6a..649a3be 100644
>>>>>>> --- a/security/selinux/hooks.c
>>>>>>> +++ b/security/selinux/hooks.c
>>>>>>> @@ -4602,10 +4602,16 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>>>>>>>                  * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
>>>>>>>                  */
>>>>>>>                 switch (address->sa_family) {
>>>>>>> +               case AF_UNSPEC:
>>>>>>>                 case AF_INET:
>>>>>>>                         if (addrlen < sizeof(struct sockaddr_in))
>>>>>>>                                 return -EINVAL;
>>>>>>>                         addr4 = (struct sockaddr_in *)address;
>>>>>>> +
>>>>>>> +                       if (address->sa_family == AF_UNSPEC &&
>>>>>>> +                           addr4->sin_addr.s_addr != htonl(INADDR_ANY))
>>>>>>> +                               return -EAFNOSUPPORT;
>>>>>>> +
>>>>>>>                         snum = ntohs(addr4->sin_port);
>>>>>>>                         addrp = (char *)&addr4->sin_addr.s_addr;
>>>>>>>                         break;
>>>>>>> @@ -4681,10 +4687,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>>>>>>>                 ad.u.net->sport = htons(snum);
>>>>>>>                 ad.u.net->family = family;
>>>>>>>
>>>>>>> -               if (address->sa_family == AF_INET)
>>>>>>> -                       ad.u.net->v4info.saddr = addr4->sin_addr.s_addr;
>>>>>>> -               else
>>>>>>> +               if (address->sa_family == AF_INET6)
>>>>>>>                         ad.u.net->v6info.saddr = addr6->sin6_addr;
>>>>>>> +               else
>>>>>>> +                       ad.u.net->v4info.saddr = addr4->sin_addr.s_addr;
>>>>>>>
>>>>>>>                 err = avc_has_perm(&selinux_state,
>>>>>>>                                    sksec->sid, sid,
>>>>>>> --
>>>>>>> 1.8.3.1
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>



-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: linux-next: Tree for May 9 (mlx5)
From: Randy Dunlap @ 2018-05-09 15:31 UTC (permalink / raw)
  To: Stephen Rothwell, Linux-Next Mailing List
  Cc: Linux Kernel Mailing List, netdev@vger.kernel.org, Saeed Mahameed,
	Matan Barak, Leon Romanovsky
In-Reply-To: <20180509212133.41a59f2f@canb.auug.org.au>

On 05/09/2018 04:21 AM, Stephen Rothwell wrote:
> Hi all,
> 
> Changes since 20180508:
> 

on x86_64:
# CONFIG_SMP is not set

In file included from ../drivers/net/ethernet/mellanox/mlx5/core/main.c:43:0:
../include/linux/mlx5/driver.h: In function 'mlx5_get_vector_affinity_hint':
../include/linux/mlx5/driver.h:1299:13: error: 'struct irq_desc' has no member named 'affinity_hint'
  return desc->affinity_hint;
             ^


-- 
~Randy

^ permalink raw reply

* [PATCH net-next 3/3] net/mlx4_core: Use msi_x module param to limit num of MSI-X irqs
From: Tariq Toukan @ 2018-05-09 15:29 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Eran Ben Elisha, Tariq Toukan, Ajaykumar Hotchandani
In-Reply-To: <1525879744-1858-1-git-send-email-tariqt@mellanox.com>

Extend the boolean interpretation of msi_x module parameter
to numerical, as follows:

0   - Don't use MSI-X.
1   - Use MSI-X, driver decides the num of MSI-X irqs.
>=2 - Use MSI-X, limit number of MSI-X irqs to msi_x.
      In SRIOV, this limits the number of MSI-X irqs per VF.

Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Cc: Ajaykumar Hotchandani <ajaykumar.hotchandani@oracle.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index b6aaf34d6648..80a75c80a463 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -73,7 +73,7 @@
 
 static int msi_x = 1;
 module_param(msi_x, int, 0444);
-MODULE_PARM_DESC(msi_x, "attempt to use MSI-X if nonzero");
+MODULE_PARM_DESC(msi_x, "0 - don't use MSI-X, 1 - use MSI-X, >1 - limit number of MSI-X irqs to msi_x");
 
 #else /* CONFIG_PCI_MSI */
 
@@ -2815,6 +2815,9 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
 				dev->caps.num_eqs - dev->caps.reserved_eqs,
 				MAX_MSIX);
 
+		if (msi_x > 1)
+			nreq = min_t(int, nreq, msi_x);
+
 		entries = kcalloc(nreq, sizeof(*entries), GFP_KERNEL);
 		if (!entries)
 			goto no_msi;
@@ -4182,6 +4185,11 @@ static int mlx4_resume(struct pci_dev *pdev)
 
 static int __init mlx4_verify_params(void)
 {
+	if (msi_x < 0) {
+		pr_warn("mlx4_core: bad msi_x: %d\n", msi_x);
+		return -1;
+	}
+
 	if ((log_num_mac < 0) || (log_num_mac > 7)) {
 		pr_warn("mlx4_core: bad num_mac: %d\n", log_num_mac);
 		return -1;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 0/3] mlx4_core misc for 4.18
From: Tariq Toukan @ 2018-05-09 15:29 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Tariq Toukan

Hi Dave,

This patchset contains misc enhancements from the team
to the mlx4 Core driver.

Patch 1 by Eran adds driver version report in FW.
Patch 2 by Yishai implements suspend/resume PCI callbacks.
Patch 3 extends the range of an existing module param from boolean to numerical.

Series generated against net-next commit:
53a7bdfb2a27 dt-bindings: dsa: Remove unnecessary #address/#size-cells

Thanks,
Tariq.


Eran Ben Elisha (1):
  net/mlx4_core: Report driver version to FW

Tariq Toukan (1):
  net/mlx4_core: Use msi_x module param to limit num of MSI-X irqs

Yishai Hadas (1):
  net/mlx4_core: Add PCI calls for suspend/resume

 drivers/net/ethernet/mellanox/mlx4/fw.c   | 12 +++++++
 drivers/net/ethernet/mellanox/mlx4/main.c | 56 ++++++++++++++++++++++++++++++-
 drivers/net/ethernet/mellanox/mlx4/mlx4.h |  2 +-
 include/linux/mlx4/device.h               |  1 +
 4 files changed, 69 insertions(+), 2 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* [PATCH net-next 2/3] net/mlx4_core: Add PCI calls for suspend/resume
From: Tariq Toukan @ 2018-05-09 15:29 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Yishai Hadas, Tariq Toukan
In-Reply-To: <1525879744-1858-1-git-send-email-tariqt@mellanox.com>

From: Yishai Hadas <yishaih@mellanox.com>

Implement suspend/resume callbacks in struct pci_driver.

Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 46 +++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 211578ffc70d..b6aaf34d6648 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -4125,12 +4125,58 @@ static void mlx4_shutdown(struct pci_dev *pdev)
 	.resume		= mlx4_pci_resume,
 };
 
+static int mlx4_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	struct mlx4_dev_persistent *persist = pci_get_drvdata(pdev);
+	struct mlx4_dev	*dev = persist->dev;
+
+	mlx4_err(dev, "suspend was called\n");
+	mutex_lock(&persist->interface_state_mutex);
+	if (persist->interface_state & MLX4_INTERFACE_STATE_UP)
+		mlx4_unload_one(pdev);
+	mutex_unlock(&persist->interface_state_mutex);
+
+	return 0;
+}
+
+static int mlx4_resume(struct pci_dev *pdev)
+{
+	struct mlx4_dev_persistent *persist = pci_get_drvdata(pdev);
+	struct mlx4_dev	*dev = persist->dev;
+	struct mlx4_priv *priv = mlx4_priv(dev);
+	int nvfs[MLX4_MAX_PORTS + 1] = {0, 0, 0};
+	int total_vfs;
+	int ret = 0;
+
+	mlx4_err(dev, "resume was called\n");
+	total_vfs = dev->persist->num_vfs;
+	memcpy(nvfs, dev->persist->nvfs, sizeof(dev->persist->nvfs));
+
+	mutex_lock(&persist->interface_state_mutex);
+	if (!(persist->interface_state & MLX4_INTERFACE_STATE_UP)) {
+		ret = mlx4_load_one(pdev, priv->pci_dev_data, total_vfs,
+				    nvfs, priv, 1);
+		if (!ret) {
+			ret = restore_current_port_types(dev,
+					dev->persist->curr_port_type,
+					dev->persist->curr_port_poss_type);
+			if (ret)
+				mlx4_err(dev, "resume: could not restore original port types (%d)\n", ret);
+		}
+	}
+	mutex_unlock(&persist->interface_state_mutex);
+
+	return ret;
+}
+
 static struct pci_driver mlx4_driver = {
 	.name		= DRV_NAME,
 	.id_table	= mlx4_pci_table,
 	.probe		= mlx4_init_one,
 	.shutdown	= mlx4_shutdown,
 	.remove		= mlx4_remove_one,
+	.suspend	= mlx4_suspend,
+	.resume		= mlx4_resume,
 	.err_handler    = &mlx4_err_handler,
 };
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 1/3] net/mlx4_core: Report driver version to FW
From: Tariq Toukan @ 2018-05-09 15:29 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Tariq Toukan
In-Reply-To: <1525879744-1858-1-git-send-email-tariqt@mellanox.com>

From: Eran Ben Elisha <eranbe@mellanox.com>

If supported, write a driver version string to FW as part of the
INIT_HCA command.

Example of driver version: "Linux,mlx4_core,4.0-0"

Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/fw.c   | 12 ++++++++++++
 drivers/net/ethernet/mellanox/mlx4/mlx4.h |  2 +-
 include/linux/mlx4/device.h               |  1 +
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
index de6b3d416148..46dcbfbe4c5e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
@@ -165,6 +165,7 @@ static void dump_dev_cap_flags2(struct mlx4_dev *dev, u64 flags)
 		[36] = "QinQ VST mode support",
 		[37] = "sl to vl mapping table change event support",
 		[38] = "user MAC support",
+		[39] = "Report driver version to FW support",
 	};
 	int i;
 
@@ -1038,6 +1039,8 @@ int mlx4_QUERY_DEV_CAP(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
 		dev_cap->flags2 |= MLX4_DEV_CAP_FLAG2_ETH_BACKPL_AN_REP;
 	if (field32 & (1 << 7))
 		dev_cap->flags2 |= MLX4_DEV_CAP_FLAG2_RECOVERABLE_ERROR_EVENT;
+	if (field32 & (1 << 8))
+		dev_cap->flags2 |= MLX4_DEV_CAP_FLAG2_DRIVER_VERSION_TO_FW;
 	MLX4_GET(field32, outbox, QUERY_DEV_CAP_DIAG_RPRT_PER_PORT);
 	if (field32 & (1 << 17))
 		dev_cap->flags2 |= MLX4_DEV_CAP_FLAG2_DIAG_PER_PORT;
@@ -1860,6 +1863,8 @@ int mlx4_INIT_HCA(struct mlx4_dev *dev, struct mlx4_init_hca_param *param)
 #define  INIT_HCA_UC_STEERING_OFFSET	 (INIT_HCA_MCAST_OFFSET + 0x18)
 #define	 INIT_HCA_LOG_MC_TABLE_SZ_OFFSET (INIT_HCA_MCAST_OFFSET + 0x1b)
 #define  INIT_HCA_DEVICE_MANAGED_FLOW_STEERING_EN	0x6
+#define  INIT_HCA_DRIVER_VERSION_OFFSET   0x140
+#define  INIT_HCA_DRIVER_VERSION_SZ       0x40
 #define  INIT_HCA_FS_PARAM_OFFSET         0x1d0
 #define  INIT_HCA_FS_BASE_OFFSET          (INIT_HCA_FS_PARAM_OFFSET + 0x00)
 #define  INIT_HCA_FS_LOG_ENTRY_SZ_OFFSET  (INIT_HCA_FS_PARAM_OFFSET + 0x12)
@@ -1950,6 +1955,13 @@ int mlx4_INIT_HCA(struct mlx4_dev *dev, struct mlx4_init_hca_param *param)
 	if (dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_RECOVERABLE_ERROR_EVENT)
 		*(inbox + INIT_HCA_RECOVERABLE_ERROR_EVENT_OFFSET / 4) |= cpu_to_be32(1 << 31);
 
+	if (dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_DRIVER_VERSION_TO_FW) {
+		u8 *dst = (u8 *)(inbox + INIT_HCA_DRIVER_VERSION_OFFSET / 4);
+
+		strncpy(dst, DRV_NAME_FOR_FW, INIT_HCA_DRIVER_VERSION_SZ - 1);
+		mlx4_dbg(dev, "Reporting Driver Version to FW: %s\n", dst);
+	}
+
 	/* QPC/EEC/CQC/EQC/RDMARC attributes */
 
 	MLX4_PUT(inbox, param->qpc_base,      INIT_HCA_QPC_BASE_OFFSET);
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index c68da1986e51..cb9e923e8399 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -55,8 +55,8 @@
 #include "fw_qos.h"
 
 #define DRV_NAME	"mlx4_core"
-#define PFX		DRV_NAME ": "
 #define DRV_VERSION	"4.0-0"
+#define DRV_NAME_FOR_FW		"Linux," DRV_NAME "," DRV_VERSION
 
 #define MLX4_FS_UDP_UC_EN		(1 << 1)
 #define MLX4_FS_TCP_UC_EN		(1 << 2)
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 81d0799b6091..122e7e9d3091 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -225,6 +225,7 @@ enum {
 	MLX4_DEV_CAP_FLAG2_SVLAN_BY_QP          = 1ULL <<  36,
 	MLX4_DEV_CAP_FLAG2_SL_TO_VL_CHANGE_EVENT = 1ULL << 37,
 	MLX4_DEV_CAP_FLAG2_USER_MAC_EN		= 1ULL << 38,
+	MLX4_DEV_CAP_FLAG2_DRIVER_VERSION_TO_FW = 1ULL << 39,
 };
 
 enum {
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH ghak81 RFC V1 3/5] audit: use inline function to get audit context
From: Paul Moore @ 2018-05-09 15:28 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Linux-Audit Mailing List, LKML,
	Linux NetDev Upstream Mailing List, Netfilter Devel List,
	Linux Security Module list, Integrity Measurement Architecture,
	SElinux list, Eric Paris, Steve Grubb, Ingo Molnar, David Howells
In-Reply-To: <0e43c5135c197209b3189032d538244571e7443d.1525466167.git.rgb@redhat.com>

On Fri, May 4, 2018 at 4:54 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> Recognizing that the audit context is an internal audit value, use an
> access function to retrieve the audit context pointer for the task
> rather than reaching directly into the task struct to get it.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  include/linux/audit.h                | 16 ++++++++---
>  include/net/xfrm.h                   |  2 +-
>  kernel/audit.c                       |  4 +--
>  kernel/audit_watch.c                 |  2 +-
>  kernel/auditsc.c                     | 52 ++++++++++++++++++------------------
>  net/bridge/netfilter/ebtables.c      |  2 +-
>  net/core/dev.c                       |  2 +-
>  net/netfilter/x_tables.c             |  2 +-
>  net/netlabel/netlabel_user.c         |  2 +-
>  security/integrity/ima/ima_api.c     |  2 +-
>  security/integrity/integrity_audit.c |  2 +-
>  security/lsm_audit.c                 |  2 +-
>  security/selinux/hooks.c             |  4 +--
>  security/selinux/selinuxfs.c         |  6 ++---
>  security/selinux/ss/services.c       | 12 ++++-----
>  15 files changed, 60 insertions(+), 52 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 5f86f7c..93e4c61 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -235,26 +235,30 @@ extern void __audit_inode_child(struct inode *parent,
>  extern void __audit_seccomp(unsigned long syscall, long signr, int code);
>  extern void __audit_ptrace(struct task_struct *t);
>
> +static inline struct audit_context *audit_context(struct task_struct *task)
> +{
> +       return task->audit_context;
> +}

Another case where I think I agree with everything here on principle,
especially when one considers it in the larger context of the audit
container ID work.  However, I think we might be able to somply this a
bit by eliminating the parameter to the new audit_context() helper and
making it always reference the current task_struct.  Based on this
patch it would appear that this change would work for all callers
except for audit_take_context() and __audit_syscall_entry(), both of
which are contained within the core audit code and are enough of a
special case that I think it is acceptable for them to access the
context directly.  I'm trying to think of reasons why a non-audit
kernel subsystem would ever need to access the audit context of a
process other than current and I can't think of any ... removing the
task_struct pointer might help prevent mistakes/abuse in the future.

> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 6e3ceb9..a4bbdcc 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -836,7 +836,7 @@ static inline struct audit_context *audit_take_context(struct task_struct *tsk,
>                                                       int return_valid,
>                                                       long return_code)
>  {
> -       struct audit_context *context = tsk->audit_context;
> +       struct audit_context *context = audit_context(tsk);
>
>         if (!context)
>                 return NULL;
> @@ -1510,7 +1510,7 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
>                            unsigned long a3, unsigned long a4)
>  {
>         struct task_struct *tsk = current;
> -       struct audit_context *context = tsk->audit_context;
> +       struct audit_context *context = audit_context(tsk);
>         enum audit_state     state;
>
>         if (!audit_enabled || !context)

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH net-next v3 0/2] socket statistics for ss
From: Stephen Hemminger @ 2018-05-09 15:22 UTC (permalink / raw)
  To: davem, gerrit, kuznet, yoshfuji; +Cc: netdev, dccp, Stephen Hemminger
In-Reply-To: <20180507184333.32688-1-sthemmin@microsoft.com>

On Mon,  7 May 2018 11:43:31 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> The eventual goal is to remove all accesses to slab statistics
> by iproute2 ss command. This are preliminary steps to add two
> statistics which are not working now, because of slab merging
> and changes that were done years ago to allow for common inet_hash
> tables.
> 
> Stephen Hemminger (2):
>   inet: add bound ports statistic
>   socket: keep track of the number of sockets allocated
> 
> v3
>   - fix build :-(
>   - add allocated sockets statistic
> 
>  include/net/inet_hashtables.h    |  3 +++
>  include/net/inet_timewait_sock.h |  2 ++
>  net/dccp/proto.c                 |  1 +
>  net/ipv4/inet_hashtables.c       | 22 +++++++++++++++++++---
>  net/ipv4/inet_timewait_sock.c    |  8 +++++---
>  net/ipv4/proc.c                  |  5 +++--
>  net/ipv4/tcp.c                   |  1 +
>  net/socket.c                     | 21 +++++++++++++++++++--
>  8 files changed, 53 insertions(+), 10 deletions(-)
> 

I am not sure if these patches are worth applying.
The 'ss -s' command has had missing values since 2.4 kernel.
And the first complaints came in only this year.

Another alternative would be just to remove these fields from ss -s
output and move on.

^ permalink raw reply

* Re: [net-next 0/6][pull request] 100GbE Intel Wired LAN Driver Updates 2018-05-07
From: Jeff Kirsher @ 2018-05-09 15:19 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20180508.001045.1674673870907537309.davem@davemloft.net>

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]

On Tue, 2018-05-08 at 00:10 -0400, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Mon,  7 May 2018 07:45:15 -0700
> 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 
> > 100GbE
> 
> Hmmm...
> 
> [davem@localhost net-next]$ git pull --no-ff
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue
> 100GbE
> From git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-
> queue
>  * branch                      100GbE     -> FETCH_HEAD
> Already up to date.
> [davem@localhost net-next]$

Sorry about that, forgot to push the patches up to my tree on
kernel.org, guess I was too pre-occupied with whether the Capitols were
going to beat the Penguins.

Since there is a need for a v2, it appears my failure to push the
patches ended up being a good thing.  Working on v2 now...

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH net] udp: fix SO_BINDTODEVICE
From: David Ahern @ 2018-05-09 15:18 UTC (permalink / raw)
  To: Paolo Abeni, netdev; +Cc: Damir Mansurov, David Miller
In-Reply-To: <9445dd5d149af16463df4d0502b2667ee2b6f4e8.1525862461.git.pabeni@redhat.com>

On 5/9/18 4:42 AM, Paolo Abeni wrote:
> Damir reported a breakage of SO_BINDTODEVICE for UDP sockets.
> In absence of VRF devices, after commit fb74c27735f0 ("net:
> ipv4: add second dif to udp socket lookups") the dif mismatch
> isn't fatal anymore for UDP socket lookup with non null
> sk_bound_dev_if, breaking SO_BINDTODEVICE semantics.
> 
> This changeset addresses the issue making the dif match mandatory
> again in the above scenario.
> 
> Reported-by: Damir Mansurov <dnman@oktetlabs.ru>
> Fixes: fb74c27735f0 ("net: ipv4: add second dif to udp socket lookups")
> Fixes: 1801b570dd2a ("net: ipv6: add second dif to udp socket lookups")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  net/ipv4/udp.c | 4 ++--
>  net/ipv6/udp.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 

Acked-by: David Ahern <dsahern@gmail.com>

^ permalink raw reply

* Re: [PATCH ghak81 RFC V1 2/5] audit: convert sessionid unset to a macro
From: Paul Moore @ 2018-05-09 15:18 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Linux-Audit Mailing List, LKML,
	Linux NetDev Upstream Mailing List, Netfilter Devel List,
	Linux Security Module list, Integrity Measurement Architecture,
	SElinux list, David Howells, Ingo Molnar
In-Reply-To: <20180509013415.sohoc2jbofdqqw5v@madcap2.tricolour.ca>

On Tue, May 8, 2018 at 9:34 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-05-04 16:54, Richard Guy Briggs wrote:
>> Use a macro, "AUDIT_SID_UNSET", to replace each instance of
>> initialization and comparison to an audit session ID.
>>
>> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>
> There's a minor issue with this patch, adding a header include to
> init/init_task.c in this patch and removing it from patch 5.  That'll be
> in the next revision.

Okay, thanks for the heads-up.  FWIW, this patch looks reasonable in
principle; changing magic numbers to macros/constants is almost always
a step in the right direction.

> I have dynamic allocation working, so that has a good chance of
> appearing  too.

I'll comment on that in your patch 0, I just want to get through the
rest of the patches first.

>> ---
>>  include/linux/audit.h      | 2 +-
>>  include/net/xfrm.h         | 2 +-
>>  include/uapi/linux/audit.h | 1 +
>>  init/init_task.c           | 2 +-
>>  kernel/auditsc.c           | 4 ++--
>>  5 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/audit.h b/include/linux/audit.h
>> index 75d5b03..5f86f7c 100644
>> --- a/include/linux/audit.h
>> +++ b/include/linux/audit.h
>> @@ -513,7 +513,7 @@ static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
>>  }
>>  static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
>>  {
>> -     return -1;
>> +     return AUDIT_SID_UNSET;
>>  }
>>  static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
>>  { }
>> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
>> index a872379..fcce8ee 100644
>> --- a/include/net/xfrm.h
>> +++ b/include/net/xfrm.h
>> @@ -751,7 +751,7 @@ static inline void xfrm_audit_helper_usrinfo(bool task_valid,
>>                                           audit_get_loginuid(current) :
>>                                           INVALID_UID);
>>       const unsigned int ses = task_valid ? audit_get_sessionid(current) :
>> -             (unsigned int) -1;
>> +             AUDIT_SID_UNSET;
>>
>>       audit_log_format(audit_buf, " auid=%u ses=%u", auid, ses);
>>       audit_log_task_context(audit_buf);
>> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
>> index 4e61a9e..04f9bd2 100644
>> --- a/include/uapi/linux/audit.h
>> +++ b/include/uapi/linux/audit.h
>> @@ -465,6 +465,7 @@ struct audit_tty_status {
>>  };
>>
>>  #define AUDIT_UID_UNSET (unsigned int)-1
>> +#define AUDIT_SID_UNSET ((unsigned int)-1)
>>
>>  /* audit_rule_data supports filter rules with both integer and string
>>   * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
>> diff --git a/init/init_task.c b/init/init_task.c
>> index 3ac6e75..c788f91 100644
>> --- a/init/init_task.c
>> +++ b/init/init_task.c
>> @@ -119,7 +119,7 @@ struct task_struct init_task
>>       .thread_node    = LIST_HEAD_INIT(init_signals.thread_head),
>>  #ifdef CONFIG_AUDITSYSCALL
>>       .loginuid       = INVALID_UID,
>> -     .sessionid      = (unsigned int)-1,
>> +     .sessionid      = AUDIT_SID_UNSET,
>>  #endif
>>  #ifdef CONFIG_PERF_EVENTS
>>       .perf_event_mutex = __MUTEX_INITIALIZER(init_task.perf_event_mutex),
>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> index f3817d0..6e3ceb9 100644
>> --- a/kernel/auditsc.c
>> +++ b/kernel/auditsc.c
>> @@ -2050,7 +2050,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
>>  int audit_set_loginuid(kuid_t loginuid)
>>  {
>>       struct task_struct *task = current;
>> -     unsigned int oldsessionid, sessionid = (unsigned int)-1;
>> +     unsigned int oldsessionid, sessionid = AUDIT_SID_UNSET;
>>       kuid_t oldloginuid;
>>       int rc;
>>
>> @@ -2064,7 +2064,7 @@ int audit_set_loginuid(kuid_t loginuid)
>>       /* are we setting or clearing? */
>>       if (uid_valid(loginuid)) {
>>               sessionid = (unsigned int)atomic_inc_return(&session_id);
>> -             if (unlikely(sessionid == (unsigned int)-1))
>> +             if (unlikely(sessionid == AUDIT_SID_UNSET))
>>                       sessionid = (unsigned int)atomic_inc_return(&session_id);
>>       }
>>
>> --
>> 1.8.3.1
>>
>> --
>> Linux-audit mailing list
>> Linux-audit@redhat.com
>> https://www.redhat.com/mailman/listinfo/linux-audit
>
> - RGB
>
> --
> Richard Guy Briggs <rgb@redhat.com>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> IRC: rgb, SunRaycer
> Voice: +1.647.777.2635, Internal: (81) 32635
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit



-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH ghak81 RFC V1 1/5] audit: normalize loginuid read access
From: Paul Moore @ 2018-05-09 15:13 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Linux-Audit Mailing List, LKML,
	Linux NetDev Upstream Mailing List, Netfilter Devel List,
	Linux Security Module list, Integrity Measurement Architecture,
	SElinux list, Eric Paris, Steve Grubb, Ingo Molnar, David Howells
In-Reply-To: <611e9c85fca8bcdb24e6fb6da412773663c007b3.1525466167.git.rgb@redhat.com>

On Fri, May 4, 2018 at 4:54 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> Recognizing that the loginuid is an internal audit value, use an access
> function to retrieve the audit loginuid value for the task rather than
> reaching directly into the task struct to get it.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/auditsc.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 479c031..f3817d0 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -374,7 +374,7 @@ static int audit_field_compare(struct task_struct *tsk,
>         case AUDIT_COMPARE_EGID_TO_OBJ_GID:
>                 return audit_compare_gid(cred->egid, name, f, ctx);
>         case AUDIT_COMPARE_AUID_TO_OBJ_UID:
> -               return audit_compare_uid(tsk->loginuid, name, f, ctx);
> +               return audit_compare_uid(audit_get_loginuid(tsk), name, f, ctx);
>         case AUDIT_COMPARE_SUID_TO_OBJ_UID:
>                 return audit_compare_uid(cred->suid, name, f, ctx);
>         case AUDIT_COMPARE_SGID_TO_OBJ_GID:
> @@ -385,7 +385,7 @@ static int audit_field_compare(struct task_struct *tsk,
>                 return audit_compare_gid(cred->fsgid, name, f, ctx);
>         /* uid comparisons */
>         case AUDIT_COMPARE_UID_TO_AUID:
> -               return audit_uid_comparator(cred->uid, f->op, tsk->loginuid);
> +               return audit_uid_comparator(cred->uid, f->op, audit_get_loginuid(tsk));
>         case AUDIT_COMPARE_UID_TO_EUID:
>                 return audit_uid_comparator(cred->uid, f->op, cred->euid);
>         case AUDIT_COMPARE_UID_TO_SUID:
> @@ -394,11 +394,11 @@ static int audit_field_compare(struct task_struct *tsk,
>                 return audit_uid_comparator(cred->uid, f->op, cred->fsuid);
>         /* auid comparisons */
>         case AUDIT_COMPARE_AUID_TO_EUID:
> -               return audit_uid_comparator(tsk->loginuid, f->op, cred->euid);
> +               return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->euid);
>         case AUDIT_COMPARE_AUID_TO_SUID:
> -               return audit_uid_comparator(tsk->loginuid, f->op, cred->suid);
> +               return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->suid);
>         case AUDIT_COMPARE_AUID_TO_FSUID:
> -               return audit_uid_comparator(tsk->loginuid, f->op, cred->fsuid);
> +               return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->fsuid);
>         /* euid comparisons */
>         case AUDIT_COMPARE_EUID_TO_SUID:
>                 return audit_uid_comparator(cred->euid, f->op, cred->suid);
> @@ -611,7 +611,7 @@ static int audit_filter_rules(struct task_struct *tsk,
>                                 result = match_tree_refs(ctx, rule->tree);
>                         break;
>                 case AUDIT_LOGINUID:
> -                       result = audit_uid_comparator(tsk->loginuid, f->op, f->uid);
> +                       result = audit_uid_comparator(audit_get_loginuid(tsk), f->op, f->uid);
>                         break;
>                 case AUDIT_LOGINUID_SET:
>                         result = audit_comparator(audit_loginuid_set(tsk), f->op, f->val);
> @@ -2287,8 +2287,8 @@ int audit_signal_info(int sig, struct task_struct *t)
>             (sig == SIGTERM || sig == SIGHUP ||
>              sig == SIGUSR1 || sig == SIGUSR2)) {
>                 audit_sig_pid = task_tgid_nr(tsk);
> -               if (uid_valid(tsk->loginuid))
> -                       audit_sig_uid = tsk->loginuid;
> +               if (uid_valid(audit_get_loginuid(tsk)))
> +                       audit_sig_uid = audit_get_loginuid(tsk);

I realize this comment is a little silly given the nature of loginuid,
but if we are going to abstract away loginuid accesses (which I think
is good), we should probably access it once, store it in a local
variable, perform the validity check on the local variable, then
commit the local variable to audit_sig_uid.  I realize a TOCTOU
problem is unlikely here, but with this new layer of abstraction it
seems that some additional safety might be a good thing.

>                 else
>                         audit_sig_uid = uid;
>                 security_task_getsecid(tsk, &audit_sig_sid);
> --
> 1.8.3.1

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH v6 12/13] Documentation: remove stale firmware API reference
From: Mauro Carvalho Chehab @ 2018-05-09 15:12 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: gregkh, akpm, keescook, josh, maco, andy.gross, david.brown,
	bjorn.andersson, teg, wagi, hdegoede, andresx7, zohar, kubakici,
	shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt, oneukum,
	cantabile.desu, ast, hare, jejb, martin.petersen, khc
In-Reply-To: <20180508181247.19431-13-mcgrof@kernel.org>

Em Tue,  8 May 2018 11:12:46 -0700
"Luis R. Rodriguez" <mcgrof@kernel.org> escreveu:

> It refers to a pending patch, but this was merged eons ago.

Didn't know that such patch was already merged. Great!

Regards,
Mauro

> 
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
>  Documentation/dell_rbu.txt | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/Documentation/dell_rbu.txt b/Documentation/dell_rbu.txt
> index 0fdb6aa2704c..077fdc29a0d0 100644
> --- a/Documentation/dell_rbu.txt
> +++ b/Documentation/dell_rbu.txt
> @@ -121,9 +121,6 @@ read back the image downloaded.
>  
>  .. note::
>  
> -   This driver requires a patch for firmware_class.c which has the modified
> -   request_firmware_nowait function.
> -

>     Also after updating the BIOS image a user mode application needs to execute
>     code which sends the BIOS update request to the BIOS. So on the next reboot
>     the BIOS knows about the new image downloaded and it updates itself.

You should likely remove the "Also" here.

With that,

Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

Regards,
Mauro

^ permalink raw reply

* Re: [PATCH] selinux: add AF_UNSPEC and INADDR_ANY checks to selinux_socket_bind()
From: Stephen Smalley @ 2018-05-09 15:11 UTC (permalink / raw)
  To: Paul Moore
  Cc: Alexey Kodanev, Richard Haines, selinux, Eric Paris,
	linux-security-module, netdev
In-Reply-To: <CAHC9VhTNS-sEL2Az4hs1nmz7naqoyhiohzhPTrqp_M=_Ys1N6A@mail.gmail.com>

On 05/09/2018 11:01 AM, Paul Moore wrote:
> On Wed, May 9, 2018 at 8:37 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> On 05/08/2018 08:25 PM, Paul Moore wrote:
>>> On Tue, May 8, 2018 at 2:40 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>>> On 05/08/2018 01:05 PM, Paul Moore wrote:
>>>>> On Tue, May 8, 2018 at 10:05 AM, Alexey Kodanev
>>>>> <alexey.kodanev@oracle.com> wrote:
>>>>>> Commit d452930fd3b9 ("selinux: Add SCTP support") breaks compatibility
>>>>>> with the old programs that can pass sockaddr_in with AF_UNSPEC and
>>>>>> INADDR_ANY to bind(). As a result, bind() returns EAFNOSUPPORT error.
>>>>>> It was found with LTP/asapi_01 test.
>>>>>>
>>>>>> Similar to commit 29c486df6a20 ("net: ipv4: relax AF_INET check in
>>>>>> bind()"), which relaxed AF_INET check for compatibility, add AF_UNSPEC
>>>>>> case to AF_INET and make sure that the address is INADDR_ANY.
>>>>>>
>>>>>> Also, in the end of selinux_socket_bind(), instead of adding AF_UNSPEC
>>>>>> to 'address->sa_family == AF_INET', verify AF_INET6 first.
>>>>>>
>>>>>> Fixes: d452930fd3b9 ("selinux: Add SCTP support")
>>>>>> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
>>>>>> ---
>>>>>>  security/selinux/hooks.c | 12 +++++++++---
>>>>>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>>>>
>>>>> Thanks for finding and reporting this regression.
>>>>>
>>>>> I think I would prefer to avoid having to duplicate the
>>>>> AF_UNSPEC/INADDR_ANY checking logic in the SELinux hook, even though
>>>>> it is a small bit of code and unlikely to change.  I'm wondering if it
>>>>> would be better to check both the socket and sockaddr address family
>>>>> in the main if conditional inside selinux_socket_bind(), what do you
>>>>> think?  Another option would be to go back to just checking the
>>>>> soackaddr address family; we moved away from that for a reason which
>>>>> escapes at the moment (code cleanliness?), but perhaps that was a
>>>>> mistake.
>>>>
>>>> We've always used the sk->sk_family there; it was only the recent code from Richard that started
>>>> using the socket address family.
>>>
>>> Yes I know, I thought I was the one that suggested it at some point
>>> (I'll take the blame) ... although now that I'm looking at the git
>>> log, maybe I'm confusing it with something else.
>>>
>>>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>>>> index 4cafe6a19167..a3789b167667 100644
>>>>> --- a/security/selinux/hooks.c
>>>>> +++ b/security/selinux/hooks.c
>>>>> @@ -4577,6 +4577,7 @@ static int selinux_socket_bind(struct socket *sock, struc>
>>>>> {
>>>>>        struct sock *sk = sock->sk;
>>>>>        u16 family;
>>>>> +       u16 family_sa;
>>>>>        int err;
>>>>>
>>>>>        err = sock_has_perm(sk, SOCKET__BIND);
>>>>> @@ -4585,7 +4586,9 @@ static int selinux_socket_bind(struct socket *sock, struc>
>>>>>
>>>>>        /* If PF_INET or PF_INET6, check name_bind permission for the port. */
>>>>>        family = sk->sk_family;
>>>>> -       if (family == PF_INET || family == PF_INET6) {
>>>>> +       family_sa = address->sa_family;
>>>>> +       if ((family == PF_INET || family == PF_INET6) &&
>>>>> +           (family_sa == PF_INET || family_sa == PF_INET6)) {
>>>>
>>>> Wouldn't this allow bypassing the name_bind permission check by passing in AF_UNSPEC?
>>>
>>> I believe these name_bind permission checkis skipped for AF_UNSPEC
>>> already, isn't it?  The only way the name_bind check would be
>>> triggered is if the source port, snum, was non-zero and I didn't think
>>> that was really legal for AF_UNSPEC/INADDR_ANY, is it?
>>
>> 1) What in inet_bind() prevents that from occurring?
>> 2) Regardless, what about the node_bind check?
> 
> Fair enough.  As mentioned above, perhaps the right fix is to move the
> address family checking back to how it was pre-SCTP.

It isn't clear to me how to do that without breaking SCTP multiple address binding, which is why
Richard had to switch to checking address->sa_family instead of just using the sk->sk_family.
Alexey's original fix might be the simplest solution.

> 
> Alexey, is this something you want to do, or should we take care of that?
> 
>>>>>                char *addrp;
>>>>>                struct sk_security_struct *sksec = sk->sk_security;
>>>>>                struct common_audit_data ad;
>>>>> @@ -4601,7 +4604,7 @@ static int selinux_socket_bind(struct socket *sock, struc>
>>>>>                 * need to check address->sa_family as it is possible to have
>>>>>                 * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
>>>>>                 */
>>>>> -               switch (address->sa_family) {
>>>>> +               switch (family_sa) {
>>>>>                case AF_INET:
>>>>>                        if (addrlen < sizeof(struct sockaddr_in))
>>>>>                                return -EINVAL;
>>>>>
>>>>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>>>>> index 4cafe6a..649a3be 100644
>>>>>> --- a/security/selinux/hooks.c
>>>>>> +++ b/security/selinux/hooks.c
>>>>>> @@ -4602,10 +4602,16 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>>>>>>                  * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
>>>>>>                  */
>>>>>>                 switch (address->sa_family) {
>>>>>> +               case AF_UNSPEC:
>>>>>>                 case AF_INET:
>>>>>>                         if (addrlen < sizeof(struct sockaddr_in))
>>>>>>                                 return -EINVAL;
>>>>>>                         addr4 = (struct sockaddr_in *)address;
>>>>>> +
>>>>>> +                       if (address->sa_family == AF_UNSPEC &&
>>>>>> +                           addr4->sin_addr.s_addr != htonl(INADDR_ANY))
>>>>>> +                               return -EAFNOSUPPORT;
>>>>>> +
>>>>>>                         snum = ntohs(addr4->sin_port);
>>>>>>                         addrp = (char *)&addr4->sin_addr.s_addr;
>>>>>>                         break;
>>>>>> @@ -4681,10 +4687,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>>>>>>                 ad.u.net->sport = htons(snum);
>>>>>>                 ad.u.net->family = family;
>>>>>>
>>>>>> -               if (address->sa_family == AF_INET)
>>>>>> -                       ad.u.net->v4info.saddr = addr4->sin_addr.s_addr;
>>>>>> -               else
>>>>>> +               if (address->sa_family == AF_INET6)
>>>>>>                         ad.u.net->v6info.saddr = addr6->sin6_addr;
>>>>>> +               else
>>>>>> +                       ad.u.net->v4info.saddr = addr4->sin_addr.s_addr;
>>>>>>
>>>>>>                 err = avc_has_perm(&selinux_state,
>>>>>>                                    sksec->sid, sid,
>>>>>> --
>>>>>> 1.8.3.1
>>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>
> 
> 
> 

^ permalink raw reply

* Re: [PATCH v2] hv_netvsc: Fix net device attach on older Windows hosts
From: Stephen Hemminger @ 2018-05-09 15:07 UTC (permalink / raw)
  To: Mohammed Gamal; +Cc: netdev, sthemmin, haiyangz, linux-kernel, devel, vkuznets
In-Reply-To: <1525853854-8277-1-git-send-email-mgamal@redhat.com>

On Wed,  9 May 2018 10:17:34 +0200
Mohammed Gamal <mgamal@redhat.com> wrote:

> On older windows hosts the net_device instance is returned to
> the caller of rndis_filter_device_add() without having the presence
> bit set first. This would cause any subsequent calls to network device
> operations (e.g. MTU change, channel change) to fail after the device
> is detached once, returning -ENODEV.
> 
> Instead of returning the device instabce, we take the exit path where
> we call netif_device_attach()
> 
> Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
> 
> Signed-off-by: Mohammed Gamal <mgamal@redhat.com>

The standard for patch submission is no blank lines between Fixes
and Signed-off-by to make it easier for bots.

> ---
>  drivers/net/hyperv/rndis_filter.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
> index 6b127be..e7ca5b5 100644
> --- a/drivers/net/hyperv/rndis_filter.c
> +++ b/drivers/net/hyperv/rndis_filter.c
> @@ -1288,7 +1288,7 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
>  		   rndis_device->link_state ? "down" : "up");
>  
>  	if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5)
> -		return net_device;
> +		goto out;
>  
>  	rndis_filter_query_link_speed(rndis_device, net_device);
>  


Reviewed-by: Stephen Hemminger <sthemmin@microsoft.com>

^ permalink raw reply

* Re: [PATCH] selinux: add AF_UNSPEC and INADDR_ANY checks to selinux_socket_bind()
From: Paul Moore @ 2018-05-09 15:01 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Alexey Kodanev, Richard Haines, selinux, Eric Paris,
	linux-security-module, netdev
In-Reply-To: <7fdbaf13-fea2-4a2c-213d-fa291db67081@tycho.nsa.gov>

On Wed, May 9, 2018 at 8:37 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 05/08/2018 08:25 PM, Paul Moore wrote:
>> On Tue, May 8, 2018 at 2:40 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>> On 05/08/2018 01:05 PM, Paul Moore wrote:
>>>> On Tue, May 8, 2018 at 10:05 AM, Alexey Kodanev
>>>> <alexey.kodanev@oracle.com> wrote:
>>>>> Commit d452930fd3b9 ("selinux: Add SCTP support") breaks compatibility
>>>>> with the old programs that can pass sockaddr_in with AF_UNSPEC and
>>>>> INADDR_ANY to bind(). As a result, bind() returns EAFNOSUPPORT error.
>>>>> It was found with LTP/asapi_01 test.
>>>>>
>>>>> Similar to commit 29c486df6a20 ("net: ipv4: relax AF_INET check in
>>>>> bind()"), which relaxed AF_INET check for compatibility, add AF_UNSPEC
>>>>> case to AF_INET and make sure that the address is INADDR_ANY.
>>>>>
>>>>> Also, in the end of selinux_socket_bind(), instead of adding AF_UNSPEC
>>>>> to 'address->sa_family == AF_INET', verify AF_INET6 first.
>>>>>
>>>>> Fixes: d452930fd3b9 ("selinux: Add SCTP support")
>>>>> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
>>>>> ---
>>>>>  security/selinux/hooks.c | 12 +++++++++---
>>>>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> Thanks for finding and reporting this regression.
>>>>
>>>> I think I would prefer to avoid having to duplicate the
>>>> AF_UNSPEC/INADDR_ANY checking logic in the SELinux hook, even though
>>>> it is a small bit of code and unlikely to change.  I'm wondering if it
>>>> would be better to check both the socket and sockaddr address family
>>>> in the main if conditional inside selinux_socket_bind(), what do you
>>>> think?  Another option would be to go back to just checking the
>>>> soackaddr address family; we moved away from that for a reason which
>>>> escapes at the moment (code cleanliness?), but perhaps that was a
>>>> mistake.
>>>
>>> We've always used the sk->sk_family there; it was only the recent code from Richard that started
>>> using the socket address family.
>>
>> Yes I know, I thought I was the one that suggested it at some point
>> (I'll take the blame) ... although now that I'm looking at the git
>> log, maybe I'm confusing it with something else.
>>
>>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>>> index 4cafe6a19167..a3789b167667 100644
>>>> --- a/security/selinux/hooks.c
>>>> +++ b/security/selinux/hooks.c
>>>> @@ -4577,6 +4577,7 @@ static int selinux_socket_bind(struct socket *sock, struc>
>>>> {
>>>>        struct sock *sk = sock->sk;
>>>>        u16 family;
>>>> +       u16 family_sa;
>>>>        int err;
>>>>
>>>>        err = sock_has_perm(sk, SOCKET__BIND);
>>>> @@ -4585,7 +4586,9 @@ static int selinux_socket_bind(struct socket *sock, struc>
>>>>
>>>>        /* If PF_INET or PF_INET6, check name_bind permission for the port. */
>>>>        family = sk->sk_family;
>>>> -       if (family == PF_INET || family == PF_INET6) {
>>>> +       family_sa = address->sa_family;
>>>> +       if ((family == PF_INET || family == PF_INET6) &&
>>>> +           (family_sa == PF_INET || family_sa == PF_INET6)) {
>>>
>>> Wouldn't this allow bypassing the name_bind permission check by passing in AF_UNSPEC?
>>
>> I believe these name_bind permission checkis skipped for AF_UNSPEC
>> already, isn't it?  The only way the name_bind check would be
>> triggered is if the source port, snum, was non-zero and I didn't think
>> that was really legal for AF_UNSPEC/INADDR_ANY, is it?
>
> 1) What in inet_bind() prevents that from occurring?
> 2) Regardless, what about the node_bind check?

Fair enough.  As mentioned above, perhaps the right fix is to move the
address family checking back to how it was pre-SCTP.

Alexey, is this something you want to do, or should we take care of that?

>>>>                char *addrp;
>>>>                struct sk_security_struct *sksec = sk->sk_security;
>>>>                struct common_audit_data ad;
>>>> @@ -4601,7 +4604,7 @@ static int selinux_socket_bind(struct socket *sock, struc>
>>>>                 * need to check address->sa_family as it is possible to have
>>>>                 * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
>>>>                 */
>>>> -               switch (address->sa_family) {
>>>> +               switch (family_sa) {
>>>>                case AF_INET:
>>>>                        if (addrlen < sizeof(struct sockaddr_in))
>>>>                                return -EINVAL;
>>>>
>>>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>>>> index 4cafe6a..649a3be 100644
>>>>> --- a/security/selinux/hooks.c
>>>>> +++ b/security/selinux/hooks.c
>>>>> @@ -4602,10 +4602,16 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>>>>>                  * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
>>>>>                  */
>>>>>                 switch (address->sa_family) {
>>>>> +               case AF_UNSPEC:
>>>>>                 case AF_INET:
>>>>>                         if (addrlen < sizeof(struct sockaddr_in))
>>>>>                                 return -EINVAL;
>>>>>                         addr4 = (struct sockaddr_in *)address;
>>>>> +
>>>>> +                       if (address->sa_family == AF_UNSPEC &&
>>>>> +                           addr4->sin_addr.s_addr != htonl(INADDR_ANY))
>>>>> +                               return -EAFNOSUPPORT;
>>>>> +
>>>>>                         snum = ntohs(addr4->sin_port);
>>>>>                         addrp = (char *)&addr4->sin_addr.s_addr;
>>>>>                         break;
>>>>> @@ -4681,10 +4687,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>>>>>                 ad.u.net->sport = htons(snum);
>>>>>                 ad.u.net->family = family;
>>>>>
>>>>> -               if (address->sa_family == AF_INET)
>>>>> -                       ad.u.net->v4info.saddr = addr4->sin_addr.s_addr;
>>>>> -               else
>>>>> +               if (address->sa_family == AF_INET6)
>>>>>                         ad.u.net->v6info.saddr = addr6->sin6_addr;
>>>>> +               else
>>>>> +                       ad.u.net->v4info.saddr = addr4->sin_addr.s_addr;
>>>>>
>>>>>                 err = avc_has_perm(&selinux_state,
>>>>>                                    sksec->sid, sid,
>>>>> --
>>>>> 1.8.3.1
>>>>>
>>>>
>>>
>>
>>
>>
>



-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH net-next] net:sched: add gkprio scheduler
From: Jamal Hadi Salim @ 2018-05-09 14:43 UTC (permalink / raw)
  To: Cong Wang
  Cc: Michel Machado, Nishanth Devarajan, Jiri Pirko, David Miller,
	Linux Kernel Network Developers, Cody Doucette
In-Reply-To: <CAM_iQpXc4PzKZ7aQAp-sno57AEcgx21upJecQ1J1V0VaH1xM_g@mail.gmail.com>

On 08/05/18 10:27 PM, Cong Wang wrote:
> On Tue, May 8, 2018 at 6:29 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>> Have you considered using skb->prio instead of peeking into the packet
>> header.
>> Also have you looked at the dsmark qdisc?
>>
> 
> dsmark modifies ds fields, while this one just maps ds fields into
> different queues.
> 

Yeah, I was thinking more of re-using it for the purpose of mapping to
queues - but would require a lot more work.

once skbprio is set by something[1] then this qdisc could be used by
other subsystems (8021q, sockets etc); so i would argue for removal
of the embedded classification and instead maybe writing a simple
extension to skbmod to mark skbprio based on ds.

I find the cleverness in changing the highest/low prios confusing.
It looks error-prone (I guess that is why there is a BUG check)
To the authors: Is there a document/paper on the theory of this thing
as to why no explicit queues are "faster"?

Some other feedback:

1) I agree that using multiple queues as in prio qdisc would make it
more manageable; does not necessarily need to be classful if you
use implicit skbprio classification. i.e on equeue use a priority
map to select a queue; on dequeue always dequeu from highest prio
until it has no more packets to send.

2) Dropping already enqueued packets will not work well for
local feedback (__NET_XMIT_BYPASS return code is about the
packet that has been dropped from earlier enqueueing because
it is lower priority - it does not  signify anything with
current skb to which actually just got enqueud).
Perhaps (off top of my head) is to always enqueue packets on
high priority when their limit is exceeded as long as lower prio has
some space. Means youd have to increment low prio accounting if their
space is used.

cheers,
jamal

[1] something like:
tc filter add match all ip action skbmod inheritdsfield
tc filter add match all ip6 action skbmod inheritdsfield

inheritdsfield maps ds to skb->prioriry

^ permalink raw reply

* Re: [PATCH net-next] net:sched: add gkprio scheduler
From: Michel Machado @ 2018-05-09 14:09 UTC (permalink / raw)
  To: Cong Wang
  Cc: Nishanth Devarajan, Jiri Pirko, Jamal Hadi Salim, David Miller,
	Linux Kernel Network Developers, Cody Doucette
In-Reply-To: <CAM_iQpW7eGBffVMtBt1b1SHpLL5fbD1doRpUcVXW2U-rw4rOiQ@mail.gmail.com>

On 05/08/2018 10:24 PM, Cong Wang wrote:
> On Tue, May 8, 2018 at 5:59 AM, Michel Machado <michel@digirati.com.br> wrote:
>>>> Overall it looks good to me, just one thing below:
>>>>
>>>>> +struct Qdisc_ops gkprio_qdisc_ops __read_mostly = {
>>>>> +       .id             =       "gkprio",
>>>>> +       .priv_size      =       sizeof(struct gkprio_sched_data),
>>>>> +       .enqueue        =       gkprio_enqueue,
>>>>> +       .dequeue        =       gkprio_dequeue,
>>>>> +       .peek           =       qdisc_peek_dequeued,
>>>>> +       .init           =       gkprio_init,
>>>>> +       .reset          =       gkprio_reset,
>>>>> +       .change         =       gkprio_change,
>>>>> +       .dump           =       gkprio_dump,
>>>>> +       .destroy        =       gkprio_destroy,
>>>>> +       .owner          =       THIS_MODULE,
>>>>> +};
>>>>
>>>>
>>>> You probably want to add Qdisc_class_ops here so that you can
>>>> dump the stats of each internal queue.
>>
>>
>> Hi Cong,
>>
>>     In the production scenario we are targeting, this priority queue must be
>> classless; being classful would only bloat the code for us. I don't see
>> making this queue classful as a problem per se, but I suggest leaving it as
>> a future improvement for when someone can come up with a useful scenario for
>> it.
> 
> 
> Take a look at sch_prio, it is fairly simple since your internal
> queues are just an array... Per-queue stats are quite useful
> in production, we definitely want to observe which queues are
> full which are not.
> 

DSprio cannot add Qdisc_class_ops without a rewrite of other queue 
disciplines, which doesn't seem desirable. Since the method cops->leaf 
is required (see register_qdisc()), we would need to replace the array 
struct sk_buff_head qdiscs[GKPRIO_MAX_PRIORITY] in struct 
gkprio_sched_data with the array struct Qdisc 
*queues[GKPRIO_MAX_PRIORITY] to be able to return a Qdisc in 
dsprio_leaf(). The problem with this change is that Qdisc does not have 
a method to dequeue from its tail. This new method may not even make 
sense in other queue disciplines. But without this method, 
gkprio_enqueue() cannot drop the lowest priority packet when the queue 
is full and an incoming packet has higher priority.

Nevertheless, I see your point on being able to observe the distribution 
of queued packets per priority. A solution for that would be to add the 
array __u32 qlen[GKPRIO_MAX_PRIORITY] in struct tc_gkprio_qopt. This 
solution even avoids adding overhead in the critical paths of DSprio. Do 
you see a better solution?

By the way, I've used GKPRIO_MAX_PRIORITY and other names that include 
"gkprio" above to reflect the version 1 of this patch that we are 
discussing. We will rename these identifiers for version 2 of this patch 
to replace "gkprio" with "dsprio".

[ ]'s
Michel Machado

^ permalink raw reply

* [PATCH v2] net: phy: DP83TC811: Introduce support for the DP83TC811 phy
From: Dan Murphy @ 2018-05-09 14:09 UTC (permalink / raw)
  To: andrew, f.fainelli; +Cc: netdev, linux-kernel, Dan Murphy

Add support for the DP83811 phy.

The DP83811 supports both rgmii and sgmii interfaces.
There are 2 part numbers for this the DP83TC811R does not
reliably support the SGMII interface but the DP83TC811S will.

There is not a way to differentiate these parts from the
hardware or register set.  So this is controlled via the DT
to indicate which phy mode is required.  Or the part can be
strapped to a certain interface.

Data sheet can be found here:
http://www.ti.com/product/DP83TC811S-Q1/description
http://www.ti.com/product/DP83TC811R-Q1/description

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v2 - Remove extra config_init in reset, update config_init call back function
fix a checkpatch alignment issue, add SGMII check in autoneg api - https://patchwork.kernel.org/patch/10389323/

 drivers/net/phy/Kconfig     |   5 +
 drivers/net/phy/Makefile    |   1 +
 drivers/net/phy/dp83tc811.c | 350 ++++++++++++++++++++++++++++++++++++
 3 files changed, 356 insertions(+)
 create mode 100644 drivers/net/phy/dp83tc811.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index bdfbabb86ee0..810140a9e114 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -285,6 +285,11 @@ config DP83822_PHY
 	---help---
 	  Supports the DP83822 PHY.
 
+config DP83TC811_PHY
+	tristate "Texas Instruments DP83TC822 PHY"
+	---help---
+	  Supports the DP83TC822 PHY.
+
 config DP83848_PHY
 	tristate "Texas Instruments DP83848 PHY"
 	---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 01acbcb2c798..00445b61a9a8 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_CORTINA_PHY)	+= cortina.o
 obj-$(CONFIG_DAVICOM_PHY)	+= davicom.o
 obj-$(CONFIG_DP83640_PHY)	+= dp83640.o
 obj-$(CONFIG_DP83822_PHY)	+= dp83822.o
+obj-$(CONFIG_DP83TC811_PHY)	+= dp83tc811.o
 obj-$(CONFIG_DP83848_PHY)	+= dp83848.o
 obj-$(CONFIG_DP83867_PHY)	+= dp83867.o
 obj-$(CONFIG_FIXED_PHY)		+= fixed_phy.o
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
new file mode 100644
index 000000000000..10c20426bcfa
--- /dev/null
+++ b/drivers/net/phy/dp83tc811.c
@@ -0,0 +1,350 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Texas Instruments DP83TC811 PHY
+ *
+ * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ */
+
+#include <linux/ethtool.h>
+#include <linux/etherdevice.h>
+#include <linux/kernel.h>
+#include <linux/mii.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/netdevice.h>
+
+#define DP83TC811_PHY_ID	0x2000a253
+#define DP83811_DEVADDR		0x1f
+
+#define MII_DP83811_SGMII_CTRL	0x09
+#define MII_DP83811_INT_STAT1	0x12
+#define MII_DP83811_INT_STAT2	0x13
+#define MII_DP83811_RESET_CTRL	0x1f
+
+#define DP83811_HW_RESET	BIT(15)
+#define DP83811_SW_RESET	BIT(14)
+
+/* INT_STAT1 bits */
+#define DP83811_RX_ERR_HF_INT_EN	BIT(0)
+#define DP83811_MS_TRAINING_INT_EN	BIT(1)
+#define DP83811_ANEG_COMPLETE_INT_EN	BIT(2)
+#define DP83811_ESD_EVENT_INT_EN	BIT(3)
+#define DP83811_WOL_INT_EN		BIT(4)
+#define DP83811_LINK_STAT_INT_EN	BIT(5)
+#define DP83811_ENERGY_DET_INT_EN	BIT(6)
+#define DP83811_LINK_QUAL_INT_EN	BIT(7)
+
+/* INT_STAT2 bits */
+#define DP83811_JABBER_DET_INT_EN	BIT(0)
+#define DP83811_POLARITY_INT_EN		BIT(1)
+#define DP83811_SLEEP_MODE_INT_EN	BIT(2)
+#define DP83811_OVERTEMP_INT_EN		BIT(3)
+#define DP83811_OVERVOLTAGE_INT_EN	BIT(6)
+#define DP83811_UNDERVOLTAGE_INT_EN	BIT(7)
+
+#define MII_DP83811_RXSOP1	0x04a5
+#define MII_DP83811_RXSOP2	0x04a6
+#define MII_DP83811_RXSOP3	0x04a7
+
+/* WoL Registers */
+#define MII_DP83811_WOL_CFG	0x04a0
+#define MII_DP83811_WOL_STAT	0x04a1
+#define MII_DP83811_WOL_DA1	0x04a2
+#define MII_DP83811_WOL_DA2	0x04a3
+#define MII_DP83811_WOL_DA3	0x04a4
+
+/* WoL bits */
+#define DP83811_WOL_MAGIC_EN	BIT(0)
+#define DP83811_WOL_SECURE_ON	BIT(5)
+#define DP83811_WOL_EN		BIT(7)
+#define DP83811_WOL_INDICATION_SEL BIT(8)
+#define DP83811_WOL_CLR_INDICATION BIT(11)
+
+/* SGMII CTRL bits */
+#define DP83811_TDR_AUTO		BIT(8)
+#define DP83811_SGMII_EN		BIT(12)
+#define DP83811_SGMII_AUTO_NEG_EN	BIT(13)
+#define DP83811_SGMII_TX_ERR_DIS	BIT(14)
+#define DP83811_SGMII_SOFT_RESET	BIT(15)
+
+static int dp83811_ack_interrupt(struct phy_device *phydev)
+{
+	int err;
+
+	err = phy_read(phydev, MII_DP83811_INT_STAT1);
+	if (err < 0)
+		return err;
+
+	err = phy_read(phydev, MII_DP83811_INT_STAT2);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static int dp83811_set_wol(struct phy_device *phydev,
+			   struct ethtool_wolinfo *wol)
+{
+	struct net_device *ndev = phydev->attached_dev;
+	u16 value;
+	const u8 *mac;
+
+	if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE)) {
+		mac = (const u8 *)ndev->dev_addr;
+
+		if (!is_valid_ether_addr(mac))
+			return -EINVAL;
+
+		/* MAC addresses start with byte 5, but stored in mac[0].
+		 * 811 PHYs store bytes 4|5, 2|3, 0|1
+		 */
+		phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_DA1,
+			      (mac[1] << 8) | mac[0]);
+		phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_DA2,
+			      (mac[3] << 8) | mac[2]);
+		phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_DA3,
+			      (mac[5] << 8) | mac[4]);
+
+		value = phy_read_mmd(phydev, DP83811_DEVADDR,
+				     MII_DP83811_WOL_CFG);
+		if (wol->wolopts & WAKE_MAGIC)
+			value |= DP83811_WOL_MAGIC_EN;
+		else
+			value &= ~DP83811_WOL_MAGIC_EN;
+
+		if (wol->wolopts & WAKE_MAGICSECURE) {
+			phy_write_mmd(phydev, DP83811_DEVADDR,
+				      MII_DP83811_RXSOP1,
+				      (wol->sopass[1] << 8) | wol->sopass[0]);
+			phy_write_mmd(phydev, DP83811_DEVADDR,
+				      MII_DP83811_RXSOP2,
+				      (wol->sopass[3] << 8) | wol->sopass[2]);
+			phy_write_mmd(phydev, DP83811_DEVADDR,
+				      MII_DP83811_RXSOP3,
+				      (wol->sopass[5] << 8) | wol->sopass[4]);
+			value |= DP83811_WOL_SECURE_ON;
+		} else {
+			value &= ~DP83811_WOL_SECURE_ON;
+		}
+
+		value |= (DP83811_WOL_EN | DP83811_WOL_INDICATION_SEL |
+			  DP83811_WOL_CLR_INDICATION);
+		phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG,
+			      value);
+	} else {
+		value = phy_read_mmd(phydev, DP83811_DEVADDR,
+				     MII_DP83811_WOL_CFG);
+		value &= ~DP83811_WOL_EN;
+		phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG,
+			      value);
+	}
+
+	return 0;
+}
+
+static void dp83811_get_wol(struct phy_device *phydev,
+			    struct ethtool_wolinfo *wol)
+{
+	int value;
+	u16 sopass_val;
+
+	wol->supported = (WAKE_MAGIC | WAKE_MAGICSECURE);
+	wol->wolopts = 0;
+
+	value = phy_read_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG);
+
+	if (value & DP83811_WOL_MAGIC_EN)
+		wol->wolopts |= WAKE_MAGIC;
+
+	if (value & DP83811_WOL_SECURE_ON) {
+		sopass_val = phy_read_mmd(phydev, DP83811_DEVADDR,
+					  MII_DP83811_RXSOP1);
+		wol->sopass[0] = (sopass_val & 0xff);
+		wol->sopass[1] = (sopass_val >> 8);
+
+		sopass_val = phy_read_mmd(phydev, DP83811_DEVADDR,
+					  MII_DP83811_RXSOP2);
+		wol->sopass[2] = (sopass_val & 0xff);
+		wol->sopass[3] = (sopass_val >> 8);
+
+		sopass_val = phy_read_mmd(phydev, DP83811_DEVADDR,
+					  MII_DP83811_RXSOP3);
+		wol->sopass[4] = (sopass_val & 0xff);
+		wol->sopass[5] = (sopass_val >> 8);
+
+		wol->wolopts |= WAKE_MAGICSECURE;
+	}
+
+	/* WoL is not enabled so set wolopts to 0 */
+	if (!(value & DP83811_WOL_EN))
+		wol->wolopts = 0;
+}
+
+static int dp83811_config_intr(struct phy_device *phydev)
+{
+	int misr_status;
+	int err;
+
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+		misr_status = phy_read(phydev, MII_DP83811_INT_STAT1);
+		if (misr_status < 0)
+			return misr_status;
+
+		misr_status |= (DP83811_RX_ERR_HF_INT_EN |
+				DP83811_MS_TRAINING_INT_EN |
+				DP83811_ANEG_COMPLETE_INT_EN |
+				DP83811_ESD_EVENT_INT_EN |
+				DP83811_WOL_INT_EN |
+				DP83811_LINK_STAT_INT_EN |
+				DP83811_ENERGY_DET_INT_EN |
+				DP83811_LINK_QUAL_INT_EN);
+
+		err = phy_write(phydev, MII_DP83811_INT_STAT1, misr_status);
+		if (err < 0)
+			return err;
+
+		misr_status = phy_read(phydev, MII_DP83811_INT_STAT2);
+		if (misr_status < 0)
+			return misr_status;
+
+		misr_status |= (DP83811_JABBER_DET_INT_EN |
+				DP83811_POLARITY_INT_EN |
+				DP83811_SLEEP_MODE_INT_EN |
+				DP83811_OVERTEMP_INT_EN |
+				DP83811_OVERVOLTAGE_INT_EN |
+				DP83811_UNDERVOLTAGE_INT_EN);
+
+		err = phy_write(phydev, MII_DP83811_INT_STAT2, misr_status);
+
+	} else {
+		err = phy_write(phydev, MII_DP83811_INT_STAT1, 0);
+		if (err < 0)
+			return err;
+
+		err = phy_write(phydev, MII_DP83811_INT_STAT1, 0);
+	}
+
+	return err;
+}
+
+static int dp83811_config_aneg(struct phy_device *phydev)
+{
+	int err;
+	int value;
+
+	if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
+		value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
+		if (phydev->autoneg == AUTONEG_ENABLE) {
+			err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
+					(DP83811_SGMII_AUTO_NEG_EN | value));
+			if (err < 0)
+				return err;
+		} else {
+			err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
+					(~DP83811_SGMII_AUTO_NEG_EN & value));
+			if (err < 0)
+				return err;
+		}
+	}
+
+	return genphy_config_aneg(phydev);
+}
+
+static int dp83811_config_init(struct phy_device *phydev)
+{
+	int err;
+	int value;
+
+	err = genphy_config_init(phydev);
+	if (err < 0)
+		return err;
+
+	if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
+		value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
+		if (!(value & DP83811_SGMII_EN)) {
+			err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
+					(DP83811_SGMII_EN | value));
+			if (err < 0)
+				return err;
+		} else {
+			err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
+					(~DP83811_SGMII_EN & value));
+			if (err < 0)
+				return err;
+		}
+	}
+
+	value = DP83811_WOL_MAGIC_EN | DP83811_WOL_SECURE_ON | DP83811_WOL_EN;
+
+	return phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG,
+	      value);
+}
+
+static int dp83811_phy_reset(struct phy_device *phydev)
+{
+	int err;
+
+	err = phy_write(phydev, MII_DP83811_RESET_CTRL, DP83811_HW_RESET);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static int dp83811_suspend(struct phy_device *phydev)
+{
+	int value;
+
+	value = phy_read_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG);
+
+	if (!(value & DP83811_WOL_EN))
+		genphy_suspend(phydev);
+
+	return 0;
+}
+
+static int dp83811_resume(struct phy_device *phydev)
+{
+	int value;
+
+	genphy_resume(phydev);
+
+	value = phy_read_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG);
+
+	phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG, value |
+		      DP83811_WOL_CLR_INDICATION);
+
+	return 0;
+}
+
+static struct phy_driver dp83811_driver[] = {
+	{
+		.phy_id = DP83TC811_PHY_ID,
+		.phy_id_mask = 0xfffffff0,
+		.name = "TI DP83TC811",
+		.features = PHY_BASIC_FEATURES,
+		.flags = PHY_HAS_INTERRUPT,
+		.config_init = dp83811_config_init,
+		.config_aneg = dp83811_config_aneg,
+		.soft_reset = dp83811_phy_reset,
+		.get_wol = dp83811_get_wol,
+		.set_wol = dp83811_set_wol,
+		.ack_interrupt = dp83811_ack_interrupt,
+		.config_intr = dp83811_config_intr,
+		.suspend = dp83811_suspend,
+		.resume = dp83811_resume,
+	 },
+};
+module_phy_driver(dp83811_driver);
+
+static struct mdio_device_id __maybe_unused dp83811_tbl[] = {
+	{ DP83TC811_PHY_ID, 0xfffffff0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(mdio, dp83811_tbl);
+
+MODULE_DESCRIPTION("Texas Instruments DP83TC811 PHY driver");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com");
+MODULE_LICENSE("GPL");
-- 
2.17.0.582.gccdcbd54c

^ permalink raw reply related

* Re: [PATCH] net: phy: DP83TC811: Introduce support for the DP83TC811 phy
From: Dan Murphy @ 2018-05-09 14:00 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: f.fainelli, netdev, linux-kernel
In-Reply-To: <20180509135826.GG14276@lunn.ch>

Andrew

On 05/09/2018 08:58 AM, Andrew Lunn wrote:
> On Wed, May 09, 2018 at 08:50:58AM -0500, Dan Murphy wrote:
>> Andrew
>>
>> Thanks for the review
>>
>> On 05/09/2018 08:43 AM, Andrew Lunn wrote:
>>>> +static int dp83811_config_aneg(struct phy_device *phydev)
>>>> +{
>>>> +	int err;
>>>> +	int value;
>>>> +
>>>> +	value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
>>>> +	if (phydev->autoneg == AUTONEG_ENABLE) {
>>>> +		err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>>>> +				(DP83811_SGMII_AUTO_NEG_EN | value));
>>>> +		if (err < 0)
>>>> +			return err;
>>>> +	} else {
>>>> +		err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>>>> +				(~DP83811_SGMII_AUTO_NEG_EN & value));
>>>> +		if (err < 0)
>>>> +			return err;
>>>> +	}
>>>> +
>>>
>>> Hi Dan
>>>
>>> You say SGMII is unreliable on one of these devices. Should you check
>>> phydev->interface before enabling SGMII autoneg?
>>
>>
>> If SGMII enable bit(12) is not set in the device then setting auto neg has no affect on the device.
> 
> Ah, O.K. Maybe add a comment about this.
> 

I will add the check it will be clearer if written in code and explicit as opposed to explaining it.

Dan

>>>> +
>>>> +static int dp83811_config_init(struct phy_device *phydev)
>>>> +{
>>>> +	int err;
>>>> +	int value;
>>>> +
>>>> +	err = genphy_config_init(phydev);
>>>> +	if (err < 0)
>>>> +		return err;
>>>> +
>>>> +	if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
>>>> +		value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
>>>> +		if (!(value & DP83811_SGMII_EN)) {
>>>> +			err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>>>> +					(DP83811_SGMII_EN | value));
>>>> +			if (err < 0)
>>>> +				return err;
>>>> +		} else {
>>>> +			err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>>>> +				(~DP83811_SGMII_EN & value));
>>>> +			if (err < 0)
>>>> +				return err;
>>>> +		}
>>>
>>> This looks to be a duplicate of dp83811_config_aneg()?
>>
>> It is almost the same but this function sets bit 12 and aneg function sets bit 13.
>> We can have SGMII with or without auto neg.
> 
> Yep, i missed the difference.
> 
>      Andrew
> 


-- 
------------------
Dan Murphy

^ permalink raw reply

* [PATCH] ixgbe: fix memory leak on ipsec allocation
From: Colin King @ 2018-05-09 13:58 UTC (permalink / raw)
  To: Jeff Kirsher, David S . Miller, intel-wired-lan, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

The error clean up path kfree's adapter->ipsec and should be
instead kfree'ing ipsec. Fix this.  Also, the err1 error exit path
does not need to kfree ipsec because this failure path was for
the failed allocation of ipsec.

Detected by CoverityScan, CID#146424 ("Resource Leak")

Fixes: 63a67fe229ea ("ixgbe: add ipsec offload add and remove SA")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index 41af2b81e960..195c0b65eee2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -919,8 +919,8 @@ void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter)
 	kfree(ipsec->ip_tbl);
 	kfree(ipsec->rx_tbl);
 	kfree(ipsec->tx_tbl);
+	kfree(ipsec);
 err1:
-	kfree(adapter->ipsec);
 	netdev_err(adapter->netdev, "Unable to allocate memory for SA tables");
 }
 
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH] net: phy: DP83TC811: Introduce support for the DP83TC811 phy
From: Andrew Lunn @ 2018-05-09 13:58 UTC (permalink / raw)
  To: Dan Murphy; +Cc: f.fainelli, netdev, linux-kernel
In-Reply-To: <382b9d52-7cfd-ae37-244a-32c7245cb27e@ti.com>

On Wed, May 09, 2018 at 08:50:58AM -0500, Dan Murphy wrote:
> Andrew
> 
> Thanks for the review
> 
> On 05/09/2018 08:43 AM, Andrew Lunn wrote:
> >> +static int dp83811_config_aneg(struct phy_device *phydev)
> >> +{
> >> +	int err;
> >> +	int value;
> >> +
> >> +	value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
> >> +	if (phydev->autoneg == AUTONEG_ENABLE) {
> >> +		err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
> >> +				(DP83811_SGMII_AUTO_NEG_EN | value));
> >> +		if (err < 0)
> >> +			return err;
> >> +	} else {
> >> +		err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
> >> +				(~DP83811_SGMII_AUTO_NEG_EN & value));
> >> +		if (err < 0)
> >> +			return err;
> >> +	}
> >> +
> > 
> > Hi Dan
> > 
> > You say SGMII is unreliable on one of these devices. Should you check
> > phydev->interface before enabling SGMII autoneg?
> 
> 
> If SGMII enable bit(12) is not set in the device then setting auto neg has no affect on the device.

Ah, O.K. Maybe add a comment about this.

> >> +
> >> +static int dp83811_config_init(struct phy_device *phydev)
> >> +{
> >> +	int err;
> >> +	int value;
> >> +
> >> +	err = genphy_config_init(phydev);
> >> +	if (err < 0)
> >> +		return err;
> >> +
> >> +	if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> >> +		value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
> >> +		if (!(value & DP83811_SGMII_EN)) {
> >> +			err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
> >> +					(DP83811_SGMII_EN | value));
> >> +			if (err < 0)
> >> +				return err;
> >> +		} else {
> >> +			err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
> >> +				(~DP83811_SGMII_EN & value));
> >> +			if (err < 0)
> >> +				return err;
> >> +		}
> > 
> > This looks to be a duplicate of dp83811_config_aneg()?
> 
> It is almost the same but this function sets bit 12 and aneg function sets bit 13.
> We can have SGMII with or without auto neg.

Yep, i missed the difference.

     Andrew

^ permalink raw reply

* Re: [PATCH] net: phy: DP83TC811: Introduce support for the DP83TC811 phy
From: Dan Murphy @ 2018-05-09 13:50 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: f.fainelli, netdev, linux-kernel
In-Reply-To: <20180509134315.GE14276@lunn.ch>

Andrew

Thanks for the review

On 05/09/2018 08:43 AM, Andrew Lunn wrote:
>> +static int dp83811_config_aneg(struct phy_device *phydev)
>> +{
>> +	int err;
>> +	int value;
>> +
>> +	value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
>> +	if (phydev->autoneg == AUTONEG_ENABLE) {
>> +		err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>> +				(DP83811_SGMII_AUTO_NEG_EN | value));
>> +		if (err < 0)
>> +			return err;
>> +	} else {
>> +		err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>> +				(~DP83811_SGMII_AUTO_NEG_EN & value));
>> +		if (err < 0)
>> +			return err;
>> +	}
>> +
> 
> Hi Dan
> 
> You say SGMII is unreliable on one of these devices. Should you check
> phydev->interface before enabling SGMII autoneg?


If SGMII enable bit(12) is not set in the device then setting auto neg has no affect on the device.
Only the SGMII has auto negotiation capability so if we set this with other MII interfaces then it is ignored by the device.

If we want to protect this with the SGMII check I can add it but it may be over kill.

Let me know what is preferred.

> 
>> +	return genphy_config_aneg(phydev);
>> +}
>> +
>> +static int dp83811_config_init(struct phy_device *phydev)
>> +{
>> +	int err;
>> +	int value;
>> +
>> +	err = genphy_config_init(phydev);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
>> +		value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
>> +		if (!(value & DP83811_SGMII_EN)) {
>> +			err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>> +					(DP83811_SGMII_EN | value));
>> +			if (err < 0)
>> +				return err;
>> +		} else {
>> +			err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>> +				(~DP83811_SGMII_EN & value));
>> +			if (err < 0)
>> +				return err;
>> +		}
> 
> This looks to be a duplicate of dp83811_config_aneg()?

It is almost the same but this function sets bit 12 and aneg function sets bit 13.
We can have SGMII with or without auto neg.

> 
>> +	}
>> +
>> +	value = DP83811_WOL_MAGIC_EN | DP83811_WOL_SECURE_ON | DP83811_WOL_EN;
>> +
>> +	return phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG,
>> +	      value);
>> +}
>> +
>> +static int dp83811_phy_reset(struct phy_device *phydev)
>> +{
>> +	int err;
>> +
>> +	err = phy_write(phydev, MII_DP83811_RESET_CTRL, DP83811_HW_RESET);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	dp83811_config_init(phydev);
> 
> I don't think you need to initialize it here. phylib should call that
> soon after the reset.

OK I will remove it.  

> 
>> +
>> +	return 0;
>> +}
>> +
> 
>> +static struct phy_driver dp83811_driver[] = {
>> +	{
>> +		.phy_id = DP83TC811_PHY_ID,
>> +		.phy_id_mask = 0xfffffff0,
>> +		.name = "TI DP83TC811",
>> +		.features = PHY_BASIC_FEATURES,
>> +		.flags = PHY_HAS_INTERRUPT,
>> +		.config_init = genphy_config_init,
> 
> ????
> 

I missed that I must have had the config init being called in my testing through the reset function.
I will update.

> 	Andrew
> 


-- 
------------------
Dan Murphy

^ permalink raw reply

* Re: Please apply 3f1e53abff84c (" netfilter: ebtables: don't attempt to allocate 0-sized compat array") to v4.14.y and later
From: Greg Kroah-Hartman @ 2018-05-09 13:46 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: stable, dvyukov@google.com, David S. Miller, Network Development
In-Reply-To: <6a55fa40-1304-dae4-e01a-9cfb1897aa8e@roeck-us.net>

On Wed, May 09, 2018 at 06:26:29AM -0700, Guenter Roeck wrote:
> Hi Greg,
> 
> Commit 3f1e53abff84c fixes commit 7d7d7e02111e9 ("("netfilter: compat: reject huge
> allocation requests"), which has found its way into 4.14.y and 4.16.y.
> This causes syzcaller hiccups at least in 4.14.y (more specifically chromeos-4.14).
> 
> Please apply 3f1e53abff84c to v4.14.y and v4.16.y to fix the problem. Copying Dave
> and netdev in case he wants to handle it.
> 
> Sorry for the noise if this has already been queued.

I queued it up 2 hours ago :)

But thanks for asking, it's better to make sure it gets added.

thanks,

greg k-h

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox