* [PATCH net-next 08/11] r8169: remove manual padding in struct ring_info
From: Heiner Kallweit @ 2018-11-19 21:38 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <7484b166-100b-52b9-1539-22e9199a8a3a@gmail.com>
The compiler takes care of alignment and padding, I see no need to
bother him with manual hints.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 3ffd9c18b..6736b804c 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -603,7 +603,6 @@ struct RxDesc {
struct ring_info {
struct sk_buff *skb;
u32 len;
- u8 __pad[sizeof(void *) - sizeof(u32)];
};
struct rtl8169_counters {
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 07/11] r8169: remove "not PCI Express" message
From: Heiner Kallweit @ 2018-11-19 21:37 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <7484b166-100b-52b9-1539-22e9199a8a3a@gmail.com>
The ones who want to know can easily identify whether chip is PCI or
PCIe based on the chip name. I doubt there's any benefit in this
message, so remove it.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index a6b12adbc..3ffd9c18b 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7291,9 +7291,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
tp->mmio_addr = pcim_iomap_table(pdev)[region];
- if (!pci_is_pcie(pdev))
- dev_info(&pdev->dev, "not PCI Express\n");
-
/* Identify chip attached to board */
rtl8169_get_mac_version(tp, cfg->default_ver);
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 06/11] r8169: remove print_mac_version
From: Heiner Kallweit @ 2018-11-19 21:36 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <7484b166-100b-52b9-1539-22e9199a8a3a@gmail.com>
The syslog message printed on driver load allows to easily identify
the mac version number (based on chip name and XID). So we don't
need this extra debug message which is wrong anyway because e.g.
RTL_GIGA_MAC_VER_01 has value 0.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index ee6458070..a6b12adbc 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -2170,11 +2170,6 @@ static void rtl8169_get_mac_version(struct rtl8169_private *tp,
}
}
-static void rtl8169_print_mac_version(struct rtl8169_private *tp)
-{
- netif_dbg(tp, drv, tp->dev, "mac_version = 0x%02x\n", tp->mac_version);
-}
-
struct phy_reg {
u16 reg;
u16 val;
@@ -3897,8 +3892,6 @@ static void rtl_hw_phy_config(struct net_device *dev)
{
struct rtl8169_private *tp = netdev_priv(dev);
- rtl8169_print_mac_version(tp);
-
switch (tp->mac_version) {
case RTL_GIGA_MAC_VER_01:
break;
@@ -7340,8 +7333,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
rtl_init_mdio_ops(tp);
rtl_init_jumbo_ops(tp);
- rtl8169_print_mac_version(tp);
-
chipset = tp->mac_version;
rc = rtl_alloc_irq(tp);
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 05/11] r8169: use PCI_VDEVICE macro
From: Heiner Kallweit @ 2018-11-19 21:35 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <7484b166-100b-52b9-1539-22e9199a8a3a@gmail.com>
Using macro PCI_VDEVICE helps to simplify the PCI ID table.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 81bdd2fe0..ee6458070 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -212,24 +212,24 @@ enum cfg_version {
};
static const struct pci_device_id rtl8169_pci_tbl[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8129), 0, 0, RTL_CFG_0 },
- { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8136), 0, 0, RTL_CFG_2 },
- { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8161), 0, 0, RTL_CFG_1 },
- { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8167), 0, 0, RTL_CFG_0 },
- { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8168), 0, 0, RTL_CFG_1 },
- { PCI_DEVICE(PCI_VENDOR_ID_NCUBE, 0x8168), 0, 0, RTL_CFG_1 },
- { PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8169), 0, 0, RTL_CFG_0 },
- { PCI_VENDOR_ID_DLINK, 0x4300,
- PCI_VENDOR_ID_DLINK, 0x4b10, 0, 0, RTL_CFG_1 },
- { PCI_DEVICE(PCI_VENDOR_ID_DLINK, 0x4300), 0, 0, RTL_CFG_0 },
- { PCI_DEVICE(PCI_VENDOR_ID_DLINK, 0x4302), 0, 0, RTL_CFG_0 },
- { PCI_DEVICE(PCI_VENDOR_ID_AT, 0xc107), 0, 0, RTL_CFG_0 },
- { PCI_DEVICE(PCI_VENDOR_ID_USR, 0x0116), 0, 0, RTL_CFG_0 },
+ { PCI_VDEVICE(REALTEK, 0x8129), RTL_CFG_0 },
+ { PCI_VDEVICE(REALTEK, 0x8136), RTL_CFG_2 },
+ { PCI_VDEVICE(REALTEK, 0x8161), RTL_CFG_1 },
+ { PCI_VDEVICE(REALTEK, 0x8167), RTL_CFG_0 },
+ { PCI_VDEVICE(REALTEK, 0x8168), RTL_CFG_1 },
+ { PCI_VDEVICE(NCUBE, 0x8168), RTL_CFG_1 },
+ { PCI_VDEVICE(REALTEK, 0x8169), RTL_CFG_0 },
+ { PCI_VENDOR_ID_DLINK, 0x4300,
+ PCI_VENDOR_ID_DLINK, 0x4b10, 0, 0, RTL_CFG_1 },
+ { PCI_VDEVICE(DLINK, 0x4300), RTL_CFG_0 },
+ { PCI_VDEVICE(DLINK, 0x4302), RTL_CFG_0 },
+ { PCI_VDEVICE(AT, 0xc107), RTL_CFG_0 },
+ { PCI_VDEVICE(USR, 0x0116), RTL_CFG_0 },
{ PCI_VENDOR_ID_LINKSYS, 0x1032,
PCI_ANY_ID, 0x0024, 0, 0, RTL_CFG_0 },
{ 0x0001, 0x8168,
PCI_ANY_ID, 0x2410, 0, 0, RTL_CFG_2 },
- {0,},
+ {}
};
MODULE_DEVICE_TABLE(pci, rtl8169_pci_tbl);
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 04/11] r8169: replace event_slow with irq_mask
From: Heiner Kallweit @ 2018-11-19 21:34 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <7484b166-100b-52b9-1539-22e9199a8a3a@gmail.com>
Recently the "slow event" handler was removed, therefore the member
name isn't appropriate any longer. In addition store the full mask,
including the RTL_EVENT_NAPI interrupt source bits.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 9e04566a2..81bdd2fe0 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -661,7 +661,7 @@ struct rtl8169_private {
struct ring_info tx_skb[NUM_TX_DESC]; /* Tx data buffers */
u16 cp_cmd;
- u16 event_slow;
+ u16 irq_mask;
const struct rtl_coalesce_info *coalesce_info;
struct clk *clk;
@@ -1340,7 +1340,7 @@ static void rtl_irq_disable(struct rtl8169_private *tp)
static void rtl_irq_enable(struct rtl8169_private *tp)
{
- RTL_W16(tp, IntrMask, RTL_EVENT_NAPI | tp->event_slow);
+ RTL_W16(tp, IntrMask, tp->irq_mask);
}
static void rtl8169_irq_mask_and_ack(struct rtl8169_private *tp)
@@ -5389,8 +5389,8 @@ static void rtl_hw_start_8168(struct rtl8169_private *tp)
/* Work around for RxFIFO overflow. */
if (tp->mac_version == RTL_GIGA_MAC_VER_11) {
- tp->event_slow |= RxFIFOOver;
- tp->event_slow &= ~RxOverflow;
+ tp->irq_mask |= RxFIFOOver;
+ tp->irq_mask &= ~RxOverflow;
}
switch (tp->mac_version) {
@@ -5627,7 +5627,7 @@ static void rtl_hw_start_8106(struct rtl8169_private *tp)
static void rtl_hw_start_8101(struct rtl8169_private *tp)
{
if (tp->mac_version >= RTL_GIGA_MAC_VER_30)
- tp->event_slow &= ~RxFIFOOver;
+ tp->irq_mask &= ~RxFIFOOver;
if (tp->mac_version == RTL_GIGA_MAC_VER_13 ||
tp->mac_version == RTL_GIGA_MAC_VER_16)
@@ -6456,7 +6456,7 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
struct rtl8169_private *tp = dev_instance;
u16 status = rtl_get_events(tp);
- if (status == 0xffff || !(status & (RTL_EVENT_NAPI | tp->event_slow)))
+ if (status == 0xffff || !(status & tp->irq_mask))
return IRQ_NONE;
if (unlikely(status & SYSErr)) {
@@ -7013,28 +7013,28 @@ static const struct net_device_ops rtl_netdev_ops = {
static const struct rtl_cfg_info {
void (*hw_start)(struct rtl8169_private *tp);
- u16 event_slow;
+ u16 irq_mask;
unsigned int has_gmii:1;
const struct rtl_coalesce_info *coalesce_info;
u8 default_ver;
} rtl_cfg_infos [] = {
[RTL_CFG_0] = {
.hw_start = rtl_hw_start_8169,
- .event_slow = SYSErr | LinkChg | RxOverflow | RxFIFOOver,
+ .irq_mask = SYSErr | LinkChg | RxOverflow | RxFIFOOver,
.has_gmii = 1,
.coalesce_info = rtl_coalesce_info_8169,
.default_ver = RTL_GIGA_MAC_VER_01,
},
[RTL_CFG_1] = {
.hw_start = rtl_hw_start_8168,
- .event_slow = LinkChg | RxOverflow,
+ .irq_mask = LinkChg | RxOverflow,
.has_gmii = 1,
.coalesce_info = rtl_coalesce_info_8168_8136,
.default_ver = RTL_GIGA_MAC_VER_11,
},
[RTL_CFG_2] = {
.hw_start = rtl_hw_start_8101,
- .event_slow = LinkChg | RxOverflow | RxFIFOOver,
+ .irq_mask = LinkChg | RxOverflow | RxFIFOOver,
.coalesce_info = rtl_coalesce_info_8168_8136,
.default_ver = RTL_GIGA_MAC_VER_13,
}
@@ -7415,7 +7415,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
dev->max_mtu = jumbo_max;
tp->hw_start = cfg->hw_start;
- tp->event_slow = cfg->event_slow;
+ tp->irq_mask = RTL_EVENT_NAPI | cfg->irq_mask;
tp->coalesce_info = cfg->coalesce_info;
tp->rtl_fw = RTL_FIRMWARE_UNKNOWN;
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 03/11] r8169: remove unused interrupt sources
From: Heiner Kallweit @ 2018-11-19 21:33 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <7484b166-100b-52b9-1539-22e9199a8a3a@gmail.com>
Setting PCSTimeout interrupt source was copied from the vendor driver
which uses the chip programmable timer interrupt. The mainline driver
doesn't use this timer interrupt.
SYSErr indicates a PCI error and isn't defined on the PCIe models.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index fa6349b5d..9e04566a2 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -5389,7 +5389,7 @@ static void rtl_hw_start_8168(struct rtl8169_private *tp)
/* Work around for RxFIFO overflow. */
if (tp->mac_version == RTL_GIGA_MAC_VER_11) {
- tp->event_slow |= RxFIFOOver | PCSTimeout;
+ tp->event_slow |= RxFIFOOver;
tp->event_slow &= ~RxOverflow;
}
@@ -7027,15 +7027,14 @@ static const struct rtl_cfg_info {
},
[RTL_CFG_1] = {
.hw_start = rtl_hw_start_8168,
- .event_slow = SYSErr | LinkChg | RxOverflow,
+ .event_slow = LinkChg | RxOverflow,
.has_gmii = 1,
.coalesce_info = rtl_coalesce_info_8168_8136,
.default_ver = RTL_GIGA_MAC_VER_11,
},
[RTL_CFG_2] = {
.hw_start = rtl_hw_start_8101,
- .event_slow = SYSErr | LinkChg | RxOverflow | RxFIFOOver |
- PCSTimeout,
+ .event_slow = LinkChg | RxOverflow | RxFIFOOver,
.coalesce_info = rtl_coalesce_info_8168_8136,
.default_ver = RTL_GIGA_MAC_VER_13,
}
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 02/11] r8169: use dev_get_drvdata where possible
From: Heiner Kallweit @ 2018-11-19 21:32 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <7484b166-100b-52b9-1539-22e9199a8a3a@gmail.com>
Using dev_get_drvdata directly is simpler here.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index ff85f0282..fa6349b5d 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6819,8 +6819,7 @@ static void rtl8169_net_suspend(struct net_device *dev)
static int rtl8169_suspend(struct device *device)
{
- struct pci_dev *pdev = to_pci_dev(device);
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(device);
struct rtl8169_private *tp = netdev_priv(dev);
rtl8169_net_suspend(dev);
@@ -6850,8 +6849,7 @@ static void __rtl8169_resume(struct net_device *dev)
static int rtl8169_resume(struct device *device)
{
- struct pci_dev *pdev = to_pci_dev(device);
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(device);
struct rtl8169_private *tp = netdev_priv(dev);
clk_prepare_enable(tp->clk);
@@ -6864,8 +6862,7 @@ static int rtl8169_resume(struct device *device)
static int rtl8169_runtime_suspend(struct device *device)
{
- struct pci_dev *pdev = to_pci_dev(device);
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(device);
struct rtl8169_private *tp = netdev_priv(dev);
if (!tp->TxDescArray)
@@ -6886,8 +6883,7 @@ static int rtl8169_runtime_suspend(struct device *device)
static int rtl8169_runtime_resume(struct device *device)
{
- struct pci_dev *pdev = to_pci_dev(device);
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(device);
struct rtl8169_private *tp = netdev_priv(dev);
rtl_rar_set(tp, dev->dev_addr);
@@ -6905,8 +6901,7 @@ static int rtl8169_runtime_resume(struct device *device)
static int rtl8169_runtime_idle(struct device *device)
{
- struct pci_dev *pdev = to_pci_dev(device);
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(device);
if (!netif_running(dev) || !netif_carrier_ok(dev))
pm_schedule_suspend(device, 10000);
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 01/11] r8169: merge rtl_irq_enable and rtl_irq_enable_all
From: Heiner Kallweit @ 2018-11-19 21:31 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <7484b166-100b-52b9-1539-22e9199a8a3a@gmail.com>
After the recent changes to the interrupt handler rtl_irq_enable and
rtl_irq_enable_all can be merged.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 100731c86..ff85f0282 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1334,18 +1334,13 @@ static void rtl_irq_disable(struct rtl8169_private *tp)
mmiowb();
}
-static void rtl_irq_enable(struct rtl8169_private *tp, u16 bits)
-{
- RTL_W16(tp, IntrMask, bits);
-}
-
#define RTL_EVENT_NAPI_RX (RxOK | RxErr)
#define RTL_EVENT_NAPI_TX (TxOK | TxErr)
#define RTL_EVENT_NAPI (RTL_EVENT_NAPI_RX | RTL_EVENT_NAPI_TX)
-static void rtl_irq_enable_all(struct rtl8169_private *tp)
+static void rtl_irq_enable(struct rtl8169_private *tp)
{
- rtl_irq_enable(tp, RTL_EVENT_NAPI | tp->event_slow);
+ RTL_W16(tp, IntrMask, RTL_EVENT_NAPI | tp->event_slow);
}
static void rtl8169_irq_mask_and_ack(struct rtl8169_private *tp)
@@ -4643,7 +4638,7 @@ static void rtl_hw_start(struct rtl8169_private *tp)
rtl_set_rx_mode(tp->dev);
/* no early-rx interrupts */
RTL_W16(tp, MultiIntr, RTL_R16(tp, MultiIntr) & 0xf000);
- rtl_irq_enable_all(tp);
+ rtl_irq_enable(tp);
}
static void rtl_hw_start_8169(struct rtl8169_private *tp)
@@ -6533,7 +6528,7 @@ static int rtl8169_poll(struct napi_struct *napi, int budget)
if (work_done < budget) {
napi_complete_done(napi, work_done);
- rtl_irq_enable_all(tp);
+ rtl_irq_enable(tp);
mmiowb();
}
--
2.19.1
^ permalink raw reply related
* Re: [RFCv3 PATCH 1/6] uacce: Add documents for WarpDrive/uacce
From: Jason Gunthorpe @ 2018-11-19 21:41 UTC (permalink / raw)
To: Jerome Glisse
Cc: Tim Sell, linux-doc, Alexander Shishkin, Zaibo Xu, zhangfei.gao,
linuxarm, haojian.zhuang, Christoph Lameter, Hao Fang,
Gavin Schenk, Leon Romanovsky, RDMA mailing list, Vinod Koul,
Doug Ledford, Uwe Kleine-König, David Kershner, Kenneth Lee,
Johan Hovold, Cyrille Pitchen, Sagar Dharia
In-Reply-To: <20181119213320.GG4593@redhat.com>
On Mon, Nov 19, 2018 at 04:33:20PM -0500, Jerome Glisse wrote:
> On Mon, Nov 19, 2018 at 02:26:38PM -0700, Jason Gunthorpe wrote:
> > On Mon, Nov 19, 2018 at 03:26:15PM -0500, Jerome Glisse wrote:
> > > On Mon, Nov 19, 2018 at 01:11:56PM -0700, Jason Gunthorpe wrote:
> > > > On Mon, Nov 19, 2018 at 02:46:32PM -0500, Jerome Glisse wrote:
> > > >
> > > > > > ?? How can O_DIRECT be fine but RDMA not? They use exactly the same
> > > > > > get_user_pages flow, right? Can we do what O_DIRECT does in RDMA and
> > > > > > be fine too?
> > > > > >
> > > > > > AFAIK the only difference is the length of the race window. You'd have
> > > > > > to fork and fault during the shorter time O_DIRECT has get_user_pages
> > > > > > open.
> > > > >
> > > > > Well in O_DIRECT case there is only one page table, the CPU
> > > > > page table and it gets updated during fork() so there is an
> > > > > ordering there and the race window is small.
> > > >
> > > > Not really, in O_DIRECT case there is another 'page table', we just
> > > > call it a DMA scatter/gather list and it is sent directly to the block
> > > > device's DMA HW. The sgl plays exactly the same role as the various HW
> > > > page list data structures that underly RDMA MRs.
> > > >
> > > > It is not a page table that matters here, it is if the DMA address of
> > > > the page is active for DMA on HW.
> > > >
> > > > Like you say, the only difference is that the race is hopefully small
> > > > with O_DIRECT (though that is not really small, NVMeof for instance
> > > > has windows as large as connection timeouts, if you try hard enough)
> > > >
> > > > So we probably can trigger this trouble with O_DIRECT and fork(), and
> > > > I would call it a bug :(
> > >
> > > I can not think of any scenario that would be a bug with O_DIRECT.
> > > Do you have one in mind ? When you fork() and do other syscall that
> > > affect the memory of your process in another thread you should
> > > expect non consistant results. Kernel is not here to provide a fully
> > > safe environement to user, user can shoot itself in the foot and
> > > that's fine as long as it only affect the process itself and no one
> > > else. We should not be in the business of making everything baby
> > > proof :)
> >
> > Sure, I setup AIO with O_DIRECT and launch a read.
> >
> > Then I fork and dirty the READ target memory using the CPU in the
> > child.
> >
> > As you described in this case the fork will retain the physical page
> > that is undergoing O_DIRECT DMA, and the parent gets a new copy'd page.
> >
> > The DMA completes, and the child gets the DMA'd to page. The parent
> > gets an unchanged copy'd page.
> >
> > The parent gets the AIO completion, but can't see the data.
> >
> > I'd call that a bug with O_DIRECT. The only correct outcome is that
> > the parent will always see the O_DIRECT data. Fork should not cause
> > the *parent* to malfunction. I agree the child cannot make any
> > prediction what memory it will see.
> >
> > I assume the same flow is possible using threads and read()..
> >
> > It is really no different than the RDMA bug with fork.
> >
>
> Yes and that's expected behavior :) If you fork() and have anything
> still in flight at time of fork that can change your process address
> space (including data in it) then all bets are of.
>
> At least this is my reading of fork() syscall.
Not mine.. I can't think of anything else that would have this
behavior.
All traditional syscalls, will properly dirty the pages of the
parent. ie if I call read() in a thread and do fork in another thread,
then not seeing the data after read() completes is clearly a bug. All
other syscalls are the same.
It is bonkers that opening the file with O_DIRECT would change this
basic behavior. I'm calling it a bug :)
Jason
^ permalink raw reply
* Re: [PATCH 10/11] nvmet-tcp: add NVMe over TCP target driver
From: Sagi Grimberg @ 2018-11-19 21:37 UTC (permalink / raw)
To: Max Gurtovoy, David Miller, sagi
Cc: linux-block, netdev, keith.busch, hch, linux-nvme
In-Reply-To: <59335e55-7e19-3201-2f8b-beb681aba810@mellanox.com>
>>> +static unsigned nvmet_tcp_recv_budget = 8;
>>> +module_param_named(recv_budget, nvmet_tcp_recv_budget, int, S_IRUGO
>>> | S_IWUSR);
>>> +MODULE_PARM_DESC(recv_budget, "recvs budget");
>>> +
>>> +static unsigned nvmet_tcp_send_budget = 8;
>>> +module_param_named(send_budget, nvmet_tcp_send_budget, int, S_IRUGO
>>> | S_IWUSR);
>>> +MODULE_PARM_DESC(send_budget, "sends budget");
>>> +
>>> +static unsigned nvmet_tcp_io_work_budget = 64;
>>> +module_param_named(io_work_budget, nvmet_tcp_io_work_budget, int,
>>> S_IRUGO | S_IWUSR);
>>> +MODULE_PARM_DESC(io_work_budget, "io work budget");
>> I strongly suggest moving away from module parameters for this stuff.
>
> agree here.
>
> also, Sagi, can you explain about the performance trade-offs seen during
> your development for these values ?
>
> are they HCA/NIC dependent ?
>
> should send/recv ratio be 1:1 ?
>
> should total/send/recv ratio be 8:1:1 ?
These are not really HW dependent at all, its more about the tradeoff
between aggregation vs. fairness multiplexing. The budgets are designed
to control how much a specific workload (e.g. nvme queue) can hog the
cpu/wire when nvmet is servicing a large number of hosts.
There no constraints about the ratios of the budgets. Its advised though
that the io_work_budget would be able to catch at least a few
sends/recvs for reasonable aggregation.
I commented to Dave that I prefer not to expose them at this point given
that they are not trivial and would require an additional interface to
the driver (and its corresponding tool chain).
^ permalink raw reply
* [PATCH bpf-next] bpf: libbpf: retry map creation without the name
From: Stanislav Fomichev @ 2018-11-19 21:35 UTC (permalink / raw)
To: netdev, ast, daniel; +Cc: vladum, Stanislav Fomichev
Since commit 88cda1c9da02 ("bpf: libbpf: Provide basic API support
to specify BPF obj name"), libbpf unconditionally sets bpf_attr->name
for maps. Pre v4.14 kernels don't know about map names and return an
error about unexpected non-zero data. Retry sys_bpf without a map
name to cover older kernels.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/bpf.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 03f9bcc4ef50..673175bc06ee 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -69,6 +69,7 @@ int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr)
{
__u32 name_len = create_attr->name ? strlen(create_attr->name) : 0;
union bpf_attr attr;
+ int ret;
memset(&attr, '\0', sizeof(attr));
@@ -86,7 +87,15 @@ int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr)
attr.map_ifindex = create_attr->map_ifindex;
attr.inner_map_fd = create_attr->inner_map_fd;
- return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+ ret = sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+ if (ret < 0 && create_attr->name) {
+ /* Retry the same syscall, but without the name.
+ * Pre v4.14 kernels don't support map names.
+ */
+ memset(attr.map_name, 0, sizeof(attr.map_name));
+ return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+ }
+ return ret;
}
int bpf_create_map_node(enum bpf_map_type map_type, const char *name,
--
2.19.1.1215.g8438c0b245-goog
^ permalink raw reply related
* Re: [RFCv3 PATCH 1/6] uacce: Add documents for WarpDrive/uacce
From: Jerome Glisse @ 2018-11-19 21:33 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Tim Sell, linux-doc, Alexander Shishkin, Zaibo Xu, zhangfei.gao,
linuxarm, haojian.zhuang, Christoph Lameter, Hao Fang,
Gavin Schenk, Leon Romanovsky, RDMA mailing list, Vinod Koul,
Doug Ledford, Uwe Kleine-König, David Kershner, Kenneth Lee,
Johan Hovold, Cyrille Pitchen, Sagar Dharia
In-Reply-To: <20181119212638.GI4890@ziepe.ca>
On Mon, Nov 19, 2018 at 02:26:38PM -0700, Jason Gunthorpe wrote:
> On Mon, Nov 19, 2018 at 03:26:15PM -0500, Jerome Glisse wrote:
> > On Mon, Nov 19, 2018 at 01:11:56PM -0700, Jason Gunthorpe wrote:
> > > On Mon, Nov 19, 2018 at 02:46:32PM -0500, Jerome Glisse wrote:
> > >
> > > > > ?? How can O_DIRECT be fine but RDMA not? They use exactly the same
> > > > > get_user_pages flow, right? Can we do what O_DIRECT does in RDMA and
> > > > > be fine too?
> > > > >
> > > > > AFAIK the only difference is the length of the race window. You'd have
> > > > > to fork and fault during the shorter time O_DIRECT has get_user_pages
> > > > > open.
> > > >
> > > > Well in O_DIRECT case there is only one page table, the CPU
> > > > page table and it gets updated during fork() so there is an
> > > > ordering there and the race window is small.
> > >
> > > Not really, in O_DIRECT case there is another 'page table', we just
> > > call it a DMA scatter/gather list and it is sent directly to the block
> > > device's DMA HW. The sgl plays exactly the same role as the various HW
> > > page list data structures that underly RDMA MRs.
> > >
> > > It is not a page table that matters here, it is if the DMA address of
> > > the page is active for DMA on HW.
> > >
> > > Like you say, the only difference is that the race is hopefully small
> > > with O_DIRECT (though that is not really small, NVMeof for instance
> > > has windows as large as connection timeouts, if you try hard enough)
> > >
> > > So we probably can trigger this trouble with O_DIRECT and fork(), and
> > > I would call it a bug :(
> >
> > I can not think of any scenario that would be a bug with O_DIRECT.
> > Do you have one in mind ? When you fork() and do other syscall that
> > affect the memory of your process in another thread you should
> > expect non consistant results. Kernel is not here to provide a fully
> > safe environement to user, user can shoot itself in the foot and
> > that's fine as long as it only affect the process itself and no one
> > else. We should not be in the business of making everything baby
> > proof :)
>
> Sure, I setup AIO with O_DIRECT and launch a read.
>
> Then I fork and dirty the READ target memory using the CPU in the
> child.
>
> As you described in this case the fork will retain the physical page
> that is undergoing O_DIRECT DMA, and the parent gets a new copy'd page.
>
> The DMA completes, and the child gets the DMA'd to page. The parent
> gets an unchanged copy'd page.
>
> The parent gets the AIO completion, but can't see the data.
>
> I'd call that a bug with O_DIRECT. The only correct outcome is that
> the parent will always see the O_DIRECT data. Fork should not cause
> the *parent* to malfunction. I agree the child cannot make any
> prediction what memory it will see.
>
> I assume the same flow is possible using threads and read()..
>
> It is really no different than the RDMA bug with fork.
>
Yes and that's expected behavior :) If you fork() and have anything
still in flight at time of fork that can change your process address
space (including data in it) then all bets are of.
At least this is my reading of fork() syscall.
Cheers,
Jérôme
^ permalink raw reply
* [PATCH net-next 00/11] r8169: series with further smaller improvements
From: Heiner Kallweit @ 2018-11-19 21:30 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
Again nothing exciting, just smaller improvements.
Heiner Kallweit (11):
r8169: merge rtl_irq_enable and rtl_irq_enable_all
r8169: use dev_get_drvdata where possible
r8169: remove unused interrupt sources
r8169: replace event_slow with irq_mask
r8169: use PCI_VDEVICE macro
r8169: remove print_mac_version
r8169: remove "not PCI Express" info message
r8169: remove manual padding in struct ring_info
r8169: remove workaround for ancient gcc bug
r8169: simplify ocp functions
r8169: improve chip version identification
drivers/net/ethernet/realtek/r8169.c | 283 +++++++++++----------------
1 file changed, 112 insertions(+), 171 deletions(-)
--
2.19.1
^ permalink raw reply
* Re: [PATCH 10/11] nvmet-tcp: add NVMe over TCP target driver
From: Sagi Grimberg @ 2018-11-19 21:26 UTC (permalink / raw)
To: David Miller, sagi; +Cc: linux-block, netdev, keith.busch, hch, linux-nvme
In-Reply-To: <20181117.121510.1754018619653365187.davem@davemloft.net>
>> +static unsigned nvmet_tcp_recv_budget = 8;
>> +module_param_named(recv_budget, nvmet_tcp_recv_budget, int, S_IRUGO | S_IWUSR);
>> +MODULE_PARM_DESC(recv_budget, "recvs budget");
>> +
>> +static unsigned nvmet_tcp_send_budget = 8;
>> +module_param_named(send_budget, nvmet_tcp_send_budget, int, S_IRUGO | S_IWUSR);
>> +MODULE_PARM_DESC(send_budget, "sends budget");
>> +
>> +static unsigned nvmet_tcp_io_work_budget = 64;
>> +module_param_named(io_work_budget, nvmet_tcp_io_work_budget, int, S_IRUGO | S_IWUSR);
>> +MODULE_PARM_DESC(io_work_budget, "io work budget");
>
> I strongly suggest moving away from module parameters for this stuff.
>
> Create a genetlink socket family and allow run time configuration of these knobs
> by the user.
Thanks for the feedback Dave.
Given that these are non-trivial knobs to configure at runtime, I think
that I'll degenerate them to sane defaults for the initial submission
and figure out what interface makes sense afterwards.
Now that I have your attention ;)
I would love you to look at skb_copy_and_hash_datagram_iter as these
changes will require an ack from you.
^ permalink raw reply
* Re: [PATCH 05/11] nvmet: Add install_queue callout
From: Sagi Grimberg @ 2018-11-19 21:21 UTC (permalink / raw)
To: Max Gurtovoy, Sagi Grimberg, linux-nvme
Cc: linux-block, netdev, Keith Busch, Christoph Hellwig
In-Reply-To: <af370da9-3d56-e1a8-27db-c0a8e9291417@mellanox.com>
>> diff --git a/drivers/nvme/target/fabrics-cmd.c
>> b/drivers/nvme/target/fabrics-cmd.c
>> index 1f05d8507e35..a84668e8939c 100644
>> --- a/drivers/nvme/target/fabrics-cmd.c
>> +++ b/drivers/nvme/target/fabrics-cmd.c
>> @@ -117,6 +117,15 @@ static u16 nvmet_install_queue(struct nvmet_ctrl
>> *ctrl, struct nvmet_req *req)
>> nvmet_sq_setup(ctrl, req->sq, qid, sqsize + 1,
>> !!(c->cattr & NVME_CONNECT_DISABLE_SQFLOW));
>
> what is your code base ? does SQFLOW must for TCP ?
Not really, just happens to sit on top..
>> + if (ctrl->ops->install_queue) {
>> + int ret = ctrl->ops->install_queue(req->sq);
>> + if (ret) {
>> + pr_err("failed to install queue %d cntlid %d ret %d\n",
>> + qid, ret, ctrl->cntlid);
>> + return NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR;
>
> I'm not sure regarding the INVALID_PARAM rc.
>
> maybe use NVME_SC_INTERNAL ?
You're right, perhaps we should propagate ->install_queue() status.
^ permalink raw reply
* [PATCH net-next 6/8] net: hns3: Add pause config info query function
From: Salil Mehta @ 2018-11-19 21:18 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linuxarm, liuzhongzhu
In-Reply-To: <20181119211845.23168-1-salil.mehta@huawei.com>
From: liuzhongzhu <liuzhongzhu@huawei.com>
This patch prints qos pause config information.
debugfs command:
echo dump qos pause cfg > cmd
Sample Command:
root@(none)# echo dump qos pause cfg > cmd
hns3 0000:7d:00.0: dump qos pause cfg
hns3 0000:7d:00.0: pause_trans_gap: 0xff
hns3 0000:7d:00.0: pause_trans_time: 0xffff
root@(none)#
Signed-off-by: liuzhongzhu <liuzhongzhu@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
.../ethernet/hisilicon/hns3/hns3_debugfs.c | 1 +
.../hisilicon/hns3/hns3pf/hclge_debugfs.c | 25 +++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
index 5e938879ca66..83ca76849be4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
@@ -132,6 +132,7 @@ static void hns3_dbg_help(struct hnae3_handle *h)
dev_info(&h->pdev->dev, "dump fd tcam\n");
dev_info(&h->pdev->dev, "dump tc\n");
dev_info(&h->pdev->dev, "dump tm\n");
+ dev_info(&h->pdev->dev, "dump qos pause cfg\n");
}
static ssize_t hns3_dbg_cmd_read(struct file *filp, char __user *buffer,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
index e6f70260fd9d..3dc1c69269ee 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
@@ -242,6 +242,29 @@ static void hclge_dbg_dump_tm(struct hclge_dev *hdev)
cmd, ret);
}
+static void hclge_dbg_dump_qos_pause_cfg(struct hclge_dev *hdev)
+{
+ struct hclge_cfg_pause_param_cmd *pause_param;
+ struct hclge_desc desc;
+ int ret;
+
+ hclge_cmd_setup_basic_desc(&desc, HCLGE_OPC_CFG_MAC_PARA, true);
+
+ ret = hclge_cmd_send(&hdev->hw, &desc, 1);
+ if (ret) {
+ dev_err(&hdev->pdev->dev, "dump checksum fail, status is %d.\n",
+ ret);
+ return;
+ }
+
+ pause_param = (struct hclge_cfg_pause_param_cmd *)desc.data;
+ dev_info(&hdev->pdev->dev, "dump qos pause cfg\n");
+ dev_info(&hdev->pdev->dev, "pause_trans_gap: 0x%x\n",
+ pause_param->pause_trans_gap);
+ dev_info(&hdev->pdev->dev, "pause_trans_time: 0x%x\n",
+ pause_param->pause_trans_time);
+}
+
static void hclge_dbg_fd_tcam_read(struct hclge_dev *hdev, u8 stage,
bool sel_x, u32 loc)
{
@@ -307,6 +330,8 @@ int hclge_dbg_run_cmd(struct hnae3_handle *handle, char *cmd_buf)
hclge_dbg_dump_tc(hdev);
} else if (strncmp(cmd_buf, "dump tm", 7) == 0) {
hclge_dbg_dump_tm(hdev);
+ } else if (strncmp(cmd_buf, "dump qos pause cfg", 18) == 0) {
+ hclge_dbg_dump_qos_pause_cfg(hdev);
} else {
dev_info(&hdev->pdev->dev, "unknown command\n");
return -EINVAL;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 00/12 net-next,v2] add flow_rule infrastructure
From: Or Gerlitz @ 2018-11-19 21:19 UTC (permalink / raw)
To: David Miller
Cc: Pablo Neira Ayuso, Linux Netdev List, thomas.lendacky,
Florian Fainelli, Ariel Elior, Michael Chan, santosh,
madalin.bucur, Zhuangyuzeng (Yisen), Salil Mehta, Jeff Kirsher,
Tariq Toukan, Saeed Mahameed, Jiri Pirko, Ido Schimmel,
Jakub Kicinski, peppe.cavallaro, grygorii.strashko, Andrew Lunn,
Vivien Didelot, alexandre.torgue, joabr
In-Reply-To: <20181119.121229.2109900411029672811.davem@davemloft.net>
On Mon, Nov 19, 2018 at 10:14 PM David Miller <davem@davemloft.net> wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Mon, 19 Nov 2018 01:15:07 +0100
>
> > This patchset introduces a kernel intermediate representation (IR) to
> > express ACL hardware offloads, as already described in previous RFC and
> > v1 patchset [1] [2]. The idea is to normalize the frontend U/APIs to use
> > the flow dissectors and the flow actions so drivers can reuse the
> > existing TC offload driver codebase - that has been converted to use the
> > flow_rule infrastructure.
>
> I'm go to bring up the elephant in the room.
> I think the real motivation here is to offload netfilter rules to HW,
> and you should be completely honest about that.
Thanks Dave for clarifying.
So.. (A) why the TC path isn't enough for CT offloading? if we could have
just do it, that would have sound really cool. (B) why do we have to deal
with EIRs (Elephants In Rooms)? (C) who can address A && B?
Or.
^ permalink raw reply
* Re: [PATCH] xfrm: policy: add missing indentation
From: Steffen Klassert @ 2018-11-20 7:35 UTC (permalink / raw)
To: Colin King
Cc: Herbert Xu, David S . Miller, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20181113142842.10531-1-colin.king@canonical.com>
On Tue, Nov 13, 2018 at 02:28:42PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There is a missing indentation before the goto statement. Add it.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Also applied, thanks!
^ permalink raw reply
* Re: [PATCH] af_key: fix indentation on declaration statement
From: Steffen Klassert @ 2018-11-20 7:34 UTC (permalink / raw)
To: Colin King
Cc: Herbert Xu, David S . Miller, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20181113142328.8322-1-colin.king@canonical.com>
On Tue, Nov 13, 2018 at 02:23:28PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There is an indentation issue before the declaration of xfrm_ctx. Remove
> spaces and replace with a tab.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to ipsec-next.
^ permalink raw reply
* Re: netns_id in bpf_sk_lookup_{tcp,udp}
From: Joe Stringer @ 2018-11-19 20:54 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: David Ahern, Joe Stringer, netdev, daniel
In-Reply-To: <55816e4b-1a00-a1f3-6514-f755b992a049@6wind.com>
On Mon, 19 Nov 2018 at 12:29, Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>
> Le 19/11/2018 à 20:54, David Ahern a écrit :
> > On 11/19/18 12:47 PM, Joe Stringer wrote:
> >> On Mon, 19 Nov 2018 at 10:39, David Ahern <dsahern@gmail.com> wrote:
> >>>
> >>> On 11/19/18 11:36 AM, Joe Stringer wrote:
> >>>> Hi David, thanks for pointing this out.
> >>>>
> >>>> This is more of an oversight through iterations, the runtime lookup
> >>>> will fail to find a socket if the netns value is greater than the
> >>>> range of a uint32 so I think it would actually make more sense to drop
> >>>> the parameter size to u32 rather than u64 so that this would be
> >>>> validated at load time rather than silently returning NULL because of
> >>>> a bad parameter.
> >>>
> >>> ok. I was wondering if it was a u64 to handle nsid of 0 which as I
> >>> understand it is a legal nsid. If you drop to u32, how do you know when
> >>> nsid has been set?
> >>
> >> I was operating under the assumption that 0 represents the root netns
> >> id, and cannot be assigned to another non-root netns.
> >>
> >> Looking at __peernet2id_alloc(), it seems to me like it attempts to
> >> find a netns and if it cannot find one, returns 0, which then leads to
> >> a scroll over the idr starting from 0 to INT_MAX to find a legitimate
> >> id for the netns, so I think this is a fair assumption?
> The NET_ID_ZERO trick is used to manage nsid 0 in net_eq_idr() (idr_for_each()
> stops when the callback returns != 0).
>
> >>
> >
> > Maybe Nicolas can give a definitive answer; as I recall he added the
> > NSID option. I have not had time to walk the code. But I do recall
> > seeing an id of 0. e.g, on my dev box:
> > $ ip netns
> > vms (id: 0)
> >
> > And include/uapi/linux/net_namespace.h shows -1 as not assigned.
> Yes, 0 is a valid value and can be assigned to any netns.
> nsid are signed 32 bit values. Note that -1 (NETNSA_NSID_NOT_ASSIGNED) is used
> by the kernel to express that the nsid is not assigned. It can also be used by
> the user to let the kernel chooses a nsid.
>
> $ ip netns add foo
> $ ip netns add bar
> $ ip netns
> bar
> foo
> $ ip netns set foo 0
> $ ip netns set bar auto
> $ ip netns
> bar (id: 1)
> foo (id: 0)
OK, I'll fix this up then.
^ permalink raw reply
* Re: [PATCH net] Revert "sctp: remove sctp_transport_pmtu_check"
From: David Miller @ 2018-11-19 20:44 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman
In-Reply-To: <722a49900dadb13e3a97a63d54561418db28e1cc.1542528887.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Sun, 18 Nov 2018 16:14:47 +0800
> This reverts commit 22d7be267eaa8114dcc28d66c1c347f667d7878a.
>
> The dst's mtu in transport can be updated by a non sctp place like
> in xfrm where the MTU information didn't get synced between asoc,
> transport and dst, so it is still needed to do the pmtu check
> in sctp_packet_config.
Applied and queued up for -stable.
Please provide a proper signoff for reverts in the future.
Thanks.
^ permalink raw reply
* Re: [PATCHv2 net] sctp: not allow to set asoc prsctp_enable by sockopt
From: David Miller @ 2018-11-19 20:42 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman
In-Reply-To: <44569c3eada8633c11f17c63e2d5ea9f3936f59c.1542525713.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Sun, 18 Nov 2018 15:21:53 +0800
> As rfc7496#section4.5 says about SCTP_PR_SUPPORTED:
>
> This socket option allows the enabling or disabling of the
> negotiation of PR-SCTP support for future associations. For existing
> associations, it allows one to query whether or not PR-SCTP support
> was negotiated on a particular association.
>
> It means only sctp sock's prsctp_enable can be set.
>
> Note that for the limitation of SCTP_{CURRENT|ALL}_ASSOC, we will
> add it when introducing SCTP_{FUTURE|CURRENT|ALL}_ASSOC for linux
> sctp in another patchset.
>
> v1->v2:
> - drop the params.assoc_id check as Neil suggested.
>
> Fixes: 28aa4c26fce2 ("sctp: add SCTP_PR_SUPPORTED on sctp sockopt")
> Reported-by: Ying Xu <yinxu@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] sctp: count sk_wmem_alloc by skb truesize in sctp_packet_transmit
From: David Miller @ 2018-11-19 20:39 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman
In-Reply-To: <b3325c7aa29d1981dabd897b25eae421f7055b0d.1542524858.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Sun, 18 Nov 2018 15:07:38 +0800
> Now sctp increases sk_wmem_alloc by 1 when doing set_owner_w for the
> skb allocked in sctp_packet_transmit and decreases by 1 when freeing
> this skb.
>
> But when this skb goes through networking stack, some subcomponents
> might change skb->truesize and add the same amount on sk_wmem_alloc.
> However sctp doesn't know the amount to decrease by, it would cause
> a leak on sk->sk_wmem_alloc and the sock can never be freed.
>
> Xiumei found this issue when it hit esp_output_head() by using sctp
> over ipsec, where skb->truesize is added and so is sk->sk_wmem_alloc.
>
> Since sctp has used sk_wmem_queued to count for writable space since
> Commit cd305c74b0f8 ("sctp: use sk_wmem_queued to check for writable
> space"), it's ok to fix it by counting sk_wmem_alloc by skb truesize
> in sctp_packet_transmit.
>
> Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible")
> Reported-by: Xiumei Mu <xmu@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next 2/6] net/bpf: split VLAN_PRESENT bit handling from VLAN_TCI
From: Michał Mirosław @ 2018-11-19 20:30 UTC (permalink / raw)
To: Daniel Borkmann
Cc: netdev, Alexei Starovoitov, Benjamin Herrenschmidt,
David S. Miller, James Hogan, linux-mips, linuxppc-dev,
Michael Ellerman, Paul Burton, Paul Mackerras, Ralf Baechle,
sparclinux
In-Reply-To: <c420b7a8-73df-3e9c-962a-a295ad0a6139@iogearbox.net>
On Mon, Nov 19, 2018 at 12:26:46PM +0100, Daniel Borkmann wrote:
> On 11/10/2018 07:58 PM, Michał Mirosław wrote:
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>
> Why you have empty commit messages for non-trivial changes like this in
> 4 out of 6 of your patches ...
>
> How was it tested on the JITs you were changing? Did you test on both,
> big and little endian machines?
I have only x86 boxes currently so didn't try to test others. I hope
upstreaming the series through net-next will allow us to find any
fallouts, if any. The changes are very simple, though: they move
code around (the "splitting" part) and eventually change a vlan_present
flag's position in a skbuff structure. Dependency on CPU endianness is
removed by using byte loads for the flag.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: netns_id in bpf_sk_lookup_{tcp,udp}
From: Nicolas Dichtel @ 2018-11-19 20:28 UTC (permalink / raw)
To: David Ahern, Joe Stringer; +Cc: netdev, daniel
In-Reply-To: <c9400c6a-67a7-dd89-cf64-4f1b8ece89a2@gmail.com>
Le 19/11/2018 à 20:54, David Ahern a écrit :
> On 11/19/18 12:47 PM, Joe Stringer wrote:
>> On Mon, 19 Nov 2018 at 10:39, David Ahern <dsahern@gmail.com> wrote:
>>>
>>> On 11/19/18 11:36 AM, Joe Stringer wrote:
>>>> Hi David, thanks for pointing this out.
>>>>
>>>> This is more of an oversight through iterations, the runtime lookup
>>>> will fail to find a socket if the netns value is greater than the
>>>> range of a uint32 so I think it would actually make more sense to drop
>>>> the parameter size to u32 rather than u64 so that this would be
>>>> validated at load time rather than silently returning NULL because of
>>>> a bad parameter.
>>>
>>> ok. I was wondering if it was a u64 to handle nsid of 0 which as I
>>> understand it is a legal nsid. If you drop to u32, how do you know when
>>> nsid has been set?
>>
>> I was operating under the assumption that 0 represents the root netns
>> id, and cannot be assigned to another non-root netns.
>>
>> Looking at __peernet2id_alloc(), it seems to me like it attempts to
>> find a netns and if it cannot find one, returns 0, which then leads to
>> a scroll over the idr starting from 0 to INT_MAX to find a legitimate
>> id for the netns, so I think this is a fair assumption?
The NET_ID_ZERO trick is used to manage nsid 0 in net_eq_idr() (idr_for_each()
stops when the callback returns != 0).
>>
>
> Maybe Nicolas can give a definitive answer; as I recall he added the
> NSID option. I have not had time to walk the code. But I do recall
> seeing an id of 0. e.g, on my dev box:
> $ ip netns
> vms (id: 0)
>
> And include/uapi/linux/net_namespace.h shows -1 as not assigned.
Yes, 0 is a valid value and can be assigned to any netns.
nsid are signed 32 bit values. Note that -1 (NETNSA_NSID_NOT_ASSIGNED) is used
by the kernel to express that the nsid is not assigned. It can also be used by
the user to let the kernel chooses a nsid.
$ ip netns add foo
$ ip netns add bar
$ ip netns
bar
foo
$ ip netns set foo 0
$ ip netns set bar auto
$ ip netns
bar (id: 1)
foo (id: 0)
Regards,
Nicolas
^ 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