* Re: [PATCH net-next] net: NET_VENDOR_MICROSEMI should default ot N
From: David Miller @ 2018-05-17 16:32 UTC (permalink / raw)
To: dsahern; +Cc: netdev, alexandre.belloni
In-Reply-To: <20180517154330.10678-1-dsahern@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Thu, 17 May 2018 08:43:30 -0700
> Other ethernet drivers default to N. There is no reason for Microsemi
> to default to y. I believe Linus has set the bar at a feature that cures
> cancer can be enabled by default. [1]
>
> [1] https://lkml.org/lkml/2010/3/2/366
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
For "drivers" yes, those should default to N. But for vendor guards
like this, they should default to Y.
^ permalink raw reply
* [net-next 6/6] ice: Update NVM AQ command functions
From: Jeff Kirsher @ 2018-05-17 16:37 UTC (permalink / raw)
To: davem
Cc: Anirudh Venkataramanan, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20180517163732.30910-1-jeffrey.t.kirsher@intel.com>
From: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
This patch updates the NVM read/erase/update AQ commands to align with
the latest specification.
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Tested-by: Tony Brelinski <tonyx.brelinski@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_adminq_cmd.h | 13 +++++++------
drivers/net/ethernet/intel/ice/ice_nvm.c | 7 ++++---
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 7dc5f045e969..7541ec2270b3 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1049,7 +1049,9 @@ struct ice_aqc_set_event_mask {
* NVM Update commands (indirect 0x0703)
*/
struct ice_aqc_nvm {
- u8 cmd_flags;
+ __le16 offset_low;
+ u8 offset_high;
+ u8 cmd_flags;
#define ICE_AQC_NVM_LAST_CMD BIT(0)
#define ICE_AQC_NVM_PCIR_REQ BIT(0) /* Used by NVM Update reply */
#define ICE_AQC_NVM_PRESERVATION_S 1
@@ -1058,12 +1060,11 @@ struct ice_aqc_nvm {
#define ICE_AQC_NVM_PRESERVE_ALL BIT(1)
#define ICE_AQC_NVM_PRESERVE_SELECTED (3 << CSR_AQ_NVM_PRESERVATION_S)
#define ICE_AQC_NVM_FLASH_ONLY BIT(7)
- u8 module_typeid;
- __le16 length;
+ __le16 module_typeid;
+ __le16 length;
#define ICE_AQC_NVM_ERASE_LEN 0xFFFF
- __le32 offset;
- __le32 addr_high;
- __le32 addr_low;
+ __le32 addr_high;
+ __le32 addr_low;
};
/* Get/Set RSS key (indirect 0x0B04/0x0B02) */
diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.c b/drivers/net/ethernet/intel/ice/ice_nvm.c
index fa7a69ac92b0..92da0a626ce0 100644
--- a/drivers/net/ethernet/intel/ice/ice_nvm.c
+++ b/drivers/net/ethernet/intel/ice/ice_nvm.c
@@ -16,7 +16,7 @@
* Read the NVM using the admin queue commands (0x0701)
*/
static enum ice_status
-ice_aq_read_nvm(struct ice_hw *hw, u8 module_typeid, u32 offset, u16 length,
+ice_aq_read_nvm(struct ice_hw *hw, u16 module_typeid, u32 offset, u16 length,
void *data, bool last_command, struct ice_sq_cd *cd)
{
struct ice_aq_desc desc;
@@ -33,8 +33,9 @@ ice_aq_read_nvm(struct ice_hw *hw, u8 module_typeid, u32 offset, u16 length,
/* If this is the last command in a series, set the proper flag. */
if (last_command)
cmd->cmd_flags |= ICE_AQC_NVM_LAST_CMD;
- cmd->module_typeid = module_typeid;
- cmd->offset = cpu_to_le32(offset);
+ cmd->module_typeid = cpu_to_le16(module_typeid);
+ cmd->offset_low = cpu_to_le16(offset & 0xFFFF);
+ cmd->offset_high = (offset >> 16) & 0xFF;
cmd->length = cpu_to_le16(length);
return ice_aq_send_cmd(hw, &desc, data, length, cd);
--
2.17.0
^ permalink raw reply related
* [net-next 3/6] ixgbe: release lock for the duration of ixgbe_suspend_close()
From: Jeff Kirsher @ 2018-05-17 16:37 UTC (permalink / raw)
To: davem; +Cc: Pavel Tatashin, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180517163732.30910-1-jeffrey.t.kirsher@intel.com>
From: Pavel Tatashin <pasha.tatashin@oracle.com>
Currently, during device_shutdown() ixgbe holds rtnl_lock for the duration
of lengthy ixgbe_close_suspend(). On machines with multiple ixgbe cards
this lock prevents scaling if device_shutdown() function is multi-threaded.
It is not necessary to hold this lock during ixgbe_close_suspend()
as it is not held when ixgbe_close() is called also during shutdown but for
kexec case.
Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index a52d92e182ee..5ddfb93ed491 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6698,8 +6698,15 @@ static int __ixgbe_shutdown(struct pci_dev *pdev, bool *enable_wake)
rtnl_lock();
netif_device_detach(netdev);
- if (netif_running(netdev))
+ if (netif_running(netdev)) {
+ /* Suspend takes a long time, device_shutdown may be
+ * parallelized this function, so drop lock for the
+ * duration of this call.
+ */
+ rtnl_unlock();
ixgbe_close_suspend(adapter);
+ rtnl_lock();
+ }
ixgbe_clear_interrupt_scheme(adapter);
rtnl_unlock();
--
2.17.0
^ permalink raw reply related
* [net-next 5/6] ixgbevf: fix MAC address changes through ixgbevf_set_mac()
From: Jeff Kirsher @ 2018-05-17 16:37 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180517163732.30910-1-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
Set hw->mac.perm_addr in ixgbevf_set_mac() in order to avoid losing the
custom MAC on reset. This can happen in the following case:
>ip link set $vf address $mac
>ethtool -r $vf
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 9a939dcaf727..083041129539 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -4164,6 +4164,7 @@ static int ixgbevf_set_mac(struct net_device *netdev, void *p)
return -EPERM;
ether_addr_copy(hw->mac.addr, addr->sa_data);
+ ether_addr_copy(hw->mac.perm_addr, addr->sa_data);
ether_addr_copy(netdev->dev_addr, addr->sa_data);
return 0;
--
2.17.0
^ permalink raw reply related
* [net-next 4/6] ixgbe: force VF to grab new MAC on driver reload
From: Jeff Kirsher @ 2018-05-17 16:37 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180517163732.30910-1-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
Do not validate the MAC address during a reset, unless the MAC was set on
the host. This way the VF will get a new MAC address every time it reloads.
Remove the "no MAC address assigned" message since it will get spammed on
reset and it doesn't help much as the MAC on the VF is randomly generated.
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 2649c06d877b..6f59933cdff7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -854,14 +854,11 @@ static int ixgbe_vf_reset_msg(struct ixgbe_adapter *adapter, u32 vf)
/* reply to reset with ack and vf mac address */
msgbuf[0] = IXGBE_VF_RESET;
- if (!is_zero_ether_addr(vf_mac)) {
+ if (!is_zero_ether_addr(vf_mac) && adapter->vfinfo[vf].pf_set_mac) {
msgbuf[0] |= IXGBE_VT_MSGTYPE_ACK;
memcpy(addr, vf_mac, ETH_ALEN);
} else {
msgbuf[0] |= IXGBE_VT_MSGTYPE_NACK;
- dev_warn(&adapter->pdev->dev,
- "VF %d has no MAC address assigned, you may have to assign one manually\n",
- vf);
}
/*
--
2.17.0
^ permalink raw reply related
* [net-next 0/6][pull request] 10GbE Intel Wired LAN Driver Updates 2018-05-17
From: Jeff Kirsher @ 2018-05-17 16:37 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene
This series contains updates to ixgbe, ixgbevf and ice drivers.
Cathy Zhou resolves sparse warnings by using the force attribute.
Mauro S M Rodrigues fixes a bug where IRQs were not freed if a PCI error
recovery system opts to remove the device which causes
ixgbe_io_error_detected() to return PCI_ERS_RESULT_DISCONNECT before
calling ixgbe_close_suspend() which results in IRQs not freed and
crashing when the remove handler calls pci_disable_device(). Resolved
this by calling ixgbe_close_suspend() before evaluating the PCI channel
state.
Pavel Tatashin releases the rtnl_lock during the call to
ixgbe_close_suspend() to allow scaling if device_shutdown() is
multi-threaded.
Emil modifies ixgbe to not validate the MAC address during a reset,
unless the MAC was set on the host so that the VF will get a new MAC
address every time it reloads. Also updates ixgbevf to set
hw->mac.perm_addr in order to retain the custom MAC on a reset.
Anirudh updates the ice NVM read/erase/update AQ commands to align with
the latest specification.
The following are changes since commit b9f672af148bf7a08a6031743156faffd58dbc7e:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 10GbE
Anirudh Venkataramanan (1):
ice: Update NVM AQ command functions
Cathy Zhou (1):
ixgbe: cleanup sparse warnings
Emil Tantilov (2):
ixgbe: force VF to grab new MAC on driver reload
ixgbevf: fix MAC address changes through ixgbevf_set_mac()
Mauro S M Rodrigues (1):
ixgbe/ixgbevf: Free IRQ when PCI error recovery removes the device
Pavel Tatashin (1):
ixgbe: release lock for the duration of ixgbe_suspend_close()
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 13 +++---
drivers/net/ethernet/intel/ice/ice_nvm.c | 7 +--
.../net/ethernet/intel/ixgbe/ixgbe_82599.c | 13 +++---
.../net/ethernet/intel/ixgbe/ixgbe_common.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 2 +-
.../net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 25 +++++++----
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 44 ++++++++++++-------
.../net/ethernet/intel/ixgbe/ixgbe_model.h | 16 +++----
.../net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +--
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 9 ++--
.../net/ethernet/intel/ixgbevf/ixgbevf_main.c | 7 +--
11 files changed, 82 insertions(+), 61 deletions(-)
--
2.17.0
^ permalink raw reply
* [net-next 2/6] ixgbe/ixgbevf: Free IRQ when PCI error recovery removes the device
From: Jeff Kirsher @ 2018-05-17 16:37 UTC (permalink / raw)
To: davem
Cc: Mauro S M Rodrigues, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20180517163732.30910-1-jeffrey.t.kirsher@intel.com>
From: Mauro S M Rodrigues <maurosr@linux.vnet.ibm.com>
Since commit f7f37e7ff2b9 ("ixgbe: handle close/suspend race with
netif_device_detach/present") ixgbe_close_suspend is called, from
ixgbe_close, only if the device is present, i.e. if it isn't detached.
That exposed a situation where IRQs weren't freed if a PCI error
recovery system opts to remove the device. For such case the pci channel
state is set to pci_channel_io_perm_failure and ixgbe_io_error_detected
was returning PCI_ERS_RESULT_DISCONNECT before calling
ixgbe_close_suspend consequentially not freeing IRQ and crashing when
the remove handler calls pci_disable_device, hitting a BUG_ON at
free_msi_irqs, which asserts that there is no non-free IRQ associated
with the device to be removed:
BUG_ON(irq_has_action(entry->irq + i));
The issue is fixed by calling the ixgbe_close_suspend before evaluate
the pci channel state.
Reported-by: Naresh Bannoth <nbannoth@in.ibm.com>
Reported-by: Abdul Haleem <abdhalee@in.ibm.com>
Signed-off-by: Mauro S M Rodrigues <maurosr@linux.vnet.ibm.com>
Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 +++---
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 163b34a9572d..a52d92e182ee 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -10935,14 +10935,14 @@ static pci_ers_result_t ixgbe_io_error_detected(struct pci_dev *pdev,
rtnl_lock();
netif_device_detach(netdev);
+ if (netif_running(netdev))
+ ixgbe_close_suspend(adapter);
+
if (state == pci_channel_io_perm_failure) {
rtnl_unlock();
return PCI_ERS_RESULT_DISCONNECT;
}
- if (netif_running(netdev))
- ixgbe_close_suspend(adapter);
-
if (!test_and_set_bit(__IXGBE_DISABLED, &adapter->state))
pci_disable_device(pdev);
rtnl_unlock();
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 1ccce6cd51fc..9a939dcaf727 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -4747,14 +4747,14 @@ static pci_ers_result_t ixgbevf_io_error_detected(struct pci_dev *pdev,
rtnl_lock();
netif_device_detach(netdev);
+ if (netif_running(netdev))
+ ixgbevf_close_suspend(adapter);
+
if (state == pci_channel_io_perm_failure) {
rtnl_unlock();
return PCI_ERS_RESULT_DISCONNECT;
}
- if (netif_running(netdev))
- ixgbevf_close_suspend(adapter);
-
if (!test_and_set_bit(__IXGBEVF_DISABLED, &adapter->state))
pci_disable_device(pdev);
rtnl_unlock();
--
2.17.0
^ permalink raw reply related
* [net-next 1/6] ixgbe: cleanup sparse warnings
From: Jeff Kirsher @ 2018-05-17 16:37 UTC (permalink / raw)
To: davem; +Cc: Cathy Zhou, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180517163732.30910-1-jeffrey.t.kirsher@intel.com>
From: Cathy Zhou <cathy.zhou@oracle.com>
Sparse complains valid conversions between restricted types, force
attribute is used to avoid those warnings.
Signed-off-by: Cathy Zhou <cathy.zhou@oracle.com>
Reviewed-by: Shannon Nelson <shannon.nelson@oracle.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
.../net/ethernet/intel/ixgbe/ixgbe_82599.c | 13 +++++----
.../net/ethernet/intel/ixgbe/ixgbe_common.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 2 +-
.../net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 25 ++++++++++------
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 29 +++++++++++--------
.../net/ethernet/intel/ixgbe/ixgbe_model.h | 16 +++++-----
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 9 +++---
7 files changed, 55 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
index 42f63b943ea0..1e49716f52bc 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
@@ -1436,7 +1436,8 @@ void ixgbe_atr_compute_perfect_hash_82599(union ixgbe_atr_input *input,
{
u32 hi_hash_dword, lo_hash_dword, flow_vm_vlan;
- u32 bucket_hash = 0, hi_dword = 0;
+ u32 bucket_hash = 0;
+ __be32 hi_dword = 0;
int i;
/* Apply masks to input data */
@@ -1475,7 +1476,7 @@ void ixgbe_atr_compute_perfect_hash_82599(union ixgbe_atr_input *input,
* Limit hash to 13 bits since max bucket count is 8K.
* Store result at the end of the input stream.
*/
- input->formatted.bkt_hash = bucket_hash & 0x1FFF;
+ input->formatted.bkt_hash = (__force __be16)(bucket_hash & 0x1FFF);
}
/**
@@ -1584,7 +1585,7 @@ s32 ixgbe_fdir_set_input_mask_82599(struct ixgbe_hw *hw,
return IXGBE_ERR_CONFIG;
}
- switch (input_mask->formatted.flex_bytes & 0xFFFF) {
+ switch ((__force u16)input_mask->formatted.flex_bytes & 0xFFFF) {
case 0x0000:
/* Mask Flex Bytes */
fdirm |= IXGBE_FDIRM_FLEX;
@@ -1654,13 +1655,13 @@ s32 ixgbe_fdir_write_perfect_filter_82599(struct ixgbe_hw *hw,
IXGBE_WRITE_REG(hw, IXGBE_FDIRPORT, fdirport);
/* record vlan (little-endian) and flex_bytes(big-endian) */
- fdirvlan = IXGBE_STORE_AS_BE16(input->formatted.flex_bytes);
+ fdirvlan = IXGBE_STORE_AS_BE16((__force u16)input->formatted.flex_bytes);
fdirvlan <<= IXGBE_FDIRVLAN_FLEX_SHIFT;
fdirvlan |= ntohs(input->formatted.vlan_id);
IXGBE_WRITE_REG(hw, IXGBE_FDIRVLAN, fdirvlan);
/* configure FDIRHASH register */
- fdirhash = input->formatted.bkt_hash;
+ fdirhash = (__force u32)input->formatted.bkt_hash;
fdirhash |= soft_id << IXGBE_FDIRHASH_SIG_SW_INDEX_SHIFT;
IXGBE_WRITE_REG(hw, IXGBE_FDIRHASH, fdirhash);
@@ -1698,7 +1699,7 @@ s32 ixgbe_fdir_erase_perfect_filter_82599(struct ixgbe_hw *hw,
s32 err;
/* configure FDIRHASH register */
- fdirhash = input->formatted.bkt_hash;
+ fdirhash = (__force u32)input->formatted.bkt_hash;
fdirhash |= soft_id << IXGBE_FDIRHASH_SIG_SW_INDEX_SHIFT;
IXGBE_WRITE_REG(hw, IXGBE_FDIRHASH, fdirhash);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index 8d038837f72b..3f5c350716bb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -3626,7 +3626,7 @@ s32 ixgbe_hic_unlocked(struct ixgbe_hw *hw, u32 *buffer, u32 length,
*/
for (i = 0; i < dword_len; i++)
IXGBE_WRITE_REG_ARRAY(hw, IXGBE_FLEX_MNG,
- i, cpu_to_le32(buffer[i]));
+ i, (__force u32)cpu_to_le32(buffer[i]));
/* Setting this bit tells the ARC that a new command is pending. */
IXGBE_WRITE_REG(hw, IXGBE_HICR, hicr | IXGBE_HICR_C);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index b5219e024f70..94b3165ff543 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -440,7 +440,7 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter,
case cpu_to_le32(IXGBE_RXDADV_STAT_FCSTAT_FCPRSP):
dma_unmap_sg(&adapter->pdev->dev, ddp->sgl,
ddp->sgc, DMA_FROM_DEVICE);
- ddp->err = ddp_err;
+ ddp->err = (__force u32)ddp_err;
ddp->sgl = NULL;
ddp->sgc = 0;
/* fall through */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index 195c0b65eee2..99b170f1efd1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -19,8 +19,9 @@ static void ixgbe_ipsec_set_tx_sa(struct ixgbe_hw *hw, u16 idx,
int i;
for (i = 0; i < 4; i++)
- IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(i), cpu_to_be32(key[3 - i]));
- IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, cpu_to_be32(salt));
+ IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(i),
+ (__force u32)cpu_to_be32(key[3 - i]));
+ IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, (__force u32)cpu_to_be32(salt));
IXGBE_WRITE_FLUSH(hw);
reg = IXGBE_READ_REG(hw, IXGBE_IPSTXIDX);
@@ -69,7 +70,8 @@ static void ixgbe_ipsec_set_rx_sa(struct ixgbe_hw *hw, u16 idx, __be32 spi,
int i;
/* store the SPI (in bigendian) and IPidx */
- IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI, cpu_to_le32(spi));
+ IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI,
+ (__force u32)cpu_to_le32((__force u32)spi));
IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX, ip_idx);
IXGBE_WRITE_FLUSH(hw);
@@ -77,8 +79,9 @@ static void ixgbe_ipsec_set_rx_sa(struct ixgbe_hw *hw, u16 idx, __be32 spi,
/* store the key, salt, and mode */
for (i = 0; i < 4; i++)
- IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(i), cpu_to_be32(key[3 - i]));
- IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, cpu_to_be32(salt));
+ IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(i),
+ (__force u32)cpu_to_be32(key[3 - i]));
+ IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, (__force u32)cpu_to_be32(salt));
IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, mode);
IXGBE_WRITE_FLUSH(hw);
@@ -97,7 +100,8 @@ static void ixgbe_ipsec_set_rx_ip(struct ixgbe_hw *hw, u16 idx, __be32 addr[])
/* store the ip address */
for (i = 0; i < 4; i++)
- IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(i), cpu_to_le32(addr[i]));
+ IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(i),
+ (__force u32)cpu_to_le32((__force u32)addr[i]));
IXGBE_WRITE_FLUSH(hw);
ixgbe_ipsec_set_rx_item(hw, idx, ips_rx_ip_tbl);
@@ -367,7 +371,8 @@ static struct xfrm_state *ixgbe_ipsec_find_rx_state(struct ixgbe_ipsec *ipsec,
struct xfrm_state *ret = NULL;
rcu_read_lock();
- hash_for_each_possible_rcu(ipsec->rx_sa_list, rsa, hlist, spi)
+ hash_for_each_possible_rcu(ipsec->rx_sa_list, rsa, hlist,
+ (__force u32)spi) {
if (spi == rsa->xs->id.spi &&
((ip4 && *daddr == rsa->xs->id.daddr.a4) ||
(!ip4 && !memcmp(daddr, &rsa->xs->id.daddr.a6,
@@ -377,6 +382,7 @@ static struct xfrm_state *ixgbe_ipsec_find_rx_state(struct ixgbe_ipsec *ipsec,
xfrm_state_hold(ret);
break;
}
+ }
rcu_read_unlock();
return ret;
}
@@ -569,7 +575,7 @@ static int ixgbe_ipsec_add_sa(struct xfrm_state *xs)
/* hash the new entry for faster search in Rx path */
hash_add_rcu(ipsec->rx_sa_list, &ipsec->rx_tbl[sa_idx].hlist,
- rsa.xs->id.spi);
+ (__force u64)rsa.xs->id.spi);
} else {
struct tx_sa tsa;
@@ -653,7 +659,8 @@ static void ixgbe_ipsec_del_sa(struct xfrm_state *xs)
if (!ipsec->ip_tbl[ipi].ref_cnt) {
memset(&ipsec->ip_tbl[ipi], 0,
sizeof(struct rx_ip_sa));
- ixgbe_ipsec_set_rx_ip(hw, ipi, zerobuf);
+ ixgbe_ipsec_set_rx_ip(hw, ipi,
+ (__force __be32 *)zerobuf);
}
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 6652b201df5b..163b34a9572d 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -727,8 +727,8 @@ static void ixgbe_dump(struct ixgbe_adapter *adapter)
ring_desc = "";
pr_info("T [0x%03X] %016llX %016llX %016llX %08X %p %016llX %p%s",
i,
- le64_to_cpu(u0->a),
- le64_to_cpu(u0->b),
+ le64_to_cpu((__force __le64)u0->a),
+ le64_to_cpu((__force __le64)u0->b),
(u64)dma_unmap_addr(tx_buffer, dma),
dma_unmap_len(tx_buffer, len),
tx_buffer->next_to_watch,
@@ -839,15 +839,15 @@ static void ixgbe_dump(struct ixgbe_adapter *adapter)
/* Descriptor Done */
pr_info("RWB[0x%03X] %016llX %016llX ---------------- %p%s\n",
i,
- le64_to_cpu(u0->a),
- le64_to_cpu(u0->b),
+ le64_to_cpu((__force __le64)u0->a),
+ le64_to_cpu((__force __le64)u0->b),
rx_buffer_info->skb,
ring_desc);
} else {
pr_info("R [0x%03X] %016llX %016llX %016llX %p%s\n",
i,
- le64_to_cpu(u0->a),
- le64_to_cpu(u0->b),
+ le64_to_cpu((__force __le64)u0->a),
+ le64_to_cpu((__force __le64)u0->b),
(u64)rx_buffer_info->dma,
rx_buffer_info->skb,
ring_desc);
@@ -7751,7 +7751,7 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring,
/* remove payload length from inner checksum */
paylen = skb->len - l4_offset;
- csum_replace_by_diff(&l4.tcp->check, htonl(paylen));
+ csum_replace_by_diff(&l4.tcp->check, (__force __wsum)htonl(paylen));
/* update gso size and bytecount with header size */
first->gso_segs = skb_shinfo(skb)->gso_segs;
@@ -9104,7 +9104,8 @@ static int ixgbe_clsu32_build_input(struct ixgbe_fdir_filter *input,
for (j = 0; field_ptr[j].val; j++) {
if (field_ptr[j].off == off) {
- field_ptr[j].val(input, mask, val, m);
+ field_ptr[j].val(input, mask, (__force u32)val,
+ (__force u32)m);
input->filter.formatted.flow_type |=
field_ptr[j].type;
found_entry = true;
@@ -9113,8 +9114,10 @@ static int ixgbe_clsu32_build_input(struct ixgbe_fdir_filter *input,
}
if (nexthdr) {
if (nexthdr->off == cls->knode.sel->keys[i].off &&
- nexthdr->val == cls->knode.sel->keys[i].val &&
- nexthdr->mask == cls->knode.sel->keys[i].mask)
+ nexthdr->val ==
+ (__force u32)cls->knode.sel->keys[i].val &&
+ nexthdr->mask ==
+ (__force u32)cls->knode.sel->keys[i].mask)
found_jump_field = true;
else
continue;
@@ -9218,7 +9221,8 @@ static int ixgbe_configure_clsu32(struct ixgbe_adapter *adapter,
for (i = 0; nexthdr[i].jump; i++) {
if (nexthdr[i].o != cls->knode.sel->offoff ||
nexthdr[i].s != cls->knode.sel->offshift ||
- nexthdr[i].m != cls->knode.sel->offmask)
+ nexthdr[i].m !=
+ (__force u32)cls->knode.sel->offmask)
return err;
jump = kzalloc(sizeof(*jump), GFP_KERNEL);
@@ -9991,7 +9995,8 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
}
} else {
for (i = 0; i < adapter->num_rx_queues; i++)
- xchg(&adapter->rx_ring[i]->xdp_prog, adapter->xdp_prog);
+ (void)xchg(&adapter->rx_ring[i]->xdp_prog,
+ adapter->xdp_prog);
}
if (old_prog)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h
index fcae520647ce..1e6cf220f543 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_model.h
@@ -29,8 +29,8 @@ static inline int ixgbe_mat_prgm_sip(struct ixgbe_fdir_filter *input,
union ixgbe_atr_input *mask,
u32 val, u32 m)
{
- input->filter.formatted.src_ip[0] = val;
- mask->formatted.src_ip[0] = m;
+ input->filter.formatted.src_ip[0] = (__force __be32)val;
+ mask->formatted.src_ip[0] = (__force __be32)m;
return 0;
}
@@ -38,8 +38,8 @@ static inline int ixgbe_mat_prgm_dip(struct ixgbe_fdir_filter *input,
union ixgbe_atr_input *mask,
u32 val, u32 m)
{
- input->filter.formatted.dst_ip[0] = val;
- mask->formatted.dst_ip[0] = m;
+ input->filter.formatted.dst_ip[0] = (__force __be32)val;
+ mask->formatted.dst_ip[0] = (__force __be32)m;
return 0;
}
@@ -55,10 +55,10 @@ static inline int ixgbe_mat_prgm_ports(struct ixgbe_fdir_filter *input,
union ixgbe_atr_input *mask,
u32 val, u32 m)
{
- input->filter.formatted.src_port = val & 0xffff;
- mask->formatted.src_port = m & 0xffff;
- input->filter.formatted.dst_port = val >> 16;
- mask->formatted.dst_port = m >> 16;
+ input->filter.formatted.src_port = (__force __be16)(val & 0xffff);
+ mask->formatted.src_port = (__force __be16)(m & 0xffff);
+ input->filter.formatted.dst_port = (__force __be16)(val >> 16);
+ mask->formatted.dst_port = (__force __be16)(m >> 16);
return 0;
};
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index ac71ed76a54b..a8148c7126e5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -878,8 +878,9 @@ static s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw,
buffer.hdr.req.checksum = FW_DEFAULT_CHECKSUM;
/* convert offset from words to bytes */
- buffer.address = cpu_to_be32((offset + current_word) * 2);
- buffer.length = cpu_to_be16(words_to_read * 2);
+ buffer.address = (__force u32)cpu_to_be32((offset +
+ current_word) * 2);
+ buffer.length = (__force u16)cpu_to_be16(words_to_read * 2);
buffer.pad2 = 0;
buffer.pad3 = 0;
@@ -1089,9 +1090,9 @@ static s32 ixgbe_read_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, u16 *data)
buffer.hdr.req.checksum = FW_DEFAULT_CHECKSUM;
/* convert offset from words to bytes */
- buffer.address = cpu_to_be32(offset * 2);
+ buffer.address = (__force u32)cpu_to_be32(offset * 2);
/* one word */
- buffer.length = cpu_to_be16(sizeof(u16));
+ buffer.length = (__force u16)cpu_to_be16(sizeof(u16));
status = hw->mac.ops.acquire_swfw_sync(hw, mask);
if (status)
--
2.17.0
^ permalink raw reply related
* Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
From: Jiri Pirko @ 2018-05-17 16:37 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, netdev, davem, idosch, jakub.kicinski, mlxsw,
vivien.didelot, michael.chan, ganeshgr, saeedm, simon.horman,
pieter.jansenvanvuuren, john.hurley, dirk.vandermerwe,
alexander.h.duyck, ogerlitz, dsahern, vijaya.guvva,
satananda.burla, raghu.vatsavayi, felix.manlunas, gospo,
sathya.perla, vasundhara-v.volam, tariqt, eranbe,
jeffrey.t.kirsher
In-Reply-To: <20180517144828.GC23601@lunn.ch>
Thu, May 17, 2018 at 04:48:28PM CEST, andrew@lunn.ch wrote:
>> >Yes, modprobe dsa-loop-bdinfo first, which will create the
>>
>> That is compiled inside "fixed_phy", isn't it?
>
>Nope.
>
>It follows a pattern seen with I2C and SPI subsystem. A bus driver
>provides a bus to Linux. But i2c and SPI, unlike PCI or USB, you
>cannot enumerate the devices on the bus, you need to know what devices
>are there. So the board file registers an info structure, listing what
>devices are on the bus. When the bus pops into existence, the core
>links the bus to the info structure about devices on the bus and then
>probes the devices.
>
>The same is happening here. The fixed_phy driver provides an MDIO bus.
>
>The info structure in dsa-loop-bdinfo says there is an dsa-loop device
>at address 31 on that bus.
>
>Combine the two causes the dsa-loop device to actually probe.
I understand. Yet I have no dsa_loop_bdinfo.ko module. In my .config I
have:
CONFIG_FIXED_PHY=y
CONFIG_NET_DSA_LOOP=m
I had to do this:
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index 15c2a831edf1..2d773d3a7d49 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -2,9 +2,7 @@
obj-$(CONFIG_NET_DSA_BCM_SF2) += bcm-sf2.o
bcm-sf2-objs := bcm_sf2.o bcm_sf2_cfp.o
obj-$(CONFIG_NET_DSA_LOOP) += dsa_loop.o
-ifdef CONFIG_NET_DSA_LOOP
-obj-$(CONFIG_FIXED_PHY) += dsa_loop_bdinfo.o
-endif
+obj-$(CONFIG_NET_DSA_LOOP) += dsa_loop_bdinfo.o
obj-$(CONFIG_NET_DSA_MT7530) += mt7530.o
obj-$(CONFIG_NET_DSA_MV88E6060) += mv88e6060.o
obj-$(CONFIG_NET_DSA_QCA8K) += qca8k.o
Now dsa_loop_bdinfo.ko gets compiled. I have no clue why it does not
work without the patch :O "obj-$(CONFIG_FIXED_PHY)" doesn't work.
^ permalink raw reply related
* Re: [PATCH net-next 3/4] tcp: add SACK compression
From: Neal Cardwell @ 2018-05-17 16:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric Dumazet, David Miller, Netdev,
Toke Høiland-Jørgensen, Yuchung Cheng,
Soheil Hassas Yeganeh
In-Reply-To: <58bcf9c0-e4f0-691d-8d6a-40ff3629f500@gmail.com>
On Thu, May 17, 2018 at 11:40 AM Eric Dumazet <eric.dumazet@gmail.com>
wrote:
> On 05/17/2018 08:14 AM, Neal Cardwell wrote:
> > Is there a particular motivation for the cap of 127? IMHO 127 ACKs is
quite
> > a few to compress. Experience seems to show that it works well to have
one
> > GRO ACK for ~64KBytes that triggers a single TSO skb of ~64KBytes. It
might
> > be nice to try to match those dynamics in this SACK compression case,
so it
> > might be nice to cap the number of compressed ACKs at something like 44?
> > (0xffff / 1448 - 1). That way for high-speed paths we could try to keep
> > the ACK clock going with ACKs for ~64KBytes that trigger a single TSO
skb
> > of ~64KBytes, no matter whether we are sending SACKs or cumulative ACKs.
> 127 was chosen because the field is u8, and since skb allocation for the
ACK
> can fail, we could have cases were the field goes above 127.
> Ultimately, I believe a followup patch would add a sysctl, so that we can
fine-tune
> this, and eventually disable ACK compression if this sysctl is set to 0
OK, a sysctl sounds good. I would still vote for a default of 44. :-)
> >> + if (hrtimer_is_queued(&tp->compressed_ack_timer))
> >> + return;
> >> +
> >> + /* compress ack timer : 5 % of srtt, but no more than 2.5 ms */
> >> +
> >> + delay = min_t(unsigned long, 2500 * NSEC_PER_USEC,
> >> + tp->rcv_rtt_est.rtt_us * (NSEC_PER_USEC >>
3)/20);
> >
> > Any particular motivation for the 2.5ms here? It might be nice to match
the
> > existing TSO autosizing dynamics and use 1ms here instead of having a
> > separate new constant of 2.5ms. Smaller time scales here should lead to
> > less burstiness and queue pressure from data packets in the network,
and we
> > know from experience that the CPU overhead of 1ms chunks is acceptable.
> This came from my tests on wifi really :)
> I also had the idea to make this threshold adjustable for wifi, like we
did for sk_pacing_shift.
> (On wifi, we might want to increase the max delay between ACK)
> So maybe use 1ms delay, when sk_pacing_shift == 10, but increase it if
sk_pacing_shift has been lowered.
Sounds good to me.
Thanks for implementing this! Overall this patch seems nice to me.
Acked-by: Neal Cardwell <ncardwell@google.com>
BTW, I guess we should spread the word to maintainers of other major TCP
stacks that they need to be prepared for what may be a much higher degree
of compression/aggregation in the SACK stream. Linux stacks going back many
years should be fine with this, but I'm not sure about the other major OSes
(they may only allow sending one MSS per ACK-with-SACKs received).
neal
^ permalink raw reply
* Re: [PATCH v2 net] stmmac: strip vlan tag on reception only for 8021q tagged frames
From: David Miller @ 2018-05-17 16:43 UTC (permalink / raw)
To: eladv6; +Cc: makita.toshiaki, netdev, peppe.cavallaro, alexandre.torgue
In-Reply-To: <35b608aa-0e74-c375-b5e8-35ddc07e0f59@gmail.com>
From: Elad Nachman <eladv6@gmail.com>
Date: Fri, 11 May 2018 10:31:40 +0300
> stmmac reception handler calls stmmac_rx_vlan() to strip the vlan
> before calling napi_gro_receive().
>
> The function assumes VLAN tagged frames are always tagged with
> 802.1Q protocol, and assigns ETH_P_8021Q to the skb by hard-coding
> the parameter on call to __vlan_hwaccel_put_tag() without checking
> the actual VLAN tag.
>
> This causes packets not to be passed to the VLAN slave if it was
> created with 802.1AD protocol (ip link add link eth0 eth0.100 type
> vlan proto 802.1ad id 100).
>
> This fix only strips the VLAN tag for 802.1Q tagged
> protocols. 802.1AD frames will be handled later by skb_vlan_untag().
>
> Signed-off-by: Elad Nachman <eladn@gilat.com>
Giuseppe and Alexandre, please review this patch.
^ permalink raw reply
* Re: [PATCH net] net: test tailroom before appending to linear skb
From: Eric Dumazet @ 2018-05-17 16:44 UTC (permalink / raw)
To: Willem de Bruijn, netdev; +Cc: davem, Willem de Bruijn
In-Reply-To: <20180517155437.120414-1-willemdebruijn.kernel@gmail.com>
On 05/17/2018 08:54 AM, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Device features may change during transmission. In particular with
> corking, a device may toggle scatter-gather in between allocating
> and writing to an skb.
>
> Do not unconditionally assume that !NETIF_F_SG at write time implies
> that the same held at alloc time and thus the skb has sufficient
> tailroom.
>
> This issue predates git history.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> net/ipv4/ip_output.c | 3 ++-
> net/ipv6/ip6_output.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 83c73bab2c3d..c15204ec2eb0 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -1045,7 +1045,8 @@ static int __ip_append_data(struct sock *sk,
> if (copy > length)
> copy = length;
>
> - if (!(rt->dst.dev->features&NETIF_F_SG)) {
> + if (!(rt->dst.dev->features&NETIF_F_SG) &&
> + skb_tailroom(skb) > copy) {
On second thought, maybe use >= for the test ?
> unsigned int off;
>
> off = skb->len;
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 2e891d2c30ef..7b6d1689087b 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1503,7 +1503,8 @@ static int __ip6_append_data(struct sock *sk,
> if (copy > length)
> copy = length;
>
> - if (!(rt->dst.dev->features&NETIF_F_SG)) {
> + if (!(rt->dst.dev->features&NETIF_F_SG) &&
> + skb_tailroom(skb) >= copy) {
> unsigned int off;
>
> off = skb->len;
>
^ permalink raw reply
* Re: [PATCH net-next] erspan: set bso bit based on mirrored packet's len
From: William Tu @ 2018-05-17 16:57 UTC (permalink / raw)
To: Tobin C. Harding; +Cc: Linux Kernel Network Developers
In-Reply-To: <20180516222400.GB25268@eros>
On Wed, May 16, 2018 at 3:24 PM, Tobin C. Harding <tobin@apporbit.com> wrote:
> On Wed, May 16, 2018 at 07:05:34AM -0700, William Tu wrote:
>> On Mon, May 14, 2018 at 10:33 PM, Tobin C. Harding <tobin@apporbit.com> wrote:
>> > On Mon, May 14, 2018 at 04:54:36PM -0700, William Tu wrote:
>> >> Before the patch, the erspan BSO bit (Bad/Short/Oversized) is not
>> >> handled. BSO has 4 possible values:
>> >> 00 --> Good frame with no error, or unknown integrity
>> >> 11 --> Payload is a Bad Frame with CRC or Alignment Error
>> >> 01 --> Payload is a Short Frame
>> >> 10 --> Payload is an Oversized Frame
>> >>
>> >> Based the short/oversized definitions in RFC1757, the patch sets
>> >> the bso bit based on the mirrored packet's size.
>> >>
>> >> Reported-by: Xiaoyan Jin <xiaoyanj@vmware.com>
>> >> Signed-off-by: William Tu <u9012063@gmail.com>
>> >> ---
>> >> include/net/erspan.h | 25 +++++++++++++++++++++++++
>> >> 1 file changed, 25 insertions(+)
>> >>
>> >> diff --git a/include/net/erspan.h b/include/net/erspan.h
>> >> index d044aa60cc76..5eb95f78ad45 100644
>> >> --- a/include/net/erspan.h
>> >> +++ b/include/net/erspan.h
>> >> @@ -219,6 +219,30 @@ static inline __be32 erspan_get_timestamp(void)
>> >> return htonl((u32)h_usecs);
>> >> }
>> >>
>> >> +/* ERSPAN BSO (Bad/Short/Oversized)
>> >> + * 00b --> Good frame with no error, or unknown integrity
>> >> + * 01b --> Payload is a Short Frame
>> >> + * 10b --> Payload is an Oversized Frame
>> >> + * 11b --> Payload is a Bad Frame with CRC or Alignment Error
>> >> + */
>> >> +enum erspan_bso {
>> >> + BSO_NOERROR,
>> >> + BSO_SHORT,
>> >> + BSO_OVERSIZED,
>> >> + BSO_BAD,
>> >> +};
>> >
>> > If we are relying on the values perhaps this would be clearer
>> >
>> > BSO_NOERROR = 0x00,
>> > BSO_SHORT = 0x01,
>> > BSO_OVERSIZED = 0x02,
>> > BSO_BAD = 0x03,
>> >
>>
>> Yes, thanks. I will change in v2.
>>
>> >> +
>> >> +static inline u8 erspan_detect_bso(struct sk_buff *skb)
>> >> +{
>> >> + if (skb->len < ETH_ZLEN)
>> >> + return BSO_SHORT;
>> >> +
>> >> + if (skb->len > ETH_FRAME_LEN)
>> >> + return BSO_OVERSIZED;
>> >> +
>> >> + return BSO_NOERROR;
>> >> +}
>> >
>> > Without having much contextual knowledge around this patch; should we be
>> > doing some check on CRC or alignment (at some stage)? Having BSO_BAD
>> > seems to imply so?
>> >
>>
>> The definition of BSO_BAD:
>> etherStatsCRCAlignErrors OBJECT-TYPE
>> SYNTAX Counter
>> ACCESS read-only
>> STATUS mandatory
>> DESCRIPTION
>> "The total number of packets received that
>> had a length (excluding framing bits, but
>> including FCS octets) of between 64 and 1518
>> octets, inclusive, but but had either a bad
>> Frame Check Sequence (FCS) with an integral
>> number of octets (FCS Error) or a bad FCS with
>> a non-integral number of octets (Alignment Error)."
>>
>> But I don't know how to check CRC error at this code point.
>> Isn't it done by the NIC hardware?
>
> I'll just start with; I don't know anything about ERSPAN
>
> "ERSPAN is a Cisco proprietary feature and is available only to
> Catalyst 6500, 7600, Nexus, and ASR 1000 platforms to date. The
> ASR 1000 supports ERSPAN source (monitoring) only on Fast
> Ethernet, Gigabit Ethernet, and port-channel interfaces."
>
> https://supportforums.cisco.com/t5/network-infrastructure-documents/understanding-span-rspan-and-erspan/ta-p/3144951
>
> I dug around a bit and none of the files that currently import erspan.h
> actually use the 'bso' field
>
> $ grep bso $(git grep -l 'erspan\.h')
> include/net/erspan.h: u8 bso = 0; /* Bad/Short/Oversized */
> include/net/erspan.h: ershdr->en = bso;
> net/ipv4/ip_gre.c: ICMP in the real Internet is absolutely infeasible.
> net/ipv4/ip_gre.c: * ICMP in the real Internet is absolutely infeasible.
>
Yes, that's expected.
>
> Normally, AFAICT, the FCS does not get passed to the operating system
> since its a link layer mechanism. If ERSPAN is passing the FCS when it
> mirrors frames (does it mirror frames or packets, I don't know?) then
> surely ERSPAN should provide a function to return the BSO value.
It mirrors layer 2 ethernet frame, so no FCS is passing.
>
> So IMHO this patch seems like a just pretense and not really doing
> anything.
>
The purpose is to set the BSO bit according to the spec, so that
ERSPAN monitor can interpret the mirrored traffic.
Thanks,
William
^ permalink raw reply
* Re: [PATCH net-next 3/4] tcp: add SACK compression
From: Yuchung Cheng @ 2018-05-17 16:59 UTC (permalink / raw)
To: Neal Cardwell
Cc: Eric Dumazet, Eric Dumazet, David Miller, Netdev,
Toke Høiland-Jørgensen, Soheil Hassas Yeganeh,
Christoph Paasch
In-Reply-To: <CADVnQymTZ-dfU65kJYAVrb4eEWkou_+bd8ZOCxE9sPUn=ZCtxg@mail.gmail.com>
On Thu, May 17, 2018 at 9:41 AM, Neal Cardwell <ncardwell@google.com> wrote:
>
> On Thu, May 17, 2018 at 11:40 AM Eric Dumazet <eric.dumazet@gmail.com>
> wrote:
> > On 05/17/2018 08:14 AM, Neal Cardwell wrote:
> > > Is there a particular motivation for the cap of 127? IMHO 127 ACKs is
> quite
> > > a few to compress. Experience seems to show that it works well to have
> one
> > > GRO ACK for ~64KBytes that triggers a single TSO skb of ~64KBytes. It
> might
> > > be nice to try to match those dynamics in this SACK compression case,
> so it
> > > might be nice to cap the number of compressed ACKs at something like 44?
> > > (0xffff / 1448 - 1). That way for high-speed paths we could try to keep
> > > the ACK clock going with ACKs for ~64KBytes that trigger a single TSO
> skb
> > > of ~64KBytes, no matter whether we are sending SACKs or cumulative ACKs.
>
> > 127 was chosen because the field is u8, and since skb allocation for the
> ACK
> > can fail, we could have cases were the field goes above 127.
>
> > Ultimately, I believe a followup patch would add a sysctl, so that we can
> fine-tune
> > this, and eventually disable ACK compression if this sysctl is set to 0
>
> OK, a sysctl sounds good. I would still vote for a default of 44. :-)
>
>
> > >> + if (hrtimer_is_queued(&tp->compressed_ack_timer))
> > >> + return;
> > >> +
> > >> + /* compress ack timer : 5 % of srtt, but no more than 2.5 ms */
> > >> +
> > >> + delay = min_t(unsigned long, 2500 * NSEC_PER_USEC,
> > >> + tp->rcv_rtt_est.rtt_us * (NSEC_PER_USEC >>
> 3)/20);
> > >
> > > Any particular motivation for the 2.5ms here? It might be nice to match
> the
> > > existing TSO autosizing dynamics and use 1ms here instead of having a
> > > separate new constant of 2.5ms. Smaller time scales here should lead to
> > > less burstiness and queue pressure from data packets in the network,
> and we
> > > know from experience that the CPU overhead of 1ms chunks is acceptable.
>
> > This came from my tests on wifi really :)
>
> > I also had the idea to make this threshold adjustable for wifi, like we
> did for sk_pacing_shift.
>
> > (On wifi, we might want to increase the max delay between ACK)
>
> > So maybe use 1ms delay, when sk_pacing_shift == 10, but increase it if
> sk_pacing_shift has been lowered.
>
> Sounds good to me.
>
> Thanks for implementing this! Overall this patch seems nice to me.
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
>
> BTW, I guess we should spread the word to maintainers of other major TCP
> stacks that they need to be prepared for what may be a much higher degree
> of compression/aggregation in the SACK stream. Linux stacks going back many
> years should be fine with this, but I'm not sure about the other major OSes
> (they may only allow sending one MSS per ACK-with-SACKs received).
Patch looks really good but Neal's comment just reminds me a potential
legacy issue.
I recall at least Apple and Windows TCP stacks still need 3+ DUPACKs
(!= a SACK covering 3+ packets) to trigger fast recovery. Will we have
an issue there interacting w/ these stacks?
>
> neal
^ permalink raw reply
* Re: [PATCH v3 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT
From: Y Song @ 2018-05-17 17:02 UTC (permalink / raw)
To: Sean Young
Cc: linux-media, linux-kernel, Alexei Starovoitov,
Mauro Carvalho Chehab, Daniel Borkmann, netdev, Matthias Reichl,
Devin Heitmueller
In-Reply-To: <92785c791057185fa5691f78cecfa4beae7fc336.1526504511.git.sean@mess.org>
On Wed, May 16, 2018 at 2:04 PM, Sean Young <sean@mess.org> wrote:
> Add support for BPF_PROG_RAWIR_EVENT. This type of BPF program can call
> rc_keydown() to reported decoded IR scancodes, or rc_repeat() to report
> that the last key should be repeated.
>
> The bpf program can be attached to using the bpf(BPF_PROG_ATTACH) syscall;
> the target_fd must be the /dev/lircN device.
>
> Signed-off-by: Sean Young <sean@mess.org>
> ---
> drivers/media/rc/Kconfig | 13 ++
> drivers/media/rc/Makefile | 1 +
> drivers/media/rc/bpf-rawir-event.c | 363 +++++++++++++++++++++++++++++
> drivers/media/rc/lirc_dev.c | 24 ++
> drivers/media/rc/rc-core-priv.h | 24 ++
> drivers/media/rc/rc-ir-raw.c | 14 +-
> include/linux/bpf_rcdev.h | 30 +++
> include/linux/bpf_types.h | 3 +
> include/uapi/linux/bpf.h | 55 ++++-
> kernel/bpf/syscall.c | 7 +
> 10 files changed, 531 insertions(+), 3 deletions(-)
> create mode 100644 drivers/media/rc/bpf-rawir-event.c
> create mode 100644 include/linux/bpf_rcdev.h
>
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index eb2c3b6eca7f..2172d65b0213 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -25,6 +25,19 @@ config LIRC
> passes raw IR to and from userspace, which is needed for
> IR transmitting (aka "blasting") and for the lirc daemon.
>
> +config BPF_RAWIR_EVENT
> + bool "Support for eBPF programs attached to lirc devices"
> + depends on BPF_SYSCALL
> + depends on RC_CORE=y
> + depends on LIRC
> + help
> + Allow attaching eBPF programs to a lirc device using the bpf(2)
> + syscall command BPF_PROG_ATTACH. This is supported for raw IR
> + receivers.
> +
> + These eBPF programs can be used to decode IR into scancodes, for
> + IR protocols not supported by the kernel decoders.
> +
> menuconfig RC_DECODERS
> bool "Remote controller decoders"
> depends on RC_CORE
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index 2e1c87066f6c..74907823bef8 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -5,6 +5,7 @@ obj-y += keymaps/
> obj-$(CONFIG_RC_CORE) += rc-core.o
> rc-core-y := rc-main.o rc-ir-raw.o
> rc-core-$(CONFIG_LIRC) += lirc_dev.o
> +rc-core-$(CONFIG_BPF_RAWIR_EVENT) += bpf-rawir-event.o
> obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o
> obj-$(CONFIG_IR_RC5_DECODER) += ir-rc5-decoder.o
> obj-$(CONFIG_IR_RC6_DECODER) += ir-rc6-decoder.o
> diff --git a/drivers/media/rc/bpf-rawir-event.c b/drivers/media/rc/bpf-rawir-event.c
> new file mode 100644
> index 000000000000..7cb48b8d87b5
> --- /dev/null
> +++ b/drivers/media/rc/bpf-rawir-event.c
> @@ -0,0 +1,363 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// bpf-rawir-event.c - handles bpf
> +//
> +// Copyright (C) 2018 Sean Young <sean@mess.org>
> +
> +#include <linux/bpf.h>
> +#include <linux/filter.h>
> +#include <linux/bpf_rcdev.h>
> +#include "rc-core-priv.h"
> +
> +/*
> + * BPF interface for raw IR
> + */
> +const struct bpf_prog_ops rawir_event_prog_ops = {
> +};
> +
> +BPF_CALL_1(bpf_rc_repeat, struct bpf_rawir_event*, event)
> +{
> + struct ir_raw_event_ctrl *ctrl;
> +
> + ctrl = container_of(event, struct ir_raw_event_ctrl, bpf_rawir_event);
> +
> + rc_repeat(ctrl->dev);
> +
> + return 0;
> +}
> +
> +static const struct bpf_func_proto rc_repeat_proto = {
> + .func = bpf_rc_repeat,
> + .gpl_only = true, /* rc_repeat is EXPORT_SYMBOL_GPL */
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> +};
> +
> +BPF_CALL_4(bpf_rc_keydown, struct bpf_rawir_event*, event, u32, protocol,
> + u32, scancode, u32, toggle)
> +{
> + struct ir_raw_event_ctrl *ctrl;
> +
> + ctrl = container_of(event, struct ir_raw_event_ctrl, bpf_rawir_event);
> +
> + rc_keydown(ctrl->dev, protocol, scancode, toggle != 0);
> +
> + return 0;
> +}
> +
> +static const struct bpf_func_proto rc_keydown_proto = {
> + .func = bpf_rc_keydown,
> + .gpl_only = true, /* rc_keydown is EXPORT_SYMBOL_GPL */
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_ANYTHING,
> + .arg3_type = ARG_ANYTHING,
> + .arg4_type = ARG_ANYTHING,
> +};
> +
> +static const struct bpf_func_proto *
> +rawir_event_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> +{
> + switch (func_id) {
> + case BPF_FUNC_rc_repeat:
> + return &rc_repeat_proto;
> + case BPF_FUNC_rc_keydown:
> + return &rc_keydown_proto;
> + case BPF_FUNC_map_lookup_elem:
> + return &bpf_map_lookup_elem_proto;
> + case BPF_FUNC_map_update_elem:
> + return &bpf_map_update_elem_proto;
> + case BPF_FUNC_map_delete_elem:
> + return &bpf_map_delete_elem_proto;
> + case BPF_FUNC_ktime_get_ns:
> + return &bpf_ktime_get_ns_proto;
> + case BPF_FUNC_tail_call:
> + return &bpf_tail_call_proto;
> + case BPF_FUNC_get_prandom_u32:
> + return &bpf_get_prandom_u32_proto;
> + case BPF_FUNC_trace_printk:
> + if (capable(CAP_SYS_ADMIN))
> + return bpf_get_trace_printk_proto();
> + /* fall through */
> + default:
> + return NULL;
> + }
> +}
> +
> +static bool rawir_event_is_valid_access(int off, int size,
> + enum bpf_access_type type,
> + const struct bpf_prog *prog,
> + struct bpf_insn_access_aux *info)
> +{
> + /* struct bpf_rawir_event has two u32 fields */
> + if (type == BPF_WRITE)
> + return false;
> +
> + if (size != sizeof(__u32))
> + return false;
> +
> + if (!(off == offsetof(struct bpf_rawir_event, duration) ||
> + off == offsetof(struct bpf_rawir_event, type)))
> + return false;
> +
> + return true;
> +}
> +
> +const struct bpf_verifier_ops rawir_event_verifier_ops = {
> + .get_func_proto = rawir_event_func_proto,
> + .is_valid_access = rawir_event_is_valid_access
> +};
> +
> +#define BPF_MAX_PROGS 64
> +
> +static int rc_dev_bpf_attach(struct rc_dev *rcdev, struct bpf_prog *prog)
> +{
> + struct ir_raw_event_ctrl *raw;
> + struct bpf_prog_list *pl;
> + int ret, size;
> +
> + if (rcdev->driver_type != RC_DRIVER_IR_RAW)
> + return -EINVAL;
> +
> + ret = mutex_lock_interruptible(&ir_raw_handler_lock);
> + if (ret)
> + return ret;
> +
> + raw = rcdev->raw;
> + if (!raw) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + size = 0;
> + list_for_each_entry(pl, &raw->progs, node) {
> + if (pl->prog == prog) {
> + ret = -EEXIST;
> + goto out;
> + }
> +
> + size++;
> + }
> +
> + if (size >= BPF_MAX_PROGS) {
> + ret = -E2BIG;
> + goto out;
> + }
> +
> + pl = kmalloc(sizeof(*pl), GFP_KERNEL);
> + if (!pl) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + pl->prog = prog;
> + list_add(&pl->node, &raw->progs);
> +out:
> + mutex_unlock(&ir_raw_handler_lock);
> + return ret;
> +}
> +
> +static int rc_dev_bpf_detach(struct rc_dev *rcdev, struct bpf_prog *prog)
> +{
> + struct ir_raw_event_ctrl *raw;
> + struct bpf_prog_list *pl, *tmp;
> + int ret;
> +
> + if (rcdev->driver_type != RC_DRIVER_IR_RAW)
> + return -EINVAL;
> +
> + ret = mutex_lock_interruptible(&ir_raw_handler_lock);
> + if (ret)
> + return ret;
> +
> + raw = rcdev->raw;
> + if (!raw) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + ret = -ENOENT;
> +
> + list_for_each_entry_safe(pl, tmp, &raw->progs, node) {
> + if (pl->prog == prog) {
> + list_del(&pl->node);
> + kfree(pl);
> + bpf_prog_put(prog);
> + ret = 0;
> + goto out;
> + }
> + }
> +out:
> + mutex_unlock(&ir_raw_handler_lock);
> + return ret;
> +}
> +
> +void rc_dev_bpf_init(struct rc_dev *rcdev)
> +{
> + INIT_LIST_HEAD(&rcdev->raw->progs);
> +}
> +
> +void rc_dev_bpf_run(struct rc_dev *rcdev, struct ir_raw_event ev)
> +{
> + struct ir_raw_event_ctrl *raw = rcdev->raw;
> + struct bpf_prog_list *pl;
> +
> + if (list_empty(&raw->progs))
> + return;
> +
> + if (unlikely(ev.carrier_report)) {
> + raw->bpf_rawir_event.carrier = ev.carrier;
> + raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_CARRIER;
> + } else {
> + raw->bpf_rawir_event.duration = ev.duration;
> +
> + if (ev.pulse)
> + raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_PULSE;
> + else if (ev.timeout)
> + raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_TIMEOUT;
> + else if (ev.reset)
> + raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_RESET;
> + else
> + raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_SPACE;
> + }
> +
> + list_for_each_entry(pl, &raw->progs, node)
> + BPF_PROG_RUN(pl->prog, &raw->bpf_rawir_event);
Is the raw->progs protected by locks? It is possible that attaching/detaching
could manipulate raw->progs at the same time? In perf/cgroup prog array
case, the prog array run is protected by rcu and the dummy prog idea is
to avoid the prog is skipped by reshuffling.
Also, the original idea about using prog array is the least overhead since you
want to BPF programs to execute as soon as possible.
> +}
> +
> +void rc_dev_bpf_free(struct rc_dev *rcdev)
> +{
> + struct bpf_prog_list *pl, *tmp;
> +
> + list_for_each_entry_safe(pl, tmp, &rcdev->raw->progs, node) {
> + list_del(&pl->node);
> + bpf_prog_put(pl->prog);
> + kfree(pl);
> + }
> +}
> +
> +int rc_dev_prog_attach(const union bpf_attr *attr)
> +{
> + struct bpf_prog *prog;
> + struct rc_dev *rcdev;
> + int ret;
> +
> + if (attr->attach_flags)
> + return -EINVAL;
> +
> + prog = bpf_prog_get_type(attr->attach_bpf_fd,
> + BPF_PROG_TYPE_RAWIR_EVENT);
> + if (IS_ERR(prog))
> + return PTR_ERR(prog);
> +
> + rcdev = rc_dev_get_from_fd(attr->target_fd);
> + if (IS_ERR(rcdev)) {
> + bpf_prog_put(prog);
> + return PTR_ERR(rcdev);
> + }
> +
> + ret = rc_dev_bpf_attach(rcdev, prog);
> + if (ret)
> + bpf_prog_put(prog);
> +
> + put_device(&rcdev->dev);
> +
> + return ret;
> +}
> +
> +int rc_dev_prog_detach(const union bpf_attr *attr)
> +{
> + struct bpf_prog *prog;
> + struct rc_dev *rcdev;
> + int ret;
> +
> + if (attr->attach_flags)
> + return -EINVAL;
> +
> + prog = bpf_prog_get_type(attr->attach_bpf_fd,
> + BPF_PROG_TYPE_RAWIR_EVENT);
> + if (IS_ERR(prog))
> + return PTR_ERR(prog);
> +
> + rcdev = rc_dev_get_from_fd(attr->target_fd);
> + if (IS_ERR(rcdev)) {
> + bpf_prog_put(prog);
> + return PTR_ERR(rcdev);
> + }
> +
> + ret = rc_dev_bpf_detach(rcdev, prog);
> +
> + bpf_prog_put(prog);
> + put_device(&rcdev->dev);
> +
> + return ret;
> +}
> +
> +int rc_dev_prog_query(const union bpf_attr *attr, union bpf_attr __user *uattr)
> +{
> + __u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
> + struct ir_raw_event_ctrl *raw;
> + struct bpf_prog_list *pl;
> + struct rc_dev *rcdev;
> + u32 cnt, flags = 0, *ids, i;
> + int ret;
> +
> + if (attr->query.query_flags)
> + return -EINVAL;
> +
> + rcdev = rc_dev_get_from_fd(attr->query.target_fd);
> + if (IS_ERR(rcdev))
> + return PTR_ERR(rcdev);
> +
> + if (rcdev->driver_type != RC_DRIVER_IR_RAW) {
> + ret = -EINVAL;
> + goto out;
mutex_lock_interruptible() has not been called. You can just return here.
> + }
> +
> + ret = mutex_lock_interruptible(&ir_raw_handler_lock);
> + if (ret)
> + goto out;
Maybe you can rename label "out" to "unlock" since it
is really an unlock and out?
> +
> + raw = rcdev->raw;
> + if (!raw) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + cnt = 0;
> + list_for_each_entry(pl, &raw->progs, node)
> + cnt++;
> +
> + if (copy_to_user(&uattr->query.prog_cnt, &cnt, sizeof(cnt))) {
> + ret = -EFAULT;
> + goto out;
> + }
> + if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags))) {
> + ret = -EFAULT;
> + goto out;
> + }
> +
> + if (attr->query.prog_cnt != 0 && prog_ids && cnt) {
> + if (attr->query.prog_cnt < cnt)
> + cnt = attr->query.prog_cnt;
> +
> + ids = kmalloc_array(cnt, sizeof(u32), GFP_KERNEL);
> + if (!ids) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + i = 0;
> + list_for_each_entry(pl, &raw->progs, node) {
> + ids[i++] = pl->prog->aux->id;
> + if (i == cnt)
> + break;
> + }
> +
> + ret = copy_to_user(prog_ids, ids, cnt * sizeof(u32));
Do you want to give user a chance to know that the "cnt" is not big enough
by return -ENOSPC if cnt is smaller than the number of progs in the array?
> + }
> +out:
> + mutex_unlock(&ir_raw_handler_lock);
> + put_device(&rcdev->dev);
> +
> + return ret;
> +}
> diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
> index 24e9fbb80e81..193540ded626 100644
> --- a/drivers/media/rc/lirc_dev.c
> +++ b/drivers/media/rc/lirc_dev.c
> @@ -20,6 +20,7 @@
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/device.h>
> +#include <linux/file.h>
> #include <linux/idr.h>
> #include <linux/poll.h>
> #include <linux/sched.h>
> @@ -816,4 +817,27 @@ void __exit lirc_dev_exit(void)
> unregister_chrdev_region(lirc_base_dev, RC_DEV_MAX);
> }
>
> +struct rc_dev *rc_dev_get_from_fd(int fd)
> +{
> + struct fd f = fdget(fd);
> + struct lirc_fh *fh;
> + struct rc_dev *dev;
> +
> + if (!f.file)
> + return ERR_PTR(-EBADF);
> +
> + if (f.file->f_op != &lirc_fops) {
> + fdput(f);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + fh = f.file->private_data;
> + dev = fh->rc;
> +
> + get_device(&dev->dev);
> + fdput(f);
> +
> + return dev;
> +}
> +
> MODULE_ALIAS("lirc_dev");
> diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
> index e0e6a17460f6..148db73cfa0c 100644
> --- a/drivers/media/rc/rc-core-priv.h
> +++ b/drivers/media/rc/rc-core-priv.h
> @@ -13,6 +13,7 @@
> #define MAX_IR_EVENT_SIZE 512
>
> #include <linux/slab.h>
> +#include <uapi/linux/bpf.h>
> #include <media/rc-core.h>
>
> /**
> @@ -57,6 +58,11 @@ struct ir_raw_event_ctrl {
> /* raw decoder state follows */
> struct ir_raw_event prev_ev;
> struct ir_raw_event this_ev;
> +
> +#ifdef CONFIG_BPF_RAWIR_EVENT
> + struct bpf_rawir_event bpf_rawir_event;
> + struct list_head progs;
> +#endif
> struct nec_dec {
> int state;
> unsigned count;
> @@ -126,6 +132,9 @@ struct ir_raw_event_ctrl {
> } imon;
> };
>
> +/* Mutex for locking raw IR processing and handler change */
> +extern struct mutex ir_raw_handler_lock;
> +
> /* macros for IR decoders */
> static inline bool geq_margin(unsigned d1, unsigned d2, unsigned margin)
> {
> @@ -288,6 +297,7 @@ void ir_lirc_raw_event(struct rc_dev *dev, struct ir_raw_event ev);
> void ir_lirc_scancode_event(struct rc_dev *dev, struct lirc_scancode *lsc);
> int ir_lirc_register(struct rc_dev *dev);
> void ir_lirc_unregister(struct rc_dev *dev);
> +struct rc_dev *rc_dev_get_from_fd(int fd);
> #else
> static inline int lirc_dev_init(void) { return 0; }
> static inline void lirc_dev_exit(void) {}
> @@ -299,4 +309,18 @@ static inline int ir_lirc_register(struct rc_dev *dev) { return 0; }
> static inline void ir_lirc_unregister(struct rc_dev *dev) { }
> #endif
>
> +/*
> + * bpf interface
> + */
> +#ifdef CONFIG_BPF_RAWIR_EVENT
> +void rc_dev_bpf_init(struct rc_dev *dev);
> +void rc_dev_bpf_free(struct rc_dev *dev);
> +void rc_dev_bpf_run(struct rc_dev *dev, struct ir_raw_event ev);
> +#else
> +static inline void rc_dev_bpf_init(struct rc_dev *dev) { }
> +static inline void rc_dev_bpf_free(struct rc_dev *dev) { }
> +static inline void rc_dev_bpf_run(struct rc_dev *dev, struct ir_raw_event ev)
> +{ }
> +#endif
> +
> #endif /* _RC_CORE_PRIV */
> diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
> index 374f83105a23..e68cdd4fbf5d 100644
> --- a/drivers/media/rc/rc-ir-raw.c
> +++ b/drivers/media/rc/rc-ir-raw.c
> @@ -14,7 +14,7 @@
> static LIST_HEAD(ir_raw_client_list);
>
> /* Used to handle IR raw handler extensions */
> -static DEFINE_MUTEX(ir_raw_handler_lock);
> +DEFINE_MUTEX(ir_raw_handler_lock);
> static LIST_HEAD(ir_raw_handler_list);
> static atomic64_t available_protocols = ATOMIC64_INIT(0);
>
> @@ -32,6 +32,7 @@ static int ir_raw_event_thread(void *data)
> handler->protocols || !handler->protocols)
> handler->decode(raw->dev, ev);
> ir_lirc_raw_event(raw->dev, ev);
> + rc_dev_bpf_run(raw->dev, ev);
> raw->prev_ev = ev;
> }
> mutex_unlock(&ir_raw_handler_lock);
> @@ -572,6 +573,7 @@ int ir_raw_event_prepare(struct rc_dev *dev)
> spin_lock_init(&dev->raw->edge_spinlock);
> timer_setup(&dev->raw->edge_handle, ir_raw_edge_handle, 0);
> INIT_KFIFO(dev->raw->kfifo);
> + rc_dev_bpf_init(dev);
>
> return 0;
> }
> @@ -621,9 +623,17 @@ void ir_raw_event_unregister(struct rc_dev *dev)
> list_for_each_entry(handler, &ir_raw_handler_list, list)
> if (handler->raw_unregister)
> handler->raw_unregister(dev);
> - mutex_unlock(&ir_raw_handler_lock);
> +
> + rc_dev_bpf_free(dev);
>
> ir_raw_event_free(dev);
> +
> + /*
> + * A user can be calling bpf(BPF_PROG_{QUERY|ATTACH|DETACH}), so
> + * ensure that the raw member is null on unlock; this is how
> + * "device gone" is checked.
> + */
> + mutex_unlock(&ir_raw_handler_lock);
> }
>
> /*
> diff --git a/include/linux/bpf_rcdev.h b/include/linux/bpf_rcdev.h
> new file mode 100644
> index 000000000000..17a30f30436a
> --- /dev/null
> +++ b/include/linux/bpf_rcdev.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _BPF_RCDEV_H
> +#define _BPF_RCDEV_H
> +
> +#include <linux/bpf.h>
> +#include <uapi/linux/bpf.h>
> +
> +#ifdef CONFIG_BPF_RAWIR_EVENT
> +int rc_dev_prog_attach(const union bpf_attr *attr);
> +int rc_dev_prog_detach(const union bpf_attr *attr);
> +int rc_dev_prog_query(const union bpf_attr *attr, union bpf_attr __user *uattr);
> +#else
> +static inline int rc_dev_prog_attach(const union bpf_attr *attr)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int rc_dev_prog_detach(const union bpf_attr *attr)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int rc_dev_prog_query(const union bpf_attr *attr,
> + union bpf_attr __user *uattr)
> +{
> + return -EINVAL;
> +}
> +#endif
> +
> +#endif /* _BPF_RCDEV_H */
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index b67f8793de0d..e2b1b12474d4 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -25,6 +25,9 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_RAW_TRACEPOINT, raw_tracepoint)
> #ifdef CONFIG_CGROUP_BPF
> BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_DEVICE, cg_dev)
> #endif
> +#ifdef CONFIG_BPF_RAWIR_EVENT
> +BPF_PROG_TYPE(BPF_PROG_TYPE_RAWIR_EVENT, rawir_event)
> +#endif
>
> BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
> BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index d94d333a8225..243e141e8a5b 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -141,6 +141,7 @@ enum bpf_prog_type {
> BPF_PROG_TYPE_SK_MSG,
> BPF_PROG_TYPE_RAW_TRACEPOINT,
> BPF_PROG_TYPE_CGROUP_SOCK_ADDR,
> + BPF_PROG_TYPE_RAWIR_EVENT,
> };
>
> enum bpf_attach_type {
> @@ -158,6 +159,7 @@ enum bpf_attach_type {
> BPF_CGROUP_INET6_CONNECT,
> BPF_CGROUP_INET4_POST_BIND,
> BPF_CGROUP_INET6_POST_BIND,
> + BPF_RAWIR_EVENT,
> __MAX_BPF_ATTACH_TYPE
> };
>
> @@ -1902,6 +1904,35 @@ union bpf_attr {
> * egress otherwise). This is the only flag supported for now.
> * Return
> * **SK_PASS** on success, or **SK_DROP** on error.
> + *
> + * int bpf_rc_keydown(void *ctx, u32 protocol, u32 scancode, u32 toggle)
> + * Description
> + * Report decoded scancode with toggle value. For use in
> + * BPF_PROG_TYPE_RAWIR_EVENT, to report a successfully
> + * decoded scancode. This is will generate a keydown event,
> + * and a keyup event once the scancode is no longer repeated.
> + *
> + * *ctx* pointer to bpf_rawir_event, *protocol* is decoded
> + * protocol (see RC_PROTO_* enum).
> + *
> + * Some protocols include a toggle bit, in case the button
> + * was released and pressed again between consecutive scancodes,
> + * copy this bit into *toggle* if it exists, else set to 0.
> + *
> + * Return
> + * Always return 0 (for now)
> + *
> + * int bpf_rc_repeat(void *ctx)
> + * Description
> + * Repeat the last decoded scancode; some IR protocols like
> + * NEC have a special IR message for repeat last button,
> + * in case user is holding a button down; the scancode is
> + * not repeated.
> + *
> + * *ctx* pointer to bpf_rawir_event.
> + *
> + * Return
> + * Always return 0 (for now)
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -1976,7 +2007,9 @@ union bpf_attr {
> FN(fib_lookup), \
> FN(sock_hash_update), \
> FN(msg_redirect_hash), \
> - FN(sk_redirect_hash),
> + FN(sk_redirect_hash), \
> + FN(rc_repeat), \
> + FN(rc_keydown),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> @@ -2043,6 +2076,26 @@ enum bpf_hdr_start_off {
> BPF_HDR_START_NET,
> };
>
> +/*
> + * user accessible mirror of in-kernel ir_raw_event
> + */
> +#define BPF_RAWIR_EVENT_SPACE 0
> +#define BPF_RAWIR_EVENT_PULSE 1
> +#define BPF_RAWIR_EVENT_TIMEOUT 2
> +#define BPF_RAWIR_EVENT_RESET 3
> +#define BPF_RAWIR_EVENT_CARRIER 4
> +#define BPF_RAWIR_EVENT_DUTY_CYCLE 5
> +
> +struct bpf_rawir_event {
> + union {
> + __u32 duration;
> + __u32 carrier;
> + __u32 duty_cycle;
> + };
> +
> + __u32 type;
> +};
> +
> /* user accessible mirror of in-kernel sk_buff.
> * new fields can only be added to the end of this structure
> */
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index e2aeb5e89f44..75c089f407c8 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -11,6 +11,7 @@
> */
> #include <linux/bpf.h>
> #include <linux/bpf_trace.h>
> +#include <linux/bpf_rcdev.h>
> #include <linux/btf.h>
> #include <linux/syscalls.h>
> #include <linux/slab.h>
> @@ -1567,6 +1568,8 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> case BPF_SK_SKB_STREAM_PARSER:
> case BPF_SK_SKB_STREAM_VERDICT:
> return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_SKB, true);
> + case BPF_RAWIR_EVENT:
> + return rc_dev_prog_attach(attr);
> default:
> return -EINVAL;
> }
> @@ -1637,6 +1640,8 @@ static int bpf_prog_detach(const union bpf_attr *attr)
> case BPF_SK_SKB_STREAM_PARSER:
> case BPF_SK_SKB_STREAM_VERDICT:
> return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_SKB, false);
> + case BPF_RAWIR_EVENT:
> + return rc_dev_prog_detach(attr);
> default:
> return -EINVAL;
> }
> @@ -1684,6 +1689,8 @@ static int bpf_prog_query(const union bpf_attr *attr,
> case BPF_CGROUP_SOCK_OPS:
> case BPF_CGROUP_DEVICE:
> break;
> + case BPF_RAWIR_EVENT:
> + return rc_dev_prog_query(attr, uattr);
> default:
> return -EINVAL;
> }
> --
> 2.17.0
>
^ permalink raw reply
* Re: [PATCH net] net: test tailroom before appending to linear skb
From: Willem de Bruijn @ 2018-05-17 17:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Network Development, David Miller, Willem de Bruijn
In-Reply-To: <69013f90-5b09-06e9-b0e4-2869bce6515c@gmail.com>
On Thu, May 17, 2018 at 12:44 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 05/17/2018 08:54 AM, Willem de Bruijn wrote:
>> From: Willem de Bruijn <willemb@google.com>
>>
>> Device features may change during transmission. In particular with
>> corking, a device may toggle scatter-gather in between allocating
>> and writing to an skb.
>>
>> Do not unconditionally assume that !NETIF_F_SG at write time implies
>> that the same held at alloc time and thus the skb has sufficient
>> tailroom.
>>
>> This issue predates git history.
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Reported-by: Eric Dumazet <edumazet@google.com>
>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>> ---
>> net/ipv4/ip_output.c | 3 ++-
>> net/ipv6/ip6_output.c | 3 ++-
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
>> index 83c73bab2c3d..c15204ec2eb0 100644
>> --- a/net/ipv4/ip_output.c
>> +++ b/net/ipv4/ip_output.c
>> @@ -1045,7 +1045,8 @@ static int __ip_append_data(struct sock *sk,
>> if (copy > length)
>> copy = length;
>>
>> - if (!(rt->dst.dev->features&NETIF_F_SG)) {
>> + if (!(rt->dst.dev->features&NETIF_F_SG) &&
>> + skb_tailroom(skb) > copy) {
>
> On second thought, maybe use >= for the test ?
Ai, yes of course. Will send a v2.
^ permalink raw reply
* Re: [PATCH net-next 0/2] sched: refactor NOLOCK qdiscs
From: David Miller @ 2018-05-17 17:06 UTC (permalink / raw)
To: pabeni; +Cc: netdev, jhs, xiyou.wangcong, jiri, john.fastabend, mst
In-Reply-To: <cover.1526392746.git.pabeni@redhat.com>
From: Paolo Abeni <pabeni@redhat.com>
Date: Tue, 15 May 2018 16:24:35 +0200
> With the introduction of NOLOCK qdiscs, pfifo_fast performances in the
> uncontended scenario degraded measurably, especially after the commit
> eb82a9944792 ("net: sched, fix OOO packets with pfifo_fast").
>
> This series restore the pfifo_fast performances in such scenario back the
> previous level, mainly reducing the number of atomic operations required to
> perform the qdisc_run() call. Even performances in the contended scenario
> increase measurably.
>
> Note: This series is on top of:
>
> sched: manipulate __QDISC_STATE_RUNNING in qdisc_run_* helpers
Series applied, thank you.
^ permalink raw reply
* [PATCH net v2] net: test tailroom before appending to linear skb
From: Willem de Bruijn @ 2018-05-17 17:13 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Device features may change during transmission. In particular with
corking, a device may toggle scatter-gather in between allocating
and writing to an skb.
Do not unconditionally assume that !NETIF_F_SG at write time implies
that the same held at alloc time and thus the skb has sufficient
tailroom.
This issue predates git history.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
v2: fix ipv4 boundary condition
---
net/ipv4/ip_output.c | 3 ++-
net/ipv6/ip6_output.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 83c73bab2c3d..d54abc097800 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1045,7 +1045,8 @@ static int __ip_append_data(struct sock *sk,
if (copy > length)
copy = length;
- if (!(rt->dst.dev->features&NETIF_F_SG)) {
+ if (!(rt->dst.dev->features&NETIF_F_SG) &&
+ skb_tailroom(skb) >= copy) {
unsigned int off;
off = skb->len;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 2e891d2c30ef..7b6d1689087b 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1503,7 +1503,8 @@ static int __ip6_append_data(struct sock *sk,
if (copy > length)
copy = length;
- if (!(rt->dst.dev->features&NETIF_F_SG)) {
+ if (!(rt->dst.dev->features&NETIF_F_SG) &&
+ skb_tailroom(skb) >= copy) {
unsigned int off;
off = skb->len;
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* Re: [PATCH net-next 2/4] tcp: do not force quickack when receiving out-of-order packets
From: Soheil Hassas Yeganeh @ 2018-05-17 17:14 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Toke Høiland-Jørgensen,
Neal Cardwell, Yuchung Cheng, Eric Dumazet
In-Reply-To: <20180517121213.43559-3-edumazet@google.com>
On Thu, May 17, 2018 at 8:12 AM, Eric Dumazet <edumazet@google.com> wrote:
> As explained in commit 9f9843a751d0 ("tcp: properly handle stretch
> acks in slow start"), TCP stacks have to consider how many packets
> are acknowledged in one single ACK, because of GRO, but also
> because of ACK compression or losses.
>
> We plan to add SACK compression in the following patch, we
> must therefore not call tcp_enter_quickack_mode()
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Thank you, Eric!
^ permalink raw reply
* Re: [PATCH net-next 3/4] tcp: add SACK compression
From: Eric Dumazet @ 2018-05-17 17:15 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Neal Cardwell, Eric Dumazet, David Miller, netdev,
Toke Høiland-Jørgensen, Soheil Hassas Yeganeh,
Christoph Paasch
In-Reply-To: <CAK6E8=e=L3oDLwAJze0Af_YXYBOBvpWTZHpBSPcGjUmuvLCKXQ@mail.gmail.com>
On Thu, May 17, 2018 at 9:59 AM Yuchung Cheng <ycheng@google.com> wrote:
> >
> > Thanks for implementing this! Overall this patch seems nice to me.
> >
> > Acked-by: Neal Cardwell <ncardwell@google.com>
> >
> > BTW, I guess we should spread the word to maintainers of other major TCP
> > stacks that they need to be prepared for what may be a much higher
degree
> > of compression/aggregation in the SACK stream. Linux stacks going back
many
> > years should be fine with this, but I'm not sure about the other major
OSes
> > (they may only allow sending one MSS per ACK-with-SACKs received).
> Patch looks really good but Neal's comment just reminds me a potential
> legacy issue.
> I recall at least Apple and Windows TCP stacks still need 3+ DUPACKs
> (!= a SACK covering 3+ packets) to trigger fast recovery. Will we have
> an issue there interacting w/ these stacks?
Then we should revert GRO :)
Really it is time for these stacks to catch up, or give up to QUIC :/
^ permalink raw reply
* Re: [PATCH net-next 3/4] tcp: add SACK compression
From: Yuchung Cheng @ 2018-05-17 17:15 UTC (permalink / raw)
To: Neal Cardwell
Cc: Eric Dumazet, Eric Dumazet, David Miller, Netdev,
Toke Høiland-Jørgensen, Soheil Hassas Yeganeh,
Christoph Paasch
In-Reply-To: <CAK6E8=e=L3oDLwAJze0Af_YXYBOBvpWTZHpBSPcGjUmuvLCKXQ@mail.gmail.com>
On Thu, May 17, 2018 at 9:59 AM, Yuchung Cheng <ycheng@google.com> wrote:
> On Thu, May 17, 2018 at 9:41 AM, Neal Cardwell <ncardwell@google.com> wrote:
>>
>> On Thu, May 17, 2018 at 11:40 AM Eric Dumazet <eric.dumazet@gmail.com>
>> wrote:
>> > On 05/17/2018 08:14 AM, Neal Cardwell wrote:
>> > > Is there a particular motivation for the cap of 127? IMHO 127 ACKs is
>> quite
>> > > a few to compress. Experience seems to show that it works well to have
>> one
>> > > GRO ACK for ~64KBytes that triggers a single TSO skb of ~64KBytes. It
>> might
>> > > be nice to try to match those dynamics in this SACK compression case,
>> so it
>> > > might be nice to cap the number of compressed ACKs at something like 44?
>> > > (0xffff / 1448 - 1). That way for high-speed paths we could try to keep
>> > > the ACK clock going with ACKs for ~64KBytes that trigger a single TSO
>> skb
>> > > of ~64KBytes, no matter whether we are sending SACKs or cumulative ACKs.
>>
>> > 127 was chosen because the field is u8, and since skb allocation for the
>> ACK
>> > can fail, we could have cases were the field goes above 127.
>>
>> > Ultimately, I believe a followup patch would add a sysctl, so that we can
>> fine-tune
>> > this, and eventually disable ACK compression if this sysctl is set to 0
>>
>> OK, a sysctl sounds good. I would still vote for a default of 44. :-)
>>
>>
>> > >> + if (hrtimer_is_queued(&tp->compressed_ack_timer))
>> > >> + return;
>> > >> +
>> > >> + /* compress ack timer : 5 % of srtt, but no more than 2.5 ms */
>> > >> +
>> > >> + delay = min_t(unsigned long, 2500 * NSEC_PER_USEC,
>> > >> + tp->rcv_rtt_est.rtt_us * (NSEC_PER_USEC >>
>> 3)/20);
>> > >
>> > > Any particular motivation for the 2.5ms here? It might be nice to match
>> the
>> > > existing TSO autosizing dynamics and use 1ms here instead of having a
>> > > separate new constant of 2.5ms. Smaller time scales here should lead to
>> > > less burstiness and queue pressure from data packets in the network,
>> and we
>> > > know from experience that the CPU overhead of 1ms chunks is acceptable.
>>
>> > This came from my tests on wifi really :)
>>
>> > I also had the idea to make this threshold adjustable for wifi, like we
>> did for sk_pacing_shift.
>>
>> > (On wifi, we might want to increase the max delay between ACK)
>>
>> > So maybe use 1ms delay, when sk_pacing_shift == 10, but increase it if
>> sk_pacing_shift has been lowered.
>>
>> Sounds good to me.
>>
>> Thanks for implementing this! Overall this patch seems nice to me.
>>
>> Acked-by: Neal Cardwell <ncardwell@google.com>
>>
>> BTW, I guess we should spread the word to maintainers of other major TCP
>> stacks that they need to be prepared for what may be a much higher degree
>> of compression/aggregation in the SACK stream. Linux stacks going back many
>> years should be fine with this, but I'm not sure about the other major OSes
>> (they may only allow sending one MSS per ACK-with-SACKs received).
> Patch looks really good but Neal's comment just reminds me a potential
> legacy issue.
>
> I recall at least Apple and Windows TCP stacks still need 3+ DUPACKs
> (!= a SACK covering 3+ packets) to trigger fast recovery. Will we have
> an issue there interacting w/ these stacks?
Offline chat w/ Eric: actually the problem already exists with GRO: a
Linux receiver could receive a OOO skb of say 5 pkts and returns one
(DUP)ACK w/ sack option covering 5 pkts.
Since no issues have been reported my concern is probably not big
deal. Hopefully other stacks can improve their sack / recovery
handling there.
>
>>
>> neal
^ permalink raw reply
* Re: [PATCH v3 2/2] bpf: add selftest for rawir_event type program
From: Y Song @ 2018-05-17 17:17 UTC (permalink / raw)
To: Sean Young
Cc: linux-media, linux-kernel, Alexei Starovoitov,
Mauro Carvalho Chehab, Daniel Borkmann, netdev, Matthias Reichl,
Devin Heitmueller
In-Reply-To: <78945f2bf82e9f16695f72bed3930d1302d38e29.1526504511.git.sean@mess.org>
On Wed, May 16, 2018 at 2:04 PM, Sean Young <sean@mess.org> wrote:
> This is simple test over rc-loopback.
>
> Signed-off-by: Sean Young <sean@mess.org>
> ---
> tools/bpf/bpftool/prog.c | 1 +
> tools/include/uapi/linux/bpf.h | 57 +++++++-
> tools/lib/bpf/libbpf.c | 1 +
> tools/testing/selftests/bpf/Makefile | 8 +-
> tools/testing/selftests/bpf/bpf_helpers.h | 6 +
> tools/testing/selftests/bpf/test_rawir.sh | 37 +++++
> .../selftests/bpf/test_rawir_event_kern.c | 26 ++++
> .../selftests/bpf/test_rawir_event_user.c | 130 ++++++++++++++++++
> 8 files changed, 261 insertions(+), 5 deletions(-)
> create mode 100755 tools/testing/selftests/bpf/test_rawir.sh
> create mode 100644 tools/testing/selftests/bpf/test_rawir_event_kern.c
> create mode 100644 tools/testing/selftests/bpf/test_rawir_event_user.c
Could you copy include/uapi/linux/lirc.h file to tools directory as well.
Otherwise, I will get the following compilation error:
gcc -Wall -O2 -I../../../include/uapi -I../../../lib
-I../../../lib/bpf -I../../../../include/generated -I../../../include
test_rawir_event_user.c
/home/yhs/work/bpf-next/tools/testing/selftests/bpf/libbpf.a -lcap
-lelf -lrt -lpthread -o
/home/yhs/work/bpf-next/tools/testing/selftests/bpf/test_rawir_event_user
test_rawir_event_user.c: In function ‘main’:
test_rawir_event_user.c:60:15: error: ‘LIRC_MODE_SCANCODE’ undeclared
(first use in this function); did you mean ‘LIRC_MODE_LIRCCODE’?
mode = LIRC_MODE_SCANCODE;
^~~~~~~~~~~~~~~~~~
LIRC_MODE_LIRCCODE
test_rawir_event_user.c:60:15: note: each undeclared identifier is
reported only once for each function it appears in
test_rawir_event_user.c:93:29: error: storage size of ‘lsc’ isn’t known
struct lirc_scancode lsc;
^~~
test_rawir_event_user.c:93:29: warning: unused variable ‘lsc’
[-Wunused-variable]
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 9bdfdf2d3fbe..8889a4ee8577 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -71,6 +71,7 @@ static const char * const prog_type_name[] = {
> [BPF_PROG_TYPE_SK_MSG] = "sk_msg",
> [BPF_PROG_TYPE_RAW_TRACEPOINT] = "raw_tracepoint",
> [BPF_PROG_TYPE_CGROUP_SOCK_ADDR] = "cgroup_sock_addr",
> + [BPF_PROG_TYPE_RAWIR_EVENT] = "rawir_event",
> };
>
> static void print_boot_time(__u64 nsecs, char *buf, unsigned int size)
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 1205d86a7a29..243e141e8a5b 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -141,6 +141,7 @@ enum bpf_prog_type {
> BPF_PROG_TYPE_SK_MSG,
> BPF_PROG_TYPE_RAW_TRACEPOINT,
> BPF_PROG_TYPE_CGROUP_SOCK_ADDR,
> + BPF_PROG_TYPE_RAWIR_EVENT,
> };
>
> enum bpf_attach_type {
> @@ -158,6 +159,7 @@ enum bpf_attach_type {
> BPF_CGROUP_INET6_CONNECT,
> BPF_CGROUP_INET4_POST_BIND,
> BPF_CGROUP_INET6_POST_BIND,
> + BPF_RAWIR_EVENT,
> __MAX_BPF_ATTACH_TYPE
> };
>
> @@ -1829,7 +1831,6 @@ union bpf_attr {
> * Return
> * 0 on success, or a negative error in case of failure.
> *
> - *
> * int bpf_fib_lookup(void *ctx, struct bpf_fib_lookup *params, int plen, u32 flags)
> * Description
> * Do FIB lookup in kernel tables using parameters in *params*.
> @@ -1856,6 +1857,7 @@ union bpf_attr {
> * Egress device index on success, 0 if packet needs to continue
> * up the stack for further processing or a negative error in case
> * of failure.
> + *
> * int bpf_sock_hash_update(struct bpf_sock_ops_kern *skops, struct bpf_map *map, void *key, u64 flags)
> * Description
> * Add an entry to, or update a sockhash *map* referencing sockets.
> @@ -1902,6 +1904,35 @@ union bpf_attr {
> * egress otherwise). This is the only flag supported for now.
> * Return
> * **SK_PASS** on success, or **SK_DROP** on error.
> + *
> + * int bpf_rc_keydown(void *ctx, u32 protocol, u32 scancode, u32 toggle)
> + * Description
> + * Report decoded scancode with toggle value. For use in
> + * BPF_PROG_TYPE_RAWIR_EVENT, to report a successfully
> + * decoded scancode. This is will generate a keydown event,
> + * and a keyup event once the scancode is no longer repeated.
> + *
> + * *ctx* pointer to bpf_rawir_event, *protocol* is decoded
> + * protocol (see RC_PROTO_* enum).
> + *
> + * Some protocols include a toggle bit, in case the button
> + * was released and pressed again between consecutive scancodes,
> + * copy this bit into *toggle* if it exists, else set to 0.
> + *
> + * Return
> + * Always return 0 (for now)
> + *
> + * int bpf_rc_repeat(void *ctx)
> + * Description
> + * Repeat the last decoded scancode; some IR protocols like
> + * NEC have a special IR message for repeat last button,
> + * in case user is holding a button down; the scancode is
> + * not repeated.
> + *
> + * *ctx* pointer to bpf_rawir_event.
> + *
> + * Return
> + * Always return 0 (for now)
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -1976,7 +2007,9 @@ union bpf_attr {
> FN(fib_lookup), \
> FN(sock_hash_update), \
> FN(msg_redirect_hash), \
> - FN(sk_redirect_hash),
> + FN(sk_redirect_hash), \
> + FN(rc_repeat), \
> + FN(rc_keydown),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> @@ -2043,6 +2076,26 @@ enum bpf_hdr_start_off {
> BPF_HDR_START_NET,
> };
>
> +/*
> + * user accessible mirror of in-kernel ir_raw_event
> + */
> +#define BPF_RAWIR_EVENT_SPACE 0
> +#define BPF_RAWIR_EVENT_PULSE 1
> +#define BPF_RAWIR_EVENT_TIMEOUT 2
> +#define BPF_RAWIR_EVENT_RESET 3
> +#define BPF_RAWIR_EVENT_CARRIER 4
> +#define BPF_RAWIR_EVENT_DUTY_CYCLE 5
> +
> +struct bpf_rawir_event {
> + union {
> + __u32 duration;
> + __u32 carrier;
> + __u32 duty_cycle;
> + };
> +
> + __u32 type;
> +};
> +
> /* user accessible mirror of in-kernel sk_buff.
> * new fields can only be added to the end of this structure
> */
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index df54c4c9e48a..372269e9053d 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1455,6 +1455,7 @@ static bool bpf_prog_type__needs_kver(enum bpf_prog_type type)
> case BPF_PROG_TYPE_CGROUP_DEVICE:
> case BPF_PROG_TYPE_SK_MSG:
> case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> + case BPF_PROG_TYPE_RAWIR_EVENT:
> return false;
> case BPF_PROG_TYPE_UNSPEC:
> case BPF_PROG_TYPE_KPROBE:
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 1eb0fa2aba92..b84e36d05d34 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -24,7 +24,7 @@ urandom_read: urandom_read.c
> # Order correspond to 'make run_tests' order
> TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
> test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \
> - test_sock test_btf test_sockmap
> + test_sock test_btf test_sockmap test_rawir_event_user
>
> TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test_obj_id.o \
> test_pkt_md_access.o test_xdp_redirect.o test_xdp_meta.o sockmap_parse_prog.o \
> @@ -33,7 +33,8 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
> sample_map_ret0.o test_tcpbpf_kern.o test_stacktrace_build_id.o \
> sockmap_tcp_msg_prog.o connect4_prog.o connect6_prog.o test_adjust_tail.o \
> test_btf_haskv.o test_btf_nokv.o test_sockmap_kern.o test_tunnel_kern.o \
> - test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o
> + test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o \
> + test_rawir_event_kern.o
>
> # Order correspond to 'make run_tests' order
> TEST_PROGS := test_kmod.sh \
> @@ -42,7 +43,8 @@ TEST_PROGS := test_kmod.sh \
> test_xdp_meta.sh \
> test_offload.py \
> test_sock_addr.sh \
> - test_tunnel.sh
> + test_tunnel.sh \
> + test_rawir.sh
>
> # Compile but not part of 'make run_tests'
> TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index 8f143dfb3700..26d89b7f9841 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -114,6 +114,12 @@ static int (*bpf_get_stack)(void *ctx, void *buf, int size, int flags) =
> static int (*bpf_fib_lookup)(void *ctx, struct bpf_fib_lookup *params,
> int plen, __u32 flags) =
> (void *) BPF_FUNC_fib_lookup;
> +static int (*bpf_rc_repeat)(void *ctx) =
> + (void *) BPF_FUNC_rc_repeat;
> +static int (*bpf_rc_keydown)(void *ctx, unsigned int protocol,
> + unsigned int scancode, unsigned int toggle) =
> + (void *) BPF_FUNC_rc_keydown;
> +
>
> /* llvm builtin functions that eBPF C program may use to
> * emit BPF_LD_ABS and BPF_LD_IND instructions
> diff --git a/tools/testing/selftests/bpf/test_rawir.sh b/tools/testing/selftests/bpf/test_rawir.sh
> new file mode 100755
> index 000000000000..0aa77b043ee1
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_rawir.sh
> @@ -0,0 +1,37 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# Test bpf_rawir_event over rc-loopback. Steps for the test:
> +#
> +# 1. Find the /dev/lircN device for rc-loopback
> +# 2. Attach bpf_rawir_event program which decodes some IR.
> +# 3. Send some IR to the same IR device; since it is loopback, this will
> +# end up in the bpf program
> +# 4. bpf program should decode IR and report keycode
> +# 5. We can read keycode from same /dev/lirc device
> +
> +GREEN='\033[0;92m'
> +RED='\033[0;31m'
> +NC='\033[0m' # No Color
> +
> +modprobe rc-loopback
> +
> +for i in /sys/class/rc/rc*
> +do
> + if grep -q DRV_NAME=rc-loopback $i/uevent
> + then
> + LIRCDEV=$(grep DEVNAME= $i/lirc*/uevent | sed sQDEVNAME=Q/dev/Q)
> + fi
> +done
> +
> +if [ -n $LIRCDEV ];
> +then
> + TYPE=rawir_event
> + ./test_rawir_event_user $LIRCDEV
> + ret=$?
> + if [ $ret -ne 0 ]; then
> + echo -e ${RED}"FAIL: $TYPE"${NC}
> + else
> + echo -e ${GREEN}"PASS: $TYPE"${NC}
> + fi
> +fi
> diff --git a/tools/testing/selftests/bpf/test_rawir_event_kern.c b/tools/testing/selftests/bpf/test_rawir_event_kern.c
> new file mode 100644
> index 000000000000..33ba5d30af62
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_rawir_event_kern.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// test ir decoder
> +//
> +// Copyright (C) 2018 Sean Young <sean@mess.org>
> +
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +SEC("rawir_event")
> +int bpf_decoder(struct bpf_rawir_event *e)
> +{
> + if (e->type == BPF_RAWIR_EVENT_PULSE) {
> + /*
> + * The lirc interface is microseconds, but here we receive
> + * nanoseconds.
> + */
> + int microseconds = e->duration / 1000;
> +
> + if (microseconds & 0x10000)
> + bpf_rc_keydown(e, 0x40, microseconds & 0xffff, 0);
> + }
> +
> + return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/test_rawir_event_user.c b/tools/testing/selftests/bpf/test_rawir_event_user.c
> new file mode 100644
> index 000000000000..c3d7f2c68033
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_rawir_event_user.c
> @@ -0,0 +1,130 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// test ir decoder
> +//
> +// Copyright (C) 2018 Sean Young <sean@mess.org>
> +
> +#include <linux/bpf.h>
> +#include <linux/lirc.h>
> +#include <assert.h>
> +#include <errno.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <poll.h>
> +#include <libgen.h>
> +#include <sys/resource.h>
> +#include <sys/types.h>
> +#include <sys/ioctl.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +#include "bpf_util.h"
> +#include <bpf/bpf.h>
> +#include <bpf/libbpf.h>
> +
> +int main(int argc, char **argv)
> +{
> + struct bpf_object *obj;
> + int ret, lircfd, progfd, mode;
> + int testir = 0x1dead;
> + u32 prog_ids[10], prog_flags[10], prog_cnt;
> +
> + if (argc != 2) {
> + printf("Usage: %s /dev/lircN\n", argv[0]);
Most people probably not really familiar with lircN device. It would be
good to provide more information about how to enable this, e.g.,
CONFIG_RC_CORE=y
CONFIG_BPF_RAWIR_EVENT=y
CONFIG_RC_LOOPBACK=y
......
Thanks!
^ permalink raw reply
* Re: [PATCH net v2] net: test tailroom before appending to linear skb
From: Eric Dumazet @ 2018-05-17 17:18 UTC (permalink / raw)
To: Willem de Bruijn, netdev; +Cc: davem, Willem de Bruijn
In-Reply-To: <20180517171329.201710-1-willemdebruijn.kernel@gmail.com>
On 05/17/2018 10:13 AM, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Device features may change during transmission. In particular with
> corking, a device may toggle scatter-gather in between allocating
> and writing to an skb.
>
> Do not unconditionally assume that !NETIF_F_SG at write time implies
> that the same held at alloc time and thus the skb has sufficient
> tailroom.
>
> This issue predates git history.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH 2/2] bpf: sockmap, fix double-free
From: John Fastabend @ 2018-05-17 17:26 UTC (permalink / raw)
To: Gustavo A. R. Silva, Alexei Starovoitov, Daniel Borkmann
Cc: netdev, linux-kernel
In-Reply-To: <96e7f5e59eacddc5e32abb72b7686c3e9163a410.1526565461.git.gustavo@embeddedor.com>
On 05/17/2018 07:11 AM, Gustavo A. R. Silva wrote:
> `e' is being freed twice.
>
> Fix this by removing one of the kfree() calls.
>
> Addresses-Coverity-ID: 1468983 ("Double free")
> Fixes: 81110384441a ("bpf: sockmap, add hash map support")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> kernel/bpf/sockmap.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
> index 41b41fc..c682669 100644
> --- a/kernel/bpf/sockmap.c
> +++ b/kernel/bpf/sockmap.c
> @@ -1823,7 +1823,6 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
> write_unlock_bh(&sock->sk_callback_lock);
> return err;
> out_free:
> - kfree(e);
> smap_release_sock(psock, sock);
> out_progs:
> if (verdict)
>
Thanks. This can happen when a user tries to add a kTLS socket to a
sockmap. We need to add some tests for kTLS + sockmap cases.
Acked-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/2] bpf: sockmap, fix uninitialized variable
From: John Fastabend @ 2018-05-17 17:27 UTC (permalink / raw)
To: Gustavo A. R. Silva, Alexei Starovoitov, Daniel Borkmann
Cc: netdev, linux-kernel
In-Reply-To: <11073635016901e9e84c2f34f20d412073b26297.1526565461.git.gustavo@embeddedor.com>
On 05/17/2018 07:08 AM, Gustavo A. R. Silva wrote:
> There is a potential execution path in which variable err is
> returned without being properly initialized previously.
>
> Fix this by initializing variable err to 0.
>
> Addresses-Coverity-ID: 1468964 ("Uninitialized scalar variable")
> Fixes: e5cd3abcb31a ("bpf: sockmap, refactor sockmap routines to work
> with hashmap")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> kernel/bpf/sockmap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
> index c6de139..41b41fc 100644
> --- a/kernel/bpf/sockmap.c
> +++ b/kernel/bpf/sockmap.c
> @@ -1713,7 +1713,7 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
> struct smap_psock_map_entry *e = NULL;
> struct smap_psock *psock;
> bool new = false;
> - int err;
> + int err = 0;
>
> /* 1. If sock map has BPF programs those will be inherited by the
> * sock being added. If the sock is already attached to BPF programs
>
Thanks for catching this and the quick fix. The path to hit this case
is to add a sock to a map (without a BPF program) where the sock already
has been added to another map. I don't have any tests for the case with
socks in multiple maps so I'll add some to the selftests so I remember
this case.
The alternative fix would be to always 'return 0' at the end of the
function, but I think its probably better to init err here like above.
Acked-by: John Fastabend <john.fastabend@gmail.com>
^ 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