* [PATCH net V2 2/2] net: core: explicitly select a txq before doing l2 forwarding
From: Jason Wang @ 2014-01-09 9:37 UTC (permalink / raw)
To: davem, netdev, linux-kernel
Cc: John Fastabend, e1000-devel, Jason Wang, Neil Horman, mst
In-Reply-To: <1389260252-48591-1-git-send-email-jasowang@redhat.com>
Currently, the tx queue were selected implicitly in ndo_dfwd_start_xmit(). The
will cause several issues:
- NETIF_F_LLTX were removed for macvlan, so txq lock were done for macvlan
instead of lower device which misses the necessary txq synchronization for
lower device such as txq stopping or frozen required by dev watchdog or
control path.
- dev_hard_start_xmit() was called with NULL txq which bypasses the net device
watchdog.
- dev_hard_start_xmit() does not check txq everywhere which will lead a crash
when tso is disabled for lower device.
Fix this by explicitly introducing a new param for .ndo_select_queue() for just
selecting queues in the case of l2 forwarding offload. And also introducing
dfwd_direct_xmit() to do the queue selecting, txq holding and transmitting for
l2 forwarding.
With this fixes, NETIF_F_LLTX could be preserved for macvlan and there's no need
to check txq against NULL in dev_hard_start_xmit(). Also there's no need to keep
a dedicated ndo_dfwd_start_xmit().
In the future, it was also required for macvtap l2 forwarding support since it
provides a necessary synchronization method.
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: e1000-devel@lists.sourceforge.net
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
Changes from V1:
- Adding a new parameter to ndo_select_queue instead of a new method to select
queue for l2 forwarding.
- Remove the unnecessary ndo_dfwd_start_xmit() since txq was selected
explicitly.
- Keep NETIF_F_LLTX when netdev feature is changed.
- Shape the commit log
---
drivers/net/bonding/bond_main.c | 3 +-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 3 +-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 3 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 33 +++++++------------
drivers/net/ethernet/lantiq_etop.c | 3 +-
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 3 +-
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 3 +-
drivers/net/ethernet/tile/tilegx.c | 3 +-
drivers/net/macvlan.c | 6 +--
drivers/net/team/team.c | 3 +-
drivers/net/tun.c | 3 +-
drivers/net/wireless/mwifiex/main.c | 3 +-
drivers/staging/bcm/Bcmnet.c | 3 +-
drivers/staging/netlogic/xlr_net.c | 3 +-
drivers/staging/rtl8188eu/os_dep/os_intfs.c | 3 +-
include/linux/netdevice.h | 10 ++++--
net/core/dev.c | 39 +++++++++++++++++------
net/core/flow_dissector.c | 2 +-
net/mac80211/iface.c | 6 ++-
net/sched/sch_generic.c | 2 +-
20 files changed, 83 insertions(+), 54 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 398e299..4b8c58b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3732,7 +3732,8 @@ static inline int bond_slave_override(struct bonding *bond,
}
-static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
+static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv)
{
/*
* This helper function exists to help dev_pick_tx get the correct
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index ec96130..513fa85 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -1832,7 +1832,8 @@ void bnx2x_netif_stop(struct bnx2x *bp, int disable_hw)
bnx2x_napi_disable_cnic(bp);
}
-u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb)
+u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv)
{
struct bnx2x *bp = netdev_priv(dev);
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
index da8fcaa..41f3ca5 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h
@@ -524,7 +524,8 @@ int bnx2x_set_vf_mac(struct net_device *dev, int queue, u8 *mac);
int bnx2x_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan, u8 qos);
/* select_queue callback */
-u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb);
+u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv);
static inline void bnx2x_update_rx_prod(struct bnx2x *bp,
struct bnx2x_fastpath *fp,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index cc06854..5bcc870 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6827,12 +6827,20 @@ static inline int ixgbe_maybe_stop_tx(struct ixgbe_ring *tx_ring, u16 size)
return __ixgbe_maybe_stop_tx(tx_ring, size);
}
-#ifdef IXGBE_FCOE
-static u16 ixgbe_select_queue(struct net_device *dev, struct sk_buff *skb)
+static u16 ixgbe_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv)
{
+ struct ixgbe_fwd_adapter *fwd_adapter = accel_priv;
+#ifdef IXGBE_FCOE
struct ixgbe_adapter *adapter;
struct ixgbe_ring_feature *f;
int txq;
+#endif
+
+ if (fwd_adapter)
+ return skb->queue_mapping + fwd_adapter->tx_base_queue;
+
+#ifdef IXGBE_FCOE
/*
* only execute the code below if protocol is FCoE
@@ -6858,9 +6866,11 @@ static u16 ixgbe_select_queue(struct net_device *dev, struct sk_buff *skb)
txq -= f->indices;
return txq + f->offset;
+#else
+ return __netdev_pick_tx(dev, skb);
+#endif
}
-#endif
netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
struct ixgbe_adapter *adapter,
struct ixgbe_ring *tx_ring)
@@ -7629,27 +7639,11 @@ static void ixgbe_fwd_del(struct net_device *pdev, void *priv)
kfree(fwd_adapter);
}
-static netdev_tx_t ixgbe_fwd_xmit(struct sk_buff *skb,
- struct net_device *dev,
- void *priv)
-{
- struct ixgbe_fwd_adapter *fwd_adapter = priv;
- unsigned int queue;
- struct ixgbe_ring *tx_ring;
-
- queue = skb->queue_mapping + fwd_adapter->tx_base_queue;
- tx_ring = fwd_adapter->real_adapter->tx_ring[queue];
-
- return __ixgbe_xmit_frame(skb, dev, tx_ring);
-}
-
static const struct net_device_ops ixgbe_netdev_ops = {
.ndo_open = ixgbe_open,
.ndo_stop = ixgbe_close,
.ndo_start_xmit = ixgbe_xmit_frame,
-#ifdef IXGBE_FCOE
.ndo_select_queue = ixgbe_select_queue,
-#endif
.ndo_set_rx_mode = ixgbe_set_rx_mode,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_mac_address = ixgbe_set_mac,
@@ -7689,7 +7683,6 @@ static const struct net_device_ops ixgbe_netdev_ops = {
.ndo_bridge_getlink = ixgbe_ndo_bridge_getlink,
.ndo_dfwd_add_station = ixgbe_fwd_add,
.ndo_dfwd_del_station = ixgbe_fwd_del,
- .ndo_dfwd_start_xmit = ixgbe_fwd_xmit,
};
/**
diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
index 6a6c1f7..ec94a20 100644
--- a/drivers/net/ethernet/lantiq_etop.c
+++ b/drivers/net/ethernet/lantiq_etop.c
@@ -619,7 +619,8 @@ ltq_etop_set_multicast_list(struct net_device *dev)
}
static u16
-ltq_etop_select_queue(struct net_device *dev, struct sk_buff *skb)
+ltq_etop_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv)
{
/* we are currently only using the first queue */
return 0;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index f54ebd5..a7fcd59 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -592,7 +592,8 @@ static void build_inline_wqe(struct mlx4_en_tx_desc *tx_desc, struct sk_buff *sk
}
}
-u16 mlx4_en_select_queue(struct net_device *dev, struct sk_buff *skb)
+u16 mlx4_en_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv)
{
struct mlx4_en_priv *priv = netdev_priv(dev);
u16 rings_p_up = priv->num_tx_rings_p_up;
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index f3758de..d5758ad 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -714,7 +714,8 @@ int mlx4_en_set_cq_moder(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq);
int mlx4_en_arm_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq);
void mlx4_en_tx_irq(struct mlx4_cq *mcq);
-u16 mlx4_en_select_queue(struct net_device *dev, struct sk_buff *skb);
+u16 mlx4_en_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv);
netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev);
int mlx4_en_create_tx_ring(struct mlx4_en_priv *priv,
diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
index 628b736..0e9fb33 100644
--- a/drivers/net/ethernet/tile/tilegx.c
+++ b/drivers/net/ethernet/tile/tilegx.c
@@ -2080,7 +2080,8 @@ static int tile_net_tx(struct sk_buff *skb, struct net_device *dev)
}
/* Return subqueue id on this core (one per core). */
-static u16 tile_net_select_queue(struct net_device *dev, struct sk_buff *skb)
+static u16 tile_net_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv)
{
return smp_processor_id();
}
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 5360f73..7eb4c82 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -299,7 +299,7 @@ netdev_tx_t macvlan_start_xmit(struct sk_buff *skb,
if (vlan->fwd_priv) {
skb->dev = vlan->lowerdev;
- ret = dev_hard_start_xmit(skb, skb->dev, NULL, vlan->fwd_priv);
+ ret = dfwd_direct_xmit(skb, skb->dev, vlan->fwd_priv);
} else {
ret = macvlan_queue_xmit(skb, dev);
}
@@ -366,7 +366,6 @@ static int macvlan_open(struct net_device *dev)
if (IS_ERR_OR_NULL(vlan->fwd_priv)) {
vlan->fwd_priv = NULL;
} else {
- dev->features &= ~NETIF_F_LLTX;
return 0;
}
}
@@ -702,8 +701,7 @@ static netdev_features_t macvlan_fix_features(struct net_device *dev,
features = netdev_increment_features(vlan->lowerdev->features,
features,
mask);
- if (!vlan->fwd_priv)
- features |= NETIF_F_LLTX;
+ features |= NETIF_F_LLTX;
return features;
}
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 736050d..b75ae5b 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1647,7 +1647,8 @@ static netdev_tx_t team_xmit(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK;
}
-static u16 team_select_queue(struct net_device *dev, struct sk_buff *skb)
+static u16 team_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv)
{
/*
* This helper function exists to help dev_pick_tx get the correct
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 7c8343a..ecec802 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -348,7 +348,8 @@ unlock:
* different rxq no. here. If we could not get rxhash, then we would
* hope the rxq no. may help here.
*/
-static u16 tun_select_queue(struct net_device *dev, struct sk_buff *skb)
+static u16 tun_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv)
{
struct tun_struct *tun = netdev_priv(dev);
struct tun_flow_entry *e;
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index 78e8a66..8bb8988 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -746,7 +746,8 @@ static struct net_device_stats *mwifiex_get_stats(struct net_device *dev)
}
static u16
-mwifiex_netdev_select_wmm_queue(struct net_device *dev, struct sk_buff *skb)
+mwifiex_netdev_select_wmm_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv)
{
skb->priority = cfg80211_classify8021d(skb);
return mwifiex_1d_to_wmm_queue[skb->priority];
diff --git a/drivers/staging/bcm/Bcmnet.c b/drivers/staging/bcm/Bcmnet.c
index 53fee2f..8dfdd27 100644
--- a/drivers/staging/bcm/Bcmnet.c
+++ b/drivers/staging/bcm/Bcmnet.c
@@ -39,7 +39,8 @@ static INT bcm_close(struct net_device *dev)
return 0;
}
-static u16 bcm_select_queue(struct net_device *dev, struct sk_buff *skb)
+static u16 bcm_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv)
{
return ClassifyPacket(netdev_priv(dev), skb);
}
diff --git a/drivers/staging/netlogic/xlr_net.c b/drivers/staging/netlogic/xlr_net.c
index 235d2b1..eedffed 100644
--- a/drivers/staging/netlogic/xlr_net.c
+++ b/drivers/staging/netlogic/xlr_net.c
@@ -306,7 +306,8 @@ static netdev_tx_t xlr_net_start_xmit(struct sk_buff *skb,
return NETDEV_TX_OK;
}
-static u16 xlr_net_select_queue(struct net_device *ndev, struct sk_buff *skb)
+static u16 xlr_net_select_queue(struct net_device *ndev, struct sk_buff *skb,
+ void *accel_priv)
{
return (u16)smp_processor_id();
}
diff --git a/drivers/staging/rtl8188eu/os_dep/os_intfs.c b/drivers/staging/rtl8188eu/os_dep/os_intfs.c
index 17659bb..dd69e34 100644
--- a/drivers/staging/rtl8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/rtl8188eu/os_dep/os_intfs.c
@@ -652,7 +652,8 @@ static unsigned int rtw_classify8021d(struct sk_buff *skb)
return dscp >> 5;
}
-static u16 rtw_select_queue(struct net_device *dev, struct sk_buff *skb)
+static u16 rtw_select_queue(struct net_device *dev, struct sk_buff *skb,
+ void *accel_priv)
{
struct adapter *padapter = rtw_netdev_priv(dev);
struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5faaadb..262b04f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -769,7 +769,8 @@ struct netdev_phys_port_id {
* (can also return NETDEV_TX_LOCKED iff NETIF_F_LLTX)
* Required can not be NULL.
*
- * u16 (*ndo_select_queue)(struct net_device *dev, struct sk_buff *skb);
+ * u16 (*ndo_select_queue)(struct net_device *dev, struct sk_buff *skb,
+ * void *accel_priv);
* Called to decide which queue to when device supports multiple
* transmit queues.
*
@@ -990,7 +991,8 @@ struct net_device_ops {
netdev_tx_t (*ndo_start_xmit) (struct sk_buff *skb,
struct net_device *dev);
u16 (*ndo_select_queue)(struct net_device *dev,
- struct sk_buff *skb);
+ struct sk_buff *skb,
+ void *accel_priv);
void (*ndo_change_rx_flags)(struct net_device *dev,
int flags);
void (*ndo_set_rx_mode)(struct net_device *dev);
@@ -2425,8 +2427,10 @@ int dev_set_mac_address(struct net_device *, struct sockaddr *);
int dev_change_carrier(struct net_device *, bool new_carrier);
int dev_get_phys_port_id(struct net_device *dev,
struct netdev_phys_port_id *ppid);
+int dfwd_direct_xmit(struct sk_buff *skb, struct net_device *dev,
+ void *accel_priv);
int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
- struct netdev_queue *txq, void *accel_priv);
+ struct netdev_queue *txq);
int dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
extern int netdev_budget;
diff --git a/net/core/dev.c b/net/core/dev.c
index 4fc1722..8d276fe 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2538,8 +2538,33 @@ static inline int skb_needs_linearize(struct sk_buff *skb,
!(features & NETIF_F_SG)));
}
+int dfwd_direct_xmit(struct sk_buff *skb, struct net_device *dev,
+ void *accel_priv)
+{
+ struct netdev_queue *txq;
+ int ret = NETDEV_TX_BUSY;
+ int index;
+
+ BUG_ON(!dev->netdev_ops->ndo_select_queue);
+ index = dev->netdev_ops->ndo_select_queue(dev, skb, accel_priv);
+
+ local_bh_disable();
+
+ skb_set_queue_mapping(skb, index);
+ txq = netdev_get_tx_queue(dev, index);
+
+ HARD_TX_LOCK(dev, txq, smp_processor_id());
+ if (!netif_xmit_frozen_or_stopped(txq))
+ ret = dev_hard_start_xmit(skb, dev, txq);
+ HARD_TX_UNLOCK(dev, txq);
+
+ local_bh_enable();
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dfwd_direct_xmit);
+
int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
- struct netdev_queue *txq, void *accel_priv)
+ struct netdev_queue *txq)
{
const struct net_device_ops *ops = dev->netdev_ops;
int rc = NETDEV_TX_OK;
@@ -2605,13 +2630,10 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
dev_queue_xmit_nit(skb, dev);
skb_len = skb->len;
- if (accel_priv)
- rc = ops->ndo_dfwd_start_xmit(skb, dev, accel_priv);
- else
rc = ops->ndo_start_xmit(skb, dev);
trace_net_dev_xmit(skb, rc, dev, skb_len);
- if (rc == NETDEV_TX_OK && txq)
+ if (rc == NETDEV_TX_OK)
txq_trans_update(txq);
return rc;
}
@@ -2627,10 +2649,7 @@ gso:
dev_queue_xmit_nit(nskb, dev);
skb_len = nskb->len;
- if (accel_priv)
- rc = ops->ndo_dfwd_start_xmit(nskb, dev, accel_priv);
- else
- rc = ops->ndo_start_xmit(nskb, dev);
+ rc = ops->ndo_start_xmit(nskb, dev);
trace_net_dev_xmit(nskb, rc, dev, skb_len);
if (unlikely(rc != NETDEV_TX_OK)) {
if (rc & ~NETDEV_TX_MASK)
@@ -2863,7 +2882,7 @@ int dev_queue_xmit(struct sk_buff *skb)
if (!netif_xmit_stopped(txq)) {
__this_cpu_inc(xmit_recursion);
- rc = dev_hard_start_xmit(skb, dev, txq, NULL);
+ rc = dev_hard_start_xmit(skb, dev, txq);
__this_cpu_dec(xmit_recursion);
if (dev_xmit_complete(rc)) {
HARD_TX_UNLOCK(dev, txq);
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index d6ef173..553946d 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -402,7 +402,7 @@ struct netdev_queue *netdev_pick_tx(struct net_device *dev,
if (dev->real_num_tx_queues != 1) {
const struct net_device_ops *ops = dev->netdev_ops;
if (ops->ndo_select_queue)
- queue_index = ops->ndo_select_queue(dev, skb);
+ queue_index = ops->ndo_select_queue(dev, skb, NULL);
else
queue_index = __netdev_pick_tx(dev, skb);
queue_index = dev_cap_txqueue(dev, queue_index);
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 36c3a4c..a075791 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1061,7 +1061,8 @@ static void ieee80211_uninit(struct net_device *dev)
}
static u16 ieee80211_netdev_select_queue(struct net_device *dev,
- struct sk_buff *skb)
+ struct sk_buff *skb,
+ void *accel_priv)
{
return ieee80211_select_queue(IEEE80211_DEV_TO_SUB_IF(dev), skb);
}
@@ -1078,7 +1079,8 @@ static const struct net_device_ops ieee80211_dataif_ops = {
};
static u16 ieee80211_monitor_select_queue(struct net_device *dev,
- struct sk_buff *skb)
+ struct sk_buff *skb,
+ void *accel_priv)
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct ieee80211_local *local = sdata->local;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 922a094..7fc899a 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -126,7 +126,7 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
HARD_TX_LOCK(dev, txq, smp_processor_id());
if (!netif_xmit_frozen_or_stopped(txq))
- ret = dev_hard_start_xmit(skb, dev, txq, NULL);
+ ret = dev_hard_start_xmit(skb, dev, txq);
HARD_TX_UNLOCK(dev, txq);
--
1.7.1
------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply related
* [PATCH net V2 1/2] macvlan: forbid L2 fowarding offload for macvtap
From: Jason Wang @ 2014-01-09 9:37 UTC (permalink / raw)
To: davem, netdev, linux-kernel; +Cc: mst, Jason Wang, John Fastabend, Neil Horman
L2 fowarding offload will bypass the rx handler of real device. This will make
the packet could not be forwarded to macvtap device. Another problem is the
dev_hard_start_xmit() called for macvtap does not have any synchronization.
Fix this by forbidding L2 forwarding for macvtap.
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/macvlan.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 60406b0..5360f73 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -338,6 +338,8 @@ static const struct header_ops macvlan_hard_header_ops = {
.cache_update = eth_header_cache_update,
};
+static struct rtnl_link_ops macvlan_link_ops;
+
static int macvlan_open(struct net_device *dev)
{
struct macvlan_dev *vlan = netdev_priv(dev);
@@ -353,7 +355,8 @@ static int macvlan_open(struct net_device *dev)
goto hash_add;
}
- if (lowerdev->features & NETIF_F_HW_L2FW_DOFFLOAD) {
+ if (lowerdev->features & NETIF_F_HW_L2FW_DOFFLOAD &&
+ dev->rtnl_link_ops == &macvlan_link_ops) {
vlan->fwd_priv =
lowerdev->netdev_ops->ndo_dfwd_add_station(lowerdev, dev);
--
1.7.1
^ permalink raw reply related
* RE: [PATCH net-next 1/3] net: add skb_checksum_setup
From: Paul Durrant @ 2014-01-09 9:22 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, xen-devel@lists.xen.org, David Miller,
Eric Dumazet, Veaceslav Falico, Alexander Duyck, Nicolas Dichtel
In-Reply-To: <1389220984.31367.22.camel@edumazet-glaptop2.roam.corp.google.com>
> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: 08 January 2014 22:43
> To: Paul Durrant
> Cc: netdev@vger.kernel.org; xen-devel@lists.xen.org; David Miller; Eric
> Dumazet; Veaceslav Falico; Alexander Duyck; Nicolas Dichtel
> Subject: Re: [PATCH net-next 1/3] net: add skb_checksum_setup
>
> On Wed, 2014-01-08 at 13:58 +0000, Paul Durrant wrote:
> > This patch adds a function to set up the partial checksum offset for IP
> > packets (and optionally re-calculate the pseudo-header checksum) into the
> > core network code.
> > The implementation was previously private and duplicated between xen-
> netback
> > and xen-netfront, however it is not xen-specific and is potentially useful
> > to any network driver.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Cc: David Miller <davem@davemloft.net>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Veaceslav Falico <vfalico@redhat.com>
> > Cc: Alexander Duyck <alexander.h.duyck@intel.com>
> > Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> > ---
> > include/linux/netdevice.h | 1 +
> > net/core/dev.c | 271
> +++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 272 insertions(+)
>
> Is there any reason to put this in net/core/dev.c instead of
> net/core/skbuff.c ?
>
No, no reason. I just wasn't sure which was the better place. I'll put it in skbuff.c if that is more appropriate.
>
> Also, no inline should be used in a .c file
>
> ( skb_maybe_pull_tail )
>
Ok. I'll fix that.
Thanks,
Paul
^ permalink raw reply
* RE: [PATCH net-next v3 8/9] xen-netback: Timeout packets in RX path
From: Paul Durrant @ 2014-01-09 9:20 UTC (permalink / raw)
To: Zoltan Kiss, Ian Campbell, Wei Liu,
xen-devel@lists.xenproject.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jonathan Davies
In-Reply-To: <52CDC45D.3050509@citrix.com>
> -----Original Message-----
> From: Zoltan Kiss
> Sent: 08 January 2014 21:34
> To: Ian Campbell; Wei Liu; xen-devel@lists.xenproject.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Jonathan Davies
> Cc: Zoltan Kiss; Paul Durrant
> Subject: Re: [PATCH net-next v3 8/9] xen-netback: Timeout packets in RX
> path
>
> I just realized when answering Ma's mail that this doesn't cause the
> desired effect after Paul's flow control improvement: starting the queue
> doesn't drop the packets which cannot fit the ring. Which in fact might
> be not good.
No, that would not be good.
> We are adding the skb to vif->rx_queue even when
> xenvif_rx_ring_slots_available(vif, min_slots_needed) said there is no
> space for that. Or am I missing something? Paul?
>
That's correct. Part of the flow control improvement was to get rid of needless packet drops. For your purposes, you basically need to avoid using the queuing discipline and take packets into netback's vif->rx_queue regardless of the state of the shared ring so that you can drop them if they get beyond a certain age. So, perhaps you should never stop the netif queue, place an upper limit on vif->rx_queue (either packet or byte count) and drop when that is exceeded (i.e. mimicking pfifo or bfifo internally).
Paul
> Zoli
>
> On 08/01/14 00:10, Zoltan Kiss wrote:
> > A malicious or buggy guest can leave its queue filled indefinitely, in which
> > case qdisc start to queue packets for that VIF. If those packets came from
> an
> > another guest, it can block its slots and prevent shutdown. To avoid that,
> we
> > make sure the queue is drained in every 10 seconds.
> ...
> > diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
> netback/interface.c
> > index 95fcd63..ce032f9 100644
> > --- a/drivers/net/xen-netback/interface.c
> > +++ b/drivers/net/xen-netback/interface.c
> > @@ -114,6 +114,16 @@ static irqreturn_t xenvif_interrupt(int irq, void
> *dev_id)
> > return IRQ_HANDLED;
> > }
> >
> > +static void xenvif_wake_queue(unsigned long data)
> > +{
> > + struct xenvif *vif = (struct xenvif *)data;
> > +
> > + if (netif_queue_stopped(vif->dev)) {
> > + netdev_err(vif->dev, "draining TX queue\n");
> > + netif_wake_queue(vif->dev);
> > + }
> > +}
> > +
> > static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > {
> > struct xenvif *vif = netdev_priv(dev);
> > @@ -143,8 +153,13 @@ static int xenvif_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
> > * then turn off the queue to give the ring a chance to
> > * drain.
> > */
> > - if (!xenvif_rx_ring_slots_available(vif, min_slots_needed))
> > + if (!xenvif_rx_ring_slots_available(vif, min_slots_needed)) {
> > + vif->wake_queue.function = xenvif_wake_queue;
> > + vif->wake_queue.data = (unsigned long)vif;
> > xenvif_stop_queue(vif);
> > + mod_timer(&vif->wake_queue,
> > + jiffies + rx_drain_timeout_jiffies);
> > + }
> >
> > skb_queue_tail(&vif->rx_queue, skb);
> > xenvif_kick_thread(vif);
^ permalink raw reply
* [PATCH v4 4/5] alx: add alx_get_stats64 operation
From: Sabrina Dubroca @ 2014-01-09 9:09 UTC (permalink / raw)
To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca
In-Reply-To: <1389258571-10083-1-git-send-email-sd@queasysnail.net>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
I've modified the assignements of hw stats to netstats here as Ben asked in v3.
The other atheros drivers use the same code as v3. Should I modify
them as well in another patch?
drivers/net/ethernet/atheros/alx/alx.h | 3 ++
drivers/net/ethernet/atheros/alx/main.c | 50 +++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/drivers/net/ethernet/atheros/alx/alx.h b/drivers/net/ethernet/atheros/alx/alx.h
index d71103d..8fc93c5 100644
--- a/drivers/net/ethernet/atheros/alx/alx.h
+++ b/drivers/net/ethernet/atheros/alx/alx.h
@@ -106,6 +106,9 @@ struct alx_priv {
u16 msg_enable;
bool msi;
+
+ /* protects hw.stats */
+ spinlock_t stats_lock;
};
extern const struct ethtool_ops alx_ethtool_ops;
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index c3c4c26..e92ffd6 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1166,10 +1166,60 @@ static void alx_poll_controller(struct net_device *netdev)
}
#endif
+static struct rtnl_link_stats64 *alx_get_stats64(struct net_device *dev,
+ struct rtnl_link_stats64 *net_stats)
+{
+ struct alx_priv *alx = netdev_priv(dev);
+ struct alx_hw_stats *hw_stats = &alx->hw.stats;
+
+ spin_lock(&alx->stats_lock);
+
+ alx_update_hw_stats(&alx->hw);
+
+ net_stats->tx_bytes = hw_stats->tx_byte_cnt;
+ net_stats->rx_bytes = hw_stats->rx_byte_cnt;
+ net_stats->multicast = hw_stats->rx_mcast;
+ net_stats->collisions = hw_stats->tx_single_col +
+ hw_stats->tx_multi_col +
+ hw_stats->tx_late_col +
+ hw_stats->tx_abort_col;
+
+ net_stats->rx_errors = hw_stats->rx_frag +
+ hw_stats->rx_fcs_err +
+ hw_stats->rx_len_err +
+ hw_stats->rx_ov_sz +
+ hw_stats->rx_ov_rrd +
+ hw_stats->rx_align_err +
+ hw_stats->rx_ov_rxf;
+
+ net_stats->rx_fifo_errors = hw_stats->rx_ov_rxf;
+ net_stats->rx_length_errors = hw_stats->rx_len_err;
+ net_stats->rx_crc_errors = hw_stats->rx_fcs_err;
+ net_stats->rx_frame_errors = hw_stats->rx_align_err;
+ net_stats->rx_dropped = hw_stats->rx_ov_rrd;
+
+ net_stats->tx_errors = hw_stats->tx_late_col +
+ hw_stats->tx_abort_col +
+ hw_stats->tx_underrun +
+ hw_stats->tx_trunc;
+
+ net_stats->tx_aborted_errors = hw_stats->tx_abort_col;
+ net_stats->tx_fifo_errors = hw_stats->tx_underrun;
+ net_stats->tx_window_errors = hw_stats->tx_late_col;
+
+ net_stats->tx_packets = hw_stats->tx_ok + net_stats->tx_errors;
+ net_stats->rx_packets = hw_stats->rx_ok + net_stats->rx_errors;
+
+ spin_unlock(&alx->stats_lock);
+
+ return net_stats;
+}
+
static const struct net_device_ops alx_netdev_ops = {
.ndo_open = alx_open,
.ndo_stop = alx_stop,
.ndo_start_xmit = alx_start_xmit,
+ .ndo_get_stats64 = alx_get_stats64,
.ndo_set_rx_mode = alx_set_rx_mode,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_mac_address = alx_set_mac_address,
--
1.8.5.2
^ permalink raw reply related
* [PATCH v4 5/5] alx: add stats to ethtool
From: Sabrina Dubroca @ 2014-01-09 9:09 UTC (permalink / raw)
To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca
In-Reply-To: <1389258571-10083-1-git-send-email-sd@queasysnail.net>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/atheros/alx/ethtool.c | 101 +++++++++++++++++++++++++++++
drivers/net/ethernet/atheros/alx/hw.h | 7 +-
2 files changed, 107 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c b/drivers/net/ethernet/atheros/alx/ethtool.c
index 45b3650..08e22df 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -46,6 +46,66 @@
#include "reg.h"
#include "hw.h"
+/* The order of these strings must match the order of the fields in
+ * struct alx_hw_stats
+ * See hw.h
+ */
+static const char alx_gstrings_stats[][ETH_GSTRING_LEN] = {
+ "rx_packets",
+ "rx_bcast_packets",
+ "rx_mcast_packets",
+ "rx_pause_packets",
+ "rx_ctrl_packets",
+ "rx_fcs_errors",
+ "rx_length_errors",
+ "rx_bytes",
+ "rx_runt_packets",
+ "rx_fragments",
+ "rx_64B_or_less_packets",
+ "rx_65B_to_127B_packets",
+ "rx_128B_to_255B_packets",
+ "rx_256B_to_511B_packets",
+ "rx_512B_to_1023B_packets",
+ "rx_1024B_to_1518B_packets",
+ "rx_1519B_to_mtu_packets",
+ "rx_oversize_packets",
+ "rx_rxf_ov_drop_packets",
+ "rx_rrd_ov_drop_packets",
+ "rx_align_errors",
+ "rx_bcast_bytes",
+ "rx_mcast_bytes",
+ "rx_address_errors",
+ "tx_packets",
+ "tx_bcast_packets",
+ "tx_mcast_packets",
+ "tx_pause_packets",
+ "tx_exc_defer_packets",
+ "tx_ctrl_packets",
+ "tx_defer_packets",
+ "tx_bytes",
+ "tx_64B_or_less_packets",
+ "tx_65B_to_127B_packets",
+ "tx_128B_to_255B_packets",
+ "tx_256B_to_511B_packets",
+ "tx_512B_to_1023B_packets",
+ "tx_1024B_to_1518B_packets",
+ "tx_1519B_to_mtu_packets",
+ "tx_single_collision",
+ "tx_multiple_collisions",
+ "tx_late_collision",
+ "tx_abort_collision",
+ "tx_underrun",
+ "tx_trd_eop",
+ "tx_length_errors",
+ "tx_trunc_packets",
+ "tx_bcast_bytes",
+ "tx_mcast_bytes",
+ "tx_update",
+};
+
+#define ALX_NUM_STATS ARRAY_SIZE(alx_gstrings_stats)
+
+
static u32 alx_get_supported_speeds(struct alx_hw *hw)
{
u32 supported = SUPPORTED_10baseT_Half |
@@ -201,6 +261,44 @@ static void alx_set_msglevel(struct net_device *netdev, u32 data)
alx->msg_enable = data;
}
+static void alx_get_ethtool_stats(struct net_device *netdev,
+ struct ethtool_stats *estats, u64 *data)
+{
+ struct alx_priv *alx = netdev_priv(netdev);
+ struct alx_hw *hw = &alx->hw;
+
+ spin_lock(&alx->stats_lock);
+
+ alx_update_hw_stats(hw);
+ BUILD_BUG_ON(sizeof(hw->stats) - offsetof(struct alx_hw_stats, rx_ok) <
+ ALX_NUM_STATS * sizeof(u64));
+ memcpy(data, &hw->stats.rx_ok, ALX_NUM_STATS * sizeof(u64));
+
+ spin_unlock(&alx->stats_lock);
+}
+
+static void alx_get_strings(struct net_device *netdev, u32 stringset, u8 *buf)
+{
+ switch (stringset) {
+ case ETH_SS_STATS:
+ memcpy(buf, &alx_gstrings_stats, sizeof(alx_gstrings_stats));
+ break;
+ default:
+ WARN_ON(1);
+ break;
+ }
+}
+
+static int alx_get_sset_count(struct net_device *netdev, int sset)
+{
+ switch (sset) {
+ case ETH_SS_STATS:
+ return ALX_NUM_STATS;
+ default:
+ return -EINVAL;
+ }
+}
+
const struct ethtool_ops alx_ethtool_ops = {
.get_settings = alx_get_settings,
.set_settings = alx_set_settings,
@@ -209,4 +307,7 @@ const struct ethtool_ops alx_ethtool_ops = {
.get_msglevel = alx_get_msglevel,
.set_msglevel = alx_set_msglevel,
.get_link = ethtool_op_get_link,
+ .get_strings = alx_get_strings,
+ .get_sset_count = alx_get_sset_count,
+ .get_ethtool_stats = alx_get_ethtool_stats,
};
diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
index 854d807..1554880 100644
--- a/drivers/net/ethernet/atheros/alx/hw.h
+++ b/drivers/net/ethernet/atheros/alx/hw.h
@@ -381,7 +381,12 @@ struct alx_rrd {
ALX_ISR_RX_Q6 | \
ALX_ISR_RX_Q7)
-/* Statistics counters collected by the MAC */
+/* Statistics counters collected by the MAC
+ *
+ * The order of the fields must match the strings in alx_gstrings_stats
+ * All stats fields should be u64
+ * See ethtool.c
+ */
struct alx_hw_stats {
/* rx */
u64 rx_ok; /* good RX packets */
--
1.8.5.2
^ permalink raw reply related
* [PATCH v4 3/5] alx: add stats update function
From: Sabrina Dubroca @ 2014-01-09 9:09 UTC (permalink / raw)
To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca
In-Reply-To: <1389258571-10083-1-git-send-email-sd@queasysnail.net>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
drivers/net/ethernet/atheros/alx/hw.c | 58 +++++++++++++++++++++++++++++++++++
drivers/net/ethernet/atheros/alx/hw.h | 4 +++
2 files changed, 62 insertions(+)
diff --git a/drivers/net/ethernet/atheros/alx/hw.c b/drivers/net/ethernet/atheros/alx/hw.c
index 1e8c24a..7712f06 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -1050,3 +1050,61 @@ bool alx_get_phy_info(struct alx_hw *hw)
return true;
}
+
+void alx_update_hw_stats(struct alx_hw *hw)
+{
+ /* RX stats */
+ hw->stats.rx_ok += alx_read_mem32(hw, ALX_MIB_RX_OK);
+ hw->stats.rx_bcast += alx_read_mem32(hw, ALX_MIB_RX_BCAST);
+ hw->stats.rx_mcast += alx_read_mem32(hw, ALX_MIB_RX_MCAST);
+ hw->stats.rx_pause += alx_read_mem32(hw, ALX_MIB_RX_PAUSE);
+ hw->stats.rx_ctrl += alx_read_mem32(hw, ALX_MIB_RX_CTRL);
+ hw->stats.rx_fcs_err += alx_read_mem32(hw, ALX_MIB_RX_FCS_ERR);
+ hw->stats.rx_len_err += alx_read_mem32(hw, ALX_MIB_RX_LEN_ERR);
+ hw->stats.rx_byte_cnt += alx_read_mem32(hw, ALX_MIB_RX_BYTE_CNT);
+ hw->stats.rx_runt += alx_read_mem32(hw, ALX_MIB_RX_RUNT);
+ hw->stats.rx_frag += alx_read_mem32(hw, ALX_MIB_RX_FRAG);
+ hw->stats.rx_sz_64B += alx_read_mem32(hw, ALX_MIB_RX_SZ_64B);
+ hw->stats.rx_sz_127B += alx_read_mem32(hw, ALX_MIB_RX_SZ_127B);
+ hw->stats.rx_sz_255B += alx_read_mem32(hw, ALX_MIB_RX_SZ_255B);
+ hw->stats.rx_sz_511B += alx_read_mem32(hw, ALX_MIB_RX_SZ_511B);
+ hw->stats.rx_sz_1023B += alx_read_mem32(hw, ALX_MIB_RX_SZ_1023B);
+ hw->stats.rx_sz_1518B += alx_read_mem32(hw, ALX_MIB_RX_SZ_1518B);
+ hw->stats.rx_sz_max += alx_read_mem32(hw, ALX_MIB_RX_SZ_MAX);
+ hw->stats.rx_ov_sz += alx_read_mem32(hw, ALX_MIB_RX_OV_SZ);
+ hw->stats.rx_ov_rxf += alx_read_mem32(hw, ALX_MIB_RX_OV_RXF);
+ hw->stats.rx_ov_rrd += alx_read_mem32(hw, ALX_MIB_RX_OV_RRD);
+ hw->stats.rx_align_err += alx_read_mem32(hw, ALX_MIB_RX_ALIGN_ERR);
+ hw->stats.rx_bc_byte_cnt += alx_read_mem32(hw, ALX_MIB_RX_BCCNT);
+ hw->stats.rx_mc_byte_cnt += alx_read_mem32(hw, ALX_MIB_RX_MCCNT);
+ hw->stats.rx_err_addr += alx_read_mem32(hw, ALX_MIB_RX_ERRADDR);
+
+ /* TX stats */
+ hw->stats.tx_ok += alx_read_mem32(hw, ALX_MIB_TX_OK);
+ hw->stats.tx_bcast += alx_read_mem32(hw, ALX_MIB_TX_BCAST);
+ hw->stats.tx_mcast += alx_read_mem32(hw, ALX_MIB_TX_MCAST);
+ hw->stats.tx_pause += alx_read_mem32(hw, ALX_MIB_TX_PAUSE);
+ hw->stats.tx_exc_defer += alx_read_mem32(hw, ALX_MIB_TX_EXC_DEFER);
+ hw->stats.tx_ctrl += alx_read_mem32(hw, ALX_MIB_TX_CTRL);
+ hw->stats.tx_defer += alx_read_mem32(hw, ALX_MIB_TX_DEFER);
+ hw->stats.tx_byte_cnt += alx_read_mem32(hw, ALX_MIB_TX_BYTE_CNT);
+ hw->stats.tx_sz_64B += alx_read_mem32(hw, ALX_MIB_TX_SZ_64B);
+ hw->stats.tx_sz_127B += alx_read_mem32(hw, ALX_MIB_TX_SZ_127B);
+ hw->stats.tx_sz_255B += alx_read_mem32(hw, ALX_MIB_TX_SZ_255B);
+ hw->stats.tx_sz_511B += alx_read_mem32(hw, ALX_MIB_TX_SZ_511B);
+ hw->stats.tx_sz_1023B += alx_read_mem32(hw, ALX_MIB_TX_SZ_1023B);
+ hw->stats.tx_sz_1518B += alx_read_mem32(hw, ALX_MIB_TX_SZ_1518B);
+ hw->stats.tx_sz_max += alx_read_mem32(hw, ALX_MIB_TX_SZ_MAX);
+ hw->stats.tx_single_col += alx_read_mem32(hw, ALX_MIB_TX_SINGLE_COL);
+ hw->stats.tx_multi_col += alx_read_mem32(hw, ALX_MIB_TX_MULTI_COL);
+ hw->stats.tx_late_col += alx_read_mem32(hw, ALX_MIB_TX_LATE_COL);
+ hw->stats.tx_abort_col += alx_read_mem32(hw, ALX_MIB_TX_ABORT_COL);
+ hw->stats.tx_underrun += alx_read_mem32(hw, ALX_MIB_TX_UNDERRUN);
+ hw->stats.tx_trd_eop += alx_read_mem32(hw, ALX_MIB_TX_TRD_EOP);
+ hw->stats.tx_len_err += alx_read_mem32(hw, ALX_MIB_TX_LEN_ERR);
+ hw->stats.tx_trunc += alx_read_mem32(hw, ALX_MIB_TX_TRUNC);
+ hw->stats.tx_bc_byte_cnt += alx_read_mem32(hw, ALX_MIB_TX_BCCNT);
+ hw->stats.tx_mc_byte_cnt += alx_read_mem32(hw, ALX_MIB_TX_MCCNT);
+
+ hw->stats.update += alx_read_mem32(hw, ALX_MIB_UPDATE);
+}
diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
index 872b362..854d807 100644
--- a/drivers/net/ethernet/atheros/alx/hw.h
+++ b/drivers/net/ethernet/atheros/alx/hw.h
@@ -486,6 +486,9 @@ struct alx_hw {
/* PHY link patch flag */
bool lnk_patch;
+
+ /* cumulated stats from the hardware (registers are cleared on read) */
+ struct alx_hw_stats stats;
};
static inline int alx_hw_revision(struct alx_hw *hw)
@@ -553,6 +556,7 @@ bool alx_phy_configured(struct alx_hw *hw);
void alx_configure_basic(struct alx_hw *hw);
void alx_disable_rss(struct alx_hw *hw);
bool alx_get_phy_info(struct alx_hw *hw);
+void alx_update_hw_stats(struct alx_hw *hw);
static inline u32 alx_speed_to_ethadv(int speed, u8 duplex)
{
--
1.8.5.2
^ permalink raw reply related
* [PATCH v4 2/5] alx: add constants for the stats fields
From: Sabrina Dubroca @ 2014-01-09 9:09 UTC (permalink / raw)
To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca
In-Reply-To: <1389258571-10083-1-git-send-email-sd@queasysnail.net>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
drivers/net/ethernet/atheros/alx/reg.h | 52 +++++++++++++++++++++++++++++++---
1 file changed, 48 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/atheros/alx/reg.h b/drivers/net/ethernet/atheros/alx/reg.h
index e4358c9..af006b4 100644
--- a/drivers/net/ethernet/atheros/alx/reg.h
+++ b/drivers/net/ethernet/atheros/alx/reg.h
@@ -404,15 +404,59 @@
/* MIB */
#define ALX_MIB_BASE 0x1700
+
#define ALX_MIB_RX_OK (ALX_MIB_BASE + 0)
+#define ALX_MIB_RX_BCAST (ALX_MIB_BASE + 4)
+#define ALX_MIB_RX_MCAST (ALX_MIB_BASE + 8)
+#define ALX_MIB_RX_PAUSE (ALX_MIB_BASE + 12)
+#define ALX_MIB_RX_CTRL (ALX_MIB_BASE + 16)
+#define ALX_MIB_RX_FCS_ERR (ALX_MIB_BASE + 20)
+#define ALX_MIB_RX_LEN_ERR (ALX_MIB_BASE + 24)
+#define ALX_MIB_RX_BYTE_CNT (ALX_MIB_BASE + 28)
+#define ALX_MIB_RX_RUNT (ALX_MIB_BASE + 32)
+#define ALX_MIB_RX_FRAG (ALX_MIB_BASE + 36)
+#define ALX_MIB_RX_SZ_64B (ALX_MIB_BASE + 40)
+#define ALX_MIB_RX_SZ_127B (ALX_MIB_BASE + 44)
+#define ALX_MIB_RX_SZ_255B (ALX_MIB_BASE + 48)
+#define ALX_MIB_RX_SZ_511B (ALX_MIB_BASE + 52)
+#define ALX_MIB_RX_SZ_1023B (ALX_MIB_BASE + 56)
+#define ALX_MIB_RX_SZ_1518B (ALX_MIB_BASE + 60)
+#define ALX_MIB_RX_SZ_MAX (ALX_MIB_BASE + 64)
+#define ALX_MIB_RX_OV_SZ (ALX_MIB_BASE + 68)
+#define ALX_MIB_RX_OV_RXF (ALX_MIB_BASE + 72)
+#define ALX_MIB_RX_OV_RRD (ALX_MIB_BASE + 76)
+#define ALX_MIB_RX_ALIGN_ERR (ALX_MIB_BASE + 80)
+#define ALX_MIB_RX_BCCNT (ALX_MIB_BASE + 84)
+#define ALX_MIB_RX_MCCNT (ALX_MIB_BASE + 88)
#define ALX_MIB_RX_ERRADDR (ALX_MIB_BASE + 92)
+
#define ALX_MIB_TX_OK (ALX_MIB_BASE + 96)
+#define ALX_MIB_TX_BCAST (ALX_MIB_BASE + 100)
+#define ALX_MIB_TX_MCAST (ALX_MIB_BASE + 104)
+#define ALX_MIB_TX_PAUSE (ALX_MIB_BASE + 108)
+#define ALX_MIB_TX_EXC_DEFER (ALX_MIB_BASE + 112)
+#define ALX_MIB_TX_CTRL (ALX_MIB_BASE + 116)
+#define ALX_MIB_TX_DEFER (ALX_MIB_BASE + 120)
+#define ALX_MIB_TX_BYTE_CNT (ALX_MIB_BASE + 124)
+#define ALX_MIB_TX_SZ_64B (ALX_MIB_BASE + 128)
+#define ALX_MIB_TX_SZ_127B (ALX_MIB_BASE + 132)
+#define ALX_MIB_TX_SZ_255B (ALX_MIB_BASE + 136)
+#define ALX_MIB_TX_SZ_511B (ALX_MIB_BASE + 140)
+#define ALX_MIB_TX_SZ_1023B (ALX_MIB_BASE + 144)
+#define ALX_MIB_TX_SZ_1518B (ALX_MIB_BASE + 148)
+#define ALX_MIB_TX_SZ_MAX (ALX_MIB_BASE + 152)
+#define ALX_MIB_TX_SINGLE_COL (ALX_MIB_BASE + 156)
+#define ALX_MIB_TX_MULTI_COL (ALX_MIB_BASE + 160)
+#define ALX_MIB_TX_LATE_COL (ALX_MIB_BASE + 164)
+#define ALX_MIB_TX_ABORT_COL (ALX_MIB_BASE + 168)
+#define ALX_MIB_TX_UNDERRUN (ALX_MIB_BASE + 172)
+#define ALX_MIB_TX_TRD_EOP (ALX_MIB_BASE + 176)
+#define ALX_MIB_TX_LEN_ERR (ALX_MIB_BASE + 180)
+#define ALX_MIB_TX_TRUNC (ALX_MIB_BASE + 184)
+#define ALX_MIB_TX_BCCNT (ALX_MIB_BASE + 188)
#define ALX_MIB_TX_MCCNT (ALX_MIB_BASE + 192)
+#define ALX_MIB_UPDATE (ALX_MIB_BASE + 196)
-#define ALX_RX_STATS_BIN ALX_MIB_RX_OK
-#define ALX_RX_STATS_END ALX_MIB_RX_ERRADDR
-#define ALX_TX_STATS_BIN ALX_MIB_TX_OK
-#define ALX_TX_STATS_END ALX_MIB_TX_MCCNT
#define ALX_ISR 0x1600
#define ALX_ISR_DIS BIT(31)
--
1.8.5.2
^ permalink raw reply related
* [PATCH v4 1/5] alx: add a hardware stats structure
From: Sabrina Dubroca @ 2014-01-09 9:09 UTC (permalink / raw)
To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca
In-Reply-To: <1389258571-10083-1-git-send-email-sd@queasysnail.net>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
drivers/net/ethernet/atheros/alx/hw.h | 62 +++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
index 96f3b43..872b362 100644
--- a/drivers/net/ethernet/atheros/alx/hw.h
+++ b/drivers/net/ethernet/atheros/alx/hw.h
@@ -381,6 +381,68 @@ struct alx_rrd {
ALX_ISR_RX_Q6 | \
ALX_ISR_RX_Q7)
+/* Statistics counters collected by the MAC */
+struct alx_hw_stats {
+ /* rx */
+ u64 rx_ok; /* good RX packets */
+ u64 rx_bcast; /* good RX broadcast packets */
+ u64 rx_mcast; /* good RX multicast packets */
+ u64 rx_pause; /* RX pause frames */
+ u64 rx_ctrl; /* RX control packets other than pause frames */
+ u64 rx_fcs_err; /* RX packets with bad FCS */
+ u64 rx_len_err; /* RX packets with length != actual size */
+ u64 rx_byte_cnt; /* good bytes received. FCS is NOT included */
+ u64 rx_runt; /* RX packets < 64 bytes with good FCS */
+ u64 rx_frag; /* RX packets < 64 bytes with bad FCS */
+ u64 rx_sz_64B; /* 64 byte RX packets */
+ u64 rx_sz_127B; /* 65-127 byte RX packets */
+ u64 rx_sz_255B; /* 128-255 byte RX packets */
+ u64 rx_sz_511B; /* 256-511 byte RX packets */
+ u64 rx_sz_1023B; /* 512-1023 byte RX packets */
+ u64 rx_sz_1518B; /* 1024-1518 byte RX packets */
+ u64 rx_sz_max; /* 1519 byte to MTU RX packets */
+ u64 rx_ov_sz; /* truncated RX packets, size > MTU */
+ u64 rx_ov_rxf; /* frames dropped due to RX FIFO overflow */
+ u64 rx_ov_rrd; /* frames dropped due to RRD overflow */
+ u64 rx_align_err; /* alignment errors */
+ u64 rx_bc_byte_cnt; /* RX broadcast bytes, excluding FCS */
+ u64 rx_mc_byte_cnt; /* RX multicast bytes, excluding FCS */
+ u64 rx_err_addr; /* packets dropped due to address filtering */
+
+ /* tx */
+ u64 tx_ok; /* good TX packets */
+ u64 tx_bcast; /* good TX broadcast packets */
+ u64 tx_mcast; /* good TX multicast packets */
+ u64 tx_pause; /* TX pause frames */
+ u64 tx_exc_defer; /* TX packets deferred excessively */
+ u64 tx_ctrl; /* TX control frames, excluding pause frames */
+ u64 tx_defer; /* TX packets deferred */
+ u64 tx_byte_cnt; /* bytes transmitted, FCS is NOT included */
+ u64 tx_sz_64B; /* 64 byte TX packets */
+ u64 tx_sz_127B; /* 65-127 byte TX packets */
+ u64 tx_sz_255B; /* 128-255 byte TX packets */
+ u64 tx_sz_511B; /* 256-511 byte TX packets */
+ u64 tx_sz_1023B; /* 512-1023 byte TX packets */
+ u64 tx_sz_1518B; /* 1024-1518 byte TX packets */
+ u64 tx_sz_max; /* 1519 byte to MTU TX packets */
+ u64 tx_single_col; /* packets TX after a single collision */
+ u64 tx_multi_col; /* packets TX after multiple collisions */
+ u64 tx_late_col; /* TX packets with late collisions */
+ u64 tx_abort_col; /* TX packets aborted w/excessive collisions */
+ u64 tx_underrun; /* TX packets aborted due to TX FIFO underrun
+ * or TRD FIFO underrun
+ */
+ u64 tx_trd_eop; /* reads beyond the EOP into the next frame
+ * when TRD was not written timely
+ */
+ u64 tx_len_err; /* TX packets where length != actual size */
+ u64 tx_trunc; /* TX packets truncated due to size > MTU */
+ u64 tx_bc_byte_cnt; /* broadcast bytes transmitted, excluding FCS */
+ u64 tx_mc_byte_cnt; /* multicast bytes transmitted, excluding FCS */
+ u64 update;
+};
+
+
/* maximum interrupt vectors for msix */
#define ALX_MAX_MSIX_INTRS 16
--
1.8.5.2
^ permalink raw reply related
* [PATCH v4 0/5] alx: add statistics
From: Sabrina Dubroca @ 2014-01-09 9:09 UTC (permalink / raw)
To: davem; +Cc: bhutchings, johannes, stephen, netdev, Sabrina Dubroca
Currently, the alx driver doesn't support statistics [1,2]. The
original alx driver [3] that Johannes Berg modified provided
statistics. This patch is an adaptation of the statistics code from
the original driver to the alx driver included in the kernel.
v4:
- modified the assignements of hw stats to netstats (Ben Hutchings)
- added comments to describe the stats fields (copied from atlx)
v3:
- renamed __alx_update_hw_stats to alx_update_hw_stats (Stephen Hemminger)
v2:
- use u64 instead of unsigned long (Ben Hutchings)
- implement ndo_get_stats64 instead of ndo_get_stats (Ben Hutchings)
- use EINVAL instead of ENOTSUPP (Ben Hutchings)
- add BUILD_BUG_ON to check the size of the stats (Johannes Berg, Ben
Hutchings)
- add a comment regarding persistence of the stats (Stephen Hemminger)
- align assignments in __alx_update_hw_stats
[1] https://bugzilla.kernel.org/show_bug.cgi?id=63401
[2] http://www.spinics.net/lists/netdev/msg245544.html
[3] https://github.com/mcgrof/alx
Sabrina Dubroca (5):
alx: add a hardware stats structure
alx: add constants for the stats fields
alx: add stats update function
alx: add alx_get_stats64 operation
alx: add stats to ethtool
drivers/net/ethernet/atheros/alx/alx.h | 3 +
drivers/net/ethernet/atheros/alx/ethtool.c | 101 +++++++++++++++++++++++++++++
drivers/net/ethernet/atheros/alx/hw.c | 58 +++++++++++++++++
drivers/net/ethernet/atheros/alx/hw.h | 71 ++++++++++++++++++++
drivers/net/ethernet/atheros/alx/main.c | 50 ++++++++++++++
drivers/net/ethernet/atheros/alx/reg.h | 52 +++++++++++++--
6 files changed, 331 insertions(+), 4 deletions(-)
--
1.8.5.2
^ permalink raw reply
* RE: [PATCH net-next 0/2] Pack struct xfrm_usersa_info and struct xfrm_userpolicy_info
From: David Laight @ 2014-01-09 9:07 UTC (permalink / raw)
To: 'Fan Du'
Cc: Steffen Klassert, davem@davemloft.net, stephen@networkplumber.org,
dev@lists.strongswan.org, netdev@vger.kernel.org
In-Reply-To: <52CE5F21.1070800@windriver.com>
> From: Fan Du
> > Packing the structures is definitely wrong. Some 32bit systems (IIRC sparc)
> > align 64bit items on 8 byte boundaries. Not to mention the expensive byte
> > by byte accesses this forces on some systems.
>
> I don't know much about sparc, if I read your message right, you mean 32bit sparc
> system also has padding even if pack attribute is supplied.
Without packing:
On 32bit x86 64bit integers are aligned on 4byte boundaries.
On 32bit sparc 64bit integers are aligned on 8byte boundaries.
If the structure is packed then, for sparc (etc), the compiler has
to generate code to read everything using byte accesses.
David
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Michael Dalton @ 2014-01-09 9:02 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, lf-virt, Eric Dumazet, David S. Miller
In-Reply-To: <CANJ5vPJp-AwvFDtY1R7SJg6L2dkEU6xPivgKcg4f0+05JCMEfw@mail.gmail.com>
If the prior code snippet looks good to you, I'll use something like
that as a baseline for a v3 patchset. I don't think we need a stricter
alignment than 64 to express values in the range (1536 ... 4096), as
the code snippet shows, which is great for x86 4KB pages.
On other architectures that have larger page sizes > 4KB with <= 64b
cachelines, we may want to increase the alignment so that the max buffer
size will be >= PAGE_SIZE (max size allowed by skb_page_frag_refill).
If we use a minimum alignment of 128, our maximum theoretical
packet buffer length is 1536 + 127 * 128 = 17792. With 256 byte
alignment, we can express a maximum packet buffer size > 65536.
Given the above, I think we want to select the min buffer alignment
based on the PAGE_SIZE:
<= 4KB PAGE_SIZE: 64b min alignment
<= 16KB PAGE_SIZE: 128b min alignment
> 16KB PAGE_SIZE: 256b min alignment
So the prior code snippet would be relatively unchanged, except that
references to the previous minimum alignment of 64 would be replaced by
a #define'd constant derived from PAGE_SIZE as shown above.
This would guarantee that we use the minimum alignment necessary to
ensure that virtio-net can post a max size (PAGE_SIZE) buffer, and for
x86 this means we won't increase the alignment beyond the x86's current
L1_CACHE_BYTES value (64). Also, sorry I haven't had a chance to respond
yet to the debugfs feedback, I will get to that soon (just wanted to do
a further deep dive on some of the sysfs/debugfs tradeoffs).
Best,
Mike
^ permalink raw reply
* [PATCH net-next v2 3/3] ipv4: introduce hardened ip_no_pmtu_disc mode
From: hannes @ 2014-01-09 9:01 UTC (permalink / raw)
To: netdev
Cc: eric.dumazet, davem, johnwheffner, steffen.klassert, fweimer,
Hannes Frederic Sowa
In-Reply-To: <1389258077-23282-1-git-send-email-hannes@stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
This new ip_no_pmtu_disc mode only allowes fragmentation-needed errors
to be honored by protocols which do more stringent validation on the
ICMP's packet payload. This knob is useful for people who e.g. want to
run an unmodified DNS server in a namespace where they need to use pmtu
for TCP connections (as they are used for zone transfers or fallback
for requests) but don't want to use possibly spoofed UDP pmtu information.
Currently the whitelisted protocols are TCP, SCTP and DCCP as they check
if the returned packet is in the window or if the association is valid.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Miller <davem@davemloft.net>
Cc: John Heffner <johnwheffner@gmail.com>
Suggested-by: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
Documentation/networking/ip-sysctl.txt | 13 ++++++++++++-
include/net/protocol.h | 7 ++++++-
net/dccp/ipv4.c | 1 +
net/ipv4/af_inet.c | 1 +
net/ipv4/icmp.c | 28 ++++++++++++++++++++++++----
net/sctp/protocol.c | 1 +
6 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 0d71fa9..c97932c 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -26,7 +26,18 @@ ip_no_pmtu_disc - INTEGER
discarded. Outgoing frames are handled the same as in mode 1,
implicitly setting IP_PMTUDISC_DONT on every created socket.
- Possible values: 0-2
+ Mode 3 is a hardend pmtu discover mode. The kernel will only
+ accept fragmentation-needed errors if the underlying protocol
+ can verify them besides a plain socket lookup. Current
+ protocols for which pmtu events will be honored are TCP, SCTP
+ and DCCP as they verify e.g. the sequence number or the
+ association. This mode should not be enabled globally but is
+ only intended to secure e.g. name servers in namespaces where
+ TCP path mtu must still work but path MTU information of other
+ protocols should be discarded. If enabled globally this mode
+ could break other protocols.
+
+ Possible values: 0-3
Default: FALSE
min_pmtu - INTEGER
diff --git a/include/net/protocol.h b/include/net/protocol.h
index fbf7676..0e5f866 100644
--- a/include/net/protocol.h
+++ b/include/net/protocol.h
@@ -43,7 +43,12 @@ struct net_protocol {
int (*handler)(struct sk_buff *skb);
void (*err_handler)(struct sk_buff *skb, u32 info);
unsigned int no_policy:1,
- netns_ok:1;
+ netns_ok:1,
+ /* does the protocol do more stringent
+ * icmp tag validation than simple
+ * socket lookup?
+ */
+ icmp_strict_tag_validation:1;
};
#if IS_ENABLED(CONFIG_IPV6)
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 88299c2..22b5d81 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -989,6 +989,7 @@ static const struct net_protocol dccp_v4_protocol = {
.err_handler = dccp_v4_err,
.no_policy = 1,
.netns_ok = 1,
+ .icmp_strict_tag_validation = 1,
};
static const struct proto_ops inet_dccp_ops = {
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 6268a47..ecd2c3f 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1545,6 +1545,7 @@ static const struct net_protocol tcp_protocol = {
.err_handler = tcp_v4_err,
.no_policy = 1,
.netns_ok = 1,
+ .icmp_strict_tag_validation = 1,
};
static const struct net_protocol udp_protocol = {
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index fb3c563..0134663 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -668,6 +668,16 @@ static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
rcu_read_unlock();
}
+static bool icmp_tag_validation(int proto)
+{
+ bool ok;
+
+ rcu_read_lock();
+ ok = rcu_dereference(inet_protos[proto])->icmp_strict_tag_validation;
+ rcu_read_unlock();
+ return ok;
+}
+
/*
* Handle ICMP_DEST_UNREACH, ICMP_TIME_EXCEED, ICMP_QUENCH, and
* ICMP_PARAMETERPROB.
@@ -705,12 +715,22 @@ static void icmp_unreach(struct sk_buff *skb)
case ICMP_PORT_UNREACH:
break;
case ICMP_FRAG_NEEDED:
- if (net->ipv4.sysctl_ip_no_pmtu_disc == 2) {
- goto out;
- } else if (net->ipv4.sysctl_ip_no_pmtu_disc) {
+ /* for documentation of the ip_no_pmtu_disc
+ * values please see
+ * Documentation/networking/ip-sysctl.txt
+ */
+ switch (net->ipv4.sysctl_ip_no_pmtu_disc) {
+ default:
LIMIT_NETDEBUG(KERN_INFO pr_fmt("%pI4: fragmentation needed and DF set\n"),
&iph->daddr);
- } else {
+ break;
+ case 2:
+ goto out;
+ case 3:
+ if (!icmp_tag_validation(iph->protocol))
+ goto out;
+ /* fall through */
+ case 0:
info = ntohs(icmph->un.frag.mtu);
if (!info)
goto out;
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 34b7726..7c16108 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1030,6 +1030,7 @@ static const struct net_protocol sctp_protocol = {
.err_handler = sctp_v4_err,
.no_policy = 1,
.netns_ok = 1,
+ .icmp_strict_tag_validation = 1,
};
/* IPv4 address related functions. */
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next v3 2/3] ipv6: introduce ip6_dst_mtu_forward and protect forwarding path with it
From: hannes @ 2014-01-09 9:01 UTC (permalink / raw)
To: netdev
Cc: eric.dumazet, davem, johnwheffner, steffen.klassert, fweimer,
Hannes Frederic Sowa
In-Reply-To: <1389258077-23282-1-git-send-email-hannes@stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
In the IPv6 forwarding path we are only concerend about the outgoing
interface MTU, but also respect locked MTUs on routes. Tunnel provider
or IPSEC already have to recheck and if needed send PtB notifications
to the sending host in case the data does not fit into the packet with
added headers (we only know the final header sizes there, while also
using path MTU information).
The reason for this change is, that path MTU information can be injected
into the kernel via e.g. icmp_err protocol handler without verification
of local sockets. As such, this could cause the IPv6 forwarding path to
wrongfully emit Packet-too-Big errors and drop IPv6 packets.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Miller <davem@davemloft.net>
Cc: John Heffner <johnwheffner@gmail.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/ip6_output.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index d1de956..ef02b26 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -321,6 +321,27 @@ static inline int ip6_forward_finish(struct sk_buff *skb)
return dst_output(skb);
}
+static unsigned int ip6_dst_mtu_forward(const struct dst_entry *dst)
+{
+ unsigned int mtu;
+ struct inet6_dev *idev;
+
+ if (dst_metric_locked(dst, RTAX_MTU)) {
+ mtu = dst_metric_raw(dst, RTAX_MTU);
+ if (mtu)
+ return mtu;
+ }
+
+ mtu = IPV6_MIN_MTU;
+ rcu_read_lock();
+ idev = __in6_dev_get(dst->dev);
+ if (idev)
+ mtu = idev->cnf.mtu6;
+ rcu_read_unlock();
+
+ return mtu;
+}
+
int ip6_forward(struct sk_buff *skb)
{
struct dst_entry *dst = skb_dst(skb);
@@ -441,7 +462,7 @@ int ip6_forward(struct sk_buff *skb)
}
}
- mtu = dst_mtu(dst);
+ mtu = ip6_dst_mtu_forward(dst);
if (mtu < IPV6_MIN_MTU)
mtu = IPV6_MIN_MTU;
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next v4 1/3] ipv4: introduce ip_dst_mtu_maybe_forward and protect forwarding path against pmtu spoofing
From: hannes @ 2014-01-09 9:01 UTC (permalink / raw)
To: netdev
Cc: eric.dumazet, davem, johnwheffner, steffen.klassert, fweimer,
Hannes Frederic Sowa
In-Reply-To: <1389258077-23282-1-git-send-email-hannes@stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
While forwarding we should not use the protocol path mtu to calculate
the mtu for a forwarded packet but instead use the interface mtu.
We mark forwarded skbs in ip_forward with IPSKB_FORWARDED, which was
introduced for multicast forwarding. But as it does not conflict with
our usage in unicast code path it is perfect for reuse.
I moved the functions ip_sk_accept_pmtu, ip_sk_use_pmtu and ip_skb_dst_mtu
along with the new ip_dst_mtu_maybe_forward to net/ip.h to fix circular
dependencies because of IPSKB_FORWARDED.
Because someone might have written a software which does probe
destinations manually and expects the kernel to honour those path mtus
I introduced a new per-namespace "ip_forward_use_pmtu" knob so someone
can disable this new behaviour. We also still use mtus which are locked on a
route for forwarding.
The reason for this change is, that path mtus information can be injected
into the kernel via e.g. icmp_err protocol handler without verification
of local sockets. As such, this could cause the IPv4 forwarding path to
wrongfully emit fragmentation needed notifications or start to fragment
packets along a path.
Tunnel and ipsec output paths clear IPCB again, thus IPSKB_FORWARDED
won't be set and further fragmentation logic will use the path mtu to
determine the fragmentation size. They also recheck packet size with
help of path mtu discovery and report appropriate errors.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Miller <davem@davemloft.net>
Cc: John Heffner <johnwheffner@gmail.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
Documentation/networking/ip-sysctl.txt | 13 +++++++++++++
include/net/ip.h | 33 +++++++++++++++++++++++++++++++++
include/net/netns/ipv4.h | 1 +
include/net/route.h | 19 +++----------------
net/ipv4/ip_forward.c | 7 +++++--
net/ipv4/ip_output.c | 8 +++++---
net/ipv4/route.c | 3 ---
net/ipv4/sysctl_net_ipv4.c | 7 +++++++
8 files changed, 67 insertions(+), 24 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 7373115..0d71fa9 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -32,6 +32,19 @@ ip_no_pmtu_disc - INTEGER
min_pmtu - INTEGER
default 552 - minimum discovered Path MTU
+ip_forward_use_pmtu - BOOLEAN
+ By default we don't trust protocol path MTUs while forwarding
+ because they could be easily forged and can lead to unwanted
+ fragmentation by the router.
+ You only need to enable this if you have user-space software
+ which tries to discover path mtus by itself and depends on the
+ kernel honoring this information. This is normally not the
+ case.
+ Default: 0 (disabled)
+ Possible values:
+ 0 - disabled
+ 1 - enabled
+
route/max_size - INTEGER
Maximum number of routes allowed in the kernel. Increase
this when using large numbers of interfaces and/or routes.
diff --git a/include/net/ip.h b/include/net/ip.h
index 5356644..0dab95c 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -263,6 +263,39 @@ int ip_dont_fragment(struct sock *sk, struct dst_entry *dst)
!(dst_metric_locked(dst, RTAX_MTU)));
}
+static inline bool ip_sk_accept_pmtu(const struct sock *sk)
+{
+ return inet_sk(sk)->pmtudisc != IP_PMTUDISC_INTERFACE;
+}
+
+static inline bool ip_sk_use_pmtu(const struct sock *sk)
+{
+ return inet_sk(sk)->pmtudisc < IP_PMTUDISC_PROBE;
+}
+
+static inline unsigned int ip_dst_mtu_maybe_forward(const struct dst_entry *dst,
+ bool forwarding)
+{
+ struct net *net = dev_net(dst->dev);
+
+ if (net->ipv4.sysctl_ip_fwd_use_pmtu ||
+ dst_metric_locked(dst, RTAX_MTU) ||
+ !forwarding)
+ return dst_mtu(dst);
+
+ return min(dst->dev->mtu, IP_MAX_MTU);
+}
+
+static inline unsigned int ip_skb_dst_mtu(const struct sk_buff *skb)
+{
+ if (!skb->sk || ip_sk_use_pmtu(skb->sk)) {
+ bool forwarding = IPCB(skb)->flags & IPSKB_FORWARDED;
+ return ip_dst_mtu_maybe_forward(skb_dst(skb), forwarding);
+ } else {
+ return min(skb_dst(skb)->dev->mtu, IP_MAX_MTU);
+ }
+}
+
void __ip_select_ident(struct iphdr *iph, struct dst_entry *dst, int more);
static inline void ip_select_ident(struct sk_buff *skb, struct dst_entry *dst, struct sock *sk)
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 929a668..80f500a 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -70,6 +70,7 @@ struct netns_ipv4 {
int sysctl_tcp_ecn;
int sysctl_ip_no_pmtu_disc;
+ int sysctl_ip_fwd_use_pmtu;
kgid_t sysctl_ping_group_range[2];
diff --git a/include/net/route.h b/include/net/route.h
index 638e3eb..9d1f423 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -36,6 +36,9 @@
#include <linux/cache.h>
#include <linux/security.h>
+/* IPv4 datagram length is stored into 16bit field (tot_len) */
+#define IP_MAX_MTU 0xFFFFU
+
#define RTO_ONLINK 0x01
#define RT_CONN_FLAGS(sk) (RT_TOS(inet_sk(sk)->tos) | sock_flag(sk, SOCK_LOCALROUTE))
@@ -311,20 +314,4 @@ static inline int ip4_dst_hoplimit(const struct dst_entry *dst)
return hoplimit;
}
-static inline bool ip_sk_accept_pmtu(const struct sock *sk)
-{
- return inet_sk(sk)->pmtudisc != IP_PMTUDISC_INTERFACE;
-}
-
-static inline bool ip_sk_use_pmtu(const struct sock *sk)
-{
- return inet_sk(sk)->pmtudisc < IP_PMTUDISC_PROBE;
-}
-
-static inline int ip_skb_dst_mtu(const struct sk_buff *skb)
-{
- return (!skb->sk || ip_sk_use_pmtu(skb->sk)) ?
- dst_mtu(skb_dst(skb)) : skb_dst(skb)->dev->mtu;
-}
-
#endif /* _ROUTE_H */
diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
index 694de3b..e9f1217 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -54,6 +54,7 @@ static int ip_forward_finish(struct sk_buff *skb)
int ip_forward(struct sk_buff *skb)
{
+ u32 mtu;
struct iphdr *iph; /* Our header */
struct rtable *rt; /* Route we use */
struct ip_options *opt = &(IPCB(skb)->opt);
@@ -88,11 +89,13 @@ int ip_forward(struct sk_buff *skb)
if (opt->is_strictroute && rt->rt_uses_gateway)
goto sr_failed;
- if (unlikely(skb->len > dst_mtu(&rt->dst) && !skb_is_gso(skb) &&
+ IPCB(skb)->flags |= IPSKB_FORWARDED;
+ mtu = ip_dst_mtu_maybe_forward(&rt->dst, true);
+ if (unlikely(skb->len > mtu && !skb_is_gso(skb) &&
(ip_hdr(skb)->frag_off & htons(IP_DF))) && !skb->local_df) {
IP_INC_STATS(dev_net(rt->dst.dev), IPSTATS_MIB_FRAGFAILS);
icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
- htonl(dst_mtu(&rt->dst)));
+ htonl(mtu));
goto drop;
}
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index df18461..9a78804 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -449,6 +449,7 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
__be16 not_last_frag;
struct rtable *rt = skb_rtable(skb);
int err = 0;
+ bool forwarding = IPCB(skb)->flags & IPSKB_FORWARDED;
dev = rt->dst.dev;
@@ -458,12 +459,13 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
iph = ip_hdr(skb);
+ mtu = ip_dst_mtu_maybe_forward(&rt->dst, forwarding);
if (unlikely(((iph->frag_off & htons(IP_DF)) && !skb->local_df) ||
(IPCB(skb)->frag_max_size &&
- IPCB(skb)->frag_max_size > dst_mtu(&rt->dst)))) {
+ IPCB(skb)->frag_max_size > mtu))) {
IP_INC_STATS(dev_net(dev), IPSTATS_MIB_FRAGFAILS);
icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
- htonl(ip_skb_dst_mtu(skb)));
+ htonl(mtu));
kfree_skb(skb);
return -EMSGSIZE;
}
@@ -473,7 +475,7 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
*/
hlen = iph->ihl * 4;
- mtu = dst_mtu(&rt->dst) - hlen; /* Size of data space */
+ mtu = mtu - hlen; /* Size of data space */
#ifdef CONFIG_BRIDGE_NETFILTER
if (skb->nf_bridge)
mtu -= nf_bridge_mtu_reduction(skb);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index f8da282..25071b4 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -112,9 +112,6 @@
#define RT_FL_TOS(oldflp4) \
((oldflp4)->flowi4_tos & (IPTOS_RT_MASK | RTO_ONLINK))
-/* IPv4 datagram length is stored into 16bit field (tot_len) */
-#define IP_MAX_MTU 0xFFFF
-
#define RT_GC_TIMEOUT (300*HZ)
static int ip_rt_max_size;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 1d2480a..44eba05 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -831,6 +831,13 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec
},
+ {
+ .procname = "ip_forward_use_pmtu",
+ .data = &init_net.ipv4.sysctl_ip_fwd_use_pmtu,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
{ }
};
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next v4 0/3] path mtu hardening patches
From: hannes @ 2014-01-09 9:01 UTC (permalink / raw)
To: netdev; +Cc: eric.dumazet, davem, johnwheffner, steffen.klassert, fweimer
Hi all!
After a lot of back and forth I want to propose these changes regarding
path mtu hardening and give an outline why I think this is the best way
how to proceed:
This set contains the following patches:
* ipv4: introduce ip_dst_mtu_maybe_forward and protect forwarding path against pmtu spoofing
* ipv6: introduce ip6_dst_mtu_forward and protect forwarding path with it
* ipv4: introduce hardened ip_no_pmtu_disc mode
The first one switches the forwarding path of IPv4 to use the interface
mtu by default and ignore a possible discovered path mtu. It provides
a sysctl to switch back to the original behavior (see discussion below).
The second patch does the same thing unconditionally for IPv6. I don't
provide a knob for IPv6 to switch to original behavior (please see
below).
The third patch introduces a hardened pmtu mode, where only pmtu
information are accepted where the protocol is able to do more stringent
checks on the icmp piggyback payload (please see the patch commit msg
for further details).
Why is this change necessary?
First of all, RFC 1191 4. Router specification says:
"When a router is unable to forward a datagram because it exceeds the
MTU of the next-hop network and its Don't Fragment bit is set, the
router is required to return an ICMP Destination Unreachable message
to the source of the datagram, with the Code indicating
"fragmentation needed and DF set". ..."
For some time now fragmentation has been considered problematic, e.g.:
* http://www.hpl.hp.com/techreports/Compaq-DEC/WRL-87-3.pdf
* http://tools.ietf.org/search/rfc4963
Most of them seem to agree that fragmentation should be avoided because
of efficiency, data corruption or security concerns.
Recently it was shown possible that correctly guessing IP ids could lead
to data injection on DNS packets:
<https://sites.google.com/site/hayashulman/files/fragmentation-poisoning.pdf>
While we can try to completly stop fragmentation on the end host
(this is e.g. implemented via IP_PMTUDISC_INTERFACE), we cannot stop
fragmentation completly on the forwarding path. On the end host the
application has to deal with MTUs and has to choose fallback methods
if fragmentation could be an attack vector. This is already the case for
most DNS software, where a maximum UDP packet size can be configured. But
until recently they had no control over local fragmentation and could
thus emit fragmented packets.
On the forwarding path we can just try to delay the fragmentation to
the last hop where this is really necessary. Current kernel already does
that but only because routers don't receive feedback of path mtus, these are
only send back to the end host system. But it is possible to maliciously
insert path mtu inforamtion via ICMP packets which have an icmp echo_reply
payload, because we cannot validate those notifications against local
sockets. DHCP clients which establish an any-bound RAW-socket could also
start processing unwanted fragmentation-needed packets.
Why does IPv4 has a knob to revert to old behavior while IPv6 doesn't?
IPv4 does fragmentation on the path while IPv6 does always respond with
packet-too-big errors. The interface MTU will always be greater than
the path MTU information. So we would discard packets we could actually
forward because of malicious information. After this change we would
let the hop, which really could not forward the packet, notify the host
of this problem.
IPv4 allowes fragmentation mid-path. In case someone does use a software
which tries to discover such paths and assumes that the kernel is handling
the discovered pmtu information automatically. This should be an extremly
rare case, but because I could not exclude the possibility this knob is
provided. Also this software could insert non-locked mtu information
into the kernel. We cannot distinguish that from path mtu information
currently. Premature fragmentation could solve some problems in wrongly
configured networks, thus this switch is provided.
One frag-needed packet could reduce the path mtu down to 522 bytes
(route/min_pmtu).
Misc:
IPv6 neighbor discovery could advertise mtu information for an
interface. These information update the ipv6-specific interface mtu and
thus get used by the forwarding path.
Tunnel and xfrm output path will still honour path mtu and also respond
with Packet-too-Big or fragmentation-needed errors if needed.
Changelog for all patches:
# ipv4: introduce ip_dst_mtu_maybe_forward and protect forwarding path against pmtu spoofing
v2)
* enabled ip_forward_use_pmtu by default
* reworded
v3)
* disabled ip_forward_use_pmtu by default
* reworded
v4)
* renamed ip_dst_mtu_secure to ip_dst_mtu_maybe_forward
* updated changelog accordingly
* removed unneeded !!(... & ...) double negations
# ipv6: introduce ip6_dst_mtu_forward and protect forwarding path with it
v2)
* by default we honour pmtu information
3)
* only honor interface mtu
* rewritten and simplified
* no knob to fall back to old mode any more
# ipv4: introduce hardened ip_no_pmtu_disc mode
v2)
* reworded Documentation
Diffstat:
Documentation/networking/ip-sysctl.txt | 26 +++++++++++++++++++++++++-
include/net/ip.h | 33 +++++++++++++++++++++++++++++++++
include/net/netns/ipv4.h | 1 +
include/net/protocol.h | 7 ++++++-
include/net/route.h | 19 +++----------------
net/dccp/ipv4.c | 1 +
net/ipv4/af_inet.c | 1 +
net/ipv4/icmp.c | 28 ++++++++++++++++++++++++----
net/ipv4/ip_forward.c | 7 +++++--
net/ipv4/ip_output.c | 8 +++++---
net/ipv4/route.c | 3 ---
net/ipv4/sysctl_net_ipv4.c | 7 +++++++
net/ipv6/ip6_output.c | 23 ++++++++++++++++++++++-
net/sctp/protocol.c | 1 +
14 files changed, 134 insertions(+), 31 deletions(-)
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH net 1/2] macvlan: forbid L2 fowarding offload for macvtap
From: Jason Wang @ 2014-01-09 8:55 UTC (permalink / raw)
To: Michael S. Tsirkin, John Fastabend
Cc: John Fastabend, Neil Horman, davem, netdev, linux-kernel,
Vlad Yasevich
In-Reply-To: <20140109071721.GD19559@redhat.com>
On 01/09/2014 03:17 PM, Michael S. Tsirkin wrote:
> On Wed, Jan 08, 2014 at 11:05:42AM -0800, John Fastabend wrote:
>> [...]
>>
>>>>> OK I think I'm finally putting all the pieces together thanks.
>>>>>
>>>>> Do you know why macvtap is setting dev->tx_queue_len by default? If you
>>>>> zero this then the noqueue_qdisc is used and the q->enqueue check in
>>>>> dev_queue_xmit will fail.
>>>> It was introduced in commit 8a35747a5d13b99e076b0222729e0caa48cb69b6
>>>> ("macvtap: Limit packet queue length") to limit the length of socket
>>>> receive queue of macvtap. But I'm not sure whether the qdisc is a
>>>> byproduct of this commit, maybe we can switch to use another name
>>>> instead of just reuse dev->tx_queue_length.
>>> You mean tx_queue_len really, right?
>>>
>>> Problem is tx_queue_len can be accessed using netlink sysfs or ioctl,
>>> so if someone uses these to control or check the # of packets that
>>> can be queued by device, this will break.
>>>
>>> How about adding ndo_set_tx_queue_len then?
>>>
>>> At some point we wanted to decouple queue length from tx_queue_length
>>> for tun as well, so that would be benefitial there as well.
>> On the receive side we need to limit the receive queue and the
>> dev->tx_queue_len value was used to do this.
>>
>> However on the tx side when dev->tx_queue_len is set the default
>> qdisc pfifo_fast or mq is used depending on if there is multiqueue
>> or not. Unless the user specifies some numtxqueues when creating
>> the macvtap device then it will be a single queue device and use
>> the pfifo_fast qdisc.
>>
>> This is the default behaviour users could zero txqueuelen when
>> they create the device because it is a stacked device with
>> NETIF_F_LLTX using the lower devices qdisc makes sense but this
>> would appear (from code inspection) to break macvtap_forward()?
>>
>> if (skb_queue_len(&q->sk.sk_receive_queue) >= dev->tx_queue_len)
>> goto drop;
>>
>> I'm not sure any of this is a problem other than its a bit
>> confusing to overload tx_queue_len for the rx_queue_len but the
>> precedent has been there for sometime.
> So how about ndo ops to access tx_queue_len then?
> This way we can set tx_queue_len to 0 and use some
> other field to store the rx_queue_len without users noticing.
> Along the lines of the below (it's a partial patch just
> to give you the idea):
>
> diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
> index 5b7d0e1..e526b46 100644
> --- a/net/core/dev_ioctl.c
> +++ b/net/core/dev_ioctl.c
> @@ -167,7 +167,10 @@ static int dev_ifsioc_locked(struct net *net, struct ifreq *ifr, unsigned int cm
> return 0;
>
> case SIOCGIFTXQLEN:
> - ifr->ifr_qlen = dev->tx_queue_len;
> + if (dev->netdev_ops->ndo_get_tx_queue_len)
> + ifr->ifr_qlen = dev->netdev_ops->ndo_get_tx_queue_len(dev);
> + else
> + ifr->ifr_qlen = dev->tx_queue_len;
> return 0;
>
> default:
> @@ -296,7 +299,10 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
> case SIOCSIFTXQLEN:
> if (ifr->ifr_qlen < 0)
> return -EINVAL;
> - dev->tx_queue_len = ifr->ifr_qlen;
> + if (dev->netdev_ops->ndo_set_tx_queue_len)
> + dev->netdev_ops->ndo_set_tx_queue_len(dev, ifr->ifr_qlen);
> + else
> + dev->tx_queue_len = ifr->ifr_qlen;
> return 0;
>
> case SIOCSIFNAME:
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index d954b56..f2fd9d5 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -280,10 +280,31 @@ NETDEVICE_SHOW_RW(flags, fmt_hex);
>
> static int change_tx_queue_len(struct net_device *net, unsigned long new_len)
> {
> - net->tx_queue_len = new_len;
> + if (dev->netdev_ops->ndo_set_tx_queue_len)
> + dev->netdev_ops->ndo_set_tx_queue_len(dev, new_len);
> + else
> + dev->tx_queue_len = new_len;
> return 0;
> }
>
> +static ssize_t format_tx_queue_len(const struct net_device *net, char *buf)
> +{
> + unsigned long tx_queue_len;
> +
> + if (dev->netdev_ops->ndo_get_tx_queue_len)
> + tx_queue_len = dev->netdev_ops->ndo_get_tx_queue_len(dev);
> + else
> + tx_queue_len = dev->tx_queue_len;
> +
> + return sprintf(buf, fmt_ulong, tx_queue_len);
> +}
> +
> +static ssize_t tx_queue_len_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return netdev_show(dev, attr, buf, format_tx_queue_len);
> +}
> +
> static ssize_t tx_queue_len_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t len)
> @@ -293,7 +314,7 @@ static ssize_t tx_queue_len_store(struct device *dev,
>
> return netdev_store(dev, attr, buf, len, change_tx_queue_len);
> }
> -NETDEVICE_SHOW_RW(tx_queue_len, fmt_ulong);
> +DEVICE_ATTR_RW(tx_queue_len);
>
> static ssize_t ifalias_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t len)
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 2a0e21d..9276e17 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -876,6 +876,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
> struct nlattr *attr, *af_spec;
> struct rtnl_af_ops *af_ops;
> struct net_device *upper_dev = netdev_master_upper_dev_get(dev);
> + unsigned long tx_queue_len;
>
> ASSERT_RTNL();
> nlh = nlmsg_put(skb, pid, seq, type, sizeof(*ifm), flags);
> @@ -890,8 +891,13 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
> ifm->ifi_flags = dev_get_flags(dev);
> ifm->ifi_change = change;
>
> + if (dev->netdev_ops->ndo_get_tx_queue_len)
> + tx_queue_len = dev->netdev_ops->ndo_get_tx_queue_len(dev);
> + else
> + tx_queue_len = dev->tx_queue_len;
> +
> if (nla_put_string(skb, IFLA_IFNAME, dev->name) ||
> - nla_put_u32(skb, IFLA_TXQLEN, dev->tx_queue_len) ||
> + nla_put_u32(skb, IFLA_TXQLEN, tx_queue_len) ||
> nla_put_u8(skb, IFLA_OPERSTATE,
> netif_running(dev) ? dev->operstate : IF_OPER_DOWN) ||
> nla_put_u8(skb, IFLA_LINKMODE, dev->link_mode) ||
> @@ -1453,8 +1459,14 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
> modified = 1;
> }
>
> - if (tb[IFLA_TXQLEN])
> - dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
> + if (tb[IFLA_TXQLEN]) {
> + u32 new_len = nla_get_u32(tb[IFLA_TXQLEN]);
> +
> + if (dev->netdev_ops->ndo_set_tx_queue_len)
> + dev->netdev_ops->ndo_set_tx_queue_len(dev, new_len);
> + else
> + dev->tx_queue_len = new_len;
> + }
>
> if (tb[IFLA_OPERSTATE])
> set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]));
> @@ -1692,8 +1704,14 @@ struct net_device *rtnl_create_link(struct net *net,
> if (tb[IFLA_BROADCAST])
> memcpy(dev->broadcast, nla_data(tb[IFLA_BROADCAST]),
> nla_len(tb[IFLA_BROADCAST]));
> - if (tb[IFLA_TXQLEN])
> - dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
> + if (tb[IFLA_TXQLEN]) {
> + u32 new_len = nla_get_u32(tb[IFLA_TXQLEN]);
> +
> + if (dev->netdev_ops->ndo_set_tx_queue_len)
> + dev->netdev_ops->ndo_set_tx_queue_len(dev, new_len);
> + else
> + dev->tx_queue_len = new_len;
> + }
> if (tb[IFLA_OPERSTATE])
> set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]));
> if (tb[IFLA_LINKMODE])
>
> Anyone objects?
What if use do want a qdisc and want to change the its queue length for
tun/macvlan? And the the name tx_queue_length is misleading. For tun it
may make sense since it was used in transmission path. For macvtap it
was not. So maybe what we need is just a new ioctl for both tun/macvtap
and a new feature flag. If user create the device with new feature flag,
the socket receive queue length could be changed by ioctl instead of
dev->tx_queue_length. If not, the old behaviour could be kept.
>> It is a change in behaviour
>> though in net-next. Previously dev_queue_xmit() was not used so
>> the qdisc layer was never hit on the macvtap device. Now with
>> dev_queue_xmit() if the user attaches multiple macvlan queues to a
>> single qdisc queue this should still work but wont be optimal. Ideally
>> the user should create as many qdisc queues at creation time via
>> numtxqueues as macvtap queues will be used during runtime so that there
>> is a 1:1 mapping or use some heuristic to avoid cases where there
>> is a many to 1 mapping.
>>
>> From my perspective it would be OK to push this configuration/policy
>> to the management layer. After all it is the entity that knows how
>> to distribute system resources amongst the objects running over the
>> macvtap devices. The relevance for me is I defaulted the l2 offloaded
>> macvlans to single queue devices and wanted to note in net-next this
>> is the same policy used in the non-offloaded case.
>>
>> Bit long-winded there.
>>
>> Thanks,
>> John
> I think it's a real problem you are pointing out - a performance
> regression for multiqueue devices.
> If we really think no qdisc is typically the right thing to do,
> we should just make it the default I think, but I agree
> just making tx_queue_len 0 doesn't work without other changes.
>
> What do others think about extra ndo ops so devices can decouple
> the internal tx_queue_len from the userspace-visible value?
>
>
^ permalink raw reply
* [PATCH] HHF qdisc: fix jiffies-time conversion.
From: Terry Lam @ 2014-01-09 8:40 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Nandita Dukkipati, Stephen Hemminger, Terry Lam
This is to be compatible with the use of "get_time" (i.e. default
time unit in us) in iproute2 patch for HHF as requested by Stephen.
Signed-off-by: Terry Lam <vtlam@google.com>
---
net/sched/sch_hhf.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
index cf7f614..1cf84a9 100644
--- a/net/sched/sch_hhf.c
+++ b/net/sched/sch_hhf.c
@@ -574,18 +574,18 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt)
q->hh_flows_limit = nla_get_u32(tb[TCA_HHF_HH_FLOWS_LIMIT]);
if (tb[TCA_HHF_RESET_TIMEOUT]) {
- u32 ms = nla_get_u32(tb[TCA_HHF_RESET_TIMEOUT]);
+ u32 us = nla_get_u32(tb[TCA_HHF_RESET_TIMEOUT]);
- q->hhf_reset_timeout = msecs_to_jiffies(ms);
+ q->hhf_reset_timeout = usecs_to_jiffies(us);
}
if (tb[TCA_HHF_ADMIT_BYTES])
q->hhf_admit_bytes = nla_get_u32(tb[TCA_HHF_ADMIT_BYTES]);
if (tb[TCA_HHF_EVICT_TIMEOUT]) {
- u32 ms = nla_get_u32(tb[TCA_HHF_EVICT_TIMEOUT]);
+ u32 us = nla_get_u32(tb[TCA_HHF_EVICT_TIMEOUT]);
- q->hhf_evict_timeout = msecs_to_jiffies(ms);
+ q->hhf_evict_timeout = usecs_to_jiffies(us);
}
qlen = sch->q.qlen;
@@ -684,10 +684,10 @@ static int hhf_dump(struct Qdisc *sch, struct sk_buff *skb)
nla_put_u32(skb, TCA_HHF_QUANTUM, q->quantum) ||
nla_put_u32(skb, TCA_HHF_HH_FLOWS_LIMIT, q->hh_flows_limit) ||
nla_put_u32(skb, TCA_HHF_RESET_TIMEOUT,
- jiffies_to_msecs(q->hhf_reset_timeout)) ||
+ jiffies_to_usecs(q->hhf_reset_timeout)) ||
nla_put_u32(skb, TCA_HHF_ADMIT_BYTES, q->hhf_admit_bytes) ||
nla_put_u32(skb, TCA_HHF_EVICT_TIMEOUT,
- jiffies_to_msecs(q->hhf_evict_timeout)) ||
+ jiffies_to_usecs(q->hhf_evict_timeout)) ||
nla_put_u32(skb, TCA_HHF_NON_HH_WEIGHT, q->hhf_non_hh_weight))
goto nla_put_failure;
--
1.8.5.1
^ permalink raw reply related
* Re: [PATCH net-next 1/2] include/uapi/linux/xfrm.h: Pack struct xfrm_userpolicy_info
From: Fan Du @ 2014-01-09 8:39 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: steffen.klassert, stephen, davem, dev, netdev
In-Reply-To: <52CC851B.2060401@cogentembedded.com>
On 2014年01月08日 06:52, Sergei Shtylyov wrote:
> Hello.
>
> On 07-01-2014 10:48, Fan Du wrote:
>
>> Otherwise 64bits kernel has sizeof(struct xfrm_userpolicy_info) 168 bytes,
>> while 32bits compiled iproute2 see the same structure as 164 bytes, which
>> leading deficit xfrm policy, in turn broken IPsec connectivity.
>
>> Fix this by packing the structure.
>
> This will force byte-by-byte access to all members on some arches like ARM...
>
>> Signed-off-by: Fan Du <fan.du@windriver.com>
>> ---
>> include/uapi/linux/xfrm.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
>> index a8cd6a4..470bfae 100644
>> --- a/include/uapi/linux/xfrm.h
>> +++ b/include/uapi/linux/xfrm.h
>> @@ -405,7 +405,7 @@ struct xfrm_userpolicy_info {
>> /* Automatically expand selector to include matching ICMP payloads. */
>> #define XFRM_POLICY_ICMP 2
>> __u8 share;
>> -};
>> +} __attribute__((packed));
>
> Please use the __packed macro instead. I guess you haven't run checkpatch.pl?
Lucky me, I run checkpatch every time before sending patch out.
--
浮沉随浪只记今朝笑
--fan
_______________________________________________
Dev mailing list
Dev@lists.strongswan.org
https://lists.strongswan.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH net-next 0/2] Pack struct xfrm_usersa_info and struct xfrm_userpolicy_info
From: Fan Du @ 2014-01-09 8:34 UTC (permalink / raw)
To: David Laight
Cc: Steffen Klassert, stephen@networkplumber.org, davem@davemloft.net,
dev@lists.strongswan.org, netdev@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D453AEE@AcuExch.aculab.com>
On 2014年01月07日 18:00, David Laight wrote:
>> From: Fan Du
>> > Sent: 07 January 2014 08:00
>> > On 2014?01?07? 15:47, Steffen Klassert wrote:
>>> > > On Tue, Jan 07, 2014 at 02:48:57PM +0800, Fan Du wrote:
>>>> > >> When trying to setup IPsec configuration on a 64bits host with
>>>> > >> iproute2(32bits compiled), the intened xfrm policy and sa is
>>>> > >> either deficit or wrong when kernel trying to parse user land
>>>> > >> information.
>>>> > >>
>>>> > >> Further investigatino shows that:
>>>> > >> L: kernel
>>>> > >> R: iproute2
>>>> > >>
>>>> > >> sizeof userpolicy usersa
>>>> > >> 64bits(unpacked) 168/168 224/224
>>>> > >> 32bits(unpacked) 164/164 220/220
>>>> > >> ^ ^
>>>> > >> L R
>>>> > >>
>>>> > >> To keep kernel and user land see a consistent structure, after
>>>> > >> add packing attribute, now it looks like this:
>>>> > >>
>>>> > >> 64bits( packed) 164/164 217/217
>>>> > >> 32bits( packed) 164/164 217/217
>>>> > >> ^ ^
>>>> > >> L R
>>>> > >>
>>> > >
>>> > > We don't change userspace exported structures. This breaks
>>> > > existing userspace tools.
>>> > >
>> >
>> > Then user with 32bits iproute2 or StrongSwan has to rebuild as 64bits?
> The kernel has support (in various places) for different structure
> layouts for 32bit and 64bit processes.
> Looks like it needs one here as well (I'm not volunteering though).
>
> At a guess the structures contain a 64bit field that is on an 8n+4
> byte boundary. On i386 64bit fields are only 4 byte aligned, on
> amd64 they are 8 byte aligned.
>
> Packing the structures is definitely wrong. Some 32bit systems (IIRC sparc)
> align 64bit items on 8 byte boundaries. Not to mention the expensive byte
> by byte accesses this forces on some systems.
I don't know much about sparc, if I read your message right, you mean 32bit sparc
system also has padding even if pack attribute is supplied.
And in this scenario, the code path here is only for configuration, i.e., not hot
path, so performance issue is not so crucial here.
>
> The structure could have been defined with the 64bit field marked
> __attribute__((aligned(4))) - so using the same layout on 32bit
> and 64 bit systems , but it is too late to do that now.
Thanks for this useful information.
--
浮沉随浪只记今朝笑
--fan
_______________________________________________
Dev mailing list
Dev@lists.strongswan.org
https://lists.strongswan.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Michael Dalton @ 2014-01-09 8:28 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, lf-virt, Eric Dumazet, David S. Miller
In-Reply-To: <20140109064807.GC19559@redhat.com>
Hi Michael,
Here's a quick sketch of some code that enforces a minimum buffer
alignment of only 64, and has a maximum theoretical buffer size of
aligned GOOD_PACKET_LEN + (BUF_ALIGN - 1) * BUF_ALIGN, which is at least
1536 + 63 * 64 = 5568. On x86, we already use a 64 byte alignment, and
this code supports all current buffer sizes, from 1536 to PAGE_SIZE.
#if L1_CACHE_BYTES < 64
#define MERGEABLE_BUFFER_ALIGN 64
#define MERGEABLE_BUFFER_SHIFT 6
#else
#define MERGEABLE_BUFFER_ALIGN L1_CACHE_BYTES
#define MERGEABLE_BUFFER_SHIFT L1_CACHE_SHIFT
#endif
#define MERGEABLE_BUFFER_MIN ALIGN(GOOD_PACKET_LEN +
sizeof(virtio_net_hdr_mrg_rbuf),
MERGEABLE_BUFFER_ALIGN)
#define MERGEABLE_BUFFER_MAX min(MERGEABLE_BUFFER_MIN +
(MERGEABLE_BUFFER_ALIGN - 1) *
MERGEABLE_BUFFER_ALIGN, PAGE_SIZE)
/* Extract buffer length from a mergeable buffer context. */
static u16 get_mergeable_buf_ctx_len(void *ctx) {
u16 len = (uintptr_t)ctx & (MERGEABLE_BUFFER_ALIGN - 1);
return MERGEABLE_BUFFER_MIN + (len << MERGEABLE_BUFFER_SHIFT);
}
/* Extract buffer base address from a mergeable buffer context. */
static void *get_mergeable_buf_ctx_base(void *ctx) {
return (void *) ((uintptr)ctx & -MERGEABLE_BUFFER_ALIGN);
}
/* Convert a base address and length to a mergeable buffer context. */
static void *to_mergeable_buf_ctx(void *base, u16 len) {
len -= MERGEABLE_BUFFER_MIN;
return (void *) ((uintptr)base | (len >> MERGEABLE_BUFFER_SHIFT));
}
/* Compute the packet buffer length for a receive queue. */
static u16 get_mergeable_buffer_len(struct receive_queue *rq) {
u16 len = clamp_t(u16, MERGEABLE_BUFFER_MIN,
ewma_read(&rq->avg_pkt_len),
MERGEABLE_BUFFER_MAX);
return ALIGN(len, MERGEABLE_BUFFER_ALIGN);
}
Best,
Mike
^ permalink raw reply
* Re: [PATCH net 2/2] net: core: explicitly select a txq before doing l2 forwarding
From: Jason Wang @ 2014-01-09 8:28 UTC (permalink / raw)
To: Neil Horman; +Cc: mst, e1000-devel, netdev, linux-kernel, John Fastabend, davem
In-Reply-To: <20140108144025.GA17802@neilslaptop.think-freely.org>
On 01/08/2014 10:40 PM, Neil Horman wrote:
> On Wed, Jan 08, 2014 at 11:21:21AM +0800, Jason Wang wrote:
>> On 01/07/2014 09:17 PM, Neil Horman wrote:
>>> On Tue, Jan 07, 2014 at 11:42:24AM +0800, Jason Wang wrote:
>>>> On 01/06/2014 08:42 PM, Neil Horman wrote:
>>>>> On Mon, Jan 06, 2014 at 11:21:07AM +0800, Jason Wang wrote:
>>>>>> Currently, the tx queue were selected implicitly in ndo_dfwd_start_xmit(). The
>>>>>> will cause several issues:
>>>>>>
>>>>>> - NETIF_F_LLTX was forced for macvlan device in this case which lead extra lock
>>>>>> contention.
>>>>>> - dev_hard_start_xmit() was called with NULL txq which bypasses the net device
>>>>>> watchdog
>>>>>> - dev_hard_start_xmit() does not check txq everywhere which will lead a crash
>>>>>> when tso is disabled for lower device.
>>>>>>
>>>>>> Fix this by explicitly introducing a select queue method just for l2 forwarding
>>>>>> offload (ndo_dfwd_select_queue), and introducing dfwd_direct_xmit() to do the
>>>>>> queue selecting and transmitting for l2 forwarding.
>>>>>>
>>>>>> With this fixes, NETIF_F_LLTX could be preserved for macvlan and there's no need
>>>>>> to check txq against NULL in dev_hard_start_xmit().
>>>>>>
>>>>>> In the future, it was also required for macvtap l2 forwarding support since it
>>>>>> provides a necessary synchronization method.
>>>>>>
>>>>>> Cc: John Fastabend <john.r.fastabend@intel.com>
>>>>>> Cc: Neil Horman <nhorman@tuxdriver.com>
>>>>>> Cc: e1000-devel@lists.sourceforge.net
>>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>>>> Instead of creating another operation here to do special queue selection, why
>>>>> not just have ndo_dfwd_start_xmit include a pointer to a pointer in its argument
>>>>> list, so it can pass the txq it used back to the caller (dev_hard_start_xmit)?
>>>>> ndo_dfwd_start_xmit already knows which queue set to pick from (since their
>>>>> reserved for the device doing the transmitting). It seems more clear to me than
>>>>> creating a new netdevice operation.
>>>> See commit 8ffab51b3dfc54876f145f15b351c41f3f703195 ("macvlan: lockless
>>>> tx path"). The point is keep the tx path lockless to be efficient and
>>>> simplicity for management. And macvtap multiqueue was also implemented
>>>> with this assumption. The real contention should be done in the txq of
>>>> lower device instead of macvlan itself. This is also needed for
>>>> multiqueue macvtap.
>>> Ok, I see how you're preserving LLTX here, and thats great, but it doesn't
>>> really buy us anything that I can see. If a macvlan is using hardware
>>> acceleration, it needs to arbitrate access to that hardware. Weather thats done
>>> by locking the lowerdev's tx queue lock or by enforcing locking on the macvlan
>>> itself is equivalent. The decision to use dfwd hardware acceleration is made on
>>> open, so its not like theres any traffic that can avoid the lock, as it all goes
>>> through the hardware. All I see that this has bought us is an extra net_device
>>> method (which isn't a big deal, but not necessecary as I see it).
>> As I replied to patch 1/2, looking at the code itself again. The locking
>> on the lowerdev's tx queue is really need since we need synchronize with
>> other control path. Two examples are dev watchdog and ixgbe_down() both
>> of which will try to hold tx lock to synchronize the with transmission.
>> Without holding the lowerdev tx lock, we may have more serious issues.
>> Also, it's a little strange for a net device has two modes. Future
>> developers need to care about two different tx lock paths which is sub
>> optimal.
>>
> Ok, having looked at this for a few hours, I agree, locking in the lowerdev has
> some definiate advantages in plugging the holes you've pointed out.
>
>> For the issue of an extra net_device method, if you don't like we can
>> reuse the ndo_select_queue by also passing the accel_priv to that method.
> I do, that actually simplifies things, since it lets us use the entire
> dev_hard_start_xmit path unmodified, which gives us the locking your looking for
> without having to create a new slimmed down variant of dev_hard_start_xmit.
>
> Regards
> Neil
Right, will post V2.
------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH net-next 2/2] include/uapi/linux/xfrm.h: Pack struct xfrm_usersa_info
From: Fan Du @ 2014-01-09 8:24 UTC (permalink / raw)
To: Ben Hutchings; +Cc: steffen.klassert, stephen, davem, dev, netdev
In-Reply-To: <1389213183.1644.31.camel@bwh-desktop.uk.level5networks.com>
On 2014年01月09日 04:33, Ben Hutchings wrote:
> On Tue, 2014-01-07 at 14:48 +0800, Fan Du wrote:
>> Otherwise 64bits kernel has sizeof(struct xfrm_usersa_info) 224 bytes,
>> while 32bits compiled iproute2 see the same structure as 220 bytes, which
>> leading deficit xfrm sa, in turn broken IPsec connectivity.
>>
>> Fix this by packing the structure.
>>
>> Signed-off-by: Fan Du<fan.du@windriver.com>
>> ---
>> include/uapi/linux/xfrm.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
>> index 470bfae..61460c4 100644
>> --- a/include/uapi/linux/xfrm.h
>> +++ b/include/uapi/linux/xfrm.h
>> @@ -366,7 +366,7 @@ struct xfrm_usersa_info {
>> #define XFRM_STATE_AF_UNSPEC 32
>> #define XFRM_STATE_ALIGN4 64
>> #define XFRM_STATE_ESN 128
>> -};
>> +} __attribute__((packed));
>>
>> #define XFRM_SA_XFLAG_DONT_ENCAP_DSCP 1
>>
>
> That change will make access to the structure very slow on some
> architectures, and I suspect it will cause other compatibility problems.
>
> I think the right thing to do is to reduce the minimum length of the
> structure in the netlink policy so that padding at the end is not
> required.
Could you please be more specific about this? Thanks.
I'm afraid we can only rearrange structure member order to reduce size
on 64bits, alas that's not feasible here :(
(It looks like all field offsets will be the same on all
> 32/64-bit architecture pairs and there is only a differing amount of
> padding at the end of the structure for 32/64-bit alignment.)
>
> Ben.
>
--
浮沉随浪只记今朝笑
--fan
_______________________________________________
Dev mailing list
Dev@lists.strongswan.org
https://lists.strongswan.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH net-next 0/2] Pack struct xfrm_usersa_info and struct xfrm_userpolicy_info
From: Fan Du @ 2014-01-09 8:24 UTC (permalink / raw)
To: David Miller; +Cc: steffen.klassert, stephen, dev, netdev
In-Reply-To: <20140107.130749.215535043457683103.davem@davemloft.net>
On 2014年01月08日 02:07, David Miller wrote:
> From: Fan Du<fan.du@windriver.com>
> Date: Tue, 7 Jan 2014 14:48:57 +0800
>
>> > When trying to setup IPsec configuration on a 64bits host with
>> > iproute2(32bits compiled), the intened xfrm policy and sa is
>> > either deficit or wrong when kernel trying to parse user land
>> > information.
> You can't make this change without breaking userspace.
>
> We'll have to translate the data structures somehow with a compat
> layer like we have for 32/64 bit compatability for system calls.
Do you mean below code snippet?
compat_sys_sendmsg
-> __sys_sendmsg
-> ___sys_sendmsg
-> get_compat_msghdr <- translate the structure here
It's a bit of late in the code flow to do the translation in
xfrm_user code, or I missed what you actually mean.
--
浮沉随浪只记今朝笑
--fan
_______________________________________________
Dev mailing list
Dev@lists.strongswan.org
https://lists.strongswan.org/mailman/listinfo/dev
^ permalink raw reply
* [PATCH] iproute2 support for Heavy Hitter Filter (HHF) qdisc.
From: Terry Lam @ 2014-01-09 8:23 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Nandita Dukkipati, Terry Lam
$tc qdisc add dev eth0 hhf help
Usage: ... hhf [ limit PACKETS ] [ quantum BYTES]
[ hh_limit NUMBER ]
[ reset_timeout TIME ]
[ admit_bytes BYTES ]
[ evict_timeout TIME ]
[ non_hh_weight NUMBER ]
$tc -s -d qdisc show dev eth0
qdisc hhf 8005: root refcnt 32 limit 1000p quantum 1514 hh_limit 2048
reset_timeout 40.0ms admit_bytes 131072 evict_timeout 1.0s non_hh_weight 2
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
drop_overlimit 0 hh_overlimit 0 tot_hh 0 cur_hh 0
HHF qdisc parameters:
- limit: max number of packets in qdisc (default 1000)
- quantum: max deficit per RR round (default 1 MTU)
- hh_limit: max number of HHs to keep states (default 2048)
- reset_timeout: time to reset HHF counters (default 40ms)
- admit_bytes: counter thresh to classify as HH (default 128KB)
- evict_timeout: threshold to evict idle HHs (default 1s)
- non_hh_weight: DRR weight for mice (default 2)
Signed-off-by: Terry Lam <vtlam@google.com>
---
include/linux/pkt_sched.h | 23 ++++++
tc/Makefile | 1 +
tc/q_hhf.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 223 insertions(+)
create mode 100644 tc/q_hhf.c
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index a806687..ddd6577 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -790,4 +790,27 @@ struct tc_fq_qd_stats {
__u32 throttled_flows;
__u32 pad;
};
+
+/* Heavy-Hitter Filter */
+
+enum {
+ TCA_HHF_UNSPEC,
+ TCA_HHF_BACKLOG_LIMIT,
+ TCA_HHF_QUANTUM,
+ TCA_HHF_HH_FLOWS_LIMIT,
+ TCA_HHF_RESET_TIMEOUT,
+ TCA_HHF_ADMIT_BYTES,
+ TCA_HHF_EVICT_TIMEOUT,
+ TCA_HHF_NON_HH_WEIGHT,
+ __TCA_HHF_MAX
+};
+
+#define TCA_HHF_MAX (__TCA_HHF_MAX - 1)
+
+struct tc_hhf_xstats {
+ __u32 drop_overlimit; /* number of time qdisc packet limit was hit */
+ __u32 hh_overlimit; /* number of time max heavy-hitters was hit */
+ __u32 hh_tot_count; /* number of captured heavy-hitters so far */
+ __u32 hh_cur_count; /* number of current heavy-hitters */
+};
#endif
diff --git a/tc/Makefile b/tc/Makefile
index 84215c0..7c6e66e 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -53,6 +53,7 @@ TCMODULES += q_mqprio.o
TCMODULES += q_codel.o
TCMODULES += q_fq_codel.o
TCMODULES += q_fq.o
+TCMODULES += q_hhf.o
ifeq ($(TC_CONFIG_IPSET), y)
ifeq ($(TC_CONFIG_XT), y)
diff --git a/tc/q_hhf.c b/tc/q_hhf.c
new file mode 100644
index 0000000..06ec8a2
--- /dev/null
+++ b/tc/q_hhf.c
@@ -0,0 +1,199 @@
+/* q_hhf.c Heavy-Hitter Filter (HHF)
+ *
+ * Copyright (C) 2013 Terry Lam <vtlam@google.com>
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+
+#include "utils.h"
+#include "tc_util.h"
+
+static void explain(void)
+{
+ fprintf(stderr, "Usage: ... hhf [ limit PACKETS ] [ quantum BYTES]\n");
+ fprintf(stderr, " [ hh_limit NUMBER ]\n");
+ fprintf(stderr, " [ reset_timeout TIME ]\n");
+ fprintf(stderr, " [ admit_bytes BYTES ]\n");
+ fprintf(stderr, " [ evict_timeout TIME ]\n");
+ fprintf(stderr, " [ non_hh_weight NUMBER ]\n");
+}
+
+static int hhf_parse_opt(struct qdisc_util *qu, int argc, char **argv,
+ struct nlmsghdr *n)
+{
+ unsigned limit = 0;
+ unsigned quantum = 0;
+ unsigned hh_limit = 0;
+ unsigned reset_timeout = 0;
+ unsigned admit_bytes = 0;
+ unsigned evict_timeout = 0;
+ unsigned non_hh_weight = 0;
+ struct rtattr *tail;
+
+ while (argc > 0) {
+ if (strcmp(*argv, "limit") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&limit, *argv, 0)) {
+ fprintf(stderr, "Illegal \"limit\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "quantum") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&quantum, *argv, 0)) {
+ fprintf(stderr, "Illegal \"quantum\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "hh_limit") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&hh_limit, *argv, 0)) {
+ fprintf(stderr, "Illegal \"hh_limit\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "reset_timeout") == 0) {
+ NEXT_ARG();
+ if (get_time(&reset_timeout, *argv)) {
+ fprintf(stderr, "Illegal \"reset_timeout\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "admit_bytes") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&admit_bytes, *argv, 0)) {
+ fprintf(stderr, "Illegal \"admit_bytes\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "evict_timeout") == 0) {
+ NEXT_ARG();
+ if (get_time(&evict_timeout, *argv)) {
+ fprintf(stderr, "Illegal \"evict_timeout\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "non_hh_weight") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&non_hh_weight, *argv, 0)) {
+ fprintf(stderr, "Illegal \"non_hh_weight\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "help") == 0) {
+ explain();
+ return -1;
+ } else {
+ fprintf(stderr, "What is \"%s\"?\n", *argv);
+ explain();
+ return -1;
+ }
+ argc--; argv++;
+ }
+
+ tail = NLMSG_TAIL(n);
+ addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+ if (limit)
+ addattr_l(n, 1024, TCA_HHF_BACKLOG_LIMIT, &limit,
+ sizeof(limit));
+ if (quantum)
+ addattr_l(n, 1024, TCA_HHF_QUANTUM, &quantum, sizeof(quantum));
+ if (hh_limit)
+ addattr_l(n, 1024, TCA_HHF_HH_FLOWS_LIMIT, &hh_limit,
+ sizeof(hh_limit));
+ if (reset_timeout)
+ addattr_l(n, 1024, TCA_HHF_RESET_TIMEOUT, &reset_timeout,
+ sizeof(reset_timeout));
+ if (admit_bytes)
+ addattr_l(n, 1024, TCA_HHF_ADMIT_BYTES, &admit_bytes,
+ sizeof(admit_bytes));
+ if (evict_timeout)
+ addattr_l(n, 1024, TCA_HHF_EVICT_TIMEOUT, &evict_timeout,
+ sizeof(evict_timeout));
+ if (non_hh_weight)
+ addattr_l(n, 1024, TCA_HHF_NON_HH_WEIGHT, &non_hh_weight,
+ sizeof(non_hh_weight));
+ tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+ return 0;
+}
+
+static int hhf_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
+{
+ struct rtattr *tb[TCA_HHF_MAX + 1];
+ unsigned limit;
+ unsigned quantum;
+ unsigned hh_limit;
+ unsigned reset_timeout;
+ unsigned admit_bytes;
+ unsigned evict_timeout;
+ unsigned non_hh_weight;
+ SPRINT_BUF(b1);
+
+ if (opt == NULL)
+ return 0;
+
+ parse_rtattr_nested(tb, TCA_HHF_MAX, opt);
+
+ if (tb[TCA_HHF_BACKLOG_LIMIT] &&
+ RTA_PAYLOAD(tb[TCA_HHF_BACKLOG_LIMIT]) >= sizeof(__u32)) {
+ limit = rta_getattr_u32(tb[TCA_HHF_BACKLOG_LIMIT]);
+ fprintf(f, "limit %up ", limit);
+ }
+ if (tb[TCA_HHF_QUANTUM] &&
+ RTA_PAYLOAD(tb[TCA_HHF_QUANTUM]) >= sizeof(__u32)) {
+ quantum = rta_getattr_u32(tb[TCA_HHF_QUANTUM]);
+ fprintf(f, "quantum %u ", quantum);
+ }
+ if (tb[TCA_HHF_HH_FLOWS_LIMIT] &&
+ RTA_PAYLOAD(tb[TCA_HHF_HH_FLOWS_LIMIT]) >= sizeof(__u32)) {
+ hh_limit = rta_getattr_u32(tb[TCA_HHF_HH_FLOWS_LIMIT]);
+ fprintf(f, "hh_limit %u ", hh_limit);
+ }
+ if (tb[TCA_HHF_RESET_TIMEOUT] &&
+ RTA_PAYLOAD(tb[TCA_HHF_RESET_TIMEOUT]) >= sizeof(__u32)) {
+ reset_timeout = rta_getattr_u32(tb[TCA_HHF_RESET_TIMEOUT]);
+ fprintf(f, "reset_timeout %s ", sprint_time(reset_timeout, b1));
+ }
+ if (tb[TCA_HHF_ADMIT_BYTES] &&
+ RTA_PAYLOAD(tb[TCA_HHF_ADMIT_BYTES]) >= sizeof(__u32)) {
+ admit_bytes = rta_getattr_u32(tb[TCA_HHF_ADMIT_BYTES]);
+ fprintf(f, "admit_bytes %u ", admit_bytes);
+ }
+ if (tb[TCA_HHF_EVICT_TIMEOUT] &&
+ RTA_PAYLOAD(tb[TCA_HHF_EVICT_TIMEOUT]) >= sizeof(__u32)) {
+ evict_timeout = rta_getattr_u32(tb[TCA_HHF_EVICT_TIMEOUT]);
+ fprintf(f, "evict_timeout %s ", sprint_time(evict_timeout, b1));
+ }
+ if (tb[TCA_HHF_NON_HH_WEIGHT] &&
+ RTA_PAYLOAD(tb[TCA_HHF_NON_HH_WEIGHT]) >= sizeof(__u32)) {
+ non_hh_weight = rta_getattr_u32(tb[TCA_HHF_NON_HH_WEIGHT]);
+ fprintf(f, "non_hh_weight %u ", non_hh_weight);
+ }
+ return 0;
+}
+
+static int hhf_print_xstats(struct qdisc_util *qu, FILE *f,
+ struct rtattr *xstats)
+{
+ struct tc_hhf_xstats *st;
+
+ if (xstats == NULL)
+ return 0;
+
+ if (RTA_PAYLOAD(xstats) < sizeof(*st))
+ return -1;
+
+ st = RTA_DATA(xstats);
+
+ fprintf(f, " drop_overlimit %u hh_overlimit %u tot_hh %u cur_hh %u",
+ st->drop_overlimit, st->hh_overlimit,
+ st->hh_tot_count, st->hh_cur_count);
+ return 0;
+}
+
+struct qdisc_util hhf_qdisc_util = {
+ .id = "hhf",
+ .parse_qopt = hhf_parse_opt,
+ .print_qopt = hhf_print_opt,
+ .print_xstats = hhf_print_xstats,
+};
--
1.8.5.1
^ permalink raw reply related
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