* [net-next 2/9] e1000e: Fix wrong comment related to link detection
From: Jeff Kirsher @ 2017-10-10 17:21 UTC (permalink / raw)
To: davem; +Cc: Benjamin Poirier, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20171010172139.77914-1-jeffrey.t.kirsher@intel.com>
From: Benjamin Poirier <bpoirier@suse.com>
Reading e1000e_check_for_copper_link() shows that get_link_status is set to
false after link has been detected. Therefore, it stays TRUE until then.
Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 8436c5f2c3e8..ead4c112580e 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5074,7 +5074,7 @@ static bool e1000e_has_link(struct e1000_adapter *adapter)
/* get_link_status is set on LSC (link status) interrupt or
* Rx sequence error interrupt. get_link_status will stay
- * false until the check_for_link establishes link
+ * true until the check_for_link establishes link
* for copper adapters ONLY
*/
switch (hw->phy.media_type) {
@@ -5092,7 +5092,7 @@ static bool e1000e_has_link(struct e1000_adapter *adapter)
break;
case e1000_media_type_internal_serdes:
ret_val = hw->mac.ops.check_for_link(hw);
- link_active = adapter->hw.mac.serdes_has_link;
+ link_active = hw->mac.serdes_has_link;
break;
default:
case e1000_media_type_unknown:
--
2.14.2
^ permalink raw reply related
* [net-next 1/9] e1000e: Fix error path in link detection
From: Jeff Kirsher @ 2017-10-10 17:21 UTC (permalink / raw)
To: davem; +Cc: Benjamin Poirier, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20171010172139.77914-1-jeffrey.t.kirsher@intel.com>
From: Benjamin Poirier <bpoirier@suse.com>
In case of error from e1e_rphy(), the loop will exit early and "success"
will be set to true erroneously.
Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/phy.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c
index d78d47b41a71..86ff0969efb6 100644
--- a/drivers/net/ethernet/intel/e1000e/phy.c
+++ b/drivers/net/ethernet/intel/e1000e/phy.c
@@ -1744,6 +1744,7 @@ s32 e1000e_phy_has_link_generic(struct e1000_hw *hw, u32 iterations,
s32 ret_val = 0;
u16 i, phy_status;
+ *success = false;
for (i = 0; i < iterations; i++) {
/* Some PHYs require the MII_BMSR register to be read
* twice due to the link bit being sticky. No harm doing
@@ -1763,16 +1764,16 @@ s32 e1000e_phy_has_link_generic(struct e1000_hw *hw, u32 iterations,
ret_val = e1e_rphy(hw, MII_BMSR, &phy_status);
if (ret_val)
break;
- if (phy_status & BMSR_LSTATUS)
+ if (phy_status & BMSR_LSTATUS) {
+ *success = true;
break;
+ }
if (usec_interval >= 1000)
msleep(usec_interval / 1000);
else
udelay(usec_interval);
}
- *success = (i < iterations);
-
return ret_val;
}
--
2.14.2
^ permalink raw reply related
* [net-next 4/9] e1000e: Separate signaling for link check/link up
From: Jeff Kirsher @ 2017-10-10 17:21 UTC (permalink / raw)
To: davem; +Cc: Benjamin Poirier, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20171010172139.77914-1-jeffrey.t.kirsher@intel.com>
From: Benjamin Poirier <bpoirier@suse.com>
Lennart reported the following race condition:
\ e1000_watchdog_task
\ e1000e_has_link
\ hw->mac.ops.check_for_link() === e1000e_check_for_copper_link
/* link is up */
mac->get_link_status = false;
/* interrupt */
\ e1000_msix_other
hw->mac.get_link_status = true;
link_active = !hw->mac.get_link_status
/* link_active is false, wrongly */
This problem arises because the single flag get_link_status is used to
signal two different states: link status needs checking and link status is
down.
Avoid the problem by using the return value of .check_for_link to signal
the link status to e1000e_has_link().
Reported-by: Lennart Sorensen <lsorense@csclub.uwaterloo.ca>
Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/mac.c | 11 ++++++++---
drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/mac.c b/drivers/net/ethernet/intel/e1000e/mac.c
index b322011ec282..f457c5703d0c 100644
--- a/drivers/net/ethernet/intel/e1000e/mac.c
+++ b/drivers/net/ethernet/intel/e1000e/mac.c
@@ -410,6 +410,9 @@ void e1000e_clear_hw_cntrs_base(struct e1000_hw *hw)
* Checks to see of the link status of the hardware has changed. If a
* change in link status has been detected, then we read the PHY registers
* to get the current speed/duplex if link exists.
+ *
+ * Returns a negative error code (-E1000_ERR_*) or 0 (link down) or 1 (link
+ * up).
**/
s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
{
@@ -423,7 +426,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
* Change or Rx Sequence Error interrupt.
*/
if (!mac->get_link_status)
- return 0;
+ return 1;
/* First we want to see if the MII Status Register reports
* link. If so, then we want to get the current speed/duplex
@@ -461,10 +464,12 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
* different link partner.
*/
ret_val = e1000e_config_fc_after_link_up(hw);
- if (ret_val)
+ if (ret_val) {
e_dbg("Error configuring flow control\n");
+ return ret_val;
+ }
- return ret_val;
+ return 1;
}
/**
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index a740de6a30b0..0a5f95ab0d3c 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5081,7 +5081,7 @@ static bool e1000e_has_link(struct e1000_adapter *adapter)
case e1000_media_type_copper:
if (hw->mac.get_link_status) {
ret_val = hw->mac.ops.check_for_link(hw);
- link_active = !hw->mac.get_link_status;
+ link_active = ret_val > 0;
} else {
link_active = true;
}
--
2.14.2
^ permalink raw reply related
* [net-next 3/9] e1000e: Fix return value test
From: Jeff Kirsher @ 2017-10-10 17:21 UTC (permalink / raw)
To: davem; +Cc: Benjamin Poirier, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20171010172139.77914-1-jeffrey.t.kirsher@intel.com>
From: Benjamin Poirier <bpoirier@suse.com>
All the helpers return -E1000_ERR_PHY.
Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index ead4c112580e..a740de6a30b0 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5099,7 +5099,7 @@ static bool e1000e_has_link(struct e1000_adapter *adapter)
break;
}
- if ((ret_val == E1000_ERR_PHY) && (hw->phy.type == e1000_phy_igp_3) &&
+ if ((ret_val == -E1000_ERR_PHY) && (hw->phy.type == e1000_phy_igp_3) &&
(er32(CTRL) & E1000_PHY_CTRL_GBE_DISABLE)) {
/* See e1000_kmrn_lock_loss_workaround_ich8lan() */
e_info("Gigabit has been disabled, downgrading speed\n");
--
2.14.2
^ permalink raw reply related
* [net-next 5/9] e1000e: Avoid receiver overrun interrupt bursts
From: Jeff Kirsher @ 2017-10-10 17:21 UTC (permalink / raw)
To: davem; +Cc: Benjamin Poirier, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20171010172139.77914-1-jeffrey.t.kirsher@intel.com>
From: Benjamin Poirier <bpoirier@suse.com>
When e1000e_poll() is not fast enough to keep up with incoming traffic, the
adapter (when operating in msix mode) raises the Other interrupt to signal
Receiver Overrun.
This is a double problem because 1) at the moment e1000_msix_other()
assumes that it is only called in case of Link Status Change and 2) if the
condition persists, the interrupt is repeatedly raised again in quick
succession.
Ideally we would configure the Other interrupt to not be raised in case of
receiver overrun but this doesn't seem possible on this adapter. Instead,
we handle the first part of the problem by reverting to the practice of
reading ICR in the other interrupt handler, like before commit 16ecba59bc33
("e1000e: Do not read ICR in Other interrupt"). Thanks to commit
0a8047ac68e5 ("e1000e: Fix msi-x interrupt automask") which cleared IAME
from CTRL_EXT, reading ICR doesn't interfere with RxQ0, TxQ0 interrupts
anymore. We handle the second part of the problem by not re-enabling the
Other interrupt right away when there is overrun. Instead, we wait until
traffic subsides, napi polling mode is exited and interrupts are
re-enabled.
Reported-by: Lennart Sorensen <lsorense@csclub.uwaterloo.ca>
Fixes: 16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt")
Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/defines.h | 1 +
drivers/net/ethernet/intel/e1000e/netdev.c | 33 ++++++++++++++++++++++-------
2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/defines.h b/drivers/net/ethernet/intel/e1000e/defines.h
index 0641c0098738..afb7ebe20b24 100644
--- a/drivers/net/ethernet/intel/e1000e/defines.h
+++ b/drivers/net/ethernet/intel/e1000e/defines.h
@@ -398,6 +398,7 @@
#define E1000_ICR_LSC 0x00000004 /* Link Status Change */
#define E1000_ICR_RXSEQ 0x00000008 /* Rx sequence error */
#define E1000_ICR_RXDMT0 0x00000010 /* Rx desc min. threshold (0) */
+#define E1000_ICR_RXO 0x00000040 /* Receiver Overrun */
#define E1000_ICR_RXT0 0x00000080 /* Rx timer intr (ring 0) */
#define E1000_ICR_ECCER 0x00400000 /* Uncorrectable ECC Error */
/* If this bit asserted, the driver should claim the interrupt */
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 0a5f95ab0d3c..ee9de3500331 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1910,14 +1910,30 @@ static irqreturn_t e1000_msix_other(int __always_unused irq, void *data)
struct net_device *netdev = data;
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
+ u32 icr;
+ bool enable = true;
+
+ icr = er32(ICR);
+ if (icr & E1000_ICR_RXO) {
+ ew32(ICR, E1000_ICR_RXO);
+ enable = false;
+ /* napi poll will re-enable Other, make sure it runs */
+ if (napi_schedule_prep(&adapter->napi)) {
+ adapter->total_rx_bytes = 0;
+ adapter->total_rx_packets = 0;
+ __napi_schedule(&adapter->napi);
+ }
+ }
+ if (icr & E1000_ICR_LSC) {
+ ew32(ICR, E1000_ICR_LSC);
+ hw->mac.get_link_status = true;
+ /* guard against interrupt when we're going down */
+ if (!test_bit(__E1000_DOWN, &adapter->state))
+ mod_timer(&adapter->watchdog_timer, jiffies + 1);
+ }
- hw->mac.get_link_status = true;
-
- /* guard against interrupt when we're going down */
- if (!test_bit(__E1000_DOWN, &adapter->state)) {
- mod_timer(&adapter->watchdog_timer, jiffies + 1);
+ if (enable && !test_bit(__E1000_DOWN, &adapter->state))
ew32(IMS, E1000_IMS_OTHER);
- }
return IRQ_HANDLED;
}
@@ -2687,7 +2703,8 @@ static int e1000e_poll(struct napi_struct *napi, int weight)
napi_complete_done(napi, work_done);
if (!test_bit(__E1000_DOWN, &adapter->state)) {
if (adapter->msix_entries)
- ew32(IMS, adapter->rx_ring->ims_val);
+ ew32(IMS, adapter->rx_ring->ims_val |
+ E1000_IMS_OTHER);
else
e1000_irq_enable(adapter);
}
@@ -4204,7 +4221,7 @@ static void e1000e_trigger_lsc(struct e1000_adapter *adapter)
struct e1000_hw *hw = &adapter->hw;
if (adapter->msix_entries)
- ew32(ICS, E1000_ICS_OTHER);
+ ew32(ICS, E1000_ICS_LSC | E1000_ICS_OTHER);
else
ew32(ICS, E1000_ICS_LSC);
}
--
2.14.2
^ permalink raw reply related
* [net-next 6/9] e1000e: fix buffer overrun while the I219 is processing DMA transactions
From: Jeff Kirsher @ 2017-10-10 17:21 UTC (permalink / raw)
To: davem; +Cc: Sasha Neftin, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20171010172139.77914-1-jeffrey.t.kirsher@intel.com>
From: Sasha Neftin <sasha.neftin@intel.com>
Intel® 100/200 Series Chipset platforms reduced the round-trip
latency for the LAN Controller DMA accesses, causing in some high
performance cases a buffer overrun while the I219 LAN Connected
Device is processing the DMA transactions. I219LM and I219V devices
can fall into unrecovered Tx hang under very stressfully UDP traffic
and multiple reconnection of Ethernet cable. This Tx hang of the LAN
Controller is only recovered if the system is rebooted. Slightly slow
down DMA access by reducing the number of outstanding requests.
This workaround could have an impact on TCP traffic performance
on the platform. Disabling TSO eliminates performance loss for TCP
traffic without a noticeable impact on CPU performance.
Please, refer to I218/I219 specification update:
https://www.intel.com/content/www/us/en/embedded/products/networking/
ethernet-connection-i218-family-documentation.html
Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
Reviewed-by: Dima Ruinskiy <dima.ruinskiy@intel.com>
Reviewed-by: Raanan Avargil <raanan.avargil@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index ee9de3500331..14b096f3d1da 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -3021,8 +3021,8 @@ static void e1000_configure_tx(struct e1000_adapter *adapter)
hw->mac.ops.config_collision_dist(hw);
- /* SPT and CNP Si errata workaround to avoid data corruption */
- if (hw->mac.type >= e1000_pch_spt) {
+ /* SPT and KBL Si errata workaround to avoid data corruption */
+ if (hw->mac.type == e1000_pch_spt) {
u32 reg_val;
reg_val = er32(IOSFPC);
@@ -3030,7 +3030,9 @@ static void e1000_configure_tx(struct e1000_adapter *adapter)
ew32(IOSFPC, reg_val);
reg_val = er32(TARC(0));
- reg_val |= E1000_TARC0_CB_MULTIQ_3_REQ;
+ /* SPT and KBL Si errata workaround to avoid Tx hang */
+ reg_val &= ~BIT(28);
+ reg_val |= BIT(29);
ew32(TARC(0), reg_val);
}
}
--
2.14.2
^ permalink raw reply related
* [net-next 7/9] e1000e: apply burst mode settings only on default
From: Jeff Kirsher @ 2017-10-10 17:21 UTC (permalink / raw)
To: davem; +Cc: Willem de Bruijn, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20171010172139.77914-1-jeffrey.t.kirsher@intel.com>
From: Willem de Bruijn <willemb@google.com>
Devices that support FLAG2_DMA_BURST have different default values
for RDTR and RADV. Apply burst mode default settings only when no
explicit value was passed at module load.
The RDTR default is zero. If the module is loaded for low latency
operation with RxIntDelay=0, do not override this value with a burst
default of 32.
Move the decision to apply burst values earlier, where explicitly
initialized module variables can be distinguished from defaults.
Signed-off-by: Willem de Bruijn <willemb@google.com>
Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/e1000.h | 4 ----
drivers/net/ethernet/intel/e1000e/netdev.c | 8 --------
drivers/net/ethernet/intel/e1000e/param.c | 16 +++++++++++++++-
3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 98e68888abb1..2311b31bdcac 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -94,10 +94,6 @@ struct e1000_info;
*/
#define E1000_CHECK_RESET_COUNT 25
-#define DEFAULT_RDTR 0
-#define DEFAULT_RADV 8
-#define BURST_RDTR 0x20
-#define BURST_RADV 0x20
#define PCICFG_DESC_RING_STATUS 0xe4
#define FLUSH_DESC_REQUIRED 0x100
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 14b096f3d1da..00f48d4cabec 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -3242,14 +3242,6 @@ static void e1000_configure_rx(struct e1000_adapter *adapter)
*/
ew32(RXDCTL(0), E1000_RXDCTL_DMA_BURST_ENABLE);
ew32(RXDCTL(1), E1000_RXDCTL_DMA_BURST_ENABLE);
-
- /* override the delay timers for enabling bursting, only if
- * the value was not set by the user via module options
- */
- if (adapter->rx_int_delay == DEFAULT_RDTR)
- adapter->rx_int_delay = BURST_RDTR;
- if (adapter->rx_abs_int_delay == DEFAULT_RADV)
- adapter->rx_abs_int_delay = BURST_RADV;
}
/* set the Receive Delay Timer Register */
diff --git a/drivers/net/ethernet/intel/e1000e/param.c b/drivers/net/ethernet/intel/e1000e/param.c
index 6d8c39abee16..47da51864543 100644
--- a/drivers/net/ethernet/intel/e1000e/param.c
+++ b/drivers/net/ethernet/intel/e1000e/param.c
@@ -73,17 +73,25 @@ E1000_PARAM(TxAbsIntDelay, "Transmit Absolute Interrupt Delay");
/* Receive Interrupt Delay in units of 1.024 microseconds
* hardware will likely hang if you set this to anything but zero.
*
+ * Burst variant is used as default if device has FLAG2_DMA_BURST.
+ *
* Valid Range: 0-65535
*/
E1000_PARAM(RxIntDelay, "Receive Interrupt Delay");
+#define DEFAULT_RDTR 0
+#define BURST_RDTR 0x20
#define MAX_RXDELAY 0xFFFF
#define MIN_RXDELAY 0
/* Receive Absolute Interrupt Delay in units of 1.024 microseconds
+ *
+ * Burst variant is used as default if device has FLAG2_DMA_BURST.
*
* Valid Range: 0-65535
*/
E1000_PARAM(RxAbsIntDelay, "Receive Absolute Interrupt Delay");
+#define DEFAULT_RADV 8
+#define BURST_RADV 0x20
#define MAX_RXABSDELAY 0xFFFF
#define MIN_RXABSDELAY 0
@@ -297,6 +305,9 @@ void e1000e_check_options(struct e1000_adapter *adapter)
.max = MAX_RXDELAY } }
};
+ if (adapter->flags2 & FLAG2_DMA_BURST)
+ opt.def = BURST_RDTR;
+
if (num_RxIntDelay > bd) {
adapter->rx_int_delay = RxIntDelay[bd];
e1000_validate_option(&adapter->rx_int_delay, &opt,
@@ -307,7 +318,7 @@ void e1000e_check_options(struct e1000_adapter *adapter)
}
/* Receive Absolute Interrupt Delay */
{
- static const struct e1000_option opt = {
+ static struct e1000_option opt = {
.type = range_option,
.name = "Receive Absolute Interrupt Delay",
.err = "using default of "
@@ -317,6 +328,9 @@ void e1000e_check_options(struct e1000_adapter *adapter)
.max = MAX_RXABSDELAY } }
};
+ if (adapter->flags2 & FLAG2_DMA_BURST)
+ opt.def = BURST_RADV;
+
if (num_RxAbsIntDelay > bd) {
adapter->rx_abs_int_delay = RxAbsIntDelay[bd];
e1000_validate_option(&adapter->rx_abs_int_delay, &opt,
--
2.14.2
^ permalink raw reply related
* [net-next 8/9] e1000e: Be drop monitor friendly
From: Jeff Kirsher @ 2017-10-10 17:21 UTC (permalink / raw)
To: davem; +Cc: Florian Fainelli, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20171010172139.77914-1-jeffrey.t.kirsher@intel.com>
From: Florian Fainelli <f.fainelli@gmail.com>
e1000e_put_txbuf() can be called from normal reclamation path as well as
when a DMA mapping failure, so we need to differentiate these two cases
when freeing SKBs to be drop monitor friendly. e1000e_tx_hwtstamp_work()
and e1000_remove() are processing TX timestamped SKBs and those should
not be accounted as drops either.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 00f48d4cabec..bf8f38f76953 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1071,7 +1071,8 @@ static bool e1000_clean_rx_irq(struct e1000_ring *rx_ring, int *work_done,
}
static void e1000_put_txbuf(struct e1000_ring *tx_ring,
- struct e1000_buffer *buffer_info)
+ struct e1000_buffer *buffer_info,
+ bool drop)
{
struct e1000_adapter *adapter = tx_ring->adapter;
@@ -1085,7 +1086,10 @@ static void e1000_put_txbuf(struct e1000_ring *tx_ring,
buffer_info->dma = 0;
}
if (buffer_info->skb) {
- dev_kfree_skb_any(buffer_info->skb);
+ if (drop)
+ dev_kfree_skb_any(buffer_info->skb);
+ else
+ dev_consume_skb_any(buffer_info->skb);
buffer_info->skb = NULL;
}
buffer_info->time_stamp = 0;
@@ -1199,7 +1203,7 @@ static void e1000e_tx_hwtstamp_work(struct work_struct *work)
wmb(); /* force write prior to skb_tstamp_tx */
skb_tstamp_tx(skb, &shhwtstamps);
- dev_kfree_skb_any(skb);
+ dev_consume_skb_any(skb);
} else if (time_after(jiffies, adapter->tx_hwtstamp_start
+ adapter->tx_timeout_factor * HZ)) {
dev_kfree_skb_any(adapter->tx_hwtstamp_skb);
@@ -1254,7 +1258,7 @@ static bool e1000_clean_tx_irq(struct e1000_ring *tx_ring)
}
}
- e1000_put_txbuf(tx_ring, buffer_info);
+ e1000_put_txbuf(tx_ring, buffer_info, false);
tx_desc->upper.data = 0;
i++;
@@ -2437,7 +2441,7 @@ static void e1000_clean_tx_ring(struct e1000_ring *tx_ring)
for (i = 0; i < tx_ring->count; i++) {
buffer_info = &tx_ring->buffer_info[i];
- e1000_put_txbuf(tx_ring, buffer_info);
+ e1000_put_txbuf(tx_ring, buffer_info, false);
}
netdev_reset_queue(adapter->netdev);
@@ -5625,7 +5629,7 @@ static int e1000_tx_map(struct e1000_ring *tx_ring, struct sk_buff *skb,
i += tx_ring->count;
i--;
buffer_info = &tx_ring->buffer_info[i];
- e1000_put_txbuf(tx_ring, buffer_info);
+ e1000_put_txbuf(tx_ring, buffer_info, true);
}
return 0;
@@ -7419,7 +7423,7 @@ static void e1000_remove(struct pci_dev *pdev)
if (adapter->flags & FLAG_HAS_HW_TIMESTAMP) {
cancel_work_sync(&adapter->tx_hwtstamp_work);
if (adapter->tx_hwtstamp_skb) {
- dev_kfree_skb_any(adapter->tx_hwtstamp_skb);
+ dev_consume_skb_any(adapter->tx_hwtstamp_skb);
adapter->tx_hwtstamp_skb = NULL;
}
}
--
2.14.2
^ permalink raw reply related
* [net-next 9/9] igb: check memory allocation failure
From: Jeff Kirsher @ 2017-10-10 17:21 UTC (permalink / raw)
To: davem; +Cc: Christophe JAILLET, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20171010172139.77914-1-jeffrey.t.kirsher@intel.com>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Check memory allocation failures and return -ENOMEM in such cases, as
already done for other memory allocations in this function.
This avoids NULL pointers dereference.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Tested-by: Aaron Brown <aaron.f.brown@intel.com
Acked-by: PJ Waskiewicz <peter.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index fd4a46b03cc8..837d9b46a390 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3162,6 +3162,8 @@ static int igb_sw_init(struct igb_adapter *adapter)
/* Setup and initialize a copy of the hw vlan table array */
adapter->shadow_vfta = kcalloc(E1000_VLAN_FILTER_TBL_SIZE, sizeof(u32),
GFP_ATOMIC);
+ if (!adapter->shadow_vfta)
+ return -ENOMEM;
/* This call may decrease the number of queues */
if (igb_init_interrupt_scheme(adapter, true)) {
--
2.14.2
^ permalink raw reply related
* Re: [PATCH RFC 0/3] tun zerocopy stats
From: Stephen Hemminger @ 2017-10-10 17:23 UTC (permalink / raw)
To: Willem de Bruijn
Cc: David Miller, Network Development, Michael S. Tsirkin, Jason Wang,
Willem de Bruijn
In-Reply-To: <CAF=yD-Jc_ikdYLydYwkw+h_+izVEg2Qp66=5+URokyqM=E=zJg@mail.gmail.com>
On Tue, 10 Oct 2017 11:29:33 -0400
Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> On Mon, Oct 9, 2017 at 11:52 PM, David Miller <davem@davemloft.net> wrote:
> > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > Date: Fri, 6 Oct 2017 18:25:13 -0400
> >
> >> From: Willem de Bruijn <willemb@google.com>
> >>
> >> Add zerocopy transfer statistics to the vhost_net/tun zerocopy path.
> >>
> >> I've been using this to verify recent changes to zerocopy tuning [1].
> >> Sharing more widely, as it may be useful in similar future work.
> >>
> >> Use ethtool stats as interface, as these are defined per device
> >> driver and can easily be extended.
> >>
> >> Make the zerocopy release callback take an extra hop through the tun
> >> driver to allow the driver to increment its counters.
> >>
> >> Care must be taken to avoid adding an alloc/free to this hot path.
> >> Since the caller already must allocate a ubuf_info, make it allocate
> >> two at a time and grant one to the tun device.
> >>
> >> 1/3: introduce ethtool stats (`ethtool -S $DEV`) for tun devices
> >> 2/3: add zerocopy tx and tx_err counters
> >> 3/3: convert vhost_net to pass a pair of ubuf_info to tun
> >>
> >> [1] http://patchwork.ozlabs.org/patch/822613/
> >
> > This looks mostly fine to me, but I don't know enough about how vhost
> > and tap interact to tell whether this makes sense to upstream.
>
> Thanks for taking a look. The need for monitoring these stats has
> come up in a couple of patch evaluation discussions, so I wanted
> to share at least one implementation to get the data.
>
> Because the choice to use zerocopy is based on heuristics and
> there is a cost if it mispredicts, I think we even want to being able
> to continuously monitor this in production.
>
> The implementation is probably not ready for that as is.
Another alternative is to use tracepoints for this.
If you need statistics in production then per-cpu (or per-queue) stats
would have less impact. Tracepoints have no visible impact unless used.
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: add ct_clear action
From: Joe Stringer @ 2017-10-10 17:24 UTC (permalink / raw)
To: Eric Garver, Joe Stringer, Pravin Shelar,
Linux Kernel Network Developers, ovs dev
In-Reply-To: <20171010150949.GG26353@dev-rhel7>
On 10 October 2017 at 08:09, Eric Garver <e@erig.me> wrote:
> On Tue, Oct 10, 2017 at 05:33:48AM -0700, Joe Stringer wrote:
>> On 9 October 2017 at 21:41, Pravin Shelar <pshelar-LZ6Gd1LRuIk@public.gmane.org> wrote:
>> > On Fri, Oct 6, 2017 at 9:44 AM, Eric Garver <e@erig.me> wrote:
>> >> This adds a ct_clear action for clearing conntrack state. ct_clear is
>> >> currently implemented in OVS userspace, but is not backed by an action
>> >> in the kernel datapath. This is useful for flows that may modify a
>> >> packet tuple after a ct lookup has already occurred.
>> >>
>> >> Signed-off-by: Eric Garver <e@erig.me>
>> > Patch mostly looks good. I have following comments.
>> >
>> >> ---
>> >> include/uapi/linux/openvswitch.h | 2 ++
>> >> net/openvswitch/actions.c | 5 +++++
>> >> net/openvswitch/conntrack.c | 12 ++++++++++++
>> >> net/openvswitch/conntrack.h | 7 +++++++
>> >> net/openvswitch/flow_netlink.c | 5 +++++
>> >> 5 files changed, 31 insertions(+)
>> >>
>> >> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
>> >> index 156ee4cab82e..1b6e510e2cc6 100644
>> >> --- a/include/uapi/linux/openvswitch.h
>> >> +++ b/include/uapi/linux/openvswitch.h
>> >> @@ -806,6 +806,7 @@ struct ovs_action_push_eth {
>> >> * packet.
>> >> * @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the
>> >> * packet.
>> >> + * @OVS_ACTION_ATTR_CT_CLEAR: Clear conntrack state from the packet.
>> >> *
>> >> * Only a single header can be set with a single %OVS_ACTION_ATTR_SET. Not all
>> >> * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
>> >> @@ -835,6 +836,7 @@ enum ovs_action_attr {
>> >> OVS_ACTION_ATTR_TRUNC, /* u32 struct ovs_action_trunc. */
>> >> OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
>> >> OVS_ACTION_ATTR_POP_ETH, /* No argument. */
>> >> + OVS_ACTION_ATTR_CT_CLEAR, /* No argument. */
>> >>
>> >> __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
>> >> * from userspace. */
>> >> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>> >> index a54a556fcdb5..db9c7f2e662b 100644
>> >> --- a/net/openvswitch/actions.c
>> >> +++ b/net/openvswitch/actions.c
>> >> @@ -1203,6 +1203,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>> >> return err == -EINPROGRESS ? 0 : err;
>> >> break;
>> >>
>> >> + case OVS_ACTION_ATTR_CT_CLEAR:
>> >> + err = ovs_ct_clear(skb, key);
>> >> + break;
>> >> +
>> >> case OVS_ACTION_ATTR_PUSH_ETH:
>> >> err = push_eth(skb, key, nla_data(a));
>> >> break;
>> >> @@ -1210,6 +1214,7 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>> >> case OVS_ACTION_ATTR_POP_ETH:
>> >> err = pop_eth(skb, key);
>> >> break;
>> >> +
>> >> }
>> > Unrelated change.
>> >
>> >>
>> >> if (unlikely(err)) {
>> >> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
>> >> index d558e882ca0c..f9b73c726ad7 100644
>> >> --- a/net/openvswitch/conntrack.c
>> >> +++ b/net/openvswitch/conntrack.c
>> >> @@ -1129,6 +1129,18 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
>> >> return err;
>> >> }
>> >>
>> >> +int ovs_ct_clear(struct sk_buff *skb, struct sw_flow_key *key)
>> >> +{
>> >> + if (skb_nfct(skb)) {
>> >> + nf_conntrack_put(skb_nfct(skb));
>> >> + nf_ct_set(skb, NULL, 0);
>> > Can the new conntract state be appropriate? may be IP_CT_UNTRACKED?
>> >
>> >> + }
>> >> +
>> >> + ovs_ct_fill_key(skb, key);
>> >> +
>> > I do not see need to refill the key if there is no skb-nf-ct.
>>
>> Really this is trying to just zero the CT key fields, but reuses
>> existing functions, right? This means that subsequent upcalls, for
>
> Right.
>
>> instance, won't have the outdated view of the CT state from the
>> previous lookup (that was prior to the ct_clear). I'd expect these key
>> fields to be cleared.
>
> I assumed Pravin was saying that we don't need to clear them if there is
> no conntrack state. They should already be zero.
The conntrack calls aren't going to clear it, so I don't see what else
would clear it?
If you execute ct(),ct_clear(), then the first ct will set the
values.. what will zero them?
^ permalink raw reply
* Re: [patch net-next 0/4] net: sched: get rid of cls_flower->egress_dev
From: Or Gerlitz @ 2017-10-10 17:25 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Netdev List, David Miller, Jamal Hadi Salim, Cong Wang,
Saeed Mahameed, Matan Barak, Leon Romanovsky, mlxsw
In-Reply-To: <20171010073016.3682-1-jiri@resnulli.us>
On Tue, Oct 10, 2017 at 10:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>
>
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 3 +
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 4 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 31 ++--
Jiri,
FWIW, as I reported to you earlier, I was playing with tc encap/decap
rules on 4.14-rc+ (net) before
applying any patch of this series, and something is messy w.r.t to
decap rules. I don't see
them removed at all when user space attempts to do so. It might
(probably) mlx5 bug, which
we will have to fix for net and later rebase net-next over net. We
have short WW here so
I will not be able to do RCA this week.
^ permalink raw reply
* Re: [PATCH][net-next] ipv6: fix dereference of rt6_ex before null check error
From: Eric Dumazet @ 2017-10-10 17:26 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20171010170116.21124-1-colin.king@canonical.com>
On Tue, 2017-10-10 at 18:01 +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently rt6_ex is being dereferenced before it is null checked
> hence there is a possible null dereference bug. Fix this by only
> dereferencing rt6_ex after it has been null checked.
>
> Detected by CoverityScan, CID#1457749 ("Dereference before null check")
>
> Fixes: 81eb8447daae ("ipv6: take care of rt6_stats")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [Patch net-next] tcp: add a tracepoint for tcp_retransmit_skb()
From: Alexei Starovoitov @ 2017-10-10 17:38 UTC (permalink / raw)
To: Cong Wang
Cc: netdev, Eric Dumazet, Yuchung Cheng, Neal Cardwell,
Martin KaFai Lau, Brendan Gregg, hannes, Song Liu
In-Reply-To: <20171010053547.21162-1-xiyou.wangcong@gmail.com>
On Mon, Oct 09, 2017 at 10:35:47PM -0700, Cong Wang wrote:
> We need a real-time notification for tcp retransmission
> for monitoring.
>
> Of course we could use ftrace to dynamically instrument this
> kernel function too, however we can't retrieve the connection
> information at the same time, for example perf-tools [1] reads
> /proc/net/tcp for socket details, which is slow when we have
> a lots of connections.
>
> Therefore, this patch adds a tracepoint for tcp_retransmit_skb()
> and exposes src/dst IP addresses and ports of the connection.
> This also makes it easier to integrate into perf.
>
> Note, I expose both IPv4 and IPv6 addresses at the same time:
> for a IPv4 socket, v4 mapped address is used as IPv6 addresses,
> for a IPv6 socket, LOOPBACK4_IPV6 is already filled by kernel.
>
> Perhaps there are other interfaces to use (for example netlink),
> but tracepoint is the quickest way I can think of.
>
> 1. https://github.com/brendangregg/perf-tools/blob/master/net/tcpretrans
>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> include/trace/events/tcp.h | 63 ++++++++++++++++++++++++++++++++++++++++++++++
> net/core/net-traces.c | 1 +
> net/ipv4/tcp_output.c | 3 +++
> 3 files changed, 67 insertions(+)
> create mode 100644 include/trace/events/tcp.h
>
> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
> new file mode 100644
> index 000000000000..cb22acc8aacd
> --- /dev/null
> +++ b/include/trace/events/tcp.h
> @@ -0,0 +1,63 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM tcp
> +
> +#if !defined(_TRACE_TCP_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_TCP_H
> +
> +#include <linux/ipv6.h>
> +#include <linux/tcp.h>
> +#include <linux/tracepoint.h>
> +#include <net/ipv6.h>
> +
> +TRACE_EVENT(tcp_retransmit_skb,
> +
> + TP_PROTO(const struct sock *sk, struct sk_buff *skb, int segs),
> +
> + TP_ARGS(sk, skb, segs),
> +
> + TP_STRUCT__entry(
> + __field(__u16, sport)
> + __field(__u16, dport)
> + __array(__u8, saddr, 4)
> + __array(__u8, daddr, 4)
> + __array(__u8, saddr_v6, 16)
> + __array(__u8, daddr_v6, 16)
> + ),
> +
> + TP_fast_assign(
> + struct ipv6_pinfo *np = inet6_sk(sk);
> + struct inet_sock *inet = inet_sk(sk);
> + struct in6_addr *pin6;
> + __be32 *p32;
> +
> + __entry->sport = ntohs(inet->inet_sport);
> + __entry->dport = ntohs(inet->inet_dport);
> +
> + p32 = (__be32 *) __entry->saddr;
> + *p32 = inet->inet_saddr;
> +
> + p32 = (__be32 *) __entry->daddr;
> + *p32 = inet->inet_daddr;
> +
> + if (np) {
> + pin6 = (struct in6_addr *)__entry->saddr_v6;
> + *pin6 = np->saddr;
> + pin6 = (struct in6_addr *)__entry->daddr_v6;
> + *pin6 = *(np->daddr_cache);
> + } else {
> + pin6 = (struct in6_addr *)__entry->saddr_v6;
> + ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
> + pin6 = (struct in6_addr *)__entry->daddr_v6;
> + ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
> + }
> + ),
> +
> + TP_printk("sport=%hu, dport=%hu, saddr=%pI4, daddr=%pI4, saddrv6=%pI6, daddrv6=%pI6",
> + __entry->sport, __entry->dport, __entry->saddr, __entry->daddr,
> + __entry->saddr_v6, __entry->daddr_v6)
> +);
> +
> +#endif /* _TRACE_TCP_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> diff --git a/net/core/net-traces.c b/net/core/net-traces.c
> index 1132820c8e62..f4e4fa2db505 100644
> --- a/net/core/net-traces.c
> +++ b/net/core/net-traces.c
> @@ -31,6 +31,7 @@
> #include <trace/events/napi.h>
> #include <trace/events/sock.h>
> #include <trace/events/udp.h>
> +#include <trace/events/tcp.h>
> #include <trace/events/fib.h>
> #include <trace/events/qdisc.h>
> #if IS_ENABLED(CONFIG_IPV6)
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 696b0a168f16..e6d6e1393578 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -42,6 +42,8 @@
> #include <linux/gfp.h>
> #include <linux/module.h>
>
> +#include <trace/events/tcp.h>
> +
> /* People can turn this off for buggy TCP's found in printers etc. */
> int sysctl_tcp_retrans_collapse __read_mostly = 1;
>
> @@ -2899,6 +2901,7 @@ int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
> if (!tp->retrans_stamp)
> tp->retrans_stamp = tcp_skb_timestamp(skb);
>
> + trace_tcp_retransmit_skb(sk, skb, segs);
I'm happy to see new tracepoints being added to tcp stack, but I'm concerned
with practical usability of them.
Like the above tracepoint definition makes it not very useful from bpf point of view,
since 'sk' pointer is not recored by as part of the tracepoint.
In bpf/tracing world we prefer tracepoints to have raw pointers recorded
in TP_STRUCT__entry() and _not_ printed in TP_printk()
(since pointers are useless for userspace).
Like trace_kfree_skb() tracepoint records raw 'skb' pointer and we can
walk whatever sk_buff fields we need inside the program.
Such approach allows tracepoint to be usable in many more scenarios, since
bpf program can examine kernel datastructures.
Over the last few years we've been running tcp statistics framework (similar to web10g)
using 8 kprobes in tcp stack with bpf programs extracting the data and now we're
ready to share this experience with the community. Right now we're working on a set
of tracepoints for tcp stack to make the interface more accurate, faster and more stable.
We're planning to send an RFC patch with these new tracepoints in the comming weeks.
More concrete, if you can make this trace_tcp_retransmit_skb() to record
sk, skb pointers and err code at the end of __tcp_retransmit_skb() it will solve
our need as well.
So far our list of kprobes is:
int kprobe__tcp_validate_incoming
int kprobe__tcp_send_active_reset
int kprobe__tcp_v4_send_reset
int kprobe__tcp_v6_send_reset
int kprobe__tcp_v4_destroy_sock
int kprobe__tcp_set_state
int kprobe__tcp_retransmit_skb
int kprobe__tcp_rtx_synack
with tracepoints we can consolidate two of them into one and drop
another one for sure. Notice that tcp_retransmit_skb is on our list too
and currently we're doing extra work inside the program to make it more
accurate which will be unnecessary if this tracepoint is at the end
of __tcp_retransmit_skb().
Thanks
^ permalink raw reply
* Re: [PATCH RFC 0/3] tun zerocopy stats
From: David Miller @ 2017-10-10 17:39 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, mst, jasowang, willemb
In-Reply-To: <CAF=yD-Jc_ikdYLydYwkw+h_+izVEg2Qp66=5+URokyqM=E=zJg@mail.gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Tue, 10 Oct 2017 11:29:33 -0400
> If there is a way to expose these stats through vhost_net directly,
> instead of through tun, that may be better. But I did not see a
> suitable interface. Perhaps debugfs.
Please don't use debugfs, thank you :-)
^ permalink raw reply
* Re: [PATCH][net-next] ipv6: fix dereference of rt6_ex before null check error
From: Martin KaFai Lau @ 2017-10-10 17:40 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20171010170116.21124-1-colin.king@canonical.com>
On Tue, Oct 10, 2017 at 05:01:16PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently rt6_ex is being dereferenced before it is null checked
> hence there is a possible null dereference bug. Fix this by only
> dereferencing rt6_ex after it has been null checked.
>
> Detected by CoverityScan, CID#1457749 ("Dereference before null check")
>
> Fixes: 81eb8447daae ("ipv6: take care of rt6_stats")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
> ---
> net/ipv6/route.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 606e80325b21..6db1541eaa7b 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1152,10 +1152,12 @@ static DEFINE_SPINLOCK(rt6_exception_lock);
> static void rt6_remove_exception(struct rt6_exception_bucket *bucket,
> struct rt6_exception *rt6_ex)
> {
> - struct net *net = dev_net(rt6_ex->rt6i->dst.dev);
> + struct net *net;
>
> if (!bucket || !rt6_ex)
> return;
> +
> + net = dev_net(rt6_ex->rt6i->dst.dev);
> rt6_ex->rt6i->rt6i_node = NULL;
> hlist_del_rcu(&rt6_ex->hlist);
> rt6_release(rt6_ex->rt6i);
> --
> 2.14.1
>
^ permalink raw reply
* Re: [PATCH v5 2/2] net: phy: at803x: Change error to EINVAL for invalid MAC
From: Florian Fainelli @ 2017-10-10 17:43 UTC (permalink / raw)
To: Dan Murphy, andrew; +Cc: netdev, Woojung.Huh, afd
In-Reply-To: <20171010174256.21930-2-dmurphy@ti.com>
On 10/10/2017 10:42 AM, Dan Murphy wrote:
> Change the return error code to EINVAL if the MAC
> address is not valid in the set_wol function.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* [PATCH v5 2/2] net: phy: at803x: Change error to EINVAL for invalid MAC
From: Dan Murphy @ 2017-10-10 17:42 UTC (permalink / raw)
To: andrew, f.fainelli; +Cc: netdev, Woojung.Huh, afd, Dan Murphy
In-Reply-To: <20171010174256.21930-1-dmurphy@ti.com>
Change the return error code to EINVAL if the MAC
address is not valid in the set_wol function.
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
v5 - No changes made
v4 - Updated $subject to include the part number -
https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html
v3 - No changes made
v2 - There was no v1 on this patch this is new.
drivers/net/phy/at803x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c1e52b9dc58d..5f93e6add563 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -167,7 +167,7 @@ static int at803x_set_wol(struct phy_device *phydev,
mac = (const u8 *) ndev->dev_addr;
if (!is_valid_ether_addr(mac))
- return -EFAULT;
+ return -EINVAL;
for (i = 0; i < 3; i++) {
phy_write(phydev, AT803X_MMD_ACCESS_CONTROL,
--
2.14.0
^ permalink raw reply related
* [PATCH v5 1/2] net: phy: DP83822 initial driver submission
From: Dan Murphy @ 2017-10-10 17:42 UTC (permalink / raw)
To: andrew, f.fainelli; +Cc: netdev, Woojung.Huh, afd, Dan Murphy
Add support for the TI DP83822 10/100Mbit ethernet phy.
The DP83822 provides flexibility to connect to a MAC through a
standard MII, RMII or RGMII interface.
In addition the DP83822 needs to be removed from the DP83848 driver
as the WoL support is added here for this device.
Datasheet:
http://www.ti.com/product/DP83822I/datasheet
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
v5 - Fixed bit mask, added config_init to set WOL register bits, fixed spacing
issues, moved password storage under secure on if statement, and added INT_EN bit
setting for interrupt enable.
https://www.mail-archive.com/netdev@vger.kernel.org/msg192495.html
v4 - Squash DP83822 removal patch into this patch -
https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html
v3 - Fixed WoL indication bit and removed mutex for suspend/resume -
https://www.mail-archive.com/netdev@vger.kernel.org/msg191891.html and
https://www.mail-archive.com/netdev@vger.kernel.org/msg191665.html
v2 - Updated per comments. Removed unnessary parantheis, called genphy_suspend/
resume routines and then performing WoL changes, reworked sopass storage and reduced
the number of phy reads, and moved WOL_SECURE_ON -
https://www.mail-archive.com/netdev@vger.kernel.org/msg191392.html
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/dp83822.c | 344 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/net/phy/dp83848.c | 3 -
4 files changed, 350 insertions(+), 3 deletions(-)
create mode 100644 drivers/net/phy/dp83822.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index cd931cf9dcc2..8e78a482e09e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -277,6 +277,11 @@ config DAVICOM_PHY
---help---
Currently supports dm9161e and dm9131
+config DP83822_PHY
+ tristate "Texas Instruments DP83822 PHY"
+ ---help---
+ Supports the DP83822 PHY.
+
config DP83848_PHY
tristate "Texas Instruments DP83848 PHY"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 416df92fbf4f..df3b82ba8550 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY) += cicada.o
obj-$(CONFIG_CORTINA_PHY) += cortina.o
obj-$(CONFIG_DAVICOM_PHY) += davicom.o
obj-$(CONFIG_DP83640_PHY) += dp83640.o
+obj-$(CONFIG_DP83822_PHY) += dp83822.o
obj-$(CONFIG_DP83848_PHY) += dp83848.o
obj-$(CONFIG_DP83867_PHY) += dp83867.o
obj-$(CONFIG_FIXED_PHY) += fixed_phy.o
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
new file mode 100644
index 000000000000..14335d14e9e4
--- /dev/null
+++ b/drivers/net/phy/dp83822.c
@@ -0,0 +1,344 @@
+/*
+ * Driver for the Texas Instruments DP83822 PHY
+ *
+ * Copyright (C) 2017 Texas Instruments Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/ethtool.h>
+#include <linux/etherdevice.h>
+#include <linux/kernel.h>
+#include <linux/mii.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/netdevice.h>
+
+#define DP83822_PHY_ID 0x2000a240
+#define DP83822_DEVADDR 0x1f
+
+#define MII_DP83822_PHYSCR 0x11
+#define MII_DP83822_MISR1 0x12
+#define MII_DP83822_MISR2 0x13
+#define MII_DP83822_RESET_CTRL 0x1f
+
+#define DP83822_HW_RESET BIT(15)
+#define DP83822_SW_RESET BIT(14)
+
+/* PHYSCR Register Fields */
+#define DP83822_PHYSCR_INT_OE BIT(0) /* Interrupt Output Enable */
+#define DP83822_PHYSCR_INTEN BIT(1) /* Interrupt Enable */
+
+/* MISR1 bits */
+#define DP83822_RX_ERR_HF_INT_EN BIT(0)
+#define DP83822_FALSE_CARRIER_HF_INT_EN BIT(1)
+#define DP83822_ANEG_COMPLETE_INT_EN BIT(2)
+#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3)
+#define DP83822_SPEED_CHANGED_INT_EN BIT(4)
+#define DP83822_LINK_STAT_INT_EN BIT(5)
+#define DP83822_ENERGY_DET_INT_EN BIT(6)
+#define DP83822_LINK_QUAL_INT_EN BIT(7)
+
+/* MISR2 bits */
+#define DP83822_JABBER_DET_INT_EN BIT(0)
+#define DP83822_WOL_PKT_INT_EN BIT(1)
+#define DP83822_SLEEP_MODE_INT_EN BIT(2)
+#define DP83822_MDI_XOVER_INT_EN BIT(3)
+#define DP83822_LB_FIFO_INT_EN BIT(4)
+#define DP83822_PAGE_RX_INT_EN BIT(5)
+#define DP83822_ANEG_ERR_INT_EN BIT(6)
+#define DP83822_EEE_ERROR_CHANGE_INT_EN BIT(7)
+
+/* INT_STAT1 bits */
+#define DP83822_WOL_INT_EN BIT(4)
+#define DP83822_WOL_INT_STAT BIT(12)
+
+#define MII_DP83822_RXSOP1 0x04a5
+#define MII_DP83822_RXSOP2 0x04a6
+#define MII_DP83822_RXSOP3 0x04a7
+
+/* WoL Registers */
+#define MII_DP83822_WOL_CFG 0x04a0
+#define MII_DP83822_WOL_STAT 0x04a1
+#define MII_DP83822_WOL_DA1 0x04a2
+#define MII_DP83822_WOL_DA2 0x04a3
+#define MII_DP83822_WOL_DA3 0x04a4
+
+/* WoL bits */
+#define DP83822_WOL_MAGIC_EN BIT(0)
+#define DP83822_WOL_SECURE_ON BIT(5)
+#define DP83822_WOL_EN BIT(7)
+#define DP83822_WOL_INDICATION_SEL BIT(8)
+#define DP83822_WOL_CLR_INDICATION BIT(11)
+
+static int dp83822_ack_interrupt(struct phy_device *phydev)
+{
+ int err;
+
+ err = phy_read(phydev, MII_DP83822_MISR1);
+ if (err < 0)
+ return err;
+
+ err = phy_read(phydev, MII_DP83822_MISR2);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static int dp83822_set_wol(struct phy_device *phydev,
+ struct ethtool_wolinfo *wol)
+{
+ struct net_device *ndev = phydev->attached_dev;
+ u16 value;
+ const u8 *mac;
+
+ if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE)) {
+ mac = (const u8 *)ndev->dev_addr;
+
+ if (!is_valid_ether_addr(mac))
+ return -EINVAL;
+
+ /* MAC addresses start with byte 5, but stored in mac[0].
+ * 822 PHYs store bytes 4|5, 2|3, 0|1
+ */
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA1,
+ (mac[1] << 8) | mac[0]);
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA2,
+ (mac[3] << 8) | mac[2]);
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA3,
+ (mac[5] << 8) | mac[4]);
+
+ value = phy_read_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_WOL_CFG);
+ if (wol->wolopts & WAKE_MAGIC)
+ value |= DP83822_WOL_MAGIC_EN;
+ else
+ value &= ~DP83822_WOL_MAGIC_EN;
+
+ if (wol->wolopts & WAKE_MAGICSECURE) {
+ phy_write_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP1,
+ (wol->sopass[1] << 8) | wol->sopass[0]);
+ phy_write_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP2,
+ (wol->sopass[3] << 8) | wol->sopass[2]);
+ phy_write_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP3,
+ (wol->sopass[5] << 8) | wol->sopass[4]);
+ value |= DP83822_WOL_SECURE_ON;
+ } else {
+ value &= ~DP83822_WOL_SECURE_ON;
+ }
+
+ value |= (DP83822_WOL_EN | DP83822_WOL_INDICATION_SEL |
+ DP83822_WOL_CLR_INDICATION);
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
+ value);
+ } else {
+ value = phy_read_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_WOL_CFG);
+ value &= ~DP83822_WOL_EN;
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
+ value);
+ }
+
+ return 0;
+}
+
+static void dp83822_get_wol(struct phy_device *phydev,
+ struct ethtool_wolinfo *wol)
+{
+ int value;
+ u16 sopass_val;
+
+ wol->supported = (WAKE_MAGIC | WAKE_MAGICSECURE);
+ wol->wolopts = 0;
+
+ value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
+
+ if (value & DP83822_WOL_MAGIC_EN)
+ wol->wolopts |= WAKE_MAGIC;
+
+ if (value & DP83822_WOL_SECURE_ON) {
+ sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP1);
+ wol->sopass[0] = (sopass_val & 0xff);
+ wol->sopass[1] = (sopass_val >> 8);
+
+ sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP2);
+ wol->sopass[2] = (sopass_val & 0xff);
+ wol->sopass[3] = (sopass_val >> 8);
+
+ sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83822_RXSOP3);
+ wol->sopass[4] = (sopass_val & 0xff);
+ wol->sopass[5] = (sopass_val >> 8);
+
+ wol->wolopts |= WAKE_MAGICSECURE;
+ }
+
+ /* WoL is not enabled so set wolopts to 0 */
+ if (!(value & DP83822_WOL_EN))
+ wol->wolopts = 0;
+}
+
+static int dp83822_config_intr(struct phy_device *phydev)
+{
+ int misr_status;
+ int physcr_status;
+ int err;
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ misr_status = phy_read(phydev, MII_DP83822_MISR1);
+ if (misr_status < 0)
+ return misr_status;
+
+ misr_status |= (DP83822_RX_ERR_HF_INT_EN |
+ DP83822_FALSE_CARRIER_HF_INT_EN |
+ DP83822_ANEG_COMPLETE_INT_EN |
+ DP83822_DUP_MODE_CHANGE_INT_EN |
+ DP83822_SPEED_CHANGED_INT_EN |
+ DP83822_LINK_STAT_INT_EN |
+ DP83822_ENERGY_DET_INT_EN |
+ DP83822_LINK_QUAL_INT_EN);
+
+ err = phy_write(phydev, MII_DP83822_MISR1, misr_status);
+ if (err < 0)
+ return err;
+
+ misr_status = phy_read(phydev, MII_DP83822_MISR2);
+ if (misr_status < 0)
+ return misr_status;
+
+ misr_status |= (DP83822_JABBER_DET_INT_EN |
+ DP83822_WOL_PKT_INT_EN |
+ DP83822_SLEEP_MODE_INT_EN |
+ DP83822_MDI_XOVER_INT_EN |
+ DP83822_LB_FIFO_INT_EN |
+ DP83822_PAGE_RX_INT_EN |
+ DP83822_ANEG_ERR_INT_EN |
+ DP83822_EEE_ERROR_CHANGE_INT_EN);
+
+ err = phy_write(phydev, MII_DP83822_MISR2, misr_status);
+ if (err < 0)
+ return err;
+
+ physcr_status = phy_read(phydev, MII_DP83822_PHYSCR);
+ if (physcr_status < 0)
+ return physcr_status;
+
+ physcr_status |= DP83822_PHYSCR_INT_OE | DP83822_PHYSCR_INTEN;
+
+ } else {
+ err = phy_write(phydev, MII_DP83822_MISR1, 0);
+ if (err < 0)
+ return err;
+
+ err = phy_write(phydev, MII_DP83822_MISR1, 0);
+ if (err < 0)
+ return err;
+
+ physcr_status = phy_read(phydev, MII_DP83822_PHYSCR);
+ if (physcr_status < 0)
+ return physcr_status;
+
+ physcr_status &= ~DP83822_PHYSCR_INTEN;
+ }
+
+ return phy_write(phydev, MII_DP83822_PHYSCR, physcr_status);
+}
+
+static int dp83822_config_init(struct phy_device *phydev)
+{
+ int err;
+ int value;
+
+ err = genphy_config_init(phydev);
+ if (err < 0)
+ return err;
+
+ value = DP83822_WOL_MAGIC_EN | DP83822_WOL_SECURE_ON | DP83822_WOL_EN;
+
+ return phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
+ value);
+}
+
+static int dp83822_phy_reset(struct phy_device *phydev)
+{
+ int err;
+
+ err = phy_write(phydev, MII_DP83822_RESET_CTRL, DP83822_HW_RESET);
+ if (err < 0)
+ return err;
+
+ dp83822_config_init(phydev);
+
+ return 0;
+}
+
+static int dp83822_suspend(struct phy_device *phydev)
+{
+ int value;
+
+ value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
+
+ if (!(value & DP83822_WOL_EN))
+ genphy_suspend(phydev);
+
+ return 0;
+}
+
+static int dp83822_resume(struct phy_device *phydev)
+{
+ int value;
+
+ genphy_resume(phydev);
+
+ value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
+
+ phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG, value |
+ DP83822_WOL_CLR_INDICATION);
+
+ return 0;
+}
+
+static struct phy_driver dp83822_driver[] = {
+ {
+ .phy_id = DP83822_PHY_ID,
+ .phy_id_mask = 0xfffffff0,
+ .name = "TI DP83822",
+ .features = PHY_BASIC_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .config_init = dp83822_config_init,
+ .soft_reset = dp83822_phy_reset,
+ .get_wol = dp83822_get_wol,
+ .set_wol = dp83822_set_wol,
+ .ack_interrupt = dp83822_ack_interrupt,
+ .config_intr = dp83822_config_intr,
+ .config_aneg = genphy_config_aneg,
+ .read_status = genphy_read_status,
+ .suspend = dp83822_suspend,
+ .resume = dp83822_resume,
+ },
+};
+module_phy_driver(dp83822_driver);
+
+static struct mdio_device_id __maybe_unused dp83822_tbl[] = {
+ { DP83822_PHY_ID, 0xfffffff0 },
+ { },
+};
+MODULE_DEVICE_TABLE(mdio, dp83822_tbl);
+
+MODULE_DESCRIPTION("Texas Instruments DP83822 PHY driver");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
index 3de4fe4dda77..3966d43c5146 100644
--- a/drivers/net/phy/dp83848.c
+++ b/drivers/net/phy/dp83848.c
@@ -20,7 +20,6 @@
#define TI_DP83620_PHY_ID 0x20005ce0
#define NS_DP83848C_PHY_ID 0x20005c90
#define TLK10X_PHY_ID 0x2000a210
-#define TI_DP83822_PHY_ID 0x2000a240
/* Registers */
#define DP83848_MICR 0x11 /* MII Interrupt Control Register */
@@ -80,7 +79,6 @@ static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
{ NS_DP83848C_PHY_ID, 0xfffffff0 },
{ TI_DP83620_PHY_ID, 0xfffffff0 },
{ TLK10X_PHY_ID, 0xfffffff0 },
- { TI_DP83822_PHY_ID, 0xfffffff0 },
{ }
};
MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
@@ -110,7 +108,6 @@ static struct phy_driver dp83848_driver[] = {
DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY"),
DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY"),
DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY"),
- DP83848_PHY_DRIVER(TI_DP83822_PHY_ID, "TI DP83822 10/100 Mbps PHY"),
};
module_phy_driver(dp83848_driver);
--
2.14.0
^ permalink raw reply related
* Re: [patch net-next 1/4] net: sched: make tc_action_ops->get_dev return dev and avoid passing net
From: Cong Wang @ 2017-10-10 17:44 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Saeed Mahameed, matanb, leonro, mlxsw
In-Reply-To: <20171010073016.3682-2-jiri@resnulli.us>
On Tue, Oct 10, 2017 at 12:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> -static int tcf_mirred_device(const struct tc_action *a, struct net *net,
> - struct net_device **mirred_dev)
> +static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
> {
> - int ifindex = tcf_mirred_ifindex(a);
> + struct tcf_mirred *m = to_mirred(a);
>
> - *mirred_dev = __dev_get_by_index(net, ifindex);
> - if (!*mirred_dev)
> - return -EINVAL;
> - return 0;
> + return __dev_get_by_index(m->net, m->tcfm_ifindex);
Hmm, why not just return m->tcfm_dev?
^ permalink raw reply
* Re: [PATCH net-next v2 5/5] selinux: bpf: Add addtional check for bpf object file receive
From: Chenbo Feng @ 2017-10-10 17:48 UTC (permalink / raw)
To: Stephen Smalley
Cc: Chenbo Feng, linux-security-module, netdev, SELinux,
Daniel Borkmann, Alexei Starovoitov, Lorenzo Colitti
In-Reply-To: <1507645461.30616.9.camel@tycho.nsa.gov>
On Tue, Oct 10, 2017 at 7:24 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Mon, 2017-10-09 at 15:20 -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a bpf object related check when sending and receiving files
>> through unix domain socket as well as binder. It checks if the
>> receiving
>> process have privilege to read/write the bpf map or use the bpf
>> program.
>> This check is necessary because the bpf maps and programs are using a
>> anonymous inode as their shared inode so the normal way of checking
>> the
>> files and sockets when passing between processes cannot work properly
>> on
>> eBPF object. This check only works when the BPF_SYSCALL is
>> configured.
>> The information stored inside the file security struct is the same as
>> the information in bpf object security struct.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>> ---
>> include/linux/bpf.h | 3 +++
>> include/linux/lsm_hooks.h | 17 +++++++++++++
>> include/linux/security.h | 9 +++++++
>> kernel/bpf/syscall.c | 4 ++--
>> security/security.c | 8 +++++++
>> security/selinux/hooks.c | 61
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> 6 files changed, 100 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 225740688ab7..81d6c01b8825 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -285,6 +285,9 @@ int bpf_prog_array_copy_to_user(struct
>> bpf_prog_array __rcu *progs,
>> #ifdef CONFIG_BPF_SYSCALL
>> DECLARE_PER_CPU(int, bpf_prog_active);
>>
>> +extern const struct file_operations bpf_map_fops;
>> +extern const struct file_operations bpf_prog_fops;
>> +
>> #define BPF_PROG_TYPE(_id, _ops) \
>> extern const struct bpf_verifier_ops _ops;
>> #define BPF_MAP_TYPE(_id, _ops) \
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 7161d8e7ee79..517dea60b87b 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1385,6 +1385,19 @@
>> * @bpf_prog_free_security:
>> * Clean up the security information stored inside bpf prog.
>> *
>> + * @bpf_map_file:
>> + * When creating a bpf map fd, set up the file security
>> information with
>> + * the bpf security information stored in the map struct. So
>> when the map
>> + * fd is passed between processes, the security module can
>> directly read
>> + * the security information from file security struct rather
>> than the bpf
>> + * security struct.
>> + *
>> + * @bpf_prog_file:
>> + * When creating a bpf prog fd, set up the file security
>> information with
>> + * the bpf security information stored in the prog struct. So
>> when the prog
>> + * fd is passed between processes, the security module can
>> directly read
>> + * the security information from file security struct rather
>> than the bpf
>> + * security struct.
>> */
>> union security_list_options {
>> int (*binder_set_context_mgr)(struct task_struct *mgr);
>> @@ -1726,6 +1739,8 @@ union security_list_options {
>> void (*bpf_map_free_security)(struct bpf_map *map);
>> int (*bpf_prog_alloc_security)(struct bpf_prog_aux *aux);
>> void (*bpf_prog_free_security)(struct bpf_prog_aux *aux);
>> + void (*bpf_map_file)(struct bpf_map *map, struct file
>> *file);
>> + void (*bpf_prog_file)(struct bpf_prog_aux *aux, struct file
>> *file);
>> #endif /* CONFIG_BPF_SYSCALL */
>> };
>>
>> @@ -1954,6 +1969,8 @@ struct security_hook_heads {
>> struct list_head bpf_map_free_security;
>> struct list_head bpf_prog_alloc_security;
>> struct list_head bpf_prog_free_security;
>> + struct list_head bpf_map_file;
>> + struct list_head bpf_prog_file;
>> #endif /* CONFIG_BPF_SYSCALL */
>> } __randomize_layout;
>>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 18800b0911e5..57573b794e2d 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -1740,6 +1740,8 @@ extern int security_bpf_map_alloc(struct
>> bpf_map *map);
>> extern void security_bpf_map_free(struct bpf_map *map);
>> extern int security_bpf_prog_alloc(struct bpf_prog_aux *aux);
>> extern void security_bpf_prog_free(struct bpf_prog_aux *aux);
>> +extern void security_bpf_map_file(struct bpf_map *map, struct file
>> *file);
>> +extern void security_bpf_prog_file(struct bpf_prog_aux *aux, struct
>> file *file);
>> #else
>> static inline int security_bpf(int cmd, union bpf_attr *attr,
>> unsigned int size)
>> @@ -1772,6 +1774,13 @@ static inline int
>> security_bpf_prog_alloc(struct bpf_prog_aux *aux)
>>
>> static inline void security_bpf_prog_free(struct bpf_prog_aux *aux)
>> { }
>> +
>> +static inline void security_bpf_map_file(struct bpf_map *map, struct
>> file *file)
>> +{ }
>> +
>> +static inline void security_bpf_prog_file(struct bpf_prog_aux *aux,
>> + struct file *file)
>> +{ }
>> #endif /* CONFIG_SECURITY */
>> #endif /* CONFIG_BPF_SYSCALL */
>>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 1cf31ddd7616..b144181d3f3a 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -313,7 +313,7 @@ static ssize_t bpf_dummy_write(struct file *filp,
>> const char __user *buf,
>> return -EINVAL;
>> }
>>
>> -static const struct file_operations bpf_map_fops = {
>> +const struct file_operations bpf_map_fops = {
>> #ifdef CONFIG_PROC_FS
>> .show_fdinfo = bpf_map_show_fdinfo,
>> #endif
>> @@ -964,7 +964,7 @@ static void bpf_prog_show_fdinfo(struct seq_file
>> *m, struct file *filp)
>> }
>> #endif
>>
>> -static const struct file_operations bpf_prog_fops = {
>> +const struct file_operations bpf_prog_fops = {
>> #ifdef CONFIG_PROC_FS
>> .show_fdinfo = bpf_prog_show_fdinfo,
>> #endif
>> diff --git a/security/security.c b/security/security.c
>> index 1cd8526cb0b7..dacf649b8cfa 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -1734,4 +1734,12 @@ void security_bpf_prog_free(struct
>> bpf_prog_aux *aux)
>> {
>> call_void_hook(bpf_prog_free_security, aux);
>> }
>> +void security_bpf_map_file(struct bpf_map *map, struct file *file)
>> +{
>> + call_void_hook(bpf_map_file, map, file);
>> +}
>> +void security_bpf_prog_file(struct bpf_prog_aux *aux, struct file
>> *file)
>> +{
>> + call_void_hook(bpf_prog_file, aux, file);
>> +}
>> #endif /* CONFIG_BPF_SYSCALL */
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 41aba4e3d57c..fea88655e0ee 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -1815,6 +1815,10 @@ static inline int file_path_has_perm(const
>> struct cred *cred,
>> return inode_has_perm(cred, file_inode(file), av, &ad);
>> }
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> +static int bpf_file_check(struct file *file, u32 sid);
>> +#endif
>> +
>> /* Check whether a task can use an open file descriptor to
>> access an inode in a given way. Check access to the
>> descriptor itself, and then use dentry_has_perm to
>> @@ -1845,6 +1849,12 @@ static int file_has_perm(const struct cred
>> *cred,
>> goto out;
>> }
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> + rc = bpf_file_check(file, cred_sid(cred));
>> + if (rc)
>> + goto out;
>> +#endif
>> +
>> /* av is zero if only checking access to the descriptor. */
>> rc = 0;
>> if (av)
>> @@ -2165,6 +2175,12 @@ static int selinux_binder_transfer_file(struct
>> task_struct *from,
>> return rc;
>> }
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> + rc = bpf_file_check(file, sid);
>> + if (rc)
>> + return rc;
>> +#endif
>> +
>> if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>> return 0;
>>
>> @@ -6288,6 +6304,33 @@ static u32 bpf_map_fmode_to_av(fmode_t fmode)
>> return av;
>> }
>>
>> +/* This function will check the file pass through unix socket or
>> binder to see
>> + * if it is a bpf related object. And apply correspinding checks on
>> the bpf
>> + * object based on the type. The bpf maps and programs, not like
>> other files and
>> + * socket, are using a shared anonymous inode inside the kernel as
>> their inode.
>> + * So checking that inode cannot identify if the process have
>> privilege to
>> + * access the bpf object and that's why we have to add this
>> additional check in
>> + * selinux_file_receive and selinux_binder_transfer_files.
>> + */
>> +static int bpf_file_check(struct file *file, u32 sid)
>> +{
>> + struct file_security_struct *fsec = file->f_security;
>> + int ret;
>> +
>> + if (file->f_op == &bpf_map_fops) {
>> + ret = avc_has_perm(sid, fsec->sid, SECCLASS_BPF_MAP,
>> + bpf_map_fmode_to_av(file-
>> >f_mode), NULL);
>> + if (ret)
>> + return ret;
>> + } else if (file->f_op == &bpf_prog_fops) {
>> + ret = avc_has_perm(sid, fsec->sid,
>> SECCLASS_BPF_PROG,
>> + BPF_PROG__USE, NULL);
>> + if (ret)
>> + return ret;
>> + }
>> + return 0;
>> +}
>> +
>> static int selinux_bpf_map(struct bpf_map *map, fmode_t fmode)
>> {
>> u32 sid = current_sid();
>> @@ -6351,6 +6394,22 @@ static void selinux_bpf_prog_free(struct
>> bpf_prog_aux *aux)
>> aux->security = NULL;
>> kfree(bpfsec);
>> }
>> +
>> +static void selinux_bpf_map_file(struct bpf_map *map, struct file
>> *file)
>> +{
>> + struct bpf_security_struct *bpfsec = map->security;
>> + struct file_security_struct *fsec = file->f_security;
>> +
>> + fsec->sid = bpfsec->sid;
>> +}
>> +
>> +static void selinux_bpf_prog_file(struct bpf_prog_aux *aux, struct
>> file *file)
>> +{
>> + struct bpf_security_struct *bpfsec = aux->security;
>> + struct file_security_struct *fsec = file->f_security;
>> +
>> + fsec->sid = bpfsec->sid;
>
> I could be wrong, but isn't it the case that fsec->sid already will
> equal bpfsec->sid, because they are both created by the same thread
> during the same system call, and they each inherit the SID of the
> current task?
>
This is true when bpf object is created by the same process that
obtains the fd. But there are other ways of getting a bpf object fd
from the kernel such as bpf_obj_get and bpf_get_obj_fd_by_id. These
action will ask the kernel to allocate a new file for the bpf object
and the file sid would be the process ask for fd while the bpfsec->sid
is the sid when bpf object get created. These two could be different.
> What I expected you to do was to add and set a flags field in the
> file_security_struct to indicate that this is a bpf map or prog, and
> then test for that in your bpf_file_check() function instead of having
> to export and test the fops structures.
>
>
>> +}
>> #endif
>>
>> static struct security_hook_list selinux_hooks[] __lsm_ro_after_init
>> = {
>> @@ -6581,6 +6640,8 @@ static struct security_hook_list
>> selinux_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(bpf_prog_alloc_security,
>> selinux_bpf_prog_alloc),
>> LSM_HOOK_INIT(bpf_map_free_security, selinux_bpf_map_free),
>> LSM_HOOK_INIT(bpf_prog_free_security,
>> selinux_bpf_prog_free),
>> + LSM_HOOK_INIT(bpf_map_file, selinux_bpf_map_file),
>> + LSM_HOOK_INIT(bpf_prog_file, selinux_bpf_prog_file),
>> #endif
>> };
>>
^ permalink raw reply
* Re: [PATCH v5 1/2] net: phy: DP83822 initial driver submission
From: Andrew F. Davis @ 2017-10-10 17:49 UTC (permalink / raw)
To: Dan Murphy, andrew, f.fainelli; +Cc: netdev, Woojung.Huh
In-Reply-To: <20171010174256.21930-1-dmurphy@ti.com>
On 10/10/2017 12:42 PM, Dan Murphy wrote:
> Add support for the TI DP83822 10/100Mbit ethernet phy.
>
> The DP83822 provides flexibility to connect to a MAC through a
> standard MII, RMII or RGMII interface.
>
> In addition the DP83822 needs to be removed from the DP83848 driver
> as the WoL support is added here for this device.
>
> Datasheet:
> http://www.ti.com/product/DP83822I/datasheet
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
Looks much nicer now, thanks for dealing with my nitpicking :)
Acked-by: Andrew F. Davis <afd@ti.com>
>
> v5 - Fixed bit mask, added config_init to set WOL register bits, fixed spacing
> issues, moved password storage under secure on if statement, and added INT_EN bit
> setting for interrupt enable.
> https://www.mail-archive.com/netdev@vger.kernel.org/msg192495.html
>
> v4 - Squash DP83822 removal patch into this patch -
> https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html
>
> v3 - Fixed WoL indication bit and removed mutex for suspend/resume -
> https://www.mail-archive.com/netdev@vger.kernel.org/msg191891.html and
> https://www.mail-archive.com/netdev@vger.kernel.org/msg191665.html
>
> v2 - Updated per comments. Removed unnessary parantheis, called genphy_suspend/
> resume routines and then performing WoL changes, reworked sopass storage and reduced
> the number of phy reads, and moved WOL_SECURE_ON -
> https://www.mail-archive.com/netdev@vger.kernel.org/msg191392.html
>
> drivers/net/phy/Kconfig | 5 +
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/dp83822.c | 344 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/net/phy/dp83848.c | 3 -
> 4 files changed, 350 insertions(+), 3 deletions(-)
> create mode 100644 drivers/net/phy/dp83822.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index cd931cf9dcc2..8e78a482e09e 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -277,6 +277,11 @@ config DAVICOM_PHY
> ---help---
> Currently supports dm9161e and dm9131
>
> +config DP83822_PHY
> + tristate "Texas Instruments DP83822 PHY"
> + ---help---
> + Supports the DP83822 PHY.
> +
> config DP83848_PHY
> tristate "Texas Instruments DP83848 PHY"
> ---help---
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 416df92fbf4f..df3b82ba8550 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY) += cicada.o
> obj-$(CONFIG_CORTINA_PHY) += cortina.o
> obj-$(CONFIG_DAVICOM_PHY) += davicom.o
> obj-$(CONFIG_DP83640_PHY) += dp83640.o
> +obj-$(CONFIG_DP83822_PHY) += dp83822.o
> obj-$(CONFIG_DP83848_PHY) += dp83848.o
> obj-$(CONFIG_DP83867_PHY) += dp83867.o
> obj-$(CONFIG_FIXED_PHY) += fixed_phy.o
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> new file mode 100644
> index 000000000000..14335d14e9e4
> --- /dev/null
> +++ b/drivers/net/phy/dp83822.c
> @@ -0,0 +1,344 @@
> +/*
> + * Driver for the Texas Instruments DP83822 PHY
> + *
> + * Copyright (C) 2017 Texas Instruments Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/ethtool.h>
> +#include <linux/etherdevice.h>
> +#include <linux/kernel.h>
> +#include <linux/mii.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy.h>
> +#include <linux/netdevice.h>
> +
> +#define DP83822_PHY_ID 0x2000a240
> +#define DP83822_DEVADDR 0x1f
> +
> +#define MII_DP83822_PHYSCR 0x11
> +#define MII_DP83822_MISR1 0x12
> +#define MII_DP83822_MISR2 0x13
> +#define MII_DP83822_RESET_CTRL 0x1f
> +
> +#define DP83822_HW_RESET BIT(15)
> +#define DP83822_SW_RESET BIT(14)
> +
> +/* PHYSCR Register Fields */
> +#define DP83822_PHYSCR_INT_OE BIT(0) /* Interrupt Output Enable */
> +#define DP83822_PHYSCR_INTEN BIT(1) /* Interrupt Enable */
> +
> +/* MISR1 bits */
> +#define DP83822_RX_ERR_HF_INT_EN BIT(0)
> +#define DP83822_FALSE_CARRIER_HF_INT_EN BIT(1)
> +#define DP83822_ANEG_COMPLETE_INT_EN BIT(2)
> +#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3)
> +#define DP83822_SPEED_CHANGED_INT_EN BIT(4)
> +#define DP83822_LINK_STAT_INT_EN BIT(5)
> +#define DP83822_ENERGY_DET_INT_EN BIT(6)
> +#define DP83822_LINK_QUAL_INT_EN BIT(7)
> +
> +/* MISR2 bits */
> +#define DP83822_JABBER_DET_INT_EN BIT(0)
> +#define DP83822_WOL_PKT_INT_EN BIT(1)
> +#define DP83822_SLEEP_MODE_INT_EN BIT(2)
> +#define DP83822_MDI_XOVER_INT_EN BIT(3)
> +#define DP83822_LB_FIFO_INT_EN BIT(4)
> +#define DP83822_PAGE_RX_INT_EN BIT(5)
> +#define DP83822_ANEG_ERR_INT_EN BIT(6)
> +#define DP83822_EEE_ERROR_CHANGE_INT_EN BIT(7)
> +
> +/* INT_STAT1 bits */
> +#define DP83822_WOL_INT_EN BIT(4)
> +#define DP83822_WOL_INT_STAT BIT(12)
> +
> +#define MII_DP83822_RXSOP1 0x04a5
> +#define MII_DP83822_RXSOP2 0x04a6
> +#define MII_DP83822_RXSOP3 0x04a7
> +
> +/* WoL Registers */
> +#define MII_DP83822_WOL_CFG 0x04a0
> +#define MII_DP83822_WOL_STAT 0x04a1
> +#define MII_DP83822_WOL_DA1 0x04a2
> +#define MII_DP83822_WOL_DA2 0x04a3
> +#define MII_DP83822_WOL_DA3 0x04a4
> +
> +/* WoL bits */
> +#define DP83822_WOL_MAGIC_EN BIT(0)
> +#define DP83822_WOL_SECURE_ON BIT(5)
> +#define DP83822_WOL_EN BIT(7)
> +#define DP83822_WOL_INDICATION_SEL BIT(8)
> +#define DP83822_WOL_CLR_INDICATION BIT(11)
> +
> +static int dp83822_ack_interrupt(struct phy_device *phydev)
> +{
> + int err;
> +
> + err = phy_read(phydev, MII_DP83822_MISR1);
> + if (err < 0)
> + return err;
> +
> + err = phy_read(phydev, MII_DP83822_MISR2);
> + if (err < 0)
> + return err;
> +
> + return 0;
> +}
> +
> +static int dp83822_set_wol(struct phy_device *phydev,
> + struct ethtool_wolinfo *wol)
> +{
> + struct net_device *ndev = phydev->attached_dev;
> + u16 value;
> + const u8 *mac;
> +
> + if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE)) {
> + mac = (const u8 *)ndev->dev_addr;
> +
> + if (!is_valid_ether_addr(mac))
> + return -EINVAL;
> +
> + /* MAC addresses start with byte 5, but stored in mac[0].
> + * 822 PHYs store bytes 4|5, 2|3, 0|1
> + */
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA1,
> + (mac[1] << 8) | mac[0]);
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA2,
> + (mac[3] << 8) | mac[2]);
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA3,
> + (mac[5] << 8) | mac[4]);
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_WOL_CFG);
> + if (wol->wolopts & WAKE_MAGIC)
> + value |= DP83822_WOL_MAGIC_EN;
> + else
> + value &= ~DP83822_WOL_MAGIC_EN;
> +
> + if (wol->wolopts & WAKE_MAGICSECURE) {
> + phy_write_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP1,
> + (wol->sopass[1] << 8) | wol->sopass[0]);
> + phy_write_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP2,
> + (wol->sopass[3] << 8) | wol->sopass[2]);
> + phy_write_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP3,
> + (wol->sopass[5] << 8) | wol->sopass[4]);
> + value |= DP83822_WOL_SECURE_ON;
> + } else {
> + value &= ~DP83822_WOL_SECURE_ON;
> + }
> +
> + value |= (DP83822_WOL_EN | DP83822_WOL_INDICATION_SEL |
> + DP83822_WOL_CLR_INDICATION);
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
> + value);
> + } else {
> + value = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_WOL_CFG);
> + value &= ~DP83822_WOL_EN;
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
> + value);
> + }
> +
> + return 0;
> +}
> +
> +static void dp83822_get_wol(struct phy_device *phydev,
> + struct ethtool_wolinfo *wol)
> +{
> + int value;
> + u16 sopass_val;
> +
> + wol->supported = (WAKE_MAGIC | WAKE_MAGICSECURE);
> + wol->wolopts = 0;
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
> +
> + if (value & DP83822_WOL_MAGIC_EN)
> + wol->wolopts |= WAKE_MAGIC;
> +
> + if (value & DP83822_WOL_SECURE_ON) {
> + sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP1);
> + wol->sopass[0] = (sopass_val & 0xff);
> + wol->sopass[1] = (sopass_val >> 8);
> +
> + sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP2);
> + wol->sopass[2] = (sopass_val & 0xff);
> + wol->sopass[3] = (sopass_val >> 8);
> +
> + sopass_val = phy_read_mmd(phydev, DP83822_DEVADDR,
> + MII_DP83822_RXSOP3);
> + wol->sopass[4] = (sopass_val & 0xff);
> + wol->sopass[5] = (sopass_val >> 8);
> +
> + wol->wolopts |= WAKE_MAGICSECURE;
> + }
> +
> + /* WoL is not enabled so set wolopts to 0 */
> + if (!(value & DP83822_WOL_EN))
> + wol->wolopts = 0;
> +}
> +
> +static int dp83822_config_intr(struct phy_device *phydev)
> +{
> + int misr_status;
> + int physcr_status;
> + int err;
> +
> + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> + misr_status = phy_read(phydev, MII_DP83822_MISR1);
> + if (misr_status < 0)
> + return misr_status;
> +
> + misr_status |= (DP83822_RX_ERR_HF_INT_EN |
> + DP83822_FALSE_CARRIER_HF_INT_EN |
> + DP83822_ANEG_COMPLETE_INT_EN |
> + DP83822_DUP_MODE_CHANGE_INT_EN |
> + DP83822_SPEED_CHANGED_INT_EN |
> + DP83822_LINK_STAT_INT_EN |
> + DP83822_ENERGY_DET_INT_EN |
> + DP83822_LINK_QUAL_INT_EN);
> +
> + err = phy_write(phydev, MII_DP83822_MISR1, misr_status);
> + if (err < 0)
> + return err;
> +
> + misr_status = phy_read(phydev, MII_DP83822_MISR2);
> + if (misr_status < 0)
> + return misr_status;
> +
> + misr_status |= (DP83822_JABBER_DET_INT_EN |
> + DP83822_WOL_PKT_INT_EN |
> + DP83822_SLEEP_MODE_INT_EN |
> + DP83822_MDI_XOVER_INT_EN |
> + DP83822_LB_FIFO_INT_EN |
> + DP83822_PAGE_RX_INT_EN |
> + DP83822_ANEG_ERR_INT_EN |
> + DP83822_EEE_ERROR_CHANGE_INT_EN);
> +
> + err = phy_write(phydev, MII_DP83822_MISR2, misr_status);
> + if (err < 0)
> + return err;
> +
> + physcr_status = phy_read(phydev, MII_DP83822_PHYSCR);
> + if (physcr_status < 0)
> + return physcr_status;
> +
> + physcr_status |= DP83822_PHYSCR_INT_OE | DP83822_PHYSCR_INTEN;
> +
> + } else {
> + err = phy_write(phydev, MII_DP83822_MISR1, 0);
> + if (err < 0)
> + return err;
> +
> + err = phy_write(phydev, MII_DP83822_MISR1, 0);
> + if (err < 0)
> + return err;
> +
> + physcr_status = phy_read(phydev, MII_DP83822_PHYSCR);
> + if (physcr_status < 0)
> + return physcr_status;
> +
> + physcr_status &= ~DP83822_PHYSCR_INTEN;
> + }
> +
> + return phy_write(phydev, MII_DP83822_PHYSCR, physcr_status);
> +}
> +
> +static int dp83822_config_init(struct phy_device *phydev)
> +{
> + int err;
> + int value;
> +
> + err = genphy_config_init(phydev);
> + if (err < 0)
> + return err;
> +
> + value = DP83822_WOL_MAGIC_EN | DP83822_WOL_SECURE_ON | DP83822_WOL_EN;
> +
> + return phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
> + value);
> +}
> +
> +static int dp83822_phy_reset(struct phy_device *phydev)
> +{
> + int err;
> +
> + err = phy_write(phydev, MII_DP83822_RESET_CTRL, DP83822_HW_RESET);
> + if (err < 0)
> + return err;
> +
> + dp83822_config_init(phydev);
> +
> + return 0;
> +}
> +
> +static int dp83822_suspend(struct phy_device *phydev)
> +{
> + int value;
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
> +
> + if (!(value & DP83822_WOL_EN))
> + genphy_suspend(phydev);
> +
> + return 0;
> +}
> +
> +static int dp83822_resume(struct phy_device *phydev)
> +{
> + int value;
> +
> + genphy_resume(phydev);
> +
> + value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
> +
> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG, value |
> + DP83822_WOL_CLR_INDICATION);
> +
> + return 0;
> +}
> +
> +static struct phy_driver dp83822_driver[] = {
> + {
> + .phy_id = DP83822_PHY_ID,
> + .phy_id_mask = 0xfffffff0,
> + .name = "TI DP83822",
> + .features = PHY_BASIC_FEATURES,
> + .flags = PHY_HAS_INTERRUPT,
> + .config_init = dp83822_config_init,
> + .soft_reset = dp83822_phy_reset,
> + .get_wol = dp83822_get_wol,
> + .set_wol = dp83822_set_wol,
> + .ack_interrupt = dp83822_ack_interrupt,
> + .config_intr = dp83822_config_intr,
> + .config_aneg = genphy_config_aneg,
> + .read_status = genphy_read_status,
> + .suspend = dp83822_suspend,
> + .resume = dp83822_resume,
> + },
> +};
> +module_phy_driver(dp83822_driver);
> +
> +static struct mdio_device_id __maybe_unused dp83822_tbl[] = {
> + { DP83822_PHY_ID, 0xfffffff0 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(mdio, dp83822_tbl);
> +
> +MODULE_DESCRIPTION("Texas Instruments DP83822 PHY driver");
> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
> index 3de4fe4dda77..3966d43c5146 100644
> --- a/drivers/net/phy/dp83848.c
> +++ b/drivers/net/phy/dp83848.c
> @@ -20,7 +20,6 @@
> #define TI_DP83620_PHY_ID 0x20005ce0
> #define NS_DP83848C_PHY_ID 0x20005c90
> #define TLK10X_PHY_ID 0x2000a210
> -#define TI_DP83822_PHY_ID 0x2000a240
>
> /* Registers */
> #define DP83848_MICR 0x11 /* MII Interrupt Control Register */
> @@ -80,7 +79,6 @@ static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
> { NS_DP83848C_PHY_ID, 0xfffffff0 },
> { TI_DP83620_PHY_ID, 0xfffffff0 },
> { TLK10X_PHY_ID, 0xfffffff0 },
> - { TI_DP83822_PHY_ID, 0xfffffff0 },
> { }
> };
> MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
> @@ -110,7 +108,6 @@ static struct phy_driver dp83848_driver[] = {
> DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY"),
> DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY"),
> DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY"),
> - DP83848_PHY_DRIVER(TI_DP83822_PHY_ID, "TI DP83822 10/100 Mbps PHY"),
> };
> module_phy_driver(dp83848_driver);
>
>
^ permalink raw reply
* Re: [PATCH v5 1/2] net: phy: DP83822 initial driver submission
From: Florian Fainelli @ 2017-10-10 17:50 UTC (permalink / raw)
To: Dan Murphy, andrew; +Cc: netdev, Woojung.Huh, afd
In-Reply-To: <20171010174256.21930-1-dmurphy@ti.com>
On 10/10/2017 10:42 AM, Dan Murphy wrote:
> Add support for the TI DP83822 10/100Mbit ethernet phy.
>
> The DP83822 provides flexibility to connect to a MAC through a
> standard MII, RMII or RGMII interface.
>
> In addition the DP83822 needs to be removed from the DP83848 driver
> as the WoL support is added here for this device.
>
> Datasheet:
> http://www.ti.com/product/DP83822I/datasheet
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Thanks Dan!
--
Florian
^ permalink raw reply
* Re: [PATCH][net-next] ipv6: fix dereference of rt6_ex before null check error
From: David Miller @ 2017-10-10 17:54 UTC (permalink / raw)
To: colin.king; +Cc: kuznet, yoshfuji, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20171010170116.21124-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Tue, 10 Oct 2017 18:01:16 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently rt6_ex is being dereferenced before it is null checked
> hence there is a possible null dereference bug. Fix this by only
> dereferencing rt6_ex after it has been null checked.
>
> Detected by CoverityScan, CID#1457749 ("Dereference before null check")
>
> Fixes: 81eb8447daae ("ipv6: take care of rt6_stats")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied, thanks Colin.
^ permalink raw reply
* Re: [PATCH net-next v2 4/5] selinux: bpf: Add selinux check for eBPF syscall operations
From: Chenbo Feng @ 2017-10-10 17:54 UTC (permalink / raw)
To: Stephen Smalley
Cc: Chenbo Feng, linux-security-module, netdev, SELinux,
Alexei Starovoitov, Daniel Borkmann, Lorenzo Colitti
In-Reply-To: <1507647148.30616.14.camel@tycho.nsa.gov>
On Tue, Oct 10, 2017 at 7:52 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Tue, 2017-10-10 at 10:18 -0400, Stephen Smalley wrote:
>> On Mon, 2017-10-09 at 15:20 -0700, Chenbo Feng wrote:
>> > From: Chenbo Feng <fengc@google.com>
>> >
>> > Implement the actual checks introduced to eBPF related syscalls.
>> > This
>> > implementation use the security field inside bpf object to store a
>> > sid that
>> > identify the bpf object. And when processes try to access the
>> > object,
>> > selinux will check if processes have the right privileges. The
>> > creation
>> > of eBPF object are also checked at the general bpf check hook and
>> > new
>> > cmd introduced to eBPF domain can also be checked there.
>> >
>> > Signed-off-by: Chenbo Feng <fengc@google.com>
>> > Acked-by: Alexei Starovoitov <ast@kernel.org>
>> > ---
>> > security/selinux/hooks.c | 111
>> > ++++++++++++++++++++++++++++++++++++
>> > security/selinux/include/classmap.h | 2 +
>> > security/selinux/include/objsec.h | 4 ++
>> > 3 files changed, 117 insertions(+)
>> >
>> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> > index f5d304736852..41aba4e3d57c 100644
>> > --- a/security/selinux/hooks.c
>> > +++ b/security/selinux/hooks.c
>> > @@ -85,6 +85,7 @@
>> > #include <linux/export.h>
>> > #include <linux/msg.h>
>> > #include <linux/shm.h>
>> > +#include <linux/bpf.h>
>> >
>> > #include "avc.h"
>> > #include "objsec.h"
>> > @@ -6252,6 +6253,106 @@ static void selinux_ib_free_security(void
>> > *ib_sec)
>> > }
>> > #endif
>> >
>> > +#ifdef CONFIG_BPF_SYSCALL
>> > +static int selinux_bpf(int cmd, union bpf_attr *attr,
>> > + unsigned int size)
>> > +{
>> > + u32 sid = current_sid();
>> > + int ret;
>> > +
>> > + switch (cmd) {
>> > + case BPF_MAP_CREATE:
>> > + ret = avc_has_perm(sid, sid, SECCLASS_BPF_MAP,
>> > BPF_MAP__CREATE,
>> > + NULL);
>> > + break;
>> > + case BPF_PROG_LOAD:
>> > + ret = avc_has_perm(sid, sid, SECCLASS_BPF_PROG,
>> > BPF_PROG__LOAD,
>> > + NULL);
>> > + break;
>> > + default:
>> > + ret = 0;
>> > + break;
>> > + }
>> > +
>> > + return ret;
>> > +}
>> > +
>> > +static u32 bpf_map_fmode_to_av(fmode_t fmode)
>> > +{
>> > + u32 av = 0;
>> > +
>> > + if (f_mode & FMODE_READ)
>> > + av |= BPF_MAP__READ;
>> > + if (f_mode & FMODE_WRITE)
>> > + av |= BPF_MAP__WRITE;
>> > + return av;
>> > +}
>> > +
>> > +static int selinux_bpf_map(struct bpf_map *map, fmode_t fmode)
>> > +{
>> > + u32 sid = current_sid();
>> > + struct bpf_security_struct *bpfsec;
>> > +
>> > + bpfsec = map->security;
>> > + return avc_has_perm(sid, bpfsec->sid, SECCLASS_BPF_MAP,
>> > + bpf_map_fmode_to_av(fmode), NULL);
>> > +}
>> > +
>> > +static int selinux_bpf_prog(struct bpf_prog *prog)
>> > +{
>> > + u32 sid = current_sid();
>> > + struct bpf_security_struct *bpfsec;
>> > +
>> > + bpfsec = prog->aux->security;
>> > + return avc_has_perm(sid, bpfsec->sid, SECCLASS_BPF_PROG,
>> > + BPF_PROG__USE, NULL);
>> > +}
>> > +
>> > +static int selinux_bpf_map_alloc(struct bpf_map *map)
>> > +{
>> > + struct bpf_security_struct *bpfsec;
>> > +
>> > + bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
>> > + if (!bpfsec)
>> > + return -ENOMEM;
>> > +
>> > + bpfsec->sid = current_sid();
>> > + map->security = bpfsec;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static void selinux_bpf_map_free(struct bpf_map *map)
>> > +{
>> > + struct bpf_security_struct *bpfsec = map->security;
>> > +
>> > + map->security = NULL;
>> > + kfree(bpfsec);
>> > +}
>> > +
>> > +static int selinux_bpf_prog_alloc(struct bpf_prog_aux *aux)
>> > +{
>> > + struct bpf_security_struct *bpfsec;
>> > +
>> > + bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
>> > + if (!bpfsec)
>> > + return -ENOMEM;
>> > +
>> > + bpfsec->sid = current_sid();
>> > + aux->security = bpfsec;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static void selinux_bpf_prog_free(struct bpf_prog_aux *aux)
>> > +{
>> > + struct bpf_security_struct *bpfsec = aux->security;
>> > +
>> > + aux->security = NULL;
>> > + kfree(bpfsec);
>> > +}
>> > +#endif
>> > +
>> > static struct security_hook_list selinux_hooks[]
>> > __lsm_ro_after_init
>> > = {
>> > LSM_HOOK_INIT(binder_set_context_mgr,
>> > selinux_binder_set_context_mgr),
>> > LSM_HOOK_INIT(binder_transaction,
>> > selinux_binder_transaction),
>> > @@ -6471,6 +6572,16 @@ static struct security_hook_list
>> > selinux_hooks[] __lsm_ro_after_init = {
>> > LSM_HOOK_INIT(audit_rule_match, selinux_audit_rule_match),
>> > LSM_HOOK_INIT(audit_rule_free, selinux_audit_rule_free),
>> > #endif
>> > +
>> > +#ifdef CONFIG_BPF_SYSCALL
>> > + LSM_HOOK_INIT(bpf, selinux_bpf),
>> > + LSM_HOOK_INIT(bpf_map, selinux_bpf_map),
>> > + LSM_HOOK_INIT(bpf_prog, selinux_bpf_prog),
>> > + LSM_HOOK_INIT(bpf_map_alloc_security,
>> > selinux_bpf_map_alloc),
>> > + LSM_HOOK_INIT(bpf_prog_alloc_security,
>> > selinux_bpf_prog_alloc),
>> > + LSM_HOOK_INIT(bpf_map_free_security,
>> > selinux_bpf_map_free),
>> > + LSM_HOOK_INIT(bpf_prog_free_security,
>> > selinux_bpf_prog_free),
>> > +#endif
>> > };
>> >
>> > static __init int selinux_init(void)
>> > diff --git a/security/selinux/include/classmap.h
>> > b/security/selinux/include/classmap.h
>> > index 35ffb29a69cb..7253c5eea59c 100644
>> > --- a/security/selinux/include/classmap.h
>> > +++ b/security/selinux/include/classmap.h
>> > @@ -237,6 +237,8 @@ struct security_class_mapping secclass_map[] =
>> > {
>> > { "access", NULL } },
>> > { "infiniband_endport",
>> > { "manage_subnet", NULL } },
>> > + { "bpf_map", {"create", "read", "write"} },
>> > + { "bpf_prog", {"load", "use"} },
>>
>> Again I have to ask: do you truly need/want two separate classes, or
>> would a single class with distinct permissions suffice, ala:
>> { "bpf", { "create_map", "read_map", "write_map",
>> "prog_load",
>> "prog_use" } },
>>
>> and then allow A self:bpf { create_map read_map write_map prog_load
>> prog_use }; would be stored in a single policy avtab rule, and be
>> cached in a single AVC entry.
>
Sorry I missed to reply on this, I keep it that way because sometimes
we need to grant the permission of accessing eBPF maps and programs
separately. But keep them in a single class definitely works for me.
> I guess for consistency in naming it should be either:
> { "bpf", { "map_create", "map_read", "map_write", "prog_load",
> "prog_use" } },
>
> or:
>
> { "bpf", { "create_map", "read_map", "write_map", "load_prog",
> "use_prog" } },
>
>
>> > { NULL }
>> > };
>> >
>> > diff --git a/security/selinux/include/objsec.h
>> > b/security/selinux/include/objsec.h
>> > index 1649cd18eb0b..3d54468ce334 100644
>> > --- a/security/selinux/include/objsec.h
>> > +++ b/security/selinux/include/objsec.h
>> > @@ -150,6 +150,10 @@ struct pkey_security_struct {
>> > u32 sid; /* SID of pkey */
>> > };
>> >
>> > +struct bpf_security_struct {
>> > + u32 sid; /*SID of bpf obj creater*/
>> > +};
>> > +
>> > extern unsigned int selinux_checkreqprot;
>> >
>> > #endif /* _SELINUX_OBJSEC_H_ */
^ 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