* RE: [PATCH net-next 0/8] Hisilicon Network Subsystem 3 VF Ethernet Driver
From: Salil Mehta @ 2017-12-04 14:28 UTC (permalink / raw)
To: Or Gerlitz
Cc: davem@davemloft.net, Zhuangyuzeng (Yisen), lipeng (Y),
mehta.salil.lnk@gmail.com, netdev@vger.kernel.org,
andy@greyhouse.net, simon.horman@netronome.com, jiri@mellanox.com,
Linuxarm
In-Reply-To: <CAJ3xEMj78+9AabgeXS=1ZeiRbMyRtCr55W2zryTDbifqvzhJew@mail.gmail.com>
> -----Original Message-----
> From: Or Gerlitz [mailto:gerlitz.or@gmail.com]
> Sent: Monday, December 04, 2017 2:08 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: davem@davemloft.net; Zhuangyuzeng (Yisen)
> <yisen.zhuang@huawei.com>; lipeng (Y) <lipeng321@huawei.com>;
> mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; andy@greyhouse.net;
> simon.horman@netronome.com; jiri@mellanox.com
> Subject: Re: [PATCH net-next 0/8] Hisilicon Network Subsystem 3 VF
> Ethernet Driver
>
> On Mon, Dec 4, 2017 at 12:48 PM, Salil Mehta <salil.mehta@huawei.com>
> wrote:
> >> From: Or Gerlitz <gerlitz.or@gmail.com>
>
> >> I wasn't sure if/what PF side control plane user space APIs are
> >> supported by this series, can you elaborate on that?
>
> > Current VF driver just supports very basic functionality. Idea was to
> > push basic RX/TX/RSS/TSO support along with this version.
>
> I was referring to PF driver knobs to manage the e-switch, not to the
> VF driver.
Right. We have unified the Enet driver for VF and PF now which will have
all those hooks related to the switchdev once we decide to get down that
path. And since these are more related to PF they have to be taken care of
in separate patches.
>
> > We will support switchdev and other lined-up features after basic
> > version of the driver has gone through. We are in analysis phase
> right
> > now for all of these features.
>
> sounds good. You might want to consider supporting 1st or only the
> switchdev mode, this will make your PF driver a community leader..
Yes, we will consider it as part of our analysis.
Thanks
Salil
>
> Or.
^ permalink raw reply
* [PATCH 1/2] net: sh_eth: add support for SH7786
From: Thomas Petazzoni @ 2017-12-04 14:17 UTC (permalink / raw)
To: David S. Miller, Sergei Shtylyov, Niklas Söderlund,
Geert Uytterhoeven, Simon Horman
Cc: netdev, linux-renesas-soc, Thomas Petazzoni
In-Reply-To: <20171204141744.18599-1-thomas.petazzoni@free-electrons.com>
This commit adds the sh_eth_cpu_data structure that describes the
SH7786 variant of the IP.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 0074c5998481..a3c48b2a713c 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -710,6 +710,30 @@ static struct sh_eth_cpu_data sh7724_data = {
.rpadir_value = 0x00020000, /* NET_IP_ALIGN assumed to be 2 */
};
+static struct sh_eth_cpu_data sh7786_data = {
+ .set_duplex = sh_eth_set_duplex,
+
+ .register_type = SH_ETH_REG_FAST_SH4,
+
+ .ecsr_value = ECSR_PSRTO | ECSR_LCHNG | ECSR_ICD,
+ .ecsipr_value = ECSIPR_PSRTOIP | ECSIPR_LCHNGIP | ECSIPR_ICDIP,
+ .eesipr_value = EESIPR_RFCOFIP | EESIPR_ADEIP | EESIPR_ECIIP |
+ EESIPR_FTCIP | EESIPR_TDEIP | EESIPR_TFUFIP |
+ EESIPR_FRIP | EESIPR_RDEIP | EESIPR_RFOFIP |
+ EESIPR_RMAFIP | EESIPR_RRFIP |
+ EESIPR_RTLFIP | EESIPR_RTSFIP |
+ EESIPR_PREIP | EESIPR_CERFIP,
+
+ .tx_check = EESR_FTC | EESR_CND | EESR_DLC | EESR_CD | EESR_RTO,
+ .eesr_err_check = EESR_TWB | EESR_TABT | EESR_RABT | EESR_RFE |
+ EESR_RDE | EESR_RFRMER | EESR_TFE | EESR_TDE,
+
+ .apr = 1,
+ .mpr = 1,
+ .tpauser = 1,
+ .hw_swap = 1,
+};
+
static void sh_eth_set_rate_sh7757(struct net_device *ndev)
{
struct sh_eth_private *mdp = netdev_priv(ndev);
@@ -3418,6 +3442,7 @@ static const struct platform_device_id sh_eth_id_table[] = {
{ "sh7757-ether", (kernel_ulong_t)&sh7757_data },
{ "sh7757-gether", (kernel_ulong_t)&sh7757_data_giga },
{ "sh7763-gether", (kernel_ulong_t)&sh7763_data },
+ { "sh7786-ether", (kernel_ulong_t)&sh7786_data },
{ }
};
MODULE_DEVICE_TABLE(platform, sh_eth_id_table);
--
2.13.6
^ permalink raw reply related
* [PATCH 2/2] net: sh_eth: make work on big endian systems
From: Thomas Petazzoni @ 2017-12-04 14:17 UTC (permalink / raw)
To: David S. Miller, Sergei Shtylyov, Niklas Söderlund,
Geert Uytterhoeven, Simon Horman
Cc: netdev, linux-renesas-soc, Thomas Petazzoni
In-Reply-To: <20171204141744.18599-1-thomas.petazzoni@free-electrons.com>
The sh_eth driver uses cpu_to_le32() and le32_to_cpu() to manipulate
the fields of the DMA descriptors, making the assumption that the DMA
descriptors are little-endian.
However, testing on the Renesas SH7786 running in big-endian mode
reveals that the DMA descriptors are also big-endian when running
big-endian. Therefore, all the endianness conversion needs to be
removed for the sh_eth driver to work.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Note: I see that Sergei Shtylyov has done some work around endianness
on this driver back in 2015. I am not sure if it's used on other
non-SuperH platforms in platforms where the CPU might run big-endian
but the IP still be little-endian. If this is the case, then we would
need a different approach and more discussion.
Therefore, it would be useful to wait for an ACK from Sergei before
applying this patch.
---
drivers/net/ethernet/renesas/sh_eth.c | 64 ++++++++++++++++-------------------
1 file changed, 30 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index a3c48b2a713c..0de8fae143ab 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -1163,31 +1163,29 @@ static int sh_eth_tx_free(struct net_device *ndev, bool sent_only)
for (; mdp->cur_tx - mdp->dirty_tx > 0; mdp->dirty_tx++) {
entry = mdp->dirty_tx % mdp->num_tx_ring;
txdesc = &mdp->tx_ring[entry];
- sent = !(txdesc->status & cpu_to_le32(TD_TACT));
+ sent = !(txdesc->status & TD_TACT);
if (sent_only && !sent)
break;
/* TACT bit must be checked before all the following reads */
dma_rmb();
netif_info(mdp, tx_done, ndev,
"tx entry %d status 0x%08x\n",
- entry, le32_to_cpu(txdesc->status));
+ entry, txdesc->status);
/* Free the original skb. */
if (mdp->tx_skbuff[entry]) {
- dma_unmap_single(&mdp->pdev->dev,
- le32_to_cpu(txdesc->addr),
- le32_to_cpu(txdesc->len) >> 16,
- DMA_TO_DEVICE);
+ dma_unmap_single(&mdp->pdev->dev, txdesc->addr,
+ txdesc->len >> 16, DMA_TO_DEVICE);
dev_kfree_skb_irq(mdp->tx_skbuff[entry]);
mdp->tx_skbuff[entry] = NULL;
free_num++;
}
- txdesc->status = cpu_to_le32(TD_TFP);
+ txdesc->status = TD_TFP;
if (entry >= mdp->num_tx_ring - 1)
- txdesc->status |= cpu_to_le32(TD_TDLE);
+ txdesc->status |= TD_TDLE;
if (sent) {
ndev->stats.tx_packets++;
- ndev->stats.tx_bytes += le32_to_cpu(txdesc->len) >> 16;
+ ndev->stats.tx_bytes += txdesc->len >> 16;
}
}
return free_num;
@@ -1204,8 +1202,7 @@ static void sh_eth_ring_free(struct net_device *ndev)
if (mdp->rx_skbuff[i]) {
struct sh_eth_rxdesc *rxdesc = &mdp->rx_ring[i];
- dma_unmap_single(&mdp->pdev->dev,
- le32_to_cpu(rxdesc->addr),
+ dma_unmap_single(&mdp->pdev->dev, rxdesc->addr,
ALIGN(mdp->rx_buf_sz, 32),
DMA_FROM_DEVICE);
}
@@ -1280,9 +1277,9 @@ static void sh_eth_ring_format(struct net_device *ndev)
/* RX descriptor */
rxdesc = &mdp->rx_ring[i];
- rxdesc->len = cpu_to_le32(buf_len << 16);
- rxdesc->addr = cpu_to_le32(dma_addr);
- rxdesc->status = cpu_to_le32(RD_RACT | RD_RFP);
+ rxdesc->len = buf_len << 16;
+ rxdesc->addr = dma_addr;
+ rxdesc->status = RD_RACT | RD_RFP;
/* Rx descriptor address set */
if (i == 0) {
@@ -1297,7 +1294,7 @@ static void sh_eth_ring_format(struct net_device *ndev)
/* Mark the last entry as wrapping the ring. */
if (rxdesc)
- rxdesc->status |= cpu_to_le32(RD_RDLE);
+ rxdesc->status |= RD_RDLE;
memset(mdp->tx_ring, 0, tx_ringsize);
@@ -1305,8 +1302,8 @@ static void sh_eth_ring_format(struct net_device *ndev)
for (i = 0; i < mdp->num_tx_ring; i++) {
mdp->tx_skbuff[i] = NULL;
txdesc = &mdp->tx_ring[i];
- txdesc->status = cpu_to_le32(TD_TFP);
- txdesc->len = cpu_to_le32(0);
+ txdesc->status = TD_TFP;
+ txdesc->len = 0;
if (i == 0) {
/* Tx descriptor address set */
sh_eth_write(ndev, mdp->tx_desc_dma, TDLAR);
@@ -1316,7 +1313,7 @@ static void sh_eth_ring_format(struct net_device *ndev)
}
}
- txdesc->status |= cpu_to_le32(TD_TDLE);
+ txdesc->status |= TD_TDLE;
}
/* Get skb and descriptor buffer */
@@ -1462,7 +1459,7 @@ static void sh_eth_dev_exit(struct net_device *ndev)
* packet boundary if it's currently running
*/
for (i = 0; i < mdp->num_tx_ring; i++)
- mdp->tx_ring[i].status &= ~cpu_to_le32(TD_TACT);
+ mdp->tx_ring[i].status &= ~TD_TACT;
/* Disable TX FIFO egress to MAC */
sh_eth_rcv_snd_disable(ndev);
@@ -1502,11 +1499,11 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
boguscnt = min(boguscnt, *quota);
limit = boguscnt;
rxdesc = &mdp->rx_ring[entry];
- while (!(rxdesc->status & cpu_to_le32(RD_RACT))) {
+ while (!(rxdesc->status & RD_RACT)) {
/* RACT bit must be checked before all the following reads */
dma_rmb();
- desc_status = le32_to_cpu(rxdesc->status);
- pkt_len = le32_to_cpu(rxdesc->len) & RD_RFL;
+ desc_status = rxdesc->status;
+ pkt_len = rxdesc->len & RD_RFL;
if (--boguscnt < 0)
break;
@@ -1544,7 +1541,7 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
if (desc_status & RD_RFS10)
ndev->stats.rx_over_errors++;
} else if (skb) {
- dma_addr = le32_to_cpu(rxdesc->addr);
+ dma_addr = rxdesc->addr;
if (!mdp->cd->hw_swap)
sh_eth_soft_swap(
phys_to_virt(ALIGN(dma_addr, 4)),
@@ -1573,7 +1570,7 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
rxdesc = &mdp->rx_ring[entry];
/* The size of the buffer is 32 byte boundary. */
buf_len = ALIGN(mdp->rx_buf_sz, 32);
- rxdesc->len = cpu_to_le32(buf_len << 16);
+ rxdesc->len = buf_len << 16;
if (mdp->rx_skbuff[entry] == NULL) {
skb = netdev_alloc_skb(ndev, skbuff_size);
@@ -1589,14 +1586,13 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
mdp->rx_skbuff[entry] = skb;
skb_checksum_none_assert(skb);
- rxdesc->addr = cpu_to_le32(dma_addr);
+ rxdesc->addr = dma_addr;
}
dma_wmb(); /* RACT bit must be set after all the above writes */
if (entry >= mdp->num_rx_ring - 1)
- rxdesc->status |=
- cpu_to_le32(RD_RACT | RD_RFP | RD_RDLE);
+ rxdesc->status |= RD_RACT | RD_RFP | RD_RDLE;
else
- rxdesc->status |= cpu_to_le32(RD_RACT | RD_RFP);
+ rxdesc->status |= RD_RACT | RD_RFP;
}
/* Restart Rx engine if stopped. */
@@ -2426,8 +2422,8 @@ static void sh_eth_tx_timeout(struct net_device *ndev)
/* Free all the skbuffs in the Rx queue. */
for (i = 0; i < mdp->num_rx_ring; i++) {
rxdesc = &mdp->rx_ring[i];
- rxdesc->status = cpu_to_le32(0);
- rxdesc->addr = cpu_to_le32(0xBADF00D0);
+ rxdesc->status = 0;
+ rxdesc->addr = 0xBADF00D0;
dev_kfree_skb(mdp->rx_skbuff[i]);
mdp->rx_skbuff[i] = NULL;
}
@@ -2477,14 +2473,14 @@ static int sh_eth_start_xmit(struct sk_buff *skb, struct net_device *ndev)
kfree_skb(skb);
return NETDEV_TX_OK;
}
- txdesc->addr = cpu_to_le32(dma_addr);
- txdesc->len = cpu_to_le32(skb->len << 16);
+ txdesc->addr = dma_addr;
+ txdesc->len = skb->len << 16;
dma_wmb(); /* TACT bit must be set after all the above writes */
if (entry >= mdp->num_tx_ring - 1)
- txdesc->status |= cpu_to_le32(TD_TACT | TD_TDLE);
+ txdesc->status |= TD_TACT | TD_TDLE;
else
- txdesc->status |= cpu_to_le32(TD_TACT);
+ txdesc->status |= TD_TACT;
mdp->cur_tx++;
--
2.13.6
^ permalink raw reply related
* [PATCH 0/2] net: sh_eth: add support for SH7786 and big-endian
From: Thomas Petazzoni @ 2017-12-04 14:17 UTC (permalink / raw)
To: David S. Miller, Sergei Shtylyov, Niklas Söderlund,
Geert Uytterhoeven, Simon Horman
Cc: netdev, linux-renesas-soc, Thomas Petazzoni
Hello,
I've recently been working on an SH7786 based platform, which uses the
sh_eth network controller. One peculiarity of my setup is that the CPU
is configured big-endian (even though little-endian is more
traditional in the Linux SuperH world), and the sh_eth driver was not
ready for this.
The first patch simply adds the sh_eth_cpu_data structure that
describes the SH7786 controller.
The second patch fixes the driver for big-endian operation. However,
I'd like this patch to be carefully reviewed by Sergei Shtylyov who
already did some endianness related changes in this driver. Indeed, my
change is based on the assumption that the DMA descriptors are in the
native endianness of the CPU.
Thanks,
Thomas
Thomas Petazzoni (2):
net: sh_eth: add support for SH7786
net: sh_eth: make work on big endian systems
drivers/net/ethernet/renesas/sh_eth.c | 89 ++++++++++++++++++++++-------------
1 file changed, 55 insertions(+), 34 deletions(-)
--
2.13.6
^ permalink raw reply
* [PATCH] net: sh_eth: do not advertise Gigabit capabilities when not available
From: Thomas Petazzoni @ 2017-12-04 14:13 UTC (permalink / raw)
To: David S. Miller, Sergei Shtylyov, Niklas Söderlund,
Geert Uytterhoeven, Simon Horman
Cc: netdev, linux-renesas-soc, Thomas Petazzoni
Not all variants of the sh_eth hardware have Gigabit
support. Unfortunately, the current driver doesn't update
phydev->supported depending on the MAC capabilities. Due to this, if
you have a Gigabit capable PHY, the PHY will advertise its Gigabit
capability and establish a link at 1Gbit/s, even though the MAC
doesn't support it.
In order to avoid this, we mark phydev->supported if we're running a
non-Gigabit capable hardware.
Tested on a SH7786 platform, with a Gigabit PHY.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index db72d13cebb9..0074c5998481 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -1892,6 +1892,11 @@ static int sh_eth_phy_init(struct net_device *ndev)
return PTR_ERR(phydev);
}
+ /* mask with MAC supported features */
+ if (mdp->cd->register_type != SH_ETH_REG_GIGABIT)
+ phydev->supported &= PHY_BASIC_FEATURES;
+ phydev->advertising = phydev->supported;
+
phy_attached_info(phydev);
return 0;
--
2.13.6
^ permalink raw reply related
* Re: [PATCH net-next 0/8] Hisilicon Network Subsystem 3 VF Ethernet Driver
From: Or Gerlitz @ 2017-12-04 14:07 UTC (permalink / raw)
To: Salil Mehta
Cc: davem@davemloft.net, Zhuangyuzeng (Yisen), lipeng (Y),
mehta.salil.lnk@gmail.com, netdev@vger.kernel.org,
andy@greyhouse.net, simon.horman@netronome.com, jiri@mellanox.com
In-Reply-To: <F4CC6FACFEB3C54C9141D49AD221F7F93BB5F986@FRAEML521-MBX.china.huawei.com>
On Mon, Dec 4, 2017 at 12:48 PM, Salil Mehta <salil.mehta@huawei.com> wrote:
>> From: Or Gerlitz <gerlitz.or@gmail.com>
>> I wasn't sure if/what PF side control plane user space APIs are
>> supported by this series, can you elaborate on that?
> Current VF driver just supports very basic functionality. Idea was to
> push basic RX/TX/RSS/TSO support along with this version.
I was referring to PF driver knobs to manage the e-switch, not to the VF driver.
> We will support switchdev and other lined-up features after basic
> version of the driver has gone through. We are in analysis phase right
> now for all of these features.
sounds good. You might want to consider supporting 1st or only the
switchdev mode, this will make your PF driver a community leader..
Or.
^ permalink raw reply
* [PATCH 2/2] net: sh_eth: don't use NULL as "struct device" for the DMA mapping API
From: Thomas Petazzoni @ 2017-12-04 13:33 UTC (permalink / raw)
To: David S. Miller, Sergei Shtylyov, Niklas Söderlund,
Geert Uytterhoeven, Simon Horman
Cc: netdev, linux-renesas-soc, Thomas Petazzoni
In-Reply-To: <20171204133327.3505-1-thomas.petazzoni@free-electrons.com>
Using NULL as argument for the DMA mapping API is bogus, as the DMA
mapping API may use information from the "struct device" to perform
the DMA mapping operation. Therefore, pass the appropriate "struct
device".
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 91e918e654fe..db72d13cebb9 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -1187,7 +1187,7 @@ static void sh_eth_ring_free(struct net_device *ndev)
}
}
ringsize = sizeof(struct sh_eth_rxdesc) * mdp->num_rx_ring;
- dma_free_coherent(NULL, ringsize, mdp->rx_ring,
+ dma_free_coherent(&mdp->pdev->dev, ringsize, mdp->rx_ring,
mdp->rx_desc_dma);
mdp->rx_ring = NULL;
}
@@ -1204,7 +1204,7 @@ static void sh_eth_ring_free(struct net_device *ndev)
sh_eth_tx_free(ndev, false);
ringsize = sizeof(struct sh_eth_txdesc) * mdp->num_tx_ring;
- dma_free_coherent(NULL, ringsize, mdp->tx_ring,
+ dma_free_coherent(&mdp->pdev->dev, ringsize, mdp->tx_ring,
mdp->tx_desc_dma);
mdp->tx_ring = NULL;
}
@@ -1324,8 +1324,8 @@ static int sh_eth_ring_init(struct net_device *ndev)
/* Allocate all Rx descriptors. */
rx_ringsize = sizeof(struct sh_eth_rxdesc) * mdp->num_rx_ring;
- mdp->rx_ring = dma_alloc_coherent(NULL, rx_ringsize, &mdp->rx_desc_dma,
- GFP_KERNEL);
+ mdp->rx_ring = dma_alloc_coherent(&mdp->pdev->dev, rx_ringsize,
+ &mdp->rx_desc_dma, GFP_KERNEL);
if (!mdp->rx_ring)
goto ring_free;
@@ -1333,8 +1333,8 @@ static int sh_eth_ring_init(struct net_device *ndev)
/* Allocate all Tx descriptors. */
tx_ringsize = sizeof(struct sh_eth_txdesc) * mdp->num_tx_ring;
- mdp->tx_ring = dma_alloc_coherent(NULL, tx_ringsize, &mdp->tx_desc_dma,
- GFP_KERNEL);
+ mdp->tx_ring = dma_alloc_coherent(&mdp->pdev->dev, tx_ringsize,
+ &mdp->tx_desc_dma, GFP_KERNEL);
if (!mdp->tx_ring)
goto ring_free;
return 0;
--
2.13.6
^ permalink raw reply related
* [PATCH 0/2] net: sh_eth: DMA mapping API fixes
From: Thomas Petazzoni @ 2017-12-04 13:33 UTC (permalink / raw)
To: David S. Miller, Sergei Shtylyov, Niklas Söderlund,
Geert Uytterhoeven, Simon Horman
Cc: netdev, linux-renesas-soc, Thomas Petazzoni
Hello,
Here are two patches that fix how the sh_eth driver is using the DMA
mapping API: a bogus struct device is used in some places, or a NULL
struct device is used.
Best regards,
Thomas
Thomas Petazzoni (2):
net: sh_eth: use correct "struct device" when calling DMA mapping
functions
net: sh_eth: don't use NULL as "struct device" for the DMA mapping API
drivers/net/ethernet/renesas/sh_eth.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
--
2.13.6
^ permalink raw reply
* [PATCH 1/2] net: sh_eth: use correct "struct device" when calling DMA mapping functions
From: Thomas Petazzoni @ 2017-12-04 13:33 UTC (permalink / raw)
To: David S. Miller, Sergei Shtylyov, Niklas Söderlund,
Geert Uytterhoeven, Simon Horman
Cc: netdev, linux-renesas-soc, Thomas Petazzoni
In-Reply-To: <20171204133327.3505-1-thomas.petazzoni@free-electrons.com>
There are two types of "struct device": the one representing the
physical device on its physical bus (platform, SPI, PCI, etc.), and
the one representing the logical device in its device class (net,
etc.).
The DMA mapping API expects to receive as argument a "struct device"
representing the physical device, as the "struct device" contains
information about the bus that the DMA API needs.
However, the sh_eth driver mistakenly uses the "struct device"
representing the logical device (embedded in "struct net_device")
rather than the "struct device" representing the physical device on
its bus.
This commit fixes that by adjusting all calls to the DMA mapping API.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/net/ethernet/renesas/sh_eth.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 7e060aa9fbed..91e918e654fe 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -1149,7 +1149,8 @@ static int sh_eth_tx_free(struct net_device *ndev, bool sent_only)
entry, le32_to_cpu(txdesc->status));
/* Free the original skb. */
if (mdp->tx_skbuff[entry]) {
- dma_unmap_single(&ndev->dev, le32_to_cpu(txdesc->addr),
+ dma_unmap_single(&mdp->pdev->dev,
+ le32_to_cpu(txdesc->addr),
le32_to_cpu(txdesc->len) >> 16,
DMA_TO_DEVICE);
dev_kfree_skb_irq(mdp->tx_skbuff[entry]);
@@ -1179,7 +1180,7 @@ static void sh_eth_ring_free(struct net_device *ndev)
if (mdp->rx_skbuff[i]) {
struct sh_eth_rxdesc *rxdesc = &mdp->rx_ring[i];
- dma_unmap_single(&ndev->dev,
+ dma_unmap_single(&mdp->pdev->dev,
le32_to_cpu(rxdesc->addr),
ALIGN(mdp->rx_buf_sz, 32),
DMA_FROM_DEVICE);
@@ -1245,9 +1246,9 @@ static void sh_eth_ring_format(struct net_device *ndev)
/* The size of the buffer is a multiple of 32 bytes. */
buf_len = ALIGN(mdp->rx_buf_sz, 32);
- dma_addr = dma_map_single(&ndev->dev, skb->data, buf_len,
+ dma_addr = dma_map_single(&mdp->pdev->dev, skb->data, buf_len,
DMA_FROM_DEVICE);
- if (dma_mapping_error(&ndev->dev, dma_addr)) {
+ if (dma_mapping_error(&mdp->pdev->dev, dma_addr)) {
kfree_skb(skb);
break;
}
@@ -1527,7 +1528,7 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
mdp->rx_skbuff[entry] = NULL;
if (mdp->cd->rpadir)
skb_reserve(skb, NET_IP_ALIGN);
- dma_unmap_single(&ndev->dev, dma_addr,
+ dma_unmap_single(&mdp->pdev->dev, dma_addr,
ALIGN(mdp->rx_buf_sz, 32),
DMA_FROM_DEVICE);
skb_put(skb, pkt_len);
@@ -1555,9 +1556,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
if (skb == NULL)
break; /* Better luck next round. */
sh_eth_set_receive_align(skb);
- dma_addr = dma_map_single(&ndev->dev, skb->data,
+ dma_addr = dma_map_single(&mdp->pdev->dev, skb->data,
buf_len, DMA_FROM_DEVICE);
- if (dma_mapping_error(&ndev->dev, dma_addr)) {
+ if (dma_mapping_error(&mdp->pdev->dev, dma_addr)) {
kfree_skb(skb);
break;
}
@@ -2441,9 +2442,9 @@ static int sh_eth_start_xmit(struct sk_buff *skb, struct net_device *ndev)
/* soft swap. */
if (!mdp->cd->hw_swap)
sh_eth_soft_swap(PTR_ALIGN(skb->data, 4), skb->len + 2);
- dma_addr = dma_map_single(&ndev->dev, skb->data, skb->len,
+ dma_addr = dma_map_single(&mdp->pdev->dev, skb->data, skb->len,
DMA_TO_DEVICE);
- if (dma_mapping_error(&ndev->dev, dma_addr)) {
+ if (dma_mapping_error(&mdp->pdev->dev, dma_addr)) {
kfree_skb(skb);
return NETDEV_TX_OK;
}
--
2.13.6
^ permalink raw reply related
* [PULL] vhost: cleanups and fixes
From: Michael S. Tsirkin @ 2017-12-04 13:25 UTC (permalink / raw)
To: Linus Torvalds
Cc: mhocko, kvm, mst, penguin-kernel, netdev, linux-kernel, stable,
virtualization, zhangweiping, marcandre.lureau, jstancek
The following changes since commit c1d0c3f623ada808904dec676da0126f5b800630:
fw_cfg: fix the command line module name (2017-11-14 23:57:40 +0200)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
for you to fetch changes up to d9e427f6ab8142d6868eb719e6a7851aafea56b6:
virtio_balloon: fix increment of vb->num_pfns in fill_balloon() (2017-12-01 16:55:45 +0200)
----------------------------------------------------------------
virtio,qemu: bugfixes
A couple of bugfixes that just became ready.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
----------------------------------------------------------------
Jan Stancek (1):
virtio_balloon: fix increment of vb->num_pfns in fill_balloon()
Marc-André Lureau (1):
fw_cfg: fix driver remove
weiping zhang (1):
virtio: release virtio index when fail to device_register
drivers/firmware/qemu_fw_cfg.c | 3 ++-
drivers/virtio/virtio.c | 2 ++
drivers/virtio/virtio_balloon.c | 3 +--
3 files changed, 5 insertions(+), 3 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next 1/5] libbpf: add ability to guess program type based on section name
From: Roman Gushchin @ 2017-12-04 13:22 UTC (permalink / raw)
To: Quentin Monnet
Cc: Jakub Kicinski, netdev, linux-kernel, kernel-team, ast, daniel,
kafai
In-Reply-To: <1ff6b535-067b-26b1-7c37-f0211749af99@netronome.com>
On Mon, Dec 04, 2017 at 01:12:33PM +0000, Quentin Monnet wrote:
> 2017-12-04 12:34 UTC+0000 ~ Roman Gushchin <guro@fb.com>
> > On Fri, Dec 01, 2017 at 02:46:06PM -0800, Jakub Kicinski wrote:
> >> On Fri, 1 Dec 2017 10:22:57 +0000, Quentin Monnet wrote:
> >>> Thanks Roman!
> >>> One comment in-line.
> >>>
> >>> 2017-11-30 13:42 UTC+0000 ~ Roman Gushchin <guro@fb.com>
> >>>> The bpf_prog_load() function will guess program type if it's not
> >>>> specified explicitly. This functionality will be used to implement
> >>>> loading of different programs without asking a user to specify
> >>>> the program type. In first order it will be used by bpftool.
> >>>>
> >>>> Signed-off-by: Roman Gushchin <guro@fb.com>
> >>>> Cc: Alexei Starovoitov <ast@kernel.org>
> >>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
> >>>> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> >>>> ---
> >>>> tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>> 1 file changed, 47 insertions(+)
> >>>>
> >>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >>>> index 5aa45f89da93..9f2410beaa18 100644
> >>>> --- a/tools/lib/bpf/libbpf.c
> >>>> +++ b/tools/lib/bpf/libbpf.c
> >>>> @@ -1721,6 +1721,41 @@ BPF_PROG_TYPE_FNS(tracepoint, BPF_PROG_TYPE_TRACEPOINT);
> >>>> BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP);
> >>>> BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT);
> >>>>
> >>>> +static enum bpf_prog_type bpf_program__guess_type(struct bpf_program *prog)
> >>>> +{
> >>>> + if (!prog->section_name)
> >>>> + goto err;
> >>>> +
> >>>> + if (strncmp(prog->section_name, "socket", 6) == 0)
> >>>> + return BPF_PROG_TYPE_SOCKET_FILTER;
> >>>> + if (strncmp(prog->section_name, "kprobe/", 7) == 0)
> >>>> + return BPF_PROG_TYPE_KPROBE;
> >>>> + if (strncmp(prog->section_name, "kretprobe/", 10) == 0)
> >>>> + return BPF_PROG_TYPE_KPROBE;
> >>>> + if (strncmp(prog->section_name, "tracepoint/", 11) == 0)
> >>>> + return BPF_PROG_TYPE_TRACEPOINT;
> >>>> + if (strncmp(prog->section_name, "xdp", 3) == 0)
> >>>> + return BPF_PROG_TYPE_XDP;
> >>>> + if (strncmp(prog->section_name, "perf_event", 10) == 0)
> >>>> + return BPF_PROG_TYPE_PERF_EVENT;
> >>>> + if (strncmp(prog->section_name, "cgroup/skb", 10) == 0)
> >>>> + return BPF_PROG_TYPE_CGROUP_SKB;
> >>>> + if (strncmp(prog->section_name, "cgroup/sock", 11) == 0)
> >>>> + return BPF_PROG_TYPE_CGROUP_SOCK;
> >>>> + if (strncmp(prog->section_name, "cgroup/dev", 10) == 0)
> >>>> + return BPF_PROG_TYPE_CGROUP_DEVICE;
> >>>> + if (strncmp(prog->section_name, "sockops", 7) == 0)
> >>>> + return BPF_PROG_TYPE_SOCK_OPS;
> >>>> + if (strncmp(prog->section_name, "sk_skb", 6) == 0)
> >>>> + return BPF_PROG_TYPE_SK_SKB;
> >>>
> >>> I do not really like these hard-coded lengths, maybe we could work out
> >>> something nicer with a bit of pre-processing work? Perhaps something like:
> >>>
> >>> #define SOCKET_FILTER_SEC_PREFIX "socket"
> >>> #define KPROBE_SEC_PREFIX "kprobe/"
> >>> […]
> >>>
> >>> #define TRY_TYPE(string, __TYPE) \
> >>> do { \
> >>> if (!strncmp(string, __TYPE ## _SEC_PREFIX, \
> >>> sizeof(__TYPE ## _SEC_PREFIX))) \
> >>> return BPF_PROG_TYPE_ ## __TYPE; \
> >>> } while(0);
> >>
> >> I like the suggestion, but I think return and goto statements hiding
> >> inside macros are slightly frowned upon in the netdev. Perhaps just
> >> a macro that wraps the strncmp() with sizeof would be enough? Without
> >> the return inside?
> >
> > Hm, I'm not sure that using macroses here makes the code much easier to read.
> > Maybe we can use just strcmp() instead?
> > As we compare with hardcoded strings, there is no real difference.
> > Something like this:
> >
> > if (!strcmp("socket", prog->section_name))
> > return BPF_PROG_TYPE_SOCKET_FILTER;
> > if (!strcmp("kprobe/", prog->section_name))
> > return BPF_PROG_TYPE_KPROBE;
> > if (!strcmp("kretprobe/", prog->section_name))
> > return BPF_PROG_TYPE_KPROBE;
> > if (!strcmp("tracepoint/", prog->section_name))
> > return BPF_PROG_TYPE_TRACEPOINT;
> > if (!strcmp("xdp", prog->section_name))
> > return BPF_PROG_TYPE_XDP;
> > if (!strcmp("perf_event", prog->section_name))
> > return BPF_PROG_TYPE_PERF_EVENT;
> > if (!strcmp("cgroup/skb", prog->section_name))
> > return BPF_PROG_TYPE_CGROUP_SKB;
> > if (!strcmp("cgroup/sock", prog->section_name))
> > return BPF_PROG_TYPE_CGROUP_SOCK;
> > if (!strcmp("cgroup/dev", prog->section_name))
> > return BPF_PROG_TYPE_CGROUP_DEVICE;
> > if (!strcmp("sockops", prog->section_name))
> > return BPF_PROG_TYPE_SOCK_OPS;
> > if (!strcmp("sk_skb", prog->section_name))
> > return BPF_PROG_TYPE_SK_SKB;
> >
> > Thanks!
> >
>
> I do not believe this works. For kprobes for example, the idea was to
> compare "kprobe/" with only the beginning of prog->section_name, right?
> The string prog->section_name is supposed to be longer than this (e.g.
> "kprobe/sys_write"), and strcmp() will not detect a match in this case.
Ah, true, I've missed this part.
Ok, I'll try to master something with macroses.
Thanks!
^ permalink raw reply
* Re: [bisected] x86 boot still broken on -rc2
From: Prarit Bhargava @ 2017-12-04 13:13 UTC (permalink / raw)
To: Jakub Kicinski, LKML; +Cc: netdev@vger.kernel.org, Thomas Gleixner
In-Reply-To: <58b4f89a-63c2-b8bd-4414-fbc312c52697@redhat.com>
On 12/04/2017 07:28 AM, Prarit Bhargava wrote:
>
>
> On 12/03/2017 08:28 PM, Jakub Kicinski wrote:
>> Same thing on rc2, bisected down to:
>>
>> commit b4c0a7326f5dc0ef7a64128b0ae7d081f4b2cbd1 (refs/bisect/bad)
>> Author: Prarit Bhargava <prarit@redhat.com>
>> Date: Tue Nov 14 07:42:57 2017 -0500
>>
>> x86/smpboot: Fix __max_logical_packages estimate
>>
>> A system booted with a small number of cores enabled per package
>> panics because the estimate of __max_logical_packages is too low.
>>
>> This occurs when the total number of active cores across all packages is
>> less than the maximum core count for a single package. e.g.:
>>
>> On a 4 package system with 20 cores/package where only 4 cores are
>> enabled on each package, the value of __max_logical_packages is
>> calculated as DIV_ROUND_UP(16 / 20) = 1 and not 4.
>>
>> Calculate __max_logical_packages after the cpu enumeration has completed.
>> Use the boot cpu's data to extrapolate the number of packages.
>>
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Cc: Andi Kleen <ak@linux.intel.com>
>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Kan Liang <kan.liang@intel.com>
>> Cc: He Chen <he.chen@linux.intel.com>
>> Cc: Stephane Eranian <eranian@google.com>
>> Cc: Dave Hansen <dave.hansen@intel.com>
>> Cc: Piotr Luc <piotr.luc@intel.com>
>> Cc: Andy Lutomirski <luto@kernel.org>
>> Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
>> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: Tim Chen <tim.c.chen@linux.intel.com>
>> Cc: Mathias Krause <minipli@googlemail.com>
>> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> Link: https://lkml.kernel.org/r/20171114124257.22013-4-prarit@redhat.com
>>
>>
>> On Fri, 1 Dec 2017 16:39:54 -0800, Jakub Kicinski wrote:
>>> Hi!
>>>
>>> I'm hitting these after DaveM pulled rc1 into net-next on my Xeon
>>> E5-2630 v4 box. It also happens on linux-next. Did anyone else
>>> experience it? (.config attached)
>>>
>>> [ 5.003771] WARNING: CPU: 14 PID: 1 at ../arch/x86/events/intel/uncore.c:936 uncore_pci_probe+0x285/0x2b0
>>> [ 5.007544] Modules linked in:
>>> [ 5.007544] CPU: 14 PID: 1 Comm: swapper/0 Not tainted 4.15.0-rc1-perf-00225-gb2a4e0a76b1d #782
>>> [ 5.007544] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.3.4 11/08/2016
>
> I have a Dell R730 available for use. OOC are you booting with the default
> BIOS options?
>
Jakub, I was able to reproduce this on a similar system by DISABLING
hyperthreading in the BIOS. Doing this on other systems seems to have no
impact. What is odd about this system when booting is that the
kernel claims that hyperthreading is ENABLED:
x86: Booting SMP configuration:
.... node #0, CPUs: #1 #2 #3 #4
.... node #1, CPUs: #5 #6 #7 #8 #9
.... node #0, CPUs: #10 #11 #12 #13 #14
.... node #1, CPUs: #15 #16 #17 #18 #19
smp: Brought up 2 nodes, 20 CPUs
smpboot: Max logical packages: 1
which means that the calculation of logical packages is wrong because
ncpus = cpu_data(0).booted_cores * smp_num_siblings;
ncpus = 10 * 2;
ncpus = 20;
smp_num_siblings is defined as "The number of threads in a core" which
should be 1 if HT/SMT is disabled.
It looks like my patch has exposed a bug in the
smp_num_siblings calculation. I'm still debugging ...
FWIW, I did test this code on systems by disabling HT/SMT in BIOS on
several systems. I have tested those systems again and don't see a
problem. It is something peculiar to a few systems.
P.
^ permalink raw reply
* Re: [PATCH net-next 1/5] libbpf: add ability to guess program type based on section name
From: Quentin Monnet @ 2017-12-04 13:12 UTC (permalink / raw)
To: Roman Gushchin, Jakub Kicinski
Cc: netdev, linux-kernel, kernel-team, ast, daniel, kafai
In-Reply-To: <20171204123430.GA6644@castle.DHCP.thefacebook.com>
2017-12-04 12:34 UTC+0000 ~ Roman Gushchin <guro@fb.com>
> On Fri, Dec 01, 2017 at 02:46:06PM -0800, Jakub Kicinski wrote:
>> On Fri, 1 Dec 2017 10:22:57 +0000, Quentin Monnet wrote:
>>> Thanks Roman!
>>> One comment in-line.
>>>
>>> 2017-11-30 13:42 UTC+0000 ~ Roman Gushchin <guro@fb.com>
>>>> The bpf_prog_load() function will guess program type if it's not
>>>> specified explicitly. This functionality will be used to implement
>>>> loading of different programs without asking a user to specify
>>>> the program type. In first order it will be used by bpftool.
>>>>
>>>> Signed-off-by: Roman Gushchin <guro@fb.com>
>>>> Cc: Alexei Starovoitov <ast@kernel.org>
>>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>>>> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
>>>> ---
>>>> tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 47 insertions(+)
>>>>
>>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>>> index 5aa45f89da93..9f2410beaa18 100644
>>>> --- a/tools/lib/bpf/libbpf.c
>>>> +++ b/tools/lib/bpf/libbpf.c
>>>> @@ -1721,6 +1721,41 @@ BPF_PROG_TYPE_FNS(tracepoint, BPF_PROG_TYPE_TRACEPOINT);
>>>> BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP);
>>>> BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT);
>>>>
>>>> +static enum bpf_prog_type bpf_program__guess_type(struct bpf_program *prog)
>>>> +{
>>>> + if (!prog->section_name)
>>>> + goto err;
>>>> +
>>>> + if (strncmp(prog->section_name, "socket", 6) == 0)
>>>> + return BPF_PROG_TYPE_SOCKET_FILTER;
>>>> + if (strncmp(prog->section_name, "kprobe/", 7) == 0)
>>>> + return BPF_PROG_TYPE_KPROBE;
>>>> + if (strncmp(prog->section_name, "kretprobe/", 10) == 0)
>>>> + return BPF_PROG_TYPE_KPROBE;
>>>> + if (strncmp(prog->section_name, "tracepoint/", 11) == 0)
>>>> + return BPF_PROG_TYPE_TRACEPOINT;
>>>> + if (strncmp(prog->section_name, "xdp", 3) == 0)
>>>> + return BPF_PROG_TYPE_XDP;
>>>> + if (strncmp(prog->section_name, "perf_event", 10) == 0)
>>>> + return BPF_PROG_TYPE_PERF_EVENT;
>>>> + if (strncmp(prog->section_name, "cgroup/skb", 10) == 0)
>>>> + return BPF_PROG_TYPE_CGROUP_SKB;
>>>> + if (strncmp(prog->section_name, "cgroup/sock", 11) == 0)
>>>> + return BPF_PROG_TYPE_CGROUP_SOCK;
>>>> + if (strncmp(prog->section_name, "cgroup/dev", 10) == 0)
>>>> + return BPF_PROG_TYPE_CGROUP_DEVICE;
>>>> + if (strncmp(prog->section_name, "sockops", 7) == 0)
>>>> + return BPF_PROG_TYPE_SOCK_OPS;
>>>> + if (strncmp(prog->section_name, "sk_skb", 6) == 0)
>>>> + return BPF_PROG_TYPE_SK_SKB;
>>>
>>> I do not really like these hard-coded lengths, maybe we could work out
>>> something nicer with a bit of pre-processing work? Perhaps something like:
>>>
>>> #define SOCKET_FILTER_SEC_PREFIX "socket"
>>> #define KPROBE_SEC_PREFIX "kprobe/"
>>> […]
>>>
>>> #define TRY_TYPE(string, __TYPE) \
>>> do { \
>>> if (!strncmp(string, __TYPE ## _SEC_PREFIX, \
>>> sizeof(__TYPE ## _SEC_PREFIX))) \
>>> return BPF_PROG_TYPE_ ## __TYPE; \
>>> } while(0);
>>
>> I like the suggestion, but I think return and goto statements hiding
>> inside macros are slightly frowned upon in the netdev. Perhaps just
>> a macro that wraps the strncmp() with sizeof would be enough? Without
>> the return inside?
>
> Hm, I'm not sure that using macroses here makes the code much easier to read.
> Maybe we can use just strcmp() instead?
> As we compare with hardcoded strings, there is no real difference.
> Something like this:
>
> if (!strcmp("socket", prog->section_name))
> return BPF_PROG_TYPE_SOCKET_FILTER;
> if (!strcmp("kprobe/", prog->section_name))
> return BPF_PROG_TYPE_KPROBE;
> if (!strcmp("kretprobe/", prog->section_name))
> return BPF_PROG_TYPE_KPROBE;
> if (!strcmp("tracepoint/", prog->section_name))
> return BPF_PROG_TYPE_TRACEPOINT;
> if (!strcmp("xdp", prog->section_name))
> return BPF_PROG_TYPE_XDP;
> if (!strcmp("perf_event", prog->section_name))
> return BPF_PROG_TYPE_PERF_EVENT;
> if (!strcmp("cgroup/skb", prog->section_name))
> return BPF_PROG_TYPE_CGROUP_SKB;
> if (!strcmp("cgroup/sock", prog->section_name))
> return BPF_PROG_TYPE_CGROUP_SOCK;
> if (!strcmp("cgroup/dev", prog->section_name))
> return BPF_PROG_TYPE_CGROUP_DEVICE;
> if (!strcmp("sockops", prog->section_name))
> return BPF_PROG_TYPE_SOCK_OPS;
> if (!strcmp("sk_skb", prog->section_name))
> return BPF_PROG_TYPE_SK_SKB;
>
> Thanks!
>
I do not believe this works. For kprobes for example, the idea was to
compare "kprobe/" with only the beginning of prog->section_name, right?
The string prog->section_name is supposed to be longer than this (e.g.
"kprobe/sys_write"), and strcmp() will not detect a match in this case.
Quentin
^ permalink raw reply
* [PATCH][next] netdevsim: make functions nsim_bpf_create_prog and nsim_bpf_destroy_prog static
From: Colin King @ 2017-12-04 12:56 UTC (permalink / raw)
To: Jakub Kicinski, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Functions nsim_bpf_create_prog and nsim_bpf_destroy_prog are local to the
source and do not need to be in global scope, so make them static.
Cleans up sparse warnings:
symbol 'nsim_bpf_create_prog' was not declared. Should it be static?
symbol 'nsim_bpf_destroy_prog' was not declared. Should it be static?
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/netdevsim/bpf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 8e4398a50903..078d2c37a6c1 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -197,7 +197,7 @@ static int nsim_xdp_set_prog(struct netdevsim *ns, struct netdev_bpf *bpf)
return 0;
}
-int nsim_bpf_create_prog(struct netdevsim *ns, struct bpf_prog *prog)
+static int nsim_bpf_create_prog(struct netdevsim *ns, struct bpf_prog *prog)
{
struct nsim_bpf_bound_prog *state;
char name[16];
@@ -232,7 +232,7 @@ int nsim_bpf_create_prog(struct netdevsim *ns, struct bpf_prog *prog)
return 0;
}
-void nsim_bpf_destroy_prog(struct bpf_prog *prog)
+static void nsim_bpf_destroy_prog(struct bpf_prog *prog)
{
struct nsim_bpf_bound_prog *state;
--
2.14.1
^ permalink raw reply related
* Re: [Patch net] tipc: fix a null pointer deref on error path
From: Ying Xue @ 2017-12-04 12:43 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: tipc-discussion
In-Reply-To: <20171204035406.32031-1-xiyou.wangcong@gmail.com>
On 12/04/2017 11:54 AM, Cong Wang wrote:
> In tipc_topsrv_kern_subscr() when s->tipc_conn_new() fails
> we call tipc_close_conn() to clean up, but in this case
> con->usr_data is NULL, tipc_subscrb_delete() should be skipped.
>
> This fixes the folllowing crash:
>
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] SMP KASAN
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 0 PID: 3085 Comm: syzkaller064164 Not tainted 4.15.0-rc1+ #137
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> task: 00000000c24413a5 task.stack: 000000005e8160b5
> RIP: 0010:__lock_acquire+0xd55/0x47f0 kernel/locking/lockdep.c:3378
> RSP: 0018:ffff8801cb5474a8 EFLAGS: 00010002
> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: 0000000000000004 RSI: 0000000000000000 RDI: ffffffff85ecb400
> RBP: ffff8801cb547830 R08: 0000000000000001 R09: 0000000000000000
> R10: 0000000000000000 R11: ffffffff87489d60 R12: ffff8801cd2980c0
> R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000020
> FS: 00000000014ee880(0000) GS:ffff8801db400000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007ffee2426e40 CR3: 00000001cb85a000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:4004
> __raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
> _raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:175
> spin_lock_bh include/linux/spinlock.h:320 [inline]
> tipc_subscrb_subscrp_delete+0x8f/0x470 net/tipc/subscr.c:201
> tipc_subscrb_delete net/tipc/subscr.c:238 [inline]
> tipc_subscrb_release_cb+0x17/0x30 net/tipc/subscr.c:316
> tipc_close_conn+0x171/0x270 net/tipc/server.c:204
> tipc_topsrv_kern_subscr+0x724/0x810 net/tipc/server.c:514
> tipc_group_create+0x702/0x9c0 net/tipc/group.c:184
> tipc_sk_join net/tipc/socket.c:2747 [inline]
> tipc_setsockopt+0x249/0xc10 net/tipc/socket.c:2861
> SYSC_setsockopt net/socket.c:1851 [inline]
> SyS_setsockopt+0x189/0x360 net/socket.c:1830
> entry_SYSCALL_64_fastpath+0x1f/0x96
>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Cc: Jon Maloy <jon.maloy@ericsson.com>
> Cc: Ying Xue <ying.xue@windriver.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> net/tipc/subscr.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
> index 251065dfd8df..2402a019b8ec 100644
> --- a/net/tipc/subscr.c
> +++ b/net/tipc/subscr.c
> @@ -235,6 +235,8 @@ static struct tipc_subscriber *tipc_subscrb_create(int conid)
>
> static void tipc_subscrb_delete(struct tipc_subscriber *subscriber)
> {
> + if (!subscriber)
> + return;
The real root cause of this crash is because when it was failed to
create a connection through s->tipc_conn_new() in
tipc_topsrv_kern_subscr(), we just should release the connection
instance with conn_put() rather than calling tipc_close_conn() to close
the connection. So a proper fix might look like:
diff --git a/net/tipc/server.c b/net/tipc/server.c
index acaef80..9f7afad 100644
--- a/net/tipc/server.c
+++ b/net/tipc/server.c
@@ -508,15 +508,15 @@ bool tipc_topsrv_kern_subscr(struct net *net, u32
port, u32 type,
return false;
*conid = con->conid;
+ con->sock = NULL;
s = con->server;
scbr = s->tipc_conn_new(*conid);
if (!scbr) {
- tipc_close_conn(con);
+ conn_put(con);
return false;
}
con->usr_data = scbr;
- con->sock = NULL;
s->tipc_conn_recvmsg(net, *conid, NULL, scbr, &sub, sizeof(sub));
return true;
}
> tipc_subscrb_subscrp_delete(subscriber, NULL);
> tipc_subscrb_put(subscriber);
> }
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* [PATCH v4.1] phylib: Add device reset GPIO support
From: Geert Uytterhoeven @ 2017-12-04 12:35 UTC (permalink / raw)
To: David S . Miller, Andrew Lunn, Florian Fainelli, Simon Horman,
Magnus Damm
Cc: Sergei Shtylyov, Rob Herring, Mark Rutland, Nicolas Ferre,
Richard Leitner, netdev, devicetree, linux-renesas-soc,
linux-kernel, Geert Uytterhoeven
In-Reply-To: <1512383692-14009-2-git-send-email-geert+renesas@glider.be>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
The PHY devices sometimes do have their reset signal (maybe even power
supply?) tied to some GPIO and sometimes it also does happen that a boot
loader does not leave it deasserted. So far this issue has been attacked
from (as I believe) a wrong angle: by teaching the MAC driver to manipulate
the GPIO in question; that solution, when applied to the device trees, led
to adding the PHY reset GPIO properties to the MAC device node, with one
exception: Cadence MACB driver which could handle the "reset-gpios" prop
in a PHY device subnode. I believe that the correct approach is to teach
the 'phylib' to get the MDIO device reset GPIO from the device tree node
corresponding to this device -- which this patch is doing...
Note that I had to modify the AT803x PHY driver as it would stop working
otherwise -- it made use of the reset GPIO for its own purposes...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Acked-by: Rob Herring <robh@kernel.org>
[geert: Propagate actual errors from fwnode_get_named_gpiod()]
[geert: Avoid destroying initial setup]
[geert: Consolidate GPIO descriptor acquiring code]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v4.1:
- Fix uninitialized variable in mdiobus_register_device(),
v4:
- Remove Florian's Acked-by,
- Add missing #include <linux/gpio/consumer.h>,
- Re-add the gpiod check, as the dummy gpiod_set_value() for !GPIOLIB
does not ignore NULL, and calls WARN_ON(1),
- Do not reassert the reset signal if {mdio,phy}_probe() or
phy_device_register() succeeded, as that may destroy initial setup,
- Do not deassert the reset signal in {mdio,phy}_remove(), as it
should already be deasserted,
- Bring the PHY back into reset state in phy_device_remove(),
- Move/consolidate GPIO descriptor acquiring code from
of_mdiobus_register_phy() and of_mdiobus_register_device() to
mdiobus_register_device().
Note that this changes behavior slightly, in that the reset signal
is now also asserted when called from of_mdiobus_register_device().
v3:
- Fix fwnode_get_named_gpiod() call due to added parameters (which
allowed to eliminate the gpiod_direction_output() call),
- Undelete one blank line in the AT803x driver,
- Resolve rejects, refresh patch,
- Reword/reformat changelog,
- Take over from Sergei,
- Add Acked-by,
- Remove unneeded gpiod check, as gpiod_set_value() handles NULL fine,
- Handle fwnode_get_named_gpiod() errors correctly:
- -ENOENT is ignored (the GPIO is optional), and turned into NULL,
which allowed to remove all later !IS_ERR() checks,
- Other errors (incl. -EPROBE_DEFER) are propagated [*],
v2:
- Reformat changelog,
- Resolve rejects, refresh patch.
---
Documentation/devicetree/bindings/net/phy.txt | 2 ++
drivers/net/phy/at803x.c | 18 +++------------
drivers/net/phy/mdio_bus.c | 21 ++++++++++++++++++
drivers/net/phy/mdio_device.c | 25 +++++++++++++++++++--
drivers/net/phy/phy_device.c | 32 +++++++++++++++++++++++++--
include/linux/mdio.h | 3 +++
include/linux/phy.h | 5 +++++
7 files changed, 87 insertions(+), 19 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 77d0b2a61ffa96fc..c05479f5ac7cc837 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -53,6 +53,8 @@ Optional Properties:
to ensure the integrated PHY is used. The absence of this property indicates
the muxers should be configured so that the external PHY is used.
+- reset-gpios: The GPIO phandle and specifier for the PHY reset signal.
+
Example:
ethernet-phy@0 {
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index de7dd6566df7a364..29da7a3c7a3761c0 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -71,7 +71,6 @@ MODULE_LICENSE("GPL");
struct at803x_priv {
bool phy_reset:1;
- struct gpio_desc *gpiod_reset;
};
struct at803x_context {
@@ -254,22 +253,11 @@ static int at803x_probe(struct phy_device *phydev)
{
struct device *dev = &phydev->mdio.dev;
struct at803x_priv *priv;
- struct gpio_desc *gpiod_reset;
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
- if (phydev->drv->phy_id != ATH8030_PHY_ID)
- goto does_not_require_reset_workaround;
-
- gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(gpiod_reset))
- return PTR_ERR(gpiod_reset);
-
- priv->gpiod_reset = gpiod_reset;
-
-does_not_require_reset_workaround:
phydev->priv = priv;
return 0;
@@ -343,14 +331,14 @@ static void at803x_link_change_notify(struct phy_device *phydev)
* cannot recover from by software.
*/
if (phydev->state == PHY_NOLINK) {
- if (priv->gpiod_reset && !priv->phy_reset) {
+ if (phydev->mdio.reset && !priv->phy_reset) {
struct at803x_context context;
at803x_context_save(phydev, &context);
- gpiod_set_value(priv->gpiod_reset, 1);
+ phy_device_reset(phydev, 1);
msleep(1);
- gpiod_set_value(priv->gpiod_reset, 0);
+ phy_device_reset(phydev, 0);
msleep(1);
at803x_context_restore(phydev, &context);
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 2df7b62c1a36811e..8f8b7747c54bc478 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -38,6 +38,7 @@
#include <linux/phy.h>
#include <linux/io.h>
#include <linux/uaccess.h>
+#include <linux/gpio/consumer.h>
#include <asm/irq.h>
@@ -48,9 +49,26 @@
int mdiobus_register_device(struct mdio_device *mdiodev)
{
+ struct gpio_desc *gpiod = NULL;
+
if (mdiodev->bus->mdio_map[mdiodev->addr])
return -EBUSY;
+ /* Deassert the optional reset signal */
+ if (mdiodev->dev.of_node)
+ gpiod = fwnode_get_named_gpiod(&mdiodev->dev.of_node->fwnode,
+ "reset-gpios", 0, GPIOD_OUT_LOW,
+ "PHY reset");
+ if (PTR_ERR(gpiod) == -ENOENT)
+ gpiod = NULL;
+ else if (IS_ERR(gpiod))
+ return PTR_ERR(gpiod);
+
+ mdiodev->reset = gpiod;
+
+ /* Assert the reset signal again */
+ mdio_device_reset(mdiodev, 1);
+
mdiodev->bus->mdio_map[mdiodev->addr] = mdiodev;
return 0;
@@ -420,6 +438,9 @@ void mdiobus_unregister(struct mii_bus *bus)
if (!mdiodev)
continue;
+ if (mdiodev->reset)
+ gpiod_put(mdiodev->reset);
+
mdiodev->device_remove(mdiodev);
mdiodev->device_free(mdiodev);
}
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index e24f28924af8953d..75d97dd9fb281704 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -12,6 +12,8 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/errno.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
@@ -114,6 +116,13 @@ void mdio_device_remove(struct mdio_device *mdiodev)
}
EXPORT_SYMBOL(mdio_device_remove);
+void mdio_device_reset(struct mdio_device *mdiodev, int value)
+{
+ if (mdiodev->reset)
+ gpiod_set_value(mdiodev->reset, value);
+}
+EXPORT_SYMBOL(mdio_device_reset);
+
/**
* mdio_probe - probe an MDIO device
* @dev: device to probe
@@ -128,8 +137,16 @@ static int mdio_probe(struct device *dev)
struct mdio_driver *mdiodrv = to_mdio_driver(drv);
int err = 0;
- if (mdiodrv->probe)
+ if (mdiodrv->probe) {
+ /* Deassert the reset signal */
+ mdio_device_reset(mdiodev, 0);
+
err = mdiodrv->probe(mdiodev);
+ if (err) {
+ /* Assert the reset signal */
+ mdio_device_reset(mdiodev, 1);
+ }
+ }
return err;
}
@@ -140,9 +157,13 @@ static int mdio_remove(struct device *dev)
struct device_driver *drv = mdiodev->dev.driver;
struct mdio_driver *mdiodrv = to_mdio_driver(drv);
- if (mdiodrv->remove)
+ if (mdiodrv->remove) {
mdiodrv->remove(mdiodev);
+ /* Assert the reset signal */
+ mdio_device_reset(mdiodev, 1);
+ }
+
return 0;
}
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8154fb706751e8ad..1de5e242b8b4cda3 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -632,6 +632,9 @@ int phy_device_register(struct phy_device *phydev)
if (err)
return err;
+ /* Deassert the reset signal */
+ phy_device_reset(phydev, 0);
+
/* Run all of the fixups for this PHY */
err = phy_scan_fixups(phydev);
if (err) {
@@ -650,6 +653,9 @@ int phy_device_register(struct phy_device *phydev)
return 0;
out:
+ /* Assert the reset signal */
+ phy_device_reset(phydev, 1);
+
mdiobus_unregister_device(&phydev->mdio);
return err;
}
@@ -666,6 +672,10 @@ EXPORT_SYMBOL(phy_device_register);
void phy_device_remove(struct phy_device *phydev)
{
device_del(&phydev->mdio.dev);
+
+ /* Assert the reset signal */
+ phy_device_reset(phydev, 1);
+
mdiobus_unregister_device(&phydev->mdio);
}
EXPORT_SYMBOL(phy_device_remove);
@@ -849,6 +859,9 @@ int phy_init_hw(struct phy_device *phydev)
{
int ret = 0;
+ /* Deassert the reset signal */
+ phy_device_reset(phydev, 0);
+
if (!phydev->drv || !phydev->drv->config_init)
return 0;
@@ -1126,6 +1139,9 @@ void phy_detach(struct phy_device *phydev)
put_device(&phydev->mdio.dev);
if (ndev_owner != bus->owner)
module_put(bus->owner);
+
+ /* Assert the reset signal */
+ phy_device_reset(phydev, 1);
}
EXPORT_SYMBOL(phy_detach);
@@ -1811,8 +1827,16 @@ static int phy_probe(struct device *dev)
/* Set the state to READY by default */
phydev->state = PHY_READY;
- if (phydev->drv->probe)
+ if (phydev->drv->probe) {
+ /* Deassert the reset signal */
+ phy_device_reset(phydev, 0);
+
err = phydev->drv->probe(phydev);
+ if (err) {
+ /* Assert the reset signal */
+ phy_device_reset(phydev, 1);
+ }
+ }
mutex_unlock(&phydev->lock);
@@ -1829,8 +1853,12 @@ static int phy_remove(struct device *dev)
phydev->state = PHY_DOWN;
mutex_unlock(&phydev->lock);
- if (phydev->drv && phydev->drv->remove)
+ if (phydev->drv && phydev->drv->remove) {
phydev->drv->remove(phydev);
+
+ /* Assert the reset signal */
+ phy_device_reset(phydev, 1);
+ }
phydev->drv = NULL;
return 0;
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index ca08ab16ecdc9b78..92d4e55ffe675637 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -12,6 +12,7 @@
#include <uapi/linux/mdio.h>
#include <linux/mod_devicetable.h>
+struct gpio_desc;
struct mii_bus;
/* Multiple levels of nesting are possible. However typically this is
@@ -39,6 +40,7 @@ struct mdio_device {
/* Bus address of the MDIO device (0-31) */
int addr;
int flags;
+ struct gpio_desc *reset;
};
#define to_mdio_device(d) container_of(d, struct mdio_device, dev)
@@ -71,6 +73,7 @@ void mdio_device_free(struct mdio_device *mdiodev);
struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr);
int mdio_device_register(struct mdio_device *mdiodev);
void mdio_device_remove(struct mdio_device *mdiodev);
+void mdio_device_reset(struct mdio_device *mdiodev, int value);
int mdio_driver_register(struct mdio_driver *drv);
void mdio_driver_unregister(struct mdio_driver *drv);
int mdio_device_bus_match(struct device *dev, struct device_driver *drv);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 4962af37722ac2ea..6106ed915841c97e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -841,6 +841,11 @@ int phy_aneg_done(struct phy_device *phydev);
int phy_stop_interrupts(struct phy_device *phydev);
int phy_restart_aneg(struct phy_device *phydev);
+static inline void phy_device_reset(struct phy_device *phydev, int value)
+{
+ mdio_device_reset(&phydev->mdio, value);
+}
+
#define phydev_err(_phydev, format, args...) \
dev_err(&_phydev->mdio.dev, format, ##args)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 1/5] libbpf: add ability to guess program type based on section name
From: Roman Gushchin @ 2017-12-04 12:34 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Quentin Monnet, netdev, linux-kernel, kernel-team, ast, daniel,
kafai
In-Reply-To: <20171201144606.5ddd0272@cakuba.netronome.com>
On Fri, Dec 01, 2017 at 02:46:06PM -0800, Jakub Kicinski wrote:
> On Fri, 1 Dec 2017 10:22:57 +0000, Quentin Monnet wrote:
> > Thanks Roman!
> > One comment in-line.
> >
> > 2017-11-30 13:42 UTC+0000 ~ Roman Gushchin <guro@fb.com>
> > > The bpf_prog_load() function will guess program type if it's not
> > > specified explicitly. This functionality will be used to implement
> > > loading of different programs without asking a user to specify
> > > the program type. In first order it will be used by bpftool.
> > >
> > > Signed-off-by: Roman Gushchin <guro@fb.com>
> > > Cc: Alexei Starovoitov <ast@kernel.org>
> > > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > > Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > ---
> > > tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 47 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 5aa45f89da93..9f2410beaa18 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -1721,6 +1721,41 @@ BPF_PROG_TYPE_FNS(tracepoint, BPF_PROG_TYPE_TRACEPOINT);
> > > BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP);
> > > BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT);
> > >
> > > +static enum bpf_prog_type bpf_program__guess_type(struct bpf_program *prog)
> > > +{
> > > + if (!prog->section_name)
> > > + goto err;
> > > +
> > > + if (strncmp(prog->section_name, "socket", 6) == 0)
> > > + return BPF_PROG_TYPE_SOCKET_FILTER;
> > > + if (strncmp(prog->section_name, "kprobe/", 7) == 0)
> > > + return BPF_PROG_TYPE_KPROBE;
> > > + if (strncmp(prog->section_name, "kretprobe/", 10) == 0)
> > > + return BPF_PROG_TYPE_KPROBE;
> > > + if (strncmp(prog->section_name, "tracepoint/", 11) == 0)
> > > + return BPF_PROG_TYPE_TRACEPOINT;
> > > + if (strncmp(prog->section_name, "xdp", 3) == 0)
> > > + return BPF_PROG_TYPE_XDP;
> > > + if (strncmp(prog->section_name, "perf_event", 10) == 0)
> > > + return BPF_PROG_TYPE_PERF_EVENT;
> > > + if (strncmp(prog->section_name, "cgroup/skb", 10) == 0)
> > > + return BPF_PROG_TYPE_CGROUP_SKB;
> > > + if (strncmp(prog->section_name, "cgroup/sock", 11) == 0)
> > > + return BPF_PROG_TYPE_CGROUP_SOCK;
> > > + if (strncmp(prog->section_name, "cgroup/dev", 10) == 0)
> > > + return BPF_PROG_TYPE_CGROUP_DEVICE;
> > > + if (strncmp(prog->section_name, "sockops", 7) == 0)
> > > + return BPF_PROG_TYPE_SOCK_OPS;
> > > + if (strncmp(prog->section_name, "sk_skb", 6) == 0)
> > > + return BPF_PROG_TYPE_SK_SKB;
> >
> > I do not really like these hard-coded lengths, maybe we could work out
> > something nicer with a bit of pre-processing work? Perhaps something like:
> >
> > #define SOCKET_FILTER_SEC_PREFIX "socket"
> > #define KPROBE_SEC_PREFIX "kprobe/"
> > […]
> >
> > #define TRY_TYPE(string, __TYPE) \
> > do { \
> > if (!strncmp(string, __TYPE ## _SEC_PREFIX, \
> > sizeof(__TYPE ## _SEC_PREFIX))) \
> > return BPF_PROG_TYPE_ ## __TYPE; \
> > } while(0);
>
> I like the suggestion, but I think return and goto statements hiding
> inside macros are slightly frowned upon in the netdev. Perhaps just
> a macro that wraps the strncmp() with sizeof would be enough? Without
> the return inside?
Hm, I'm not sure that using macroses here makes the code much easier to read.
Maybe we can use just strcmp() instead?
As we compare with hardcoded strings, there is no real difference.
Something like this:
if (!strcmp("socket", prog->section_name))
return BPF_PROG_TYPE_SOCKET_FILTER;
if (!strcmp("kprobe/", prog->section_name))
return BPF_PROG_TYPE_KPROBE;
if (!strcmp("kretprobe/", prog->section_name))
return BPF_PROG_TYPE_KPROBE;
if (!strcmp("tracepoint/", prog->section_name))
return BPF_PROG_TYPE_TRACEPOINT;
if (!strcmp("xdp", prog->section_name))
return BPF_PROG_TYPE_XDP;
if (!strcmp("perf_event", prog->section_name))
return BPF_PROG_TYPE_PERF_EVENT;
if (!strcmp("cgroup/skb", prog->section_name))
return BPF_PROG_TYPE_CGROUP_SKB;
if (!strcmp("cgroup/sock", prog->section_name))
return BPF_PROG_TYPE_CGROUP_SOCK;
if (!strcmp("cgroup/dev", prog->section_name))
return BPF_PROG_TYPE_CGROUP_DEVICE;
if (!strcmp("sockops", prog->section_name))
return BPF_PROG_TYPE_SOCK_OPS;
if (!strcmp("sk_skb", prog->section_name))
return BPF_PROG_TYPE_SK_SKB;
Thanks!
^ permalink raw reply
* Re: [bisected] x86 boot still broken on -rc2
From: Prarit Bhargava @ 2017-12-04 12:28 UTC (permalink / raw)
To: Jakub Kicinski, LKML; +Cc: netdev@vger.kernel.org, Thomas Gleixner
In-Reply-To: <20171203172824.3d9b7ede@cakuba.netronome.com>
On 12/03/2017 08:28 PM, Jakub Kicinski wrote:
> Same thing on rc2, bisected down to:
>
> commit b4c0a7326f5dc0ef7a64128b0ae7d081f4b2cbd1 (refs/bisect/bad)
> Author: Prarit Bhargava <prarit@redhat.com>
> Date: Tue Nov 14 07:42:57 2017 -0500
>
> x86/smpboot: Fix __max_logical_packages estimate
>
> A system booted with a small number of cores enabled per package
> panics because the estimate of __max_logical_packages is too low.
>
> This occurs when the total number of active cores across all packages is
> less than the maximum core count for a single package. e.g.:
>
> On a 4 package system with 20 cores/package where only 4 cores are
> enabled on each package, the value of __max_logical_packages is
> calculated as DIV_ROUND_UP(16 / 20) = 1 and not 4.
>
> Calculate __max_logical_packages after the cpu enumeration has completed.
> Use the boot cpu's data to extrapolate the number of packages.
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Kan Liang <kan.liang@intel.com>
> Cc: He Chen <he.chen@linux.intel.com>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: Piotr Luc <piotr.luc@intel.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> Cc: Mathias Krause <minipli@googlemail.com>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Link: https://lkml.kernel.org/r/20171114124257.22013-4-prarit@redhat.com
>
>
> On Fri, 1 Dec 2017 16:39:54 -0800, Jakub Kicinski wrote:
>> Hi!
>>
>> I'm hitting these after DaveM pulled rc1 into net-next on my Xeon
>> E5-2630 v4 box. It also happens on linux-next. Did anyone else
>> experience it? (.config attached)
>>
>> [ 5.003771] WARNING: CPU: 14 PID: 1 at ../arch/x86/events/intel/uncore.c:936 uncore_pci_probe+0x285/0x2b0
>> [ 5.007544] Modules linked in:
>> [ 5.007544] CPU: 14 PID: 1 Comm: swapper/0 Not tainted 4.15.0-rc1-perf-00225-gb2a4e0a76b1d #782
>> [ 5.007544] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.3.4 11/08/2016
I have a Dell R730 available for use. OOC are you booting with the default
BIOS options?
P.
>> [ 5.007544] task: 000000009e842725 task.stack: 000000008a63fd2d
>> [ 5.007544] RIP: 0010:uncore_pci_probe+0x285/0x2b0
>> [ 5.007544] RSP: 0000:ffffad8580163d10 EFLAGS: 00010286
>> [ 5.007544] RAX: ffff98576cc3df30 RBX: ffffffffb08037e0 RCX: ffffffffb0c1a120
>> [ 5.007544] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffb0c1a960
>> [ 5.007544] RBP: ffff985b6c00ac00 R08: fffffffffffffffe R09: 00000000000fffff
>> [ 5.007544] R10: ffff98576f1b6018 R11: 0000000000000022 R12: ffff985b6c641000
>> [ 5.007544] R13: 0000000000000001 R14: 0000000000000001 R15: 0000000000000001
>> [ 5.007544] FS: 0000000000000000(0000) GS:ffff98576fb80000(0000) knlGS:0000000000000000
>> [ 5.007544] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 5.007544] CR2: 0000000000000000 CR3: 0000000185c09001 CR4: 00000000003606e0
>> [ 5.007544] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> [ 5.007544] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> [ 5.007544] Call Trace:
>> [ 5.007544] local_pci_probe+0x3d/0x90
>> [ 5.007544] ? pci_match_device+0xd9/0x100
>> [ 5.007544] pci_device_probe+0x122/0x180
>> [ 5.007544] driver_probe_device+0x246/0x330
>> [ 5.007544] ? set_debug_rodata+0x11/0x11
>> [ 5.007544] __driver_attach+0x8a/0x90
>> [ 5.007544] ? driver_probe_device+0x330/0x330
>> [ 5.007544] bus_for_each_dev+0x5c/0x90
>> [ 5.007544] bus_add_driver+0x196/0x220
>> [ 5.007544] driver_register+0x57/0xc0
>> [ 5.007544] intel_uncore_init+0x1e3/0x249
>> [ 5.007544] ? uncore_type_init+0x193/0x193
>> [ 5.007544] ? set_debug_rodata+0x11/0x11
>> [ 5.007544] do_one_initcall+0x4b/0x190
>> [ 5.007544] kernel_init_freeable+0x16e/0x1f5
>> [ 5.007544] ? rest_init+0xd0/0xd0
>> [ 5.007544] kernel_init+0xa/0x100
>> [ 5.007544] ret_from_fork+0x1f/0x30
>> [ 5.007544] Code: 48 8b 52 08 48 85 d2 74 0d 89 44 24 04 48 89 df ff d2 8b 44 24 04 48 89 df 89 44 24 04 e8 54 0a 1c 00 8b 44 24 0
>> [ 5.007544] ---[ end trace 4dc4c3d5f5afcd2f ]---
>> [ 5.244504] bdx_uncore: probe of 0000:ff:08.2 failed with error -22
>> [ 5.251604] bdx_uncore: probe of 0000:ff:0b.1 failed with error -22
>> [ 5.258711] bdx_uncore: probe of 0000:ff:10.1 failed with error -22
>> [ 5.265819] bdx_uncore: probe of 0000:ff:14.0 failed with error -22
>> [ 5.272919] bdx_uncore: probe of 0000:ff:14.1 failed with error -22
>> [ 5.280019] bdx_uncore: probe of 0000:ff:15.0 failed with error -22
>> [ 5.287112] bdx_uncore: probe of 0000:ff:15.1 failed with error -22
>> [ 5.294376] WARNING: CPU: 1 PID: 15 at ../arch/x86/events/intel/uncore.c:1065 uncore_change_type_ctx.isra.5+0xe6/0xf0
>> [ 5.298362] Modules linked in:
>> [ 5.298362] CPU: 1 PID: 15 Comm: cpuhp/1 Tainted: G W 4.15.0-rc1-perf-00225-gb2a4e0a76b1d #782
>> [ 5.298362] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.3.4 11/08/2016
>> [ 5.298362] task: 00000000ae78bc8f task.stack: 00000000f79660c1
>> [ 5.298362] RIP: 0010:uncore_change_type_ctx.isra.5+0xe6/0xf0
>> [ 5.298362] RSP: 0000:ffffad85833b3db8 EFLAGS: 00010213
>> [ 5.298362] RAX: 0000000000000000 RBX: ffff9857669b0200 RCX: 0000000000000001
>> [ 5.298362] RDX: ffff985b6f000000 RSI: ffff985b66580400 RDI: ffffffffb0c1ae8c
>> [ 5.298362] RBP: ffff985b66580400 R08: ffffffffb0c1ae8c R09: 0000000000000001
>> [ 5.298362] R10: 0000000000000000 R11: 00000000003d0900 R12: 0000000000000000
>> [ 5.298362] R13: ffffffffffffffff R14: 0000000000000001 R15: 0000000000000008
>> [ 5.298362] FS: 0000000000000000(0000) GS:ffff985b6f000000(0000) knlGS:0000000000000000
>> [ 5.298362] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 5.298362] CR2: 0000000000000000 CR3: 0000000185c09001 CR4: 00000000003606e0
>> [ 5.298362] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> [ 5.298362] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> [ 5.298362] Call Trace:
>> [ 5.298362] uncore_event_cpu_online+0x283/0x340
>> [ 5.298362] ? uncore_event_cpu_offline+0x180/0x180
>> [ 5.298362] cpuhp_invoke_callback+0x8c/0x620
>> [ 5.298362] ? __schedule+0x1ad/0x6c0
>> [ 5.298362] ? sort_range+0x20/0x20
>> [ 5.298362] cpuhp_thread_fun+0xbc/0x140
>> [ 5.298362] smpboot_thread_fn+0x114/0x1d0
>> [ 5.298362] kthread+0x111/0x130
>> [ 5.298362] ? kthread_create_on_node+0x40/0x40
>> [ 5.298362] ret_from_fork+0x1f/0x30
>> [ 5.298362] Code: 2a 44 89 73 10 41 83 c4 01 48 81 c5 40 01 00 00 45 3b 20 7c cf 48 83 c4 08 5b 5d 41 5c 41 5d 41 5e 41 5f c3 0f f
>> [ 5.298362] ---[ end trace 4dc4c3d5f5afcd30 ]---
>> [ 5.504808] Scanning for low memory corruption every 60 seconds
>> [ 5.512347] Initialise system trusted keyrings
>> [ 5.517470] workingset: timestamp_bits=40 max_order=23 bucket_order=0
>> [ 5.524840] BUG: unable to handle kernel paging request at 0000000023314bf4
>> [ 5.528761] IP: __kmalloc_track_caller+0xa8/0x210
>> [ 5.528761] PGD 185c0a067 P4D 185c0a067 PUD 185c0c067 PMD 0
>> [ 5.528761] Oops: 0000 [#1] PREEMPT SMP
>> [ 5.528761] Modules linked in:
>> [ 5.528761] CPU: 14 PID: 1 Comm: swapper/0 Tainted: G W 4.15.0-rc1-perf-00225-gb2a4e0a76b1d #782
>> [ 5.528761] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.3.4 11/08/2016
>> [ 5.528761] task: 000000009e842725 task.stack: 000000008a63fd2d
>> [ 5.528761] RIP: 0010:__kmalloc_track_caller+0xa8/0x210
>> [ 5.528761] RSP: 0000:ffffad8580163d58 EFLAGS: 00010286
>> [ 5.528761] RAX: 0000000000000000 RBX: ffffffffffffffff RCX: 000000000012ce0e
>> [ 5.528761] RDX: 000000000012cd0e RSI: 000000000012cd0e RDI: 000000000001dde0
>> [ 5.528761] RBP: ffff985700000001 R08: ffff98576f407c00 R09: ffffffffb071edbf
>> [ 5.528761] R10: ffffd54de1995600 R11: ffff985b6655915f R12: 0000000000000004
>> [ 5.528761] R13: 00000000014000c0 R14: ffffffffb026c239 R15: ffff98576f407c00
>> [ 5.528761] FS: 0000000000000000(0000) GS:ffff98576fb80000(0000) knlGS:0000000000000000
>> [ 5.528761] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 5.528761] CR2: ffffffffffffffff CR3: 0000000185c09001 CR4: 00000000003606e0
>> [ 5.528761] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> [ 5.528761] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> [ 5.528761] Call Trace:
>> [ 5.528761] kstrdup+0x2d/0x60
>> [ 5.528761] __kernfs_new_node+0x29/0x130
>> [ 5.528761] kernfs_new_node+0x24/0x50
>> [ 5.528761] kernfs_create_link+0x29/0x90
>> [ 5.528761] sysfs_do_create_link_sd.isra.0+0x5d/0xc0
>> [ 5.528761] sysfs_slab_add+0x1f5/0x270
>> [ 5.528761] ? set_debug_rodata+0x11/0x11
>> [ 5.528761] slab_sysfs_init+0x8b/0xfa
>> [ 5.528761] ? kmem_cache_init+0xf9/0xf9
>> [ 5.528761] do_one_initcall+0x4b/0x190
>> [ 5.528761] kernel_init_freeable+0x16e/0x1f5
>> [ 5.528761] ? rest_init+0xd0/0xd0
>> [ 5.528761] kernel_init+0xa/0x100
>> [ 5.528761] ret_from_fork+0x1f/0x30
>> [ 5.528761] Code: 49 63 47 20 49 8b 3f 48 8d 8a 00 01 00 00 48 8b 5c 05 00 48 89 e8 65 48 0f c7 0f 0f 94 c0 84 c0 74 ab 48 85 db 7
>> [ 5.528761] RIP: __kmalloc_track_caller+0xa8/0x210 RSP: ffffad8580163d58
>> [ 5.528761] CR2: ffffffffffffffff
>> [ 5.528761] ---[ end trace 4dc4c3d5f5afcd31 ]---
>> [ 5.773089] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
>> [ 5.773089]
>> [ 5.777076] Kernel Offset: 0x2f000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
>> [ 5.777076] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
>
^ permalink raw reply
* Re: Fixing CVE-2017-16939 in v4.4.y and possibly v3.18.y
From: Greg Kroah-Hartman @ 2017-12-04 12:05 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Michal Kubecek, netdev, stable
In-Reply-To: <4ac5fb52-28f9-7111-483d-f010b093d923@roeck-us.net>
On Sat, Dec 02, 2017 at 04:20:40PM -0800, Guenter Roeck wrote:
> On 12/01/2017 11:48 AM, Michal Kubecek wrote:
> > On Thu, Nov 30, 2017 at 10:37:40AM -0800, Guenter Roeck wrote:
> > > Hi,
> > >
> > > The fix for CVE-2017-16939 has been applied to v4.9.y, but not to v4.4.y
> > > and older kernels. However, I confirmed that running the published POC
> > > (see https://blogs.securiteam.com/index.php/archives/3535) does crash a 4.4
> > > kernel.
> > >
> > > I confirmed that the following two patches fix the problem in v4.4.y.
> > > Please consider applying them to v4.4.y (and possibly v3.18.y).
> > >
> > > fc9e50f5a5a4e ("netlink: add a start callback for starting a netlink dump")
> > > 1137b5e2529a8 ("ipsec: Fix aborted xfrm policy dump crash")
> > >
> > > My apologies for the noise if this is already under consideration.
> >
> > It's a bit too big hammer. As Nicolai Stange noticed when we were
>
> The hammer is just as big as the upstream hammer. Personally I prefer the
> upstream patch; I don't see a reason to deviate from upstream just because
> the upstream solution is more complex than necessary.
>
> > handling this for SLE12 (where fc9e50f5a5a4e would break kABI), it's
>
> I didn't know that this is even a concern for stable releases. Is there
> some guideline that kABI changes should be avoided in stable releases ?
Nope, for now I don't care about kABI changes in stable releases. I'd
almost always prefer to take the upstream patches exactly as-is.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 4/4] selinux: Add SCTP support
From: Richard Haines @ 2017-12-04 11:40 UTC (permalink / raw)
To: Stephen Smalley, selinux, netdev, linux-sctp,
linux-security-module
Cc: marcelo.leitner, vyasevich, nhorman, paul
In-Reply-To: <1511899154.30337.27.camel@tycho.nsa.gov>
On Tue, 2017-11-28 at 14:59 -0500, Stephen Smalley wrote:
> On Tue, 2017-11-28 at 14:39 -0500, Stephen Smalley wrote:
> > On Mon, 2017-11-27 at 19:32 +0000, Richard Haines wrote:
> > > The SELinux SCTP implementation is explained in:
> > > Documentation/security/SELinux-sctp.rst
> > >
> > > Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> > > ---
> > > Documentation/security/SELinux-sctp.rst | 104 ++++++++++++
> > > security/selinux/hooks.c | 278
> > > +++++++++++++++++++++++++++++---
> > > security/selinux/include/classmap.h | 2 +-
> > > security/selinux/include/netlabel.h | 15 +-
> > > security/selinux/include/objsec.h | 4 +
> > > security/selinux/netlabel.c | 128 +++++++++++++--
> > > 6 files changed, 499 insertions(+), 32 deletions(-)
> > > create mode 100644 Documentation/security/SELinux-sctp.rst
> > >
> > > diff --git a/Documentation/security/SELinux-sctp.rst
> > > b/Documentation/security/SELinux-sctp.rst
> > > new file mode 100644
> > > index 0000000..f6a9162
> > > --- /dev/null
> > > +++ b/Documentation/security/SELinux-sctp.rst
> > > @@ -0,0 +1,104 @@
> > > +SCTP SELinux Support
> > > +=====================
> > > +
> > > +Security Hooks
> > > +===============
> > > +
> > > +The ``Documentation/security/LSM-sctp.rst`` document describes
> > > how
> > > the
> > > +following sctp security hooks are utilised::
> > > +
> > > + security_sctp_assoc_request()
> > > + security_sctp_bind_connect()
> > > + security_sctp_sk_clone()
> > > + security_inet_conn_established()
> > > +
> > > +
> > > +Policy Statements
> > > +==================
> > > +The following class and permissions to support SCTP are
> > > available
> > > within the
> > > +kernel::
> > > +
> > > + class sctp_socket inherits socket { node_bind }
> > > +
> > > +whenever the following policy capability is enabled::
> > > +
> > > + policycap extended_socket_class;
> > > +
> > > +SELinux SCTP support adds the ``name_connect`` permission for
> > > connecting
> > > +to a specific port type and the ``association`` permission that
> > > is
> > > explained
> > > +in the section below.
> > > +
> > > +If userspace tools have been updated, SCTP will support the
> > > ``portcon``
> > > +statement as shown in the following example::
> > > +
> > > + portcon sctp 1024-1036 system_u:object_r:sctp_ports_t:s0
> > > +
> > > +
> > > +SCTP Bind, Connect and ASCONF Chunk Parameter Permission Checks
> > > +================================================================
> > > +The hook ``security_sctp_bind_connect()`` is called by SCTP to
> > > check
> > > +permissions required for ipv4/ipv6 addresses based on the ``@opt
> > > na
> > > me
> > > `` as
> > > +follows::
> > > +
> > > + --------------------------------------------------------------
> > > ----
> > > + | BIND Permission
> > > Checks |
> > > + | @optname | @address
> > > contains |
> > > + |----------------------------|--------------------------------
> > > ---|
> > > + | SCTP_SOCKOPT_BINDX_ADD | One or more ipv4 / ipv6
> > > addresses
> > > >
> > >
> > > + | SCTP_PRIMARY_ADDR | Single ipv4 or ipv6
> > > address |
> > > + | SCTP_SET_PEER_PRIMARY_ADDR | Single ipv4 or ipv6
> > > address |
> > > + --------------------------------------------------------------
> > > ----
> > > +
> > > + --------------------------------------------------------------
> > > ----
> > > + | CONNECT Permission
> > > Checks |
> > > + | @optname | @address
> > > contains |
> > > + |----------------------------|--------------------------------
> > > ---|
> > > + | SCTP_SOCKOPT_CONNECTX | One or more ipv4 / ipv6
> > > addresses
> > > >
> > >
> > > + | SCTP_PARAM_ADD_IP | One or more ipv4 / ipv6
> > > addresses
> > > >
> > >
> > > + | SCTP_SENDMSG_CONNECT | Single ipv4 or ipv6
> > > address |
> > > + | SCTP_PARAM_SET_PRIMARY | Single ipv4 or ipv6
> > > address |
> > > + --------------------------------------------------------------
> > > ----
> > > +
> > > +
> > > +SCTP Peer Labeling
> > > +===================
> > > +An SCTP socket will only have one peer label assigned to it.
> > > This
> > > will be
> > > +assigned during the establishment of the first association. Once
> > > the
> > > peer
> > > +label has been assigned, any new associations will have the
> > > ``association``
> > > +permission validated by checking the socket peer sid against the
> > > received
> > > +packets peer sid to determine whether the association should be
> > > allowed or
> > > +denied.
> > > +
> > > +NOTES:
> > > + 1) If peer labeling is not enabled, then the peer context
> > > will
> > > always be
> > > + ``SECINITSID_UNLABELED`` (``unlabeled_t`` in Reference
> > > Policy).
> > > +
> > > + 2) As SCTP can support more than one transport address per
> > > endpoint
> > > + (multi-homing) on a single socket, it is possible to
> > > configure
> > > policy
> > > + and NetLabel to provide different peer labels for each of
> > > these. As the
> > > + socket peer label is determined by the first associations
> > > transport
> > > + address, it is recommended that all peer labels are
> > > consistent.
> > > +
> > > + 3) **getpeercon**\(3) may be used by userspace to retrieve
> > > the
> > > sockets peer
> > > + context.
> > > +
> > > + 4) While not SCTP specific, be aware when using NetLabel that
> > > if
> > > a label
> > > + is assigned to a specific interface, and that interface
> > > 'goes
> > > down',
> > > + then the NetLabel service will remove the entry. Therefore
> > > ensure that
> > > + the network startup scripts call **netlabelctl**\(8) to
> > > set
> > > the required
> > > + label (see **netlabel-config**\(8) helper script for
> > > details).
> > > +
> > > + 5) The NetLabel SCTP peer labeling rules apply as discussed
> > > in
> > > the following
> > > + set of posts tagged "netlabel" at: http://www.paul-moore.c
> > > om
> > > /b
> > > log/t.
> > > +
> > > + 6) CIPSO is only supported for IPv4 addressing:
> > > ``socket(AF_INET,
> > > ...)``
> > > + CALIPSO is only supported for IPv6 addressing:
> > > ``socket(AF_INET6, ...)``
> > > +
> > > + Note the following when testing CIPSO/CALIPSO:
> > > + a) CIPSO will send an ICMP packet if an SCTP packet
> > > cannot
> > > be
> > > + delivered because of an invalid label.
> > > + b) CALIPSO does not send an ICMP packet, just silently
> > > discards it.
> > > +
> > > + 7) IPSEC is not supported as RFC 3554 - sctp/ipsec support
> > > has
> > > not been
> > > + implemented in userspace (**racoon**\(8) or
> > > **ipsec_pluto**\(8)),
> > > + although the kernel supports SCTP/IPSEC.
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index 0110bb5..7bd5886 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -67,6 +67,8 @@
> > > #include <linux/tcp.h>
> > > #include <linux/udp.h>
> > > #include <linux/dccp.h>
> > > +#include <linux/sctp.h>
> > > +#include <net/sctp/structs.h>
> > > #include <linux/quota.h>
> > > #include <linux/un.h> /* for Unix socket types */
> > > #include <net/af_unix.h> /* for Unix socket types */
> > > @@ -4136,6 +4138,23 @@ static int selinux_parse_skb_ipv4(struct
> > > sk_buff *skb,
> > > break;
> > > }
> > >
> > > +#if IS_ENABLED(CONFIG_IP_SCTP)
> > > + case IPPROTO_SCTP: {
> > > + struct sctphdr _sctph, *sh;
> > > +
> > > + if (ntohs(ih->frag_off) & IP_OFFSET)
> > > + break;
> > > +
> > > + offset += ihlen;
> > > + sh = skb_header_pointer(skb, offset,
> > > sizeof(_sctph),
> > > &_sctph);
> > > + if (sh == NULL)
> > > + break;
> > > +
> > > + ad->u.net->sport = sh->source;
> > > + ad->u.net->dport = sh->dest;
> > > + break;
> > > + }
> > > +#endif
> > > default:
> > > break;
> > > }
> > > @@ -4209,6 +4228,19 @@ static int selinux_parse_skb_ipv6(struct
> > > sk_buff *skb,
> > > break;
> > > }
> > >
> > > +#if IS_ENABLED(CONFIG_IP_SCTP)
> > > + case IPPROTO_SCTP: {
> > > + struct sctphdr _sctph, *sh;
> > > +
> > > + sh = skb_header_pointer(skb, offset,
> > > sizeof(_sctph),
> > > &_sctph);
> > > + if (sh == NULL)
> > > + break;
> > > +
> > > + ad->u.net->sport = sh->source;
> > > + ad->u.net->dport = sh->dest;
> > > + break;
> > > + }
> > > +#endif
> > > /* includes fragments */
> > > default:
> > > break;
> > > @@ -4398,6 +4430,10 @@ static int
> > > selinux_socket_post_create(struct
> > > socket *sock, int family,
> > > sksec = sock->sk->sk_security;
> > > sksec->sclass = sclass;
> > > sksec->sid = sid;
> > > + /* Allows detection of the first association on
> > > this
> > > socket */
> > > + if (sksec->sclass == SECCLASS_SCTP_SOCKET)
> > > + sksec->sctp_assoc_state =
> > > SCTP_ASSOC_UNSET;
> >
> > Same comment as before:
> > What prevents this from interleaving with
> > selinux_sctp_assoc_request()
> > accesses to sksec->sctp_assoc_state? You aren't holding any lock
> > here.
> > What ensures that this executes before
> > selinux_sctp_assoc_request()?
>
> Sorry, maybe I'm wrong. selinux_sctp_assoc_request() can't be called
> until after bind() and listen() have completed?
Correct - In tests I've never had a problem here.
>
> >
> > > +
> > > err = selinux_netlbl_socket_post_create(sock-
> > > >sk,
> > > family);
> > > }
> > >
> > > @@ -4418,11 +4454,7 @@ static int selinux_socket_bind(struct
> > > socket
> > > *sock, struct sockaddr *address, in
> > > if (err)
> > > goto out;
> > >
> > > - /*
> > > - * If PF_INET or PF_INET6, check name_bind permission
> > > for
> > > the port.
> > > - * Multiple address binding for SCTP is not supported
> > > yet:
> > > we just
> > > - * check the first address now.
> > > - */
> > > + /* If PF_INET or PF_INET6, check name_bind permission
> > > for
> > > the port. */
> > > family = sk->sk_family;
> > > if (family == PF_INET || family == PF_INET6) {
> > > char *addrp;
> > > @@ -4434,7 +4466,13 @@ static int selinux_socket_bind(struct
> > > socket
> > > *sock, struct sockaddr *address, in
> > > unsigned short snum;
> > > u32 sid, node_perm;
> > >
> > > - if (family == PF_INET) {
> > > + /*
> > > + * sctp_bindx(3) calls via
> > > selinux_sctp_bind_connect()
> > > + * that validates multiple binding addresses.
> > > Because of this
> > > + * need to check address->sa_family as it is
> > > possible to have
> > > + * sk->sk_family = PF_INET6 with addr->sa_family
> > > =
> > > AF_INET.
> > > + */
> > > + if (address->sa_family == AF_INET) {
> > > if (addrlen < sizeof(struct
> > > sockaddr_in))
> > > {
> > > err = -EINVAL;
> > > goto out;
> > > @@ -4488,6 +4526,10 @@ static int selinux_socket_bind(struct
> > > socket
> > > *sock, struct sockaddr *address, in
> > > node_perm = DCCP_SOCKET__NODE_BIND;
> > > break;
> > >
> > > + case SECCLASS_SCTP_SOCKET:
> > > + node_perm = SCTP_SOCKET__NODE_BIND;
> > > + break;
> > > +
> > > default:
> > > node_perm = RAWIP_SOCKET__NODE_BIND;
> > > break;
> > > @@ -4502,7 +4544,7 @@ static int selinux_socket_bind(struct
> > > socket
> > > *sock, struct sockaddr *address, in
> > > ad.u.net->sport = htons(snum);
> > > ad.u.net->family = family;
> > >
> > > - if (family == PF_INET)
> > > + if (address->sa_family == AF_INET)
> > > ad.u.net->v4info.saddr = addr4-
> > > > sin_addr.s_addr;
> > >
> > > else
> > > ad.u.net->v6info.saddr = addr6-
> > > >sin6_addr;
> > > @@ -4516,7 +4558,11 @@ static int selinux_socket_bind(struct
> > > socket
> > > *sock, struct sockaddr *address, in
> > > return err;
> > > }
> > >
> > > -static int selinux_socket_connect(struct socket *sock, struct
> > > sockaddr *address, int addrlen)
> > > +/* This supports connect(2) and SCTP connect services such as
> > > sctp_connectx(3)
> > > + * and sctp_sendmsg(3) as described in
> > > Documentation/security/LSM-
> > > sctp.txt
> > > + */
> > > +static int selinux_socket_connect_helper(struct socket *sock,
> > > + struct sockaddr
> > > *address,
> > > int addrlen)
> > > {
> > > struct sock *sk = sock->sk;
> > > struct sk_security_struct *sksec = sk->sk_security;
> > > @@ -4527,10 +4573,12 @@ static int selinux_socket_connect(struct
> > > socket *sock, struct sockaddr *address,
> > > return err;
> > >
> > > /*
> > > - * If a TCP or DCCP socket, check name_connect
> > > permission
> > > for the port.
> > > + * If a TCP, DCCP or SCTP socket, check name_connect
> > > permission
> > > + * for the port.
> > > */
> > > if (sksec->sclass == SECCLASS_TCP_SOCKET ||
> > > - sksec->sclass == SECCLASS_DCCP_SOCKET) {
> > > + sksec->sclass == SECCLASS_DCCP_SOCKET ||
> > > + sksec->sclass == SECCLASS_SCTP_SOCKET) {
> > > struct common_audit_data ad;
> > > struct lsm_network_audit net = {0,};
> > > struct sockaddr_in *addr4 = NULL;
> > > @@ -4538,7 +4586,12 @@ static int selinux_socket_connect(struct
> > > socket *sock, struct sockaddr *address,
> > > unsigned short snum;
> > > u32 sid, perm;
> > >
> > > - if (sk->sk_family == PF_INET) {
> > > + /* sctp_connectx(3) calls via
> > > selinux_sctp_bind_connect()
> > > + * that validates multiple connect addresses.
> > > Because of this
> > > + * need to check address->sa_family as it is
> > > possible to have
> > > + * sk->sk_family = PF_INET6 with addr->sa_family
> > > =
> > > AF_INET.
> > > + */
> > > + if (address->sa_family == AF_INET) {
> > > addr4 = (struct sockaddr_in *)address;
> > > if (addrlen < sizeof(struct
> > > sockaddr_in))
> > > return -EINVAL;
> > > @@ -4552,10 +4605,19 @@ static int selinux_socket_connect(struct
> > > socket *sock, struct sockaddr *address,
> > >
> > > err = sel_netport_sid(sk->sk_protocol, snum,
> > > &sid);
> > > if (err)
> > > - goto out;
> > > + return err;
> > >
> > > - perm = (sksec->sclass == SECCLASS_TCP_SOCKET) ?
> > > - TCP_SOCKET__NAME_CONNECT :
> > > DCCP_SOCKET__NAME_CONNECT;
> > > + switch (sksec->sclass) {
> > > + case SECCLASS_TCP_SOCKET:
> > > + perm = TCP_SOCKET__NAME_CONNECT;
> > > + break;
> > > + case SECCLASS_DCCP_SOCKET:
> > > + perm = DCCP_SOCKET__NAME_CONNECT;
> > > + break;
> > > + case SECCLASS_SCTP_SOCKET:
> > > + perm = SCTP_SOCKET__NAME_CONNECT;
> > > + break;
> > > + }
> > >
> > > ad.type = LSM_AUDIT_DATA_NET;
> > > ad.u.net = &net;
> > > @@ -4563,13 +4625,24 @@ static int selinux_socket_connect(struct
> > > socket *sock, struct sockaddr *address,
> > > ad.u.net->family = sk->sk_family;
> > > err = avc_has_perm(sksec->sid, sid, sksec-
> > > >sclass,
> > > perm, &ad);
> > > if (err)
> > > - goto out;
> > > + return err;
> > > }
> > >
> > > - err = selinux_netlbl_socket_connect(sk, address);
> > > + return 0;
> > > +}
> > >
> > > -out:
> > > - return err;
> > > +/* Supports connect(2), see comments in
> > > selinux_socket_connect_helper() */
> > > +static int selinux_socket_connect(struct socket *sock,
> > > + struct sockaddr *address, int
> > > addrlen)
> > > +{
> > > + int err;
> > > + struct sock *sk = sock->sk;
> > > +
> > > + err = selinux_socket_connect_helper(sock, address,
> > > addrlen);
> > > + if (err)
> > > + return err;
> > > +
> > > + return selinux_netlbl_socket_connect(sk, address);
> > > }
> > >
> > > static int selinux_socket_listen(struct socket *sock, int
> > > backlog)
> > > @@ -4832,7 +4905,8 @@ static int
> > > selinux_socket_getpeersec_stream(struct socket *sock, char __user
> > > *op
> > > u32 peer_sid = SECSID_NULL;
> > >
> > > if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET ||
> > > - sksec->sclass == SECCLASS_TCP_SOCKET)
> > > + sksec->sclass == SECCLASS_TCP_SOCKET ||
> > > + sksec->sclass == SECCLASS_SCTP_SOCKET)
> > > peer_sid = sksec->peer_sid;
> > > if (peer_sid == SECSID_NULL)
> > > return -ENOPROTOOPT;
> > > @@ -4945,6 +5019,169 @@ static void selinux_sock_graft(struct
> > > sock
> > > *sk, struct socket *parent)
> > > sksec->sclass = isec->sclass;
> > > }
> > >
> > > +/* Called whenever SCTP receives an INIT chunk. This happens
> > > when
> > > an
> > > incoming
> > > + * connect(2), sctp_connectx(3) or sctp_sendmsg(3) (with no
> > > association
> > > + * already present).
> > > + * The lock is to ensure sksec->sctp_assoc_state.
> > > + */
> > > +static DEFINE_SPINLOCK(assoc_lock);
> >
> > The lock needs to be taken by all entities accessing sksec-
> > > sctp_assoc_state, and you need to further ensure proper handling
> > > if
> >
> > the ordering is reversed. Also, the lock should be per-sksec, not
> > global.
> >
> > > +static int selinux_sctp_assoc_request(struct sctp_endpoint *ep,
> > > + struct sk_buff *skb)
> > > +{
> > > + struct sk_security_struct *sksec = ep->base.sk-
> > > > sk_security;
> > >
> > > + struct common_audit_data ad;
> > > + struct lsm_network_audit net = {0,};
> > > + u8 peerlbl_active;
> > > + u32 peer_sid = SECINITSID_UNLABELED;
> > > + u32 conn_sid;
> > > + int err = 0;
> > > +
> > > + if (!selinux_policycap_extsockclass)
> > > + return 0;
> > > +
> > > + spin_lock(&assoc_lock);
>
> So what is this protecting? And if needed, does it need to be
> spin_lock_bh() instead of just spin_lock()? Can multiple calls to
> selinux_sctp_assoc_request() on the same endpoint be interleaved?
>
In the RFC patch I would also call this on client side INIT_ACK which
is why I had the lock. However I've now dropped this check so the lock
can go. During tests I've not seen any interleaving so removing this
lock seems okay.
I'll submit a new patch to also include the kbuild test robot catch as
well later this week.
> > > +
> > > + peerlbl_active = selinux_peerlbl_enabled();
> > > +
> > > + if (peerlbl_active) {
> > > + /* This will return peer_sid = SECSID_NULL if
> > > there
> > > are
> > > + * no peer labels, see
> > > security_net_peersid_resolve().
> > > + */
> > > + err = selinux_skb_peerlbl_sid(skb, ep->base.sk-
> > > > sk_family,
> > >
> > > + &peer_sid);
> > > + if (err)
> > > + goto err;
> > > +
> > > + if (peer_sid == SECSID_NULL)
> > > + peer_sid = SECINITSID_UNLABELED;
> > > + }
> > > +
> > > + if (sksec->sctp_assoc_state == SCTP_ASSOC_UNSET) {
> > > + sksec->sctp_assoc_state = SCTP_ASSOC_SET;
> > > +
> > > + /* Here as first association on socket. As the
> > > peer
> > > SID
> > > + * was allowed by peer recv (and the netif/node
> > > checks),
> > > + * then it is approved by policy and used as the
> > > primary
> > > + * peer SID for getpeercon(3).
> > > + */
> > > + sksec->peer_sid = peer_sid;
> > > + } else if (sksec->peer_sid != peer_sid) {
> > > + /* Other association peer SIDs are checked to
> > > enforce
> > > + * consistency among the peer SIDs.
> > > + */
> > > + ad.type = LSM_AUDIT_DATA_NET;
> > > + ad.u.net = &net;
> > > + ad.u.net->sk = ep->base.sk;
> > > + err = avc_has_perm(sksec->peer_sid, peer_sid,
> > > sksec-
> > > > sclass,
> > >
> > > + SCTP_SOCKET__ASSOCIATION,
> > > &ad);
> > > + if (err)
> > > + goto err;
> > > + }
> > > +
> > > + /* Compute the MLS component for the connection and
> > > store
> > > + * the information in ep. This will be used by SCTP TCP
> > > type
> > > + * sockets and peeled off connections as they cause a
> > > new
> > > + * socket to be generated. selinux_sctp_sk_clone() will
> > > then
> > > + * plug this into the new socket.
> > > + */
> > > + err = selinux_conn_sid(sksec->sid, peer_sid, &conn_sid);
> > > + if (err)
> > > + goto err;
> > > +
> > > + ep->secid = conn_sid;
> > > + ep->peer_secid = peer_sid;
> > > +
> > > + /* Set any NetLabel labels including CIPSO/CALIPSO
> > > options.
> > > */
> > > + err = selinux_netlbl_sctp_assoc_request(ep, skb);
> > > +
> > > +err:
> > > + spin_unlock(&assoc_lock);
> > > + return err;
> > > +}
> > > +
> > > +/*
> > > + * Check if sctp IPv4/IPv6 addresses are valid for binding or
> > > connecting
> > > + * based on their @optname.
> > > + */
> > > +static int selinux_sctp_bind_connect(struct sock *sk, int
> > > optname,
> > > + struct sockaddr *address,
> > > + int addrlen)
> > > +{
> > > + int len, err = 0, walk_size = 0;
> > > + void *addr_buf;
> > > + struct sockaddr *addr;
> > > + struct socket *sock;
> > > +
> > > + if (!selinux_policycap_extsockclass)
> > > + return 0;
> > > +
> > > + /* Process one or more addresses that may be IPv4 or
> > > IPv6
> > > */
> > > + sock = sk->sk_socket;
> > > + addr_buf = address;
> > > +
> > > + while (walk_size < addrlen) {
> > > + addr = addr_buf;
> > > + switch (addr->sa_family) {
> > > + case AF_INET:
> > > + len = sizeof(struct sockaddr_in);
> > > + break;
> > > + case AF_INET6:
> > > + len = sizeof(struct sockaddr_in6);
> > > + break;
> > > + default:
> > > + return -EAFNOSUPPORT;
> > > + }
> > > +
> > > + err = -EINVAL;
> > > + switch (optname) {
> > > + /* Bind checks */
> > > + case SCTP_PRIMARY_ADDR:
> > > + case SCTP_SET_PEER_PRIMARY_ADDR:
> > > + case SCTP_SOCKOPT_BINDX_ADD:
> > > + err = selinux_socket_bind(sock, addr,
> > > len);
> > > + break;
> > > + /* Connect checks */
> > > + case SCTP_SOCKOPT_CONNECTX:
> > > + case SCTP_PARAM_SET_PRIMARY:
> > > + case SCTP_PARAM_ADD_IP:
> > > + case SCTP_SENDMSG_CONNECT:
> > > + err =
> > > selinux_socket_connect_helper(sock,
> > > addr, len);
> > > + if (err)
> > > + return err;
> > > +
> > > + err =
> > > selinux_netlbl_sctp_socket_connect(sk,
> > > addr);
> > > + break;
> > > + }
> > > +
> > > + if (err)
> > > + return err;
> > > +
> > > + addr_buf += len;
> > > + walk_size += len;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/* Called whenever a new socket is created by accept(2) or
> > > sctp_peeloff(3). */
> > > +static void selinux_sctp_sk_clone(struct sctp_endpoint *ep,
> > > struct
> > > sock *sk,
> > > + struct sock *newsk)
> > > +{
> > > + struct sk_security_struct *sksec = sk->sk_security;
> > > + struct sk_security_struct *newsksec = newsk-
> > > >sk_security;
> > > +
> > > + /* If policy does not support SECCLASS_SCTP_SOCKET then
> > > call
> > > + * the non-sctp clone version.
> > > + */
> > > + if (!selinux_policycap_extsockclass)
> > > + return selinux_sk_clone_security(sk, newsk);
> > > +
> > > + newsksec->sid = ep->secid;
> > > + newsksec->peer_sid = ep->peer_secid;
> > > + newsksec->sclass = sksec->sclass;
> > > + newsksec->nlbl_state = sksec->nlbl_state;
> > > +}
> > > +
> > > static int selinux_inet_conn_request(struct sock *sk, struct
> > > sk_buff
> > > *skb,
> > > struct request_sock *req)
> > > {
> > > @@ -6433,6 +6670,9 @@ static struct security_hook_list
> > > selinux_hooks[] __lsm_ro_after_init = {
> > > LSM_HOOK_INIT(sk_clone_security,
> > > selinux_sk_clone_security),
> > > LSM_HOOK_INIT(sk_getsecid, selinux_sk_getsecid),
> > > LSM_HOOK_INIT(sock_graft, selinux_sock_graft),
> > > + LSM_HOOK_INIT(sctp_assoc_request,
> > > selinux_sctp_assoc_request),
> > > + LSM_HOOK_INIT(sctp_sk_clone, selinux_sctp_sk_clone),
> > > + LSM_HOOK_INIT(sctp_bind_connect,
> > > selinux_sctp_bind_connect),
> > > LSM_HOOK_INIT(inet_conn_request,
> > > selinux_inet_conn_request),
> > > LSM_HOOK_INIT(inet_csk_clone, selinux_inet_csk_clone),
> > > LSM_HOOK_INIT(inet_conn_established,
> > > selinux_inet_conn_established),
> > > diff --git a/security/selinux/include/classmap.h
> > > b/security/selinux/include/classmap.h
> > > index 35ffb29..099065e 100644
> > > --- a/security/selinux/include/classmap.h
> > > +++ b/security/selinux/include/classmap.h
> > > @@ -175,7 +175,7 @@ struct security_class_mapping secclass_map[]
> > > =
> > > {
> > > { COMMON_CAP2_PERMS, NULL } },
> > > { "sctp_socket",
> > > { COMMON_SOCK_PERMS,
> > > - "node_bind", NULL } },
> > > + "node_bind", "name_connect", "association", NULL }
> > > },
> > > { "icmp_socket",
> > > { COMMON_SOCK_PERMS,
> > > "node_bind", NULL } },
> > > diff --git a/security/selinux/include/netlabel.h
> > > b/security/selinux/include/netlabel.h
> > > index 75686d5..313c8bd 100644
> > > --- a/security/selinux/include/netlabel.h
> > > +++ b/security/selinux/include/netlabel.h
> > > @@ -33,6 +33,7 @@
> > > #include <linux/skbuff.h>
> > > #include <net/sock.h>
> > > #include <net/request_sock.h>
> > > +#include <net/sctp/structs.h>
> > >
> > > #include "avc.h"
> > > #include "objsec.h"
> > > @@ -53,7 +54,8 @@ int selinux_netlbl_skbuff_getsid(struct sk_buff
> > > *skb,
> > > int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
> > > u16 family,
> > > u32 sid);
> > > -
> > > +int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
> > > + struct sk_buff *skb);
> > > int selinux_netlbl_inet_conn_request(struct request_sock *req,
> > > u16
> > > family);
> > > void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family);
> > > int selinux_netlbl_socket_post_create(struct sock *sk, u16
> > > family);
> > > @@ -65,6 +67,7 @@ int selinux_netlbl_socket_setsockopt(struct
> > > socket
> > > *sock,
> > > int level,
> > > int optname);
> > > int selinux_netlbl_socket_connect(struct sock *sk, struct
> > > sockaddr
> > > *addr);
> > > +int selinux_netlbl_sctp_socket_connect(struct sock *sk, struct
> > > sockaddr *addr);
> > >
> > > #else
> > > static inline void selinux_netlbl_cache_invalidate(void)
> > > @@ -114,6 +117,11 @@ static inline int
> > > selinux_netlbl_conn_setsid(struct sock *sk,
> > > return 0;
> > > }
> > >
> > > +static inline int selinux_netlbl_sctp_assoc_request(struct
> > > sctp_endpoint *ep,
> > > + struct
> > > sk_buff
> > > *skb)
> > > +{
> > > + return 0;
> > > +}
> > > static inline int selinux_netlbl_inet_conn_request(struct
> > > request_sock *req,
> > > u16 family)
> > > {
> > > @@ -146,6 +154,11 @@ static inline int
> > > selinux_netlbl_socket_connect(struct sock *sk,
> > > {
> > > return 0;
> > > }
> > > +static inline int selinux_netlbl_sctp_socket_connect(struct sock
> > > *sk,
> > > + struct
> > > sockaddr
> > > *addr)
> > > +{
> > > + return 0;
> > > +}
> > > #endif /* CONFIG_NETLABEL */
> > >
> > > #endif
> > > diff --git a/security/selinux/include/objsec.h
> > > b/security/selinux/include/objsec.h
> > > index 6ebc61e..e319d5d 100644
> > > --- a/security/selinux/include/objsec.h
> > > +++ b/security/selinux/include/objsec.h
> > > @@ -130,6 +130,10 @@ struct sk_security_struct {
> > > u32 sid; /* SID of this object */
> > > u32 peer_sid; /* SID of peer */
> > > u16 sclass; /* sock security
> > > class
> > > */
> > > + enum { /* SCTP
> > > association
> > > state */
> > > + SCTP_ASSOC_UNSET = 0,
> > > + SCTP_ASSOC_SET,
> > > + } sctp_assoc_state;
> > > };
> > >
> > > struct tun_security_struct {
> > > diff --git a/security/selinux/netlabel.c
> > > b/security/selinux/netlabel.c
> > > index aaba667..ac23f29 100644
> > > --- a/security/selinux/netlabel.c
> > > +++ b/security/selinux/netlabel.c
> > > @@ -250,6 +250,7 @@ int selinux_netlbl_skbuff_setsid(struct
> > > sk_buff
> > > *skb,
> > > sk = skb_to_full_sk(skb);
> > > if (sk != NULL) {
> > > struct sk_security_struct *sksec = sk-
> > > > sk_security;
> > >
> > > +
> > > if (sksec->nlbl_state != NLBL_REQSKB)
> > > return 0;
> > > secattr = selinux_netlbl_sock_getattr(sk, sid);
> > > @@ -270,6 +271,61 @@ int selinux_netlbl_skbuff_setsid(struct
> > > sk_buff
> > > *skb,
> > > return rc;
> > > }
> > >
> > > +/**
> > > + * selinux_netlbl_sctp_assoc_request - Label an incoming sctp
> > > association.
> > > + * @ep: incoming association endpoint.
> > > + * @skb: the packet.
> > > + *
> > > + * Description:
> > > + * A new incoming connection is represented by @ep, ......
> > > + * Returns zero on success, negative values on failure.
> > > + *
> > > + */
> > > +int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
> > > + struct sk_buff *skb)
> > > +{
> > > + int rc;
> > > + struct netlbl_lsm_secattr secattr;
> > > + struct sk_security_struct *sksec = ep->base.sk-
> > > > sk_security;
> > >
> > > + struct sockaddr *addr;
> > > + struct sockaddr_in addr4;
> > > +#if IS_ENABLED(CONFIG_IPV6)
> > > + struct sockaddr_in6 addr6;
> > > +#endif
> > > +
> > > + if (ep->base.sk->sk_family != PF_INET &&
> > > + ep->base.sk->sk_family !=
> > > PF_INET6)
> > > + return 0;
> > > +
> > > + netlbl_secattr_init(&secattr);
> > > + rc = security_netlbl_sid_to_secattr(ep->secid,
> > > &secattr);
> > > + if (rc != 0)
> > > + goto assoc_request_return;
> > > +
> > > + /* Move skb hdr address info to a struct sockaddr and
> > > then
> > > call
> > > + * netlbl_conn_setattr().
> > > + */
> > > + if (ip_hdr(skb)->version == 4) {
> > > + addr4.sin_family = AF_INET;
> > > + addr4.sin_addr.s_addr = ip_hdr(skb)->saddr;
> > > + addr = (struct sockaddr *)&addr4;
> > > +#if IS_ENABLED(CONFIG_IPV6)
> > > + } else {
> > > + addr6.sin6_family = AF_INET6;
> > > + addr6.sin6_addr = ipv6_hdr(skb)->saddr;
> > > + addr = (struct sockaddr *)&addr6;
> > > +#endif
> > > + }
> > > +
> > > + rc = netlbl_conn_setattr(ep->base.sk, addr, &secattr);
> > > + if (rc == 0)
> > > + sksec->nlbl_state = NLBL_LABELED;
> > > +
> > > +assoc_request_return:
> > > + netlbl_secattr_destroy(&secattr);
> > > + return rc;
> > > +}
> > > +
> > > /**
> > > * selinux_netlbl_inet_conn_request - Label an incoming stream
> > > connection
> > > * @req: incoming connection request socket
> > > @@ -470,7 +526,8 @@ int selinux_netlbl_socket_setsockopt(struct
> > > socket *sock,
> > > }
> > >
> > > /**
> > > - * selinux_netlbl_socket_connect - Label a client-side socket on
> > > connect
> > > + * selinux_netlbl_socket_connect_helper - Help label a client-
> > > side
> > > socket on
> > > + * connect
> > > * @sk: the socket to label
> > > * @addr: the destination address
> > > *
> > > @@ -479,18 +536,13 @@ int selinux_netlbl_socket_setsockopt(struct
> > > socket *sock,
> > > * Returns zero values on success, negative values on failure.
> > > *
> > > */
> > > -int selinux_netlbl_socket_connect(struct sock *sk, struct
> > > sockaddr
> > > *addr)
> > > +static int selinux_netlbl_socket_connect_helper(struct sock *sk,
> > > + struct sockaddr
> > > *addr)
> > > {
> > > int rc;
> > > struct sk_security_struct *sksec = sk->sk_security;
> > > struct netlbl_lsm_secattr *secattr;
> > >
> > > - if (sksec->nlbl_state != NLBL_REQSKB &&
> > > - sksec->nlbl_state != NLBL_CONNLABELED)
> > > - return 0;
> > > -
> > > - lock_sock(sk);
> > > -
> > > /* connected sockets are allowed to disconnect when the
> > > address family
> > > * is set to AF_UNSPEC, if that is what is happening we
> > > want
> > > to reset
> > > * the socket */
> > > @@ -498,18 +550,72 @@ int selinux_netlbl_socket_connect(struct
> > > sock
> > > *sk, struct sockaddr *addr)
> > > netlbl_sock_delattr(sk);
> > > sksec->nlbl_state = NLBL_REQSKB;
> > > rc = 0;
> > > - goto socket_connect_return;
> > > + return rc;
> > > }
> > > secattr = selinux_netlbl_sock_genattr(sk);
> > > if (secattr == NULL) {
> > > rc = -ENOMEM;
> > > - goto socket_connect_return;
> > > + return rc;
> > > }
> > > rc = netlbl_conn_setattr(sk, addr, secattr);
> > > if (rc == 0)
> > > sksec->nlbl_state = NLBL_CONNLABELED;
> > >
> > > -socket_connect_return:
> > > + return rc;
> > > +}
> > > +
> > > +/**
> > > + * selinux_netlbl_socket_connect - Label a client-side socket on
> > > connect
> > > + * @sk: the socket to label
> > > + * @addr: the destination address
> > > + *
> > > + * Description:
> > > + * Attempt to label a connected socket with NetLabel using the
> > > given
> > > address.
> > > + * Returns zero values on success, negative values on failure.
> > > + *
> > > + */
> > > +int selinux_netlbl_socket_connect(struct sock *sk, struct
> > > sockaddr
> > > *addr)
> > > +{
> > > + int rc;
> > > + struct sk_security_struct *sksec = sk->sk_security;
> > > +
> > > + if (sksec->nlbl_state != NLBL_REQSKB &&
> > > + sksec->nlbl_state != NLBL_CONNLABELED)
> > > + return 0;
> > > +
> > > + lock_sock(sk);
> > > + rc = selinux_netlbl_socket_connect_helper(sk, addr);
> > > release_sock(sk);
> > > +
> > > + return rc;
> > > +}
> > > +
> > > +/**
> > > + * selinux_netlbl_sctp_socket_connect - Label an SCTP client-
> > > side
> > > socket on a
> > > + * connect
> > > + * @sk: the socket to label
> > > + * @addr: the destination address
> > > + *
> > > + * Description:
> > > + * Attempt to label a connected socket with NetLabel using the
> > > given
> > > address
> > > + * when called by the SCTP protocol layer. The situations
> > > handled
> > > are:
> > > + * sctp_connectx(3), sctp_sendmsg(3), sendmsg(2), whenever a new
> > > IP
> > > address
> > > + * is added or when a new primary address is selected. Note that
> > > an
> > > SCTP
> > > + * connect(2) call happens before the SCTP protocol layer and is
> > > handled via
> > > + * selinux_netlbl_socket_connect()
> > > + * Returns zero values on success, negative values on failure.
> > > + *
> > > + */
> > > +int selinux_netlbl_sctp_socket_connect(struct sock *sk, struct
> > > sockaddr *addr)
> > > +{
> > > + int rc;
> > > + struct sk_security_struct *sksec = sk->sk_security;
> > > +
> > > + if (sksec->nlbl_state != NLBL_REQSKB &&
> > > + sksec->nlbl_state != NLBL_CONNLABELED)
> > > + return 0;
> > > +
> > > + rc = selinux_netlbl_socket_connect_helper(sk, addr);
> > > +
> > > return rc;
> > > }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH net-next 0/4] Introduce NETIF_F_GRO_HW
From: Elior, Ariel @ 2017-12-04 11:38 UTC (permalink / raw)
To: Michael Chan, davem@davemloft.net; +Cc: netdev@vger.kernel.org
In-Reply-To: <1512385967-32339-1-git-send-email-michael.chan@broadcom.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> Behalf Of Michael Chan
> Sent: Monday, December 04, 2017 1:13 PM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org
> Subject: [PATCH net-next 0/4] Introduce NETIF_F_GRO_HW
>
> Introduce NETIF_F_GRO_HW feature flag and convert drivers that support
> hardware GRO to use the flag.
>
> Michael Chan (4):
> net: Introduce NETIF_F_GRO_HW
> bnxt_en: Use NETIF_F_GRO_HW.
> bnx2x: Use NETIF_F_GRO_HW.
> qede: Use NETIF_F_GRO_HW.
>
> Documentation/networking/netdev-features.txt | 7 +++++++
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 10 +++-------
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 +++-
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 8 +++++---
> drivers/net/ethernet/qlogic/qede/qede_filter.c | 9 ++-------
> drivers/net/ethernet/qlogic/qede/qede_main.c | 4 ++--
> include/linux/netdev_features.h | 5 ++++-
> net/core/dev.c | 13 +++++++++++++
> net/core/ethtool.c | 1 +
> 9 files changed, 40 insertions(+), 21 deletions(-)
>
> --
> 1.8.3.1
Michael, Thanks for adding this.
Dave, we will be reviewing and testing out the changes on bnx2x and qede.
Will provide feedback shortly.
Thanks,
Ariel
^ permalink raw reply
* Re: [e1000_shutdown] e1000 0000:00:03.0: disabling already-disabled device
From: Fengguang Wu @ 2017-12-04 11:33 UTC (permalink / raw)
To: Tushar Dave
Cc: intel-wired-lan, Jeff Kirsher, Linus Torvalds, David S. Miller,
Sasha Neftin, Dima Ruinskiy, Raanan Avargil, Jacob Keller,
Thomas Gleixner, Konstantin Khlebnikov, Bernd Faust,
stephen hemminger, netdev, linux-kernel, lkp
In-Reply-To: <97fc7c6e-cc19-dfcf-c1cc-353f7ebf1ceb@oracle.com>
Hi Tushar,
On Tue, Nov 28, 2017 at 01:01:23AM +0530, Tushar Dave wrote:
>
>
>On 11/23/2017 04:43 AM, Fengguang Wu wrote:
>> On Wed, Nov 22, 2017 at 03:40:52AM +0530, Tushar Dave wrote:
>>>
>>>
>>> On 11/21/2017 06:11 PM, Fengguang Wu wrote:
>>>> Hello,
>>>>
>>>> FYI this happens in mainline kernel 4.14.0-01330-g3c07399.
>>>> It happens since 4.13 .
>>>>
>>>> It occurs in 3 out of 162 boots.
>>>>
>>>>
>>>> [ 44.637743] advantechwdt: Unexpected close, not stopping watchdog!
>>>> [ 44.997548] input: ImExPS/2 Generic Explorer Mouse as
>>>> /devices/platform/i8042/serio1/input/input6
>>>> [ 45.013419] e1000 0000:00:03.0: disabling already-disabled device
>>>> [ 45.013447] ------------[ cut here ]------------
>>>> [ 45.014868] WARNING: CPU: 1 PID: 71 at drivers/pci/pci.c:1641
>>>> pci_disable_device+0xa1/0x105:
>>>> pci_disable_device at drivers/pci/pci.c:1640
>>>> [ 45.016171] CPU: 1 PID: 71 Comm: rcu_perf_shutdo Not tainted
>>>> 4.14.0-01330-g3c07399 #1
>>>> [ 45.017197] task: ffff88011bee9e40 task.stack: ffffc90000860000
>>>> [ 45.017987] RIP: 0010:pci_disable_device+0xa1/0x105:
>>>> pci_disable_device at drivers/pci/pci.c:1640
>>>> [ 45.018603] RSP: 0000:ffffc90000863e30 EFLAGS: 00010286
>>>> [ 45.019282] RAX: 0000000000000035 RBX: ffff88013a230008 RCX:
>>>> 0000000000000000
>>>> [ 45.020182] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
>>>> 0000000000000203
>>>> [ 45.021084] RBP: ffff88013a3f31e8 R08: 0000000000000001 R09:
>>>> 0000000000000000
>>>> [ 45.021986] R10: ffffffff827ec29c R11: 0000000000000002 R12:
>>>> 0000000000000001
>>>> [ 45.022946] R13: ffff88013a230008 R14: ffff880117802b20 R15:
>>>> ffffc90000863e8f
>>>> [ 45.023842] FS: 0000000000000000(0000) GS:ffff88013fd00000(0000)
>>>> knlGS:0000000000000000
>>>> [ 45.024863] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [ 45.025583] CR2: ffffc900006d4000 CR3: 000000000220f000 CR4:
>>>> 00000000000006a0
>>>> [ 45.026478] Call Trace:
>>>> [ 45.026811] __e1000_shutdown+0x1d4/0x1e2:
>>>> __e1000_shutdown at
>>>> drivers/net/ethernet/intel/e1000/e1000_main.c:5162
>>>> [ 45.027344] ? rcu_perf_cleanup+0x2a1/0x2a1:
>>>> rcu_perf_shutdown at kernel/rcu/rcuperf.c:627
>>>> [ 45.027883] e1000_shutdown+0x14/0x3a:
>>>> e1000_shutdown at
>>>> drivers/net/ethernet/intel/e1000/e1000_main.c:5235
>>>> [ 45.028351] device_shutdown+0x110/0x1aa:
>>>> device_shutdown at drivers/base/core.c:2807
>>>> [ 45.028858] kernel_power_off+0x31/0x64:
>>>> kernel_power_off at kernel/reboot.c:260
>>>> [ 45.029343] rcu_perf_shutdown+0x9b/0xa7:
>>>> rcu_perf_shutdown at kernel/rcu/rcuperf.c:637
>>>> [ 45.029852] ? __wake_up_common_lock+0xa2/0xa2:
>>>> autoremove_wake_function at
>>>> kernel/sched/wait.c:376
>>>> [ 45.030414] kthread+0x126/0x12e:
>>>> kthread at kernel/kthread.c:233
>>>> [ 45.030834] ? __kthread_bind_mask+0x8e/0x8e:
>>>> kthread at kernel/kthread.c:190
>>>> [ 45.031399] ? ret_from_fork+0x1f/0x30:
>>>> ret_from_fork at arch/x86/entry/entry_64.S:443
>>>> [ 45.031883] ? kernel_init+0xa/0xf5:
>>>> kernel_init at init/main.c:997
>>>> [ 45.032325] ret_from_fork+0x1f/0x30:
>>>> ret_from_fork at arch/x86/entry/entry_64.S:443
>>>> [ 45.032777] Code: 00 48 85 ed 75 07 48 8b ab a8 00 00 00 48 8d bb
>>>> 98 00 00 00 e8 aa d1 11 00 48 89 ea 48 89 c6 48 c7 c7 d8 e4 0b 82 e8
>>>> 55 7d da ff <0f> ff b9 01 00 00 00 31 d2 be 01 00 00 00 48 c7 c7 f0
>>>> b1 61 82
>>>> [ 45.035222] ---[ end trace c257137b1b1976ef ]---
>>>> [ 45.037838] ACPI: Preparing to enter system sleep state S5
>>>>
>>>> Attached the full dmesg, kconfig and reproduce scripts.
>>> Looks like e1000 pci/pxi-x device is already suspended. And therefore
>>> call to e1000_suspend() -> __e1000_shutdown() -> pci_disable_device()
>>> already had disabled the device.
>>> Disabling device again by e1000_shutdown handler during system shutdown
>>> causes warning at drivers/pci/pci.c:1641.
>>>
>>> I think function __e1000_shutdown should just return if device is
>>> already suspended!
>>>
>>> I don't have e1000 hardware to test right now. So if this seems logical
>>> to others I will send a patch.
>>
>> Tushar, it happens on QEMU boot testing, so do not rely on e1000 HW.
>> Unless you'd like to prevent regressions on real HW.
>>
>> The original report attached a reproduce script to run the QEMU test.
>> Or you may send me the patch for testing.
>Fengguang,
>
>Would you please try this patch and test. The patch is compile tested
>only. The patch is similar to how ixgbe handled the issue.
>Thanks.
>
>e1000: fix disabling already-disabled warning
>
>This patch adds check so that driver does not disable already
>disabled device.
It works! I tried 100 boots and the "e1000 0000:00:03.0: disabling
already-disabled device" error no longer show up.
Tested-by: Fengguang Wu <fengguang.wu@intel.com>
Thanks,
Fengguang
>Signed-off-by: Tushar Dave <tushar.n.dave@oracle.com>
>---
> drivers/net/ethernet/intel/e1000/e1000.h | 3 ++-
> drivers/net/ethernet/intel/e1000/e1000_main.c | 23 ++++++++++++++++++-----
> 2 files changed, 20 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/e1000/e1000.h
>b/drivers/net/ethernet/intel/e1000/e1000.h
>index d7bdea7..8fd2458 100644
>--- a/drivers/net/ethernet/intel/e1000/e1000.h
>+++ b/drivers/net/ethernet/intel/e1000/e1000.h
>@@ -331,7 +331,8 @@ struct e1000_adapter {
> enum e1000_state_t {
> __E1000_TESTING,
> __E1000_RESETTING,
>- __E1000_DOWN
>+ __E1000_DOWN,
>+ __E1000_DISABLED
> };
>
> #undef pr_fmt
>diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c
>b/drivers/net/ethernet/intel/e1000/e1000_main.c
>index 1982f79..a7de31d 100644
>--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
>+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
>@@ -945,7 +945,7 @@ static int e1000_init_hw_struct(struct e1000_adapter
>*adapter,
> static int e1000_probe(struct pci_dev *pdev, const struct
>pci_device_id *ent)
> {
> struct net_device *netdev;
>- struct e1000_adapter *adapter;
>+ struct e1000_adapter *adapter = NULL;
> struct e1000_hw *hw;
>
> static int cards_found;
>@@ -955,6 +955,7 @@ static int e1000_probe(struct pci_dev *pdev, const
>struct pci_device_id *ent)
> u16 tmp = 0;
> u16 eeprom_apme_mask = E1000_EEPROM_APME;
> int bars, need_ioport;
>+ bool disable_dev = false;
>
> /* do not allocate ioport bars when not needed */
> need_ioport = e1000_is_need_ioport(pdev);
>@@ -1259,11 +1260,13 @@ static int e1000_probe(struct pci_dev *pdev,
>const struct pci_device_id *ent)
> iounmap(hw->ce4100_gbe_mdio_base_virt);
> iounmap(hw->hw_addr);
> err_ioremap:
>+ disable_dev = !test_and_set_bit(__E1000_DISABLED, &adapter->flags);
> free_netdev(netdev);
> err_alloc_etherdev:
> pci_release_selected_regions(pdev, bars);
> err_pci_reg:
>- pci_disable_device(pdev);
>+ if (!adapter || disable_dev)
>+ pci_disable_device(pdev);
> return err;
> }
>
>@@ -1281,6 +1284,7 @@ static void e1000_remove(struct pci_dev *pdev)
> struct net_device *netdev = pci_get_drvdata(pdev);
> struct e1000_adapter *adapter = netdev_priv(netdev);
> struct e1000_hw *hw = &adapter->hw;
>+ bool disable_dev;
>
> e1000_down_and_stop(adapter);
> e1000_release_manageability(adapter);
>@@ -1299,9 +1303,11 @@ static void e1000_remove(struct pci_dev *pdev)
> iounmap(hw->flash_address);
> pci_release_selected_regions(pdev, adapter->bars);
>
>+ disable_dev = !test_and_set_bit(__E1000_DISABLED, &adapter->flags);
> free_netdev(netdev);
>
>- pci_disable_device(pdev);
>+ if (disable_dev)
>+ pci_disable_device(pdev);
> }
>
> /**
>@@ -5156,7 +5162,8 @@ static int __e1000_shutdown(struct pci_dev *pdev,
>bool *enable_wake)
> if (netif_running(netdev))
> e1000_free_irq(adapter);
>
>- pci_disable_device(pdev);
>+ if (!test_and_set_bit(__E1000_DISABLED, &adapter->flags))
>+ pci_disable_device(pdev);
>
> return 0;
> }
>@@ -5200,6 +5207,8 @@ static int e1000_resume(struct pci_dev *pdev)
> pr_err("Cannot enable PCI device from suspend\n");
> return err;
> }
>+ smp_mb__before_atomic();
>+ clear_bit(__E1000_DISABLED, &adapter->flags);
> pci_set_master(pdev);
>
> pci_enable_wake(pdev, PCI_D3hot, 0);
>@@ -5274,7 +5283,9 @@ static pci_ers_result_t
>e1000_io_error_detected(struct pci_dev *pdev,
>
> if (netif_running(netdev))
> e1000_down(adapter);
>- pci_disable_device(pdev);
>+
>+ if (!test_and_set_bit(__E1000_DISABLED, &adapter->flags))
>+ pci_disable_device(pdev);
>
> /* Request a slot slot reset. */
> return PCI_ERS_RESULT_NEED_RESET;
>@@ -5302,6 +5313,8 @@ static pci_ers_result_t e1000_io_slot_reset(struct
>pci_dev *pdev)
> pr_err("Cannot re-enable PCI device after reset.\n");
> return PCI_ERS_RESULT_DISCONNECT;
> }
>+ smp_mb__before_atomic();
>+ clear_bit(__E1000_DISABLED, &adapter->flags);
> pci_set_master(pdev);
>
> pci_enable_wake(pdev, PCI_D3hot, 0);
>--
>1.8.3.1
>
>
>-Tushar
>>
>> Thanks,
>> Fengguang
>>
>
^ permalink raw reply
* [patch net 1/2] net_sched: red: Avoid devision by zero
From: Nogah Frankel @ 2017-12-04 11:31 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, mlxsw, Nogah Frankel
In-Reply-To: <1512387071-35193-1-git-send-email-nogahf@mellanox.com>
Do not allow delta value to be zero since it is used as a divisor.
Fixes: 8af2a218de38 ("sch_red: Adaptative RED AQM")
Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
---
include/net/red.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/red.h b/include/net/red.h
index 9a93477..5918f78 100644
--- a/include/net/red.h
+++ b/include/net/red.h
@@ -179,7 +179,7 @@ static inline void red_set_parms(struct red_parms *p,
p->qth_max = qth_max << Wlog;
p->Wlog = Wlog;
p->Plog = Plog;
- if (delta < 0)
+ if (delta <= 0)
delta = 1;
p->qth_delta = delta;
if (!max_P) {
--
2.4.3
^ permalink raw reply related
* [patch net 2/2] net_sched: red: Avoid illegal values
From: Nogah Frankel @ 2017-12-04 11:31 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, mlxsw, Nogah Frankel
In-Reply-To: <1512387071-35193-1-git-send-email-nogahf@mellanox.com>
Check the qmin & qmax values doesn't overflow for the given Wlog value.
Check that qmin <= qmax.
Fixes: a783474591f2 ("[PKT_SCHED]: Generic RED layer")
Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
---
include/net/red.h | 11 +++++++++++
net/sched/sch_choke.c | 3 +++
net/sched/sch_gred.c | 3 +++
net/sched/sch_red.c | 2 ++
net/sched/sch_sfq.c | 3 +++
5 files changed, 22 insertions(+)
diff --git a/include/net/red.h b/include/net/red.h
index 5918f78..9665582 100644
--- a/include/net/red.h
+++ b/include/net/red.h
@@ -168,6 +168,17 @@ static inline void red_set_vars(struct red_vars *v)
v->qcount = -1;
}
+static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog)
+{
+ if (fls(qth_min) + Wlog > 32)
+ return false;
+ if (fls(qth_max) + Wlog > 32)
+ return false;
+ if (qth_max < qth_min)
+ return false;
+ return true;
+}
+
static inline void red_set_parms(struct red_parms *p,
u32 qth_min, u32 qth_max, u8 Wlog, u8 Plog,
u8 Scell_log, u8 *stab, u32 max_P)
diff --git a/net/sched/sch_choke.c b/net/sched/sch_choke.c
index b30a2c7..531250f 100644
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -369,6 +369,9 @@ static int choke_change(struct Qdisc *sch, struct nlattr *opt)
ctl = nla_data(tb[TCA_CHOKE_PARMS]);
+ if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+ return -EINVAL;
+
if (ctl->limit > CHOKE_MAX_QUEUE)
return -EINVAL;
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index 17c7130..bc30f91 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -356,6 +356,9 @@ static inline int gred_change_vq(struct Qdisc *sch, int dp,
struct gred_sched *table = qdisc_priv(sch);
struct gred_sched_data *q = table->tab[dp];
+ if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+ return -EINVAL;
+
if (!q) {
table->tab[dp] = q = *prealloc;
*prealloc = NULL;
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index 7f8ea9e..9d874e6 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -212,6 +212,8 @@ static int red_change(struct Qdisc *sch, struct nlattr *opt)
max_P = tb[TCA_RED_MAX_P] ? nla_get_u32(tb[TCA_RED_MAX_P]) : 0;
ctl = nla_data(tb[TCA_RED_PARMS]);
+ if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+ return -EINVAL;
if (ctl->limit > 0) {
child = fifo_create_dflt(sch, &bfifo_qdisc_ops, ctl->limit);
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 09c1203..930e5bd 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -639,6 +639,9 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
if (ctl->divisor &&
(!is_power_of_2(ctl->divisor) || ctl->divisor > 65536))
return -EINVAL;
+ if (ctl_v1 && !red_check_params(ctl_v1->qth_min, ctl_v1->qth_max,
+ ctl_v1->Wlog))
+ return -EINVAL;
if (ctl_v1 && ctl_v1->qth_min) {
p = kmalloc(sizeof(*p), GFP_KERNEL);
if (!p)
--
2.4.3
^ permalink raw reply related
* [patch net 0/2] RED qdisc fixes
From: Nogah Frankel @ 2017-12-04 11:31 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, mlxsw, Nogah Frankel
Add some input validation checks to RED qdisc.
Nogah Frankel (2):
net_sched: red: Avoid devision by zero
net_sched: red: Avoid illegal values
include/net/red.h | 13 ++++++++++++-
net/sched/sch_choke.c | 3 +++
net/sched/sch_gred.c | 3 +++
net/sched/sch_red.c | 2 ++
net/sched/sch_sfq.c | 3 +++
5 files changed, 23 insertions(+), 1 deletion(-)
--
2.4.3
^ 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