* Re: [PATCH] net: mlxsw: avoid unused variable warnings
From: David Miller @ 2016-03-23 18:38 UTC (permalink / raw)
To: andrew; +Cc: arnd, jiri, idosch, eladr, netdev, linux-kernel
In-Reply-To: <20160323165111.GJ5250@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Wed, 23 Mar 2016 17:51:11 +0100
> On Wed, Mar 23, 2016 at 05:37:38PM +0100, Arnd Bergmann wrote:
>> dev_dbg_ratelimited() is a macro that ignores its arguments when DEBUG is
>> not set, which can lead to unused variable warnings:
>>
>> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_sdq_handle':
>> ethernet/mellanox/mlxsw/pci.c:646:18: warning: unused variable 'pdev' [-Wunused-variable]
>> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_rdq_handle':
>> ethernet/mellanox/mlxsw/pci.c:671:18: warning: unused variable 'pdev' [-Wunused-variable]
>>
>> This changes the mlxsw driver to remove the local variables we get
>> warnings for and instead pass the device directly into the API.
>
> Hi Arnd
>
> Would it not be better to fix the macro?
>
> I think the issue is that dev_dbg_ratelimited calls no_printk(),
> without making use of dev. So how about:
>
> #define dev_dbg_ratelimited(dev, fmt, ...) \
> ({ \
> if (0) \
> dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
> })
>
> This follows the pattern for other macros for when DEBUG is not defined.
Yeah, this is probably a better way to fix this problem.
^ permalink raw reply
* [PATCH v2] net: Fix typos and whitespace.
From: Bjorn Helgaas @ 2016-03-23 18:47 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel
Fix typos. Capitalize CPU, NAPI, RCU consistently. Align structure
indentation. No functional change intended; only comment and whitespace
changes.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
include/linux/netdevice.h | 215 ++++++++++++++++++++++-----------------------
1 file changed, 106 insertions(+), 109 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3f6385d..a675205 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -81,8 +81,8 @@ void netdev_set_default_ethtool_ops(struct net_device *dev,
* function. Real network devices commonly used with qdiscs should only return
* the driver transmit return codes though - when qdiscs are used, the actual
* transmission happens asynchronously, so the value is not propagated to
- * higher layers. Virtual network devices transmit synchronously, in this case
- * the driver transmit return codes are consumed by dev_queue_xmit(), all
+ * higher layers. Virtual network devices transmit synchronously; in this case
+ * the driver transmit return codes are consumed by dev_queue_xmit(), and all
* others are propagated to higher layers.
*/
@@ -129,7 +129,7 @@ static inline bool dev_xmit_complete(int rc)
}
/*
- * Compute the worst case header length according to the protocols
+ * Compute the worst-case header length according to the protocols
* used.
*/
@@ -246,7 +246,7 @@ struct hh_cache {
unsigned long hh_data[HH_DATA_ALIGN(LL_MAX_HEADER) / sizeof(long)];
};
-/* Reserve HH_DATA_MOD byte aligned hard_header_len, but at least that much.
+/* Reserve HH_DATA_MOD byte-aligned hard_header_len, but at least that much.
* Alternative is:
* dev->hard_header_len ? (dev->hard_header_len +
* (HH_DATA_MOD - 1)) & ~(HH_DATA_MOD - 1) : 0
@@ -272,7 +272,7 @@ struct header_ops {
};
/* These flag bits are private to the generic network queueing
- * layer, they may not be explicitly referenced by any other
+ * layer; they may not be explicitly referenced by any other
* code.
*/
@@ -286,7 +286,7 @@ enum netdev_state_t {
/*
- * This structure holds at boot time configured netdevice settings. They
+ * This structure holds boot-time configured netdevice settings. They
* are then used in the device probing.
*/
struct netdev_boot_setup {
@@ -304,7 +304,7 @@ struct napi_struct {
/* The poll_list must only be managed by the entity which
* changes the state of the NAPI_STATE_SCHED bit. This means
* whoever atomically sets that bit can add this napi_struct
- * to the per-cpu poll_list, and whoever clears that bit
+ * to the per-CPU poll_list, and whoever clears that bit
* can remove from the list right before clearing the bit.
*/
struct list_head poll_list;
@@ -350,7 +350,7 @@ typedef enum gro_result gro_result_t;
* @RX_HANDLER_ANOTHER: Do another round in receive path. This is indicated in
* case skb->dev was changed by rx_handler.
* @RX_HANDLER_EXACT: Force exact delivery, no wildcard.
- * @RX_HANDLER_PASS: Do nothing, passe the skb as if no rx_handler was called.
+ * @RX_HANDLER_PASS: Do nothing, pass the skb as if no rx_handler was called.
*
* rx_handlers are functions called from inside __netif_receive_skb(), to do
* special processing of the skb, prior to delivery to protocol handlers.
@@ -365,19 +365,19 @@ typedef enum gro_result gro_result_t;
* Upon return, rx_handler is expected to tell __netif_receive_skb() what to
* do with the skb.
*
- * If the rx_handler consumed to skb in some way, it should return
+ * If the rx_handler consumed the skb in some way, it should return
* RX_HANDLER_CONSUMED. This is appropriate when the rx_handler arranged for
- * the skb to be delivered in some other ways.
+ * the skb to be delivered in some other way.
*
* If the rx_handler changed skb->dev, to divert the skb to another
* net_device, it should return RX_HANDLER_ANOTHER. The rx_handler for the
* new device will be called if it exists.
*
- * If the rx_handler consider the skb should be ignored, it should return
+ * If the rx_handler decides the skb should be ignored, it should return
* RX_HANDLER_EXACT. The skb will only be delivered to protocol handlers that
* are registered on exact device (ptype->dev == skb->dev).
*
- * If the rx_handler didn't changed skb->dev, but want the skb to be normally
+ * If the rx_handler didn't change skb->dev, but wants the skb to be normally
* delivered, it should return RX_HANDLER_PASS.
*
* A device without a registered rx_handler will behave as if rx_handler
@@ -402,11 +402,11 @@ static inline bool napi_disable_pending(struct napi_struct *n)
}
/**
- * napi_schedule_prep - check if napi can be scheduled
- * @n: napi context
+ * napi_schedule_prep - check if NAPI can be scheduled
+ * @n: NAPI context
*
* Test if NAPI routine is already running, and if not mark
- * it as running. This is used as a condition variable
+ * it as running. This is used as a condition variable to
* insure only one NAPI poll instance runs. We also make
* sure there is no pending NAPI disable.
*/
@@ -418,7 +418,7 @@ static inline bool napi_schedule_prep(struct napi_struct *n)
/**
* napi_schedule - schedule NAPI poll
- * @n: napi context
+ * @n: NAPI context
*
* Schedule NAPI poll routine to be called if it is not already
* running.
@@ -431,7 +431,7 @@ static inline void napi_schedule(struct napi_struct *n)
/**
* napi_schedule_irqoff - schedule NAPI poll
- * @n: napi context
+ * @n: NAPI context
*
* Variant of napi_schedule(), assuming hard irqs are masked.
*/
@@ -455,7 +455,7 @@ void __napi_complete(struct napi_struct *n);
void napi_complete_done(struct napi_struct *n, int work_done);
/**
* napi_complete - NAPI processing complete
- * @n: napi context
+ * @n: NAPI context
*
* Mark NAPI processing as complete.
* Consider using napi_complete_done() instead.
@@ -467,32 +467,32 @@ static inline void napi_complete(struct napi_struct *n)
/**
* napi_hash_add - add a NAPI to global hashtable
- * @napi: napi context
+ * @napi: NAPI context
*
- * generate a new napi_id and store a @napi under it in napi_hash
- * Used for busy polling (CONFIG_NET_RX_BUSY_POLL)
+ * Generate a new napi_id and store a @napi under it in napi_hash.
+ * Used for busy polling (CONFIG_NET_RX_BUSY_POLL).
* Note: This is normally automatically done from netif_napi_add(),
- * so might disappear in a future linux version.
+ * so might disappear in a future Linux version.
*/
void napi_hash_add(struct napi_struct *napi);
/**
* napi_hash_del - remove a NAPI from global table
- * @napi: napi context
+ * @napi: NAPI context
*
- * Warning: caller must observe rcu grace period
+ * Warning: caller must observe RCU grace period
* before freeing memory containing @napi, if
* this function returns true.
* Note: core networking stack automatically calls it
- * from netif_napi_del()
+ * from netif_napi_del().
* Drivers might want to call this helper to combine all
- * the needed rcu grace periods into a single one.
+ * the needed RCU grace periods into a single one.
*/
bool napi_hash_del(struct napi_struct *napi);
/**
* napi_disable - prevent NAPI from scheduling
- * @n: napi context
+ * @n: NAPI context
*
* Stop NAPI from being scheduled on this context.
* Waits till any outstanding processing completes.
@@ -501,7 +501,7 @@ void napi_disable(struct napi_struct *n);
/**
* napi_enable - enable NAPI scheduling
- * @n: napi context
+ * @n: NAPI context
*
* Resume NAPI from being scheduled on this context.
* Must be paired with napi_disable.
@@ -516,7 +516,7 @@ static inline void napi_enable(struct napi_struct *n)
/**
* napi_synchronize - wait until NAPI is not running
- * @n: napi context
+ * @n: NAPI context
*
* Wait until NAPI is done being scheduled on this context.
* Waits till any outstanding processing completes but
@@ -559,7 +559,7 @@ enum netdev_queue_state_t {
struct netdev_queue {
/*
- * read mostly part
+ * read-mostly part
*/
struct net_device *dev;
struct Qdisc __rcu *qdisc;
@@ -571,7 +571,7 @@ struct netdev_queue {
int numa_node;
#endif
/*
- * write mostly part
+ * write-mostly part
*/
spinlock_t _xmit_lock ____cacheline_aligned_in_smp;
int xmit_lock_owner;
@@ -648,11 +648,11 @@ struct rps_dev_flow_table {
/*
* The rps_sock_flow_table contains mappings of flows to the last CPU
* on which they were processed by the application (set in recvmsg).
- * Each entry is a 32bit value. Upper part is the high order bits
- * of flow hash, lower part is cpu number.
+ * Each entry is a 32bit value. Upper part is the high-order bits
+ * of flow hash, lower part is CPU number.
* rps_cpu_mask is used to partition the space, depending on number of
- * possible cpus : rps_cpu_mask = roundup_pow_of_two(nr_cpu_ids) - 1
- * For example, if 64 cpus are possible, rps_cpu_mask = 0x3f,
+ * possible CPUs : rps_cpu_mask = roundup_pow_of_two(nr_cpu_ids) - 1
+ * For example, if 64 CPUs are possible, rps_cpu_mask = 0x3f,
* meaning we use 32-6=26 bits for the hash.
*/
struct rps_sock_flow_table {
@@ -674,7 +674,7 @@ static inline void rps_record_sock_flow(struct rps_sock_flow_table *table,
unsigned int index = hash & table->mask;
u32 val = hash & ~rps_cpu_mask;
- /* We only give a hint, preemption can change cpu under us */
+ /* We only give a hint, preemption can change CPU under us */
val |= raw_smp_processor_id();
if (table->ents[index] != val)
@@ -807,21 +807,21 @@ struct tc_to_netdev {
* optional and can be filled with a null pointer.
*
* int (*ndo_init)(struct net_device *dev);
- * This function is called once when network device is registered.
- * The network device can use this to any late stage initializaton
- * or semantic validattion. It can fail with an error code which will
- * be propogated back to register_netdev
+ * This function is called once when a network device is registered.
+ * The network device can use this for any late stage initialization
+ * or semantic validation. It can fail with an error code which will
+ * be propagated back to register_netdev.
*
* void (*ndo_uninit)(struct net_device *dev);
* This function is called when device is unregistered or when registration
* fails. It is not called if init fails.
*
* int (*ndo_open)(struct net_device *dev);
- * This function is called when network device transistions to the up
+ * This function is called when a network device transitions to the up
* state.
*
* int (*ndo_stop)(struct net_device *dev);
- * This function is called when network device transistions to the down
+ * This function is called when a network device transitions to the down
* state.
*
* netdev_tx_t (*ndo_start_xmit)(struct sk_buff *skb,
@@ -832,7 +832,7 @@ struct tc_to_netdev {
* corner cases, but the stack really does a non-trivial amount
* of useless work if you return NETDEV_TX_BUSY.
* (can also return NETDEV_TX_LOCKED iff NETIF_F_LLTX)
- * Required can not be NULL.
+ * Required; cannot be NULL.
*
* netdev_features_t (*ndo_fix_features)(struct net_device *dev,
* netdev_features_t features);
@@ -842,34 +842,34 @@ struct tc_to_netdev {
*
* u16 (*ndo_select_queue)(struct net_device *dev, struct sk_buff *skb,
* void *accel_priv, select_queue_fallback_t fallback);
- * Called to decide which queue to when device supports multiple
+ * Called to decide which queue to use when device supports multiple
* transmit queues.
*
* void (*ndo_change_rx_flags)(struct net_device *dev, int flags);
* This function is called to allow device receiver to make
- * changes to configuration when multicast or promiscious is enabled.
+ * changes to configuration when multicast or promiscuous is enabled.
*
* void (*ndo_set_rx_mode)(struct net_device *dev);
* This function is called device changes address list filtering.
* If driver handles unicast address filtering, it should set
- * IFF_UNICAST_FLT to its priv_flags.
+ * IFF_UNICAST_FLT in its priv_flags.
*
* int (*ndo_set_mac_address)(struct net_device *dev, void *addr);
* This function is called when the Media Access Control address
* needs to be changed. If this interface is not defined, the
- * mac address can not be changed.
+ * MAC address can not be changed.
*
* int (*ndo_validate_addr)(struct net_device *dev);
* Test if Media Access Control address is valid for the device.
*
* int (*ndo_do_ioctl)(struct net_device *dev, struct ifreq *ifr, int cmd);
- * Called when a user request an ioctl which can't be handled by
- * the generic interface code. If not defined ioctl's return
+ * Called when a user requests an ioctl which can't be handled by
+ * the generic interface code. If not defined ioctls return
* not supported error code.
*
* int (*ndo_set_config)(struct net_device *dev, struct ifmap *map);
* Used to set network devices bus interface parameters. This interface
- * is retained for legacy reason, new devices should use the bus
+ * is retained for legacy reasons; new devices should use the bus
* interface (PCI) for low level management.
*
* int (*ndo_change_mtu)(struct net_device *dev, int new_mtu);
@@ -878,7 +878,7 @@ struct tc_to_netdev {
* will return an error.
*
* void (*ndo_tx_timeout)(struct net_device *dev);
- * Callback uses when the transmitter has not made any progress
+ * Callback used when the transmitter has not made any progress
* for dev->watchdog ticks.
*
* struct rtnl_link_stats64* (*ndo_get_stats64)(struct net_device *dev,
@@ -896,11 +896,11 @@ struct tc_to_netdev {
* neither operation.
*
* int (*ndo_vlan_rx_add_vid)(struct net_device *dev, __be16 proto, u16 vid);
- * If device support VLAN filtering this function is called when a
+ * If device supports VLAN filtering this function is called when a
* VLAN id is registered.
*
* int (*ndo_vlan_rx_kill_vid)(struct net_device *dev, __be16 proto, u16 vid);
- * If device support VLAN filtering this function is called when a
+ * If device supports VLAN filtering this function is called when a
* VLAN id is unregistered.
*
* void (*ndo_poll_controller)(struct net_device *dev);
@@ -920,7 +920,7 @@ struct tc_to_netdev {
*
* Enable or disable the VF ability to query its RSS Redirection Table and
* Hash Key. This is needed since on some devices VF share this information
- * with PF and querying it may adduce a theoretical security risk.
+ * with PF and querying it may introduce a theoretical security risk.
* int (*ndo_set_vf_rss_query_en)(struct net_device *dev, int vf, bool setting);
* int (*ndo_get_vf_port)(struct net_device *dev, int vf, struct sk_buff *skb);
* int (*ndo_setup_tc)(struct net_device *dev, u8 tc)
@@ -1030,20 +1030,20 @@ struct tc_to_netdev {
*
* void (*ndo_add_vxlan_port)(struct net_device *dev,
* sa_family_t sa_family, __be16 port);
- * Called by vxlan to notiy a driver about the UDP port and socket
- * address family that vxlan is listnening to. It is called only when
+ * Called by vxlan to notify a driver about the UDP port and socket
+ * address family that vxlan is listening to. It is called only when
* a new port starts listening. The operation is protected by the
* vxlan_net->sock_lock.
*
* void (*ndo_add_geneve_port)(struct net_device *dev,
- * sa_family_t sa_family, __be16 port);
+ * sa_family_t sa_family, __be16 port);
* Called by geneve to notify a driver about the UDP port and socket
* address family that geneve is listnening to. It is called only when
* a new port starts listening. The operation is protected by the
* geneve_net->sock_lock.
*
* void (*ndo_del_geneve_port)(struct net_device *dev,
- * sa_family_t sa_family, __be16 port);
+ * sa_family_t sa_family, __be16 port);
* Called by geneve to notify the driver about a UDP port and socket
* address family that geneve is not listening to anymore. The operation
* is protected by the geneve_net->sock_lock.
@@ -1072,9 +1072,9 @@ struct tc_to_netdev {
* Callback to use for xmit over the accelerated station. This
* is used in place of ndo_start_xmit on accelerated net
* devices.
- * netdev_features_t (*ndo_features_check) (struct sk_buff *skb,
- * struct net_device *dev
- * netdev_features_t features);
+ * netdev_features_t (*ndo_features_check)(struct sk_buff *skb,
+ * struct net_device *dev
+ * netdev_features_t features);
* Called by core transmit path to determine if device is capable of
* performing offload operations on a given packet. This is to give
* the device an opportunity to implement any restrictions that cannot
@@ -1088,7 +1088,7 @@ struct tc_to_netdev {
* int (*ndo_get_iflink)(const struct net_device *dev);
* Called to get the iflink value of this device.
* void (*ndo_change_proto_down)(struct net_device *dev,
- * bool proto_down);
+ * bool proto_down);
* This function is used to pass protocol port error state information
* to the switch driver. The switch driver can react to the proto_down
* by doing a phys down on the associated switch port.
@@ -1100,7 +1100,7 @@ struct tc_to_netdev {
* This function is used to specify the headroom that the skb must
* consider when allocation skb during packet reception. Setting
* appropriate rx headroom value allows avoiding skb head copy on
- * forward. Setting a negative value reset the rx headroom to the
+ * forward. Setting a negative value resets the rx headroom to the
* default value.
*
*/
@@ -1296,7 +1296,7 @@ struct net_device_ops {
*
* These are the &struct net_device, they are only set internally
* by drivers and used in the kernel. These flags are invisible to
- * userspace, this means that the order of these flags can change
+ * userspace; this means that the order of these flags can change
* during any kernel release.
*
* You should have a pretty good reason to be extending these flags.
@@ -1414,10 +1414,10 @@ enum netdev_priv_flags {
*
* @state: Generic network queuing layer state, see netdev_state_t
* @dev_list: The global list of network devices
- * @napi_list: List entry, that is used for polling napi devices
- * @unreg_list: List entry, that is used, when we are unregistering the
- * device, see the function unregister_netdev
- * @close_list: List entry, that is used, when we are closing the device
+ * @napi_list: List entry used for polling NAPI devices
+ * @unreg_list: List entry when we are unregistering the
+ * device; see the function unregister_netdev
+ * @close_list: List entry used when we are closing the device
* @ptype_all: Device-specific packet handlers for all protocols
* @ptype_specific: Device-specific, protocol-specific packet handlers
*
@@ -1437,7 +1437,7 @@ enum netdev_priv_flags {
* @mpls_features: Mask of features inheritable by MPLS
*
* @ifindex: interface index
- * @group: The group, that the device belongs to
+ * @group: The group the device belongs to
*
* @stats: Statistics struct, which was left as a legacy, use
* rtnl_link_stats64 instead
@@ -1491,7 +1491,7 @@ enum netdev_priv_flags {
* @dev_port: Used to differentiate devices that share
* the same function
* @addr_list_lock: XXX: need comments on this one
- * @uc_promisc: Counter, that indicates, that promiscuous mode
+ * @uc_promisc: Counter that indicates promiscuous mode
* has been enabled due to the need to listen to
* additional unicast addresses in a device that
* does not implement ndo_set_rx_mode()
@@ -1499,9 +1499,9 @@ enum netdev_priv_flags {
* @mc: multicast mac addresses
* @dev_addrs: list of device hw addresses
* @queues_kset: Group of all Kobjects in the Tx and RX queues
- * @promiscuity: Number of times, the NIC is told to work in
- * Promiscuous mode, if it becomes 0 the NIC will
- * exit from working in Promiscuous mode
+ * @promiscuity: Number of times the NIC is told to work in
+ * promiscuous mode; if it becomes 0 the NIC will
+ * exit promiscuous mode
* @allmulti: Counter, enables or disables allmulticast mode
*
* @vlan_info: VLAN info
@@ -1547,7 +1547,7 @@ enum netdev_priv_flags {
*
* @trans_start: Time (in jiffies) of last Tx
* @watchdog_timeo: Represents the timeout that is used by
- * the watchdog ( see dev_watchdog() )
+ * the watchdog (see dev_watchdog())
* @watchdog_timer: List of timers
*
* @pcpu_refcnt: Number of references to this device
@@ -1664,8 +1664,8 @@ struct net_device {
atomic_long_t rx_nohandler;
#ifdef CONFIG_WIRELESS_EXT
- const struct iw_handler_def * wireless_handlers;
- struct iw_public_data * wireless_data;
+ const struct iw_handler_def *wireless_handlers;
+ struct iw_public_data *wireless_data;
#endif
const struct net_device_ops *netdev_ops;
const struct ethtool_ops *ethtool_ops;
@@ -1718,7 +1718,7 @@ struct net_device {
unsigned int allmulti;
- /* Protocol specific pointers */
+ /* Protocol-specific pointers */
#if IS_ENABLED(CONFIG_VLAN_8021Q)
struct vlan_info __rcu *vlan_info;
@@ -1748,13 +1748,11 @@ struct net_device {
/* Interface address info used in eth_type_trans() */
unsigned char *dev_addr;
-
#ifdef CONFIG_SYSFS
struct netdev_rx_queue *_rx;
unsigned int num_rx_queues;
unsigned int real_num_rx_queues;
-
#endif
unsigned long gro_flush_timeout;
@@ -1846,7 +1844,7 @@ struct net_device {
struct garp_port __rcu *garp_port;
struct mrp_port __rcu *mrp_port;
- struct device dev;
+ struct device dev;
const struct attribute_group *sysfs_groups[4];
const struct attribute_group *sysfs_rx_queue_group;
@@ -1861,9 +1859,9 @@ struct net_device {
#ifdef CONFIG_DCB
const struct dcbnl_rtnl_ops *dcbnl_ops;
#endif
- u8 num_tc;
- struct netdev_tc_txq tc_to_txq[TC_MAX_QUEUE];
- u8 prio_tc_map[TC_BITMASK + 1];
+ u8 num_tc;
+ struct netdev_tc_txq tc_to_txq[TC_MAX_QUEUE];
+ u8 prio_tc_map[TC_BITMASK + 1];
#if IS_ENABLED(CONFIG_FCOE)
unsigned int fcoe_ddp_xid;
@@ -1871,9 +1869,9 @@ struct net_device {
#if IS_ENABLED(CONFIG_CGROUP_NET_PRIO)
struct netprio_map __rcu *priomap;
#endif
- struct phy_device *phydev;
- struct lock_class_key *qdisc_tx_busylock;
- bool proto_down;
+ struct phy_device *phydev;
+ struct lock_class_key *qdisc_tx_busylock;
+ bool proto_down;
};
#define to_net_dev(d) container_of(d, struct net_device, dev)
@@ -2021,7 +2019,7 @@ static inline void *netdev_priv(const struct net_device *dev)
/* Set the sysfs device type for the network logical device to allow
* fine-grained identification of different network device types. For
- * example Ethernet, Wirelss LAN, Bluetooth, WiMAX etc.
+ * example Ethernet, Wireless LAN, Bluetooth, WiMAX etc.
*/
#define SET_NETDEV_DEVTYPE(net, devtype) ((net)->dev.type = (devtype))
@@ -2031,22 +2029,22 @@ static inline void *netdev_priv(const struct net_device *dev)
#define NAPI_POLL_WEIGHT 64
/**
- * netif_napi_add - initialize a napi context
+ * netif_napi_add - initialize a NAPI context
* @dev: network device
- * @napi: napi context
+ * @napi: NAPI context
* @poll: polling function
* @weight: default weight
*
- * netif_napi_add() must be used to initialize a napi context prior to calling
- * *any* of the other napi related functions.
+ * netif_napi_add() must be used to initialize a NAPI context prior to calling
+ * *any* of the other NAPI-related functions.
*/
void netif_napi_add(struct net_device *dev, struct napi_struct *napi,
int (*poll)(struct napi_struct *, int), int weight);
/**
- * netif_tx_napi_add - initialize a napi context
+ * netif_tx_napi_add - initialize a NAPI context
* @dev: network device
- * @napi: napi context
+ * @napi: NAPI context
* @poll: polling function
* @weight: default weight
*
@@ -2064,22 +2062,22 @@ static inline void netif_tx_napi_add(struct net_device *dev,
}
/**
- * netif_napi_del - remove a napi context
- * @napi: napi context
+ * netif_napi_del - remove a NAPI context
+ * @napi: NAPI context
*
- * netif_napi_del() removes a napi context from the network device napi list
+ * netif_napi_del() removes a NAPI context from the network device NAPI list
*/
void netif_napi_del(struct napi_struct *napi);
struct napi_gro_cb {
/* Virtual address of skb_shinfo(skb)->frags[0].page + offset. */
- void *frag0;
+ void *frag0;
/* Length of frag0. */
unsigned int frag0_len;
/* This indicates where we are processing relative to skb->data. */
- int data_offset;
+ int data_offset;
/* This is non-zero if the packet cannot be merged with the new skb. */
u16 flush;
@@ -2175,7 +2173,7 @@ struct udp_offload {
struct udp_offload_callbacks callbacks;
};
-/* often modified stats are per cpu, other are shared (netdev->stats) */
+/* often modified stats are per-CPU, other are shared (netdev->stats) */
struct pcpu_sw_netstats {
u64 rx_packets;
u64 rx_bytes;
@@ -2272,7 +2270,7 @@ struct netdev_notifier_changeupper_info {
struct netdev_notifier_info info; /* must be first */
struct net_device *upper_dev; /* new upper dev */
bool master; /* is upper dev master */
- bool linking; /* is the nofication for link or unlink */
+ bool linking; /* is the notification for link or unlink */
void *upper_info; /* upper dev info */
};
@@ -2737,7 +2735,7 @@ extern int netdev_flow_limit_table_len;
#endif /* CONFIG_NET_FLOW_LIMIT */
/*
- * Incoming packets are placed on per-cpu queues
+ * Incoming packets are placed on per-CPU queues
*/
struct softnet_data {
struct list_head poll_list;
@@ -2907,7 +2905,7 @@ netif_xmit_frozen_or_drv_stopped(const struct netdev_queue *dev_queue)
* @dev_queue: pointer to transmit queue
*
* BQL enabled drivers might use this helper in their ndo_start_xmit(),
- * to give appropriate hint to the cpu.
+ * to give appropriate hint to the CPU.
*/
static inline void netdev_txq_bql_enqueue_prefetchw(struct netdev_queue *dev_queue)
{
@@ -2921,7 +2919,7 @@ static inline void netdev_txq_bql_enqueue_prefetchw(struct netdev_queue *dev_que
* @dev_queue: pointer to transmit queue
*
* BQL enabled drivers might use this helper in their TX completion path,
- * to give appropriate hint to the cpu.
+ * to give appropriate hint to the CPU.
*/
static inline void netdev_txq_bql_complete_prefetchw(struct netdev_queue *dev_queue)
{
@@ -3060,7 +3058,7 @@ static inline bool netif_running(const struct net_device *dev)
}
/*
- * Routines to manage the subqueues on a device. We only need start
+ * Routines to manage the subqueues on a device. We only need start,
* stop, and a check if it's stopped. All other device management is
* done at the overall netdevice level.
* Also test the device if we're multiqueue.
@@ -3344,7 +3342,6 @@ void netif_carrier_off(struct net_device *dev);
* in a "pending" state, waiting for some external event. For "on-
* demand" interfaces, this new state identifies the situation where the
* interface is waiting for events to place it in the up state.
- *
*/
static inline void netif_dormant_on(struct net_device *dev)
{
@@ -3679,7 +3676,7 @@ void dev_uc_init(struct net_device *dev);
*
* Add newly added addresses to the interface, and release
* addresses that have been deleted.
- **/
+ */
static inline int __dev_uc_sync(struct net_device *dev,
int (*sync)(struct net_device *,
const unsigned char *),
@@ -3695,7 +3692,7 @@ static inline int __dev_uc_sync(struct net_device *dev,
* @unsync: function to call if address should be removed
*
* Remove all addresses that were added to the device by dev_uc_sync().
- **/
+ */
static inline void __dev_uc_unsync(struct net_device *dev,
int (*unsync)(struct net_device *,
const unsigned char *))
@@ -3723,7 +3720,7 @@ void dev_mc_init(struct net_device *dev);
*
* Add newly added addresses to the interface, and release
* addresses that have been deleted.
- **/
+ */
static inline int __dev_mc_sync(struct net_device *dev,
int (*sync)(struct net_device *,
const unsigned char *),
@@ -3739,7 +3736,7 @@ static inline int __dev_mc_sync(struct net_device *dev,
* @unsync: function to call if address should be removed
*
* Remove all addresses that were added to the device by dev_mc_sync().
- **/
+ */
static inline void __dev_mc_unsync(struct net_device *dev,
int (*unsync)(struct net_device *,
const unsigned char *))
^ permalink raw reply related
* Re: [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem
From: Andrew Lunn @ 2016-03-23 18:50 UTC (permalink / raw)
To: Vishal Thanki; +Cc: f.fainelli, ujhelyi.m, netdev
In-Reply-To: <1458755500-15571-1-git-send-email-vishalthanki@gmail.com>
On Wed, Mar 23, 2016 at 06:51:37PM +0100, Vishal Thanki wrote:
> Hi all,
>
> Based on suggestions from Andrew and Florian, I have made some changes
> to expose the ethernet PHY LEDs using kernel LED subsystem. The following
> ALPHA patchset introduces two new LED triggers:
>
> 1) <phydev>-eth-phy-link:
> To monitor the PHY Link status
>
> 2) <phydev>-eth-phy-activity:
> To monitor the PHY activity status. This trigger may still some more work
> because as of now it just takes decision to set the trigger based on
> PHY state machine and triggers the blink_set with delay_on and delay_off
> parameters set to 0.
My suggestion was that the hardware needs to control the LEDs. You
have software doing it. You might be able to do this with the PHY
state machine for link. But activity is never going to be accepted if
software control.
The LED trigger attached to an LED should be used to configure the
hardware to drive the LED as wanted.
The exception to this is when there is no trigger attached, and the
brightness can set the on/off state, if the hardware supports this.
Andrew
^ permalink raw reply
* Re: [PATCH 3/3] led: at8030: Add LED driver for AT8030 ethernet PHY
From: Andrew Lunn @ 2016-03-23 18:52 UTC (permalink / raw)
To: Vishal Thanki; +Cc: f.fainelli, ujhelyi.m, netdev
In-Reply-To: <1458755500-15571-4-git-send-email-vishalthanki@gmail.com>
> +static void at803x_led_work(struct work_struct *work)
> +{
> + struct at803x_phy_led *led =
> + container_of(work, struct at803x_phy_led, work);
> + union at803x_led_manual_ctrl regval = led->regval;
> +
> + phy_write(led->led_grp->phydev, led->reg, regval.value);
> +}
> +
There has recently been a big refactoring of the LED code. You should
no longer need a work queue. The core will do that for you.
Andrew
^ permalink raw reply
* Re: [PATCH 2/3] net: phy: at8030: Expose the Link and Activity LEDs
From: Andrew Lunn @ 2016-03-23 18:56 UTC (permalink / raw)
To: Vishal Thanki; +Cc: f.fainelli, ujhelyi.m, netdev
In-Reply-To: <1458755500-15571-3-git-send-email-vishalthanki@gmail.com>
On Wed, Mar 23, 2016 at 06:51:39PM +0100, Vishal Thanki wrote:
> Expose these LEDs by means of a platform device which
> can be controlled by a suitable LED driver.
I would expose the leds directly here in this driver, not via a
separate platform device.
Andrew
^ permalink raw reply
* Re: [PATCH v2] net: Fix typos and whitespace.
From: David Miller @ 2016-03-23 19:04 UTC (permalink / raw)
To: bhelgaas; +Cc: netdev, linux-kernel
In-Reply-To: <20160323184723.2198.94949.stgit@bhelgaas-glaptop2.roam.corp.google.com>
From: Bjorn Helgaas <bhelgaas@google.com>
Date: Wed, 23 Mar 2016 13:47:23 -0500
> Fix typos. Capitalize CPU, NAPI, RCU consistently. Align structure
> indentation. No functional change intended; only comment and whitespace
> changes.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Applied, thank you.
^ permalink raw reply
* Re: [RFC PATCH 8/9] i40e/i40evf: Add support for GSO partial with UDP_TUNNEL_CSUM and GRE_CSUM
From: Jesse Gross @ 2016-03-23 19:35 UTC (permalink / raw)
To: Alexander Duyck
Cc: Edward Cree, Linux Kernel Network Developers, David Miller,
Alexander Duyck, Tom Herbert
In-Reply-To: <20160318232528.14955.87859.stgit@localhost.localdomain>
On Fri, Mar 18, 2016 at 4:25 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 39b0009253c2..ac3964a9f5c0 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -9074,7 +9075,12 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
> if (!(pf->flags & I40E_FLAG_MFP_ENABLED))
> netdev->features |= NETIF_F_NTUPLE;
> if (pf->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE)
> - netdev->features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
> + netdev->features = NETIF_F_GSO_UDP_TUNNEL_CSUM;
Looks like there might be a typo - I guess this is supposed to still
OR with the previous features instead of replacing them?
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: Request 'reset' GPIO only for AT8030 PHY
From: Sergei Shtylyov @ 2016-03-23 19:42 UTC (permalink / raw)
To: Sebastian Frias, Uwe Kleine-König
Cc: Daniel Mack, David S. Miller, netdev, lkml, mason,
Florian Fainelli, Mans Rullgard, Fabio Estevam,
Martin Blumenstingl, Linus Walleij
In-Reply-To: <56F274A5.6030502@laposte.net>
Hello.
On 03/23/2016 01:49 PM, Sebastian Frias wrote:
> This removes the dependency on GPIOLIB for non faulty PHYs.
>
> Indeed, without this patch, if GPIOLIB is not selected
> devm_gpiod_get_optional() will return -ENOSYS and the driver probe
> call will fail, regardless of the actual PHY hardware.
>
> Out of the 3 PHYs supported by this driver (AT8030, AT8031, AT8035),
> only AT8030 presents the issues that commit 13a56b449325 ("net: phy:
> at803x: Add support for hardware reset") attempts to work-around by
> using a 'reset' GPIO line.
>
> Hence, only AT8030 should depend on GPIOLIB operating properly.
>
> Fixes: 13a56b449325 ("net: phy: at803x: Add support for hardware reset")
>
> Signed-off-by: Sebastian Frias <sf84@laposte.net>
[...]
What I don't understand is why the link_change_notify() method ptr is
populated for all 3 supported chips while only being needed on 8030...
MBR, Sergei
^ permalink raw reply
* [PATCH net 1/2] net: bcmgenet: fix dev->stats.tx_bytes accounting
From: Petri Gynther @ 2016-03-23 20:01 UTC (permalink / raw)
To: netdev; +Cc: davem, f.fainelli, jaedon.shin, edumazet, Petri Gynther
1. Add bytes_compl local variable to collect transmitted bytes
- dev->stats updates can then be moved outside the while-loop
- bytes_compl is also needed for future BQL support
2. When bcmgenet device uses Tx checksum offload, each transmitted
skb has an extra 64-byte header prepended to it. This needs to
be deducted when counting transmitted bytes.
3. skb->len covers the entire length of skb, whether it is linear or
fragmented. Thus, when we clean the fragments, do not increase
transmitted bytes.
Signed-off-by: Petri Gynther <pgynther@google.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 6746fd0..9990582 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1171,6 +1171,7 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
struct enet_cb *tx_cb_ptr;
struct netdev_queue *txq;
unsigned int pkts_compl = 0;
+ unsigned int bytes_compl = 0;
unsigned int c_index;
unsigned int txbds_ready;
unsigned int txbds_processed = 0;
@@ -1193,16 +1194,14 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
tx_cb_ptr = &priv->tx_cbs[ring->clean_ptr];
if (tx_cb_ptr->skb) {
pkts_compl++;
- dev->stats.tx_packets++;
- dev->stats.tx_bytes += tx_cb_ptr->skb->len;
+ bytes_compl += tx_cb_ptr->skb->len -
+ (priv->desc_64b_en ? 64 : 0);
dma_unmap_single(&dev->dev,
dma_unmap_addr(tx_cb_ptr, dma_addr),
dma_unmap_len(tx_cb_ptr, dma_len),
DMA_TO_DEVICE);
bcmgenet_free_cb(tx_cb_ptr);
} else if (dma_unmap_addr(tx_cb_ptr, dma_addr)) {
- dev->stats.tx_bytes +=
- dma_unmap_len(tx_cb_ptr, dma_len);
dma_unmap_page(&dev->dev,
dma_unmap_addr(tx_cb_ptr, dma_addr),
dma_unmap_len(tx_cb_ptr, dma_len),
@@ -1220,6 +1219,9 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
ring->free_bds += txbds_processed;
ring->c_index = (ring->c_index + txbds_processed) & DMA_C_INDEX_MASK;
+ dev->stats.tx_packets += pkts_compl;
+ dev->stats.tx_bytes += bytes_compl;
+
if (ring->free_bds > (MAX_SKB_FRAGS + 1)) {
txq = netdev_get_tx_queue(dev, ring->queue);
if (netif_tx_queue_stopped(txq))
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH net 2/2] net: bcmgenet: fix skb_len in bcmgenet_xmit_single()
From: Petri Gynther @ 2016-03-23 20:01 UTC (permalink / raw)
To: netdev; +Cc: davem, f.fainelli, jaedon.shin, edumazet, Petri Gynther
In-Reply-To: <1458763289-95167-1-git-send-email-pgynther@google.com>
skb_len needs to be skb_headlen(skb) in bcmgenet_xmit_single().
Fragmented skbs can have only Ethernet + IP + TCP headers (14+20+20=54 bytes)
in the linear buffer, followed by the rest in fragments. Bumping skb_len to
ETH_ZLEN would be incorrect for this case, as it would introduce garbage
between TCP header and the fragment data.
This also works with regular/non-fragmented small packets < ETH_ZLEN bytes.
Successfully tested this on GENETv3 with 42-byte ARP frames.
For testing, I used:
ethtool -K eth0 tx-checksum-ipv4 off
ethtool -K eth0 tx-checksum-ipv6 off
echo 0 > /proc/sys/net/ipv4/tcp_timestamps
Signed-off-by: Petri Gynther <pgynther@google.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 9990582..00da239 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1298,7 +1298,7 @@ static int bcmgenet_xmit_single(struct net_device *dev,
tx_cb_ptr->skb = skb;
- skb_len = skb_headlen(skb) < ETH_ZLEN ? ETH_ZLEN : skb_headlen(skb);
+ skb_len = skb_headlen(skb);
mapping = dma_map_single(kdev, skb->data, skb_len, DMA_TO_DEVICE);
ret = dma_mapping_error(kdev, mapping);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: [RFC PATCH 8/9] i40e/i40evf: Add support for GSO partial with UDP_TUNNEL_CSUM and GRE_CSUM
From: Alexander Duyck @ 2016-03-23 20:21 UTC (permalink / raw)
To: Jesse Gross
Cc: Alexander Duyck, Edward Cree, Linux Kernel Network Developers,
David Miller, Tom Herbert
In-Reply-To: <CAEh+42jxjkhoTeC0GK91pmX6XUpnMAxnA-pRhbG0kkQFfa2WXQ@mail.gmail.com>
On Wed, Mar 23, 2016 at 12:35 PM, Jesse Gross <jesse@kernel.org> wrote:
> On Fri, Mar 18, 2016 at 4:25 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index 39b0009253c2..ac3964a9f5c0 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -9074,7 +9075,12 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
>> if (!(pf->flags & I40E_FLAG_MFP_ENABLED))
>> netdev->features |= NETIF_F_NTUPLE;
>> if (pf->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE)
>> - netdev->features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
>> + netdev->features = NETIF_F_GSO_UDP_TUNNEL_CSUM;
>
> Looks like there might be a typo - I guess this is supposed to still
> OR with the previous features instead of replacing them?
Yes, that was a typo. It looks like I got it right on the i40vf half
of the patch but didn't get it correct here.
Thanks for catching that, I'll have it fixed for when I push it for net-next.
- Alex
^ permalink raw reply
* Re: [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem
From: Vishal Thanki @ 2016-03-23 20:24 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, Matus Ujhelyi, netdev
In-Reply-To: <20160323185006.GL5250@lunn.ch>
Hi,
> My suggestion was that the hardware needs to control the LEDs. You
> have software doing it. You might be able to do this with the PHY
> state machine for link. But activity is never going to be accepted if
> software control.
>
> The LED trigger attached to an LED should be used to configure the
> hardware to drive the LED as wanted.
>
The eth-phy-activity trigger uses the blink_set which I think uses the
hardware acceleration if available. I am not sure how to handles LEDs
which does not have hardware acceleration for this (eth-phy-activity)
trigger.
> The exception to this is when there is no trigger attached, and the
> brightness can set the on/off state, if the hardware supports this.
>
> Andrew
^ permalink raw reply
* CISCO IP PHONES AND CPU's
From: Laison, Computech, Inc <support @ 2016-03-23 18:45 UTC (permalink / raw)
To: Recipients
Hi,
Our Stock list.
Brand NEW
96 x Cisco 7900 IP Phone
87 x Unified IP Phone 6900
12 x Unified IP Phone 8900
76 x Unified IP Phone 9900
55 x Unified IP Phone 8800
67 x Cisco 1921
67 x Cisco 1941
56 x Cisco CP-7961G 7961G
34 x Cisco CP-7971G-GE 7971G
19 x Cisco Unified IP Conference Station 7937G Model: 7937 CP-7937G
20 x Cisco CP-7975G 7975G
Brand New Sealed :
23 x CISCO1921-SEC/K9
Conditions: Brand New Sealed
Description: CISCO 1921 Security Bundle w/SEC license PAK
45 x CISCO1921/K9
Conditions: Brand New Sealed
Description: CISCO 1921 Modular Router, 2 GE, 2 EHWIC slots, 512DRAM, IP Base
(1) WS-C4500X-16SFP+
Serial number: JAE183501L3
US$2600
(1) WS-C3850-48PW-S
Serial number: FCW1823C0EW
US$2650
(1) WS-X6908-10G-2T
Serial number: SAL1620CKUB
US$3650
(1) ASR1000-ESP10
Serial number: JAE181306C3
US$3800
(1) AIR-CT5508-250-K9 (this is new but box is open!)
Serial number: FCW1521L038
US$4000
CPUs part number below
89 x X5650
975 x X5660
150 x X5680
265 x X5690
Kindly make your price offers.
Sincerely
Barbara Johnson
Laison Computech
210 N Scoring Ave,
Rialto California, 92376
Tel: +1-657-232-7047
Fax: +1-347-214-0478
Email: sales@laisoncomputertech.us
^ permalink raw reply
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Edward Cree @ 2016-03-23 21:05 UTC (permalink / raw)
To: Alexander Duyck, Or Gerlitz
Cc: Alexander Duyck, Netdev, David Miller, Tom Herbert
In-Reply-To: <CAKgT0Uei6YrWRPaov3LAFrOima=o4+V4mQcFih-rHZ+w-x4hwA@mail.gmail.com>
On 23/03/16 18:06, Alexander Duyck wrote:
> On Wed, Mar 23, 2016 at 9:27 AM, Edward Cree <ecree@solarflare.com> wrote:
>> My belief is that my way is (in the long run) simpler: ultimately it gets
>> rid of per-protocol GSO callbacks entirely. Every header gets written
>> correctly* when the packet initially traverses the stack, and then at
>> transmit time you either hand that off to TSO, or do the equivalent thing
>> in software: segment at the TCP layer while treating everything above it
>> as an opaque pseudo-L2 header.
> I'm pretty sure that isn't a safe approach to take. With GSO we are
> holding off until we are about to hand the packets off to the device
> before we segment it. If we do it earlier it will lead to more issues
> as you could have the packet route off to somewhere you were not
> expecting and having it already "soft segmented" could lead to issues
> where the packet would no longer be coherent.
Ah, yes, that is a potential problem. And now that I think about it, we
might not know what the MSS is until segmentation time, either, even if
we did know for sure we would want to segment.
So my approach doesn't work after all, the superframe has to be coherent
when it traverses the stack.
>> You mean not supported by offloads, right? We can still _create_ a nested
>> tunnel device, it just won't use hardware offloads. And in the long run,
>> if we can make tunnel-agnostic TSO as I'm proposing, we should be able to
>> support it for nested tunnels too (the giant L2 header just gets more giant).
> Right. Basically what will currently happen is that if you were to
> encapsulate an ipip tunnel inside of a VXLAN for instance the GSO
> would kick in before the VXLAN tunnel has even added the outer headers
> because the VXLAN netdev does not advertise NETIF_F_GSO_IPIP. That is
> the kind of thing we want to have happen though anyway since a
> physical device wouldn't know how to deal with such a scenario anyway.
I disagree. Surely we should be able to "soft segment" the packet just
before we give it to the physical device, and then tell it to do dumb copying
of both the VXLAN and IPIP headers? At this point, we don't have the problem
you identified above, because we've arrived at the device now.
So we can chase through some per-protocol callbacks to shorten all the outer
lengths and adjust all the outer checksums, then hand it to the device for
TSO. The device is treating the extra headers as an opaque blob, so it
doesn't know or care whether it's one layer of encapsulation or forty-two.
>> I think an L2 header extension is a better semantic match for what's
>> happening (the tunnel is an L2 device and we're sending L3 packets over
>> it). But why does it matter? Are you giving the hardware the L2 and
>> L3 headers in separate DMA descriptors or something? The way I see it,
>> all hardware needs to be told is "where to start TSO from", and how it
>> thinks of the stuff before that doesn't matter, because it's not
>> supposed to touch it anyway.
> The problem is setups like VFs where they want to know where the
> network header starts so they know where to insert a VLAN tag. Most
> hardware supports IP options or IPv6 extension headers. What I am
> doing is exploiting that in the case of the Intel hardware by telling
> it the IP header is the outer IP header and the transport header is
> the inner transport header. Then all I have to deal with is the fact
> that hardware will try to compute the entire IPv4 header checksum over
> the range so I cancel that out by seeding the IP checksum with the
> lco_csum added to the inner pseudo-header checksum.
Ok, it sounds like the interface to Intel hardware is just Very Different
to Solarflare hardware on this point: we don't tell our hardware anything
about where the various headers start, it just parses them to figure it
out. (And for new-style TSO we'd tell it where the TCP header starts, as
I described before.)
But this sounds like a driver-level thing: you have to undo some of what
your hardware will do because you're having to lie to it about what you're
giving it. So that all happens in the driver, the stack's GSO code isn't
affected. In which case I'm much less bothered by this; I thought it was
an assumption you were baking into the stack. As far as I'm concerned you
can do whatever ugly hacks you like in Intel drivers, as long as I don't
have to do them in sfc ;)
Although, why is your device computing the IPv4 header checksum? Those
aren't supposed to be offloaded, the stack always already filled them in
in software. (I think sfc makes the same mistake actually.) Is there not
a way for you to tell your device to skip IP header checksum offload?
Then you wouldn't have this problem in the first place, and you could tell
it the IP header was as big as you like without having to seed it with
this correction value.
>> Like I say, I'm assuming we'll start setting DF on outer frames.
>> Besides, it doesn't matter what happens "in transit" - as long as the
>> outer headers aren't touched by the transmitting NIC, the network can
>> do what it likes to them afterwards, without it breaking our TSO code.
> The IP ID bit is going to be something where I don't want to break
> things. One of the things I have seen is there ends up being a number
> of occasions where VXLAN gets fragmented due to incorrectly configured
> MTU. I would rather not have it get completely screwed up when this
> occurs. A performance hit for fragmenting is one thing. Having
> people start complaining because previously working tunnels suddenly
> stop functioning is another. The fact is the whole point of VXLAN is
> to support sending the tunneled frames between data centers. With the
> DF bit set on a tunnel header we end up making it so that frames get
> dropped instead of fragmented which would be a change of behavior.
I agree this isn't something we can do silently. But we _can_ make it a
condition for enabling gso-partial. And I think it's a necessary
condition for truly generic TSO. Sure, your 'L3 extension header' works
fine for a single tunnel. But if you nest tunnels, you now need to
update the outer _and_ middle IP IDs, and you can't do that because you
only have one L3 header pointer.
OTOH, for a single tunnel I think we could implement your 'L3 extension
header' trick in firmware, by making it always parse the outer packet up
to the outer L3 header and increment the IP ID in that. So I could live
with that approach if necessary.
> Yes, I am very confident of that. For Intel hardware the outer VLAN
> tag would be the one inserted via software, the inner VLAN tag is the
> one inserted via hardware.
That's really weird; why does it do that?
For sfc, the only reason we do hardware VLANs at all is to transparently
tag a VF (or non-primary PF) that's being passed through into an
(untrusted) VM. For that use case you'd always want to insert the outer
VLAN tag, because otherwise the VM can escape the isolation by inserting
a different VLAN tag in software.
>> _However_, if we don't need to update the IP IDs, then we can just take
>> the offset of the inner L4 header, and it doesn't make any difference
>> whether you choose to think of the stuff before that as L2 + giant L3
>> or as giant L2 + normal L3, because it's not part of the OS->NIC
>> interface (which is just "L4 starts here"). If your NIC needs to be
>> told where the outer L3 starts as well, then, I guess that's just a
>> wart you need in your drivers. You have skb->network_header, so that
>> shouldn't be difficult - that will always point to the outer one.
> Right. That is kind of what I was going for with this setup. The
> only issue is that the VXLAN tunnels not setting the DF bit kind of
> get in the way of the giant L3 approach.
On the contrary; your giant L3 approach is exactly what solves this case
(for non-nested tunnels) - the hardware has the outer IP and inner TCP
header offsets, which are exactly the two headers it needs to alter.
And if the hardware is sensible, it won't try to re-checksum the whole
giant L3 header, it'll just decrement the (outer) IP checksum to account
for incrementing the (outer) IP ID. If the hardware isn't sensible,
then you have to play games like you are doing in the Intel drivers ;)
> Dealing with the outer header needing the DF bit is something I have
> left unaddressed at this point. The question would be what is the
> correct approach to take for all this. I know RFC2003 for IPv4 in
> IPv4 says you must set the DF bit if the inner header has the DF bit
> set. I'm just wondering if we can apply the same type of logic to GRE
> and UDP tunnels.
I wonder if _not_ setting the DF bit in the outer header can harm TCP
congestion control at all? If so, then we'd pretty much have to set it
in that case.
> My concern is that I am not sure how much value there is to add with
> this type of code if the hardware is parsing headers. In the case of
> most of the Intel parts you specify offsets for the network and
> transport headers so it gives you some degree of flexibility. If
> however the hardware parses headers it becomes problematic as we can
> only support protocols that can be parsed by the hardware.
Solarflare parts will _normally_ parse headers to get that information.
But, when doing TSO, we do get a chance to specify some extra
information, in the TSO option descriptor. Enough of the datapath is
under firmware control that that should be enough; as long as the outer
frame is IP over Ethernet, the hardware will parse that fine, and we
*should* be able to make it just accept that it doesn't know what's
going on between that and the start of the TCP header. And, it
shouldn't matter that the hardware can parse some types of tunnel
headers, because we'll just tell it to ignore that.
Of course, that means changing the firmware; luckily we haven't got any
parts in the wild doing tunnel offloads yet, so we still have a chance
to do that without needing driver code to work around our past
mistakes...
But this stuff does definitely add value for us, it means we could TSO
any tunnel type whatsoever; even nested tunnels as long as only the
outermost IP ID needs to change.
-Ed
^ permalink raw reply
* Re: [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem
From: Andrew Lunn @ 2016-03-23 21:10 UTC (permalink / raw)
To: Vishal Thanki; +Cc: Florian Fainelli, Matus Ujhelyi, netdev
In-Reply-To: <CAC3a_SCD6aMF=triCQDJrW9GQ3HOMMH9q_J9GEw56qrnwgFXKA@mail.gmail.com>
On Wed, Mar 23, 2016 at 09:24:00PM +0100, Vishal Thanki wrote:
> Hi,
>
> > My suggestion was that the hardware needs to control the LEDs. You
> > have software doing it. You might be able to do this with the PHY
> > state machine for link. But activity is never going to be accepted if
> > software control.
> >
> > The LED trigger attached to an LED should be used to configure the
> > hardware to drive the LED as wanted.
> >
>
> The eth-phy-activity trigger uses the blink_set which I think uses the
> hardware acceleration if available. I am not sure how to handles LEDs
> which does not have hardware acceleration for this (eth-phy-activity)
> trigger.
We want the LED to blink on activity, real packets coming in and
out. The PHY can do this, so let the PHY control the LED. In this
case, the trigger is just mechanism for the user to say what the LED
should be used for. The trigger is not itself controlling the LED, it
has no idea about packets coming and going.
Andrew
^ permalink raw reply
* Re: [PATCH net 1/2] net: bcmgenet: fix dev->stats.tx_bytes accounting
From: Eric Dumazet @ 2016-03-23 21:23 UTC (permalink / raw)
To: Petri Gynther; +Cc: netdev, davem, f.fainelli, jaedon.shin, edumazet
In-Reply-To: <1458763289-95167-1-git-send-email-pgynther@google.com>
On Wed, 2016-03-23 at 13:01 -0700, Petri Gynther wrote:
> 1. Add bytes_compl local variable to collect transmitted bytes
> - dev->stats updates can then be moved outside the while-loop
> - bytes_compl is also needed for future BQL support
> 2. When bcmgenet device uses Tx checksum offload, each transmitted
> skb has an extra 64-byte header prepended to it. This needs to
> be deducted when counting transmitted bytes.
> 3. skb->len covers the entire length of skb, whether it is linear or
> fragmented. Thus, when we clean the fragments, do not increase
> transmitted bytes.
>
> Signed-off-by: Petri Gynther <pgynther@google.com>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 6746fd0..9990582 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1171,6 +1171,7 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
> struct enet_cb *tx_cb_ptr;
> struct netdev_queue *txq;
> unsigned int pkts_compl = 0;
> + unsigned int bytes_compl = 0;
> unsigned int c_index;
> unsigned int txbds_ready;
> unsigned int txbds_processed = 0;
> @@ -1193,16 +1194,14 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
> tx_cb_ptr = &priv->tx_cbs[ring->clean_ptr];
> if (tx_cb_ptr->skb) {
> pkts_compl++;
> - dev->stats.tx_packets++;
> - dev->stats.tx_bytes += tx_cb_ptr->skb->len;
> + bytes_compl += tx_cb_ptr->skb->len -
> + (priv->desc_64b_en ? 64 : 0);
Please do not use this 64 scalar, prefer sizeof(struct status_64)
I know this driver uses skb_pull(skb, 64); len -= 64; in
bcmgenet_desc_rx(), but I would prefer not adding magic values.
Thanks.
^ permalink raw reply
* Re: [PATCH net 2/2] net: bcmgenet: fix skb_len in bcmgenet_xmit_single()
From: Eric Dumazet @ 2016-03-23 21:24 UTC (permalink / raw)
To: Petri Gynther; +Cc: netdev, davem, f.fainelli, jaedon.shin, edumazet
In-Reply-To: <1458763289-95167-2-git-send-email-pgynther@google.com>
On Wed, 2016-03-23 at 13:01 -0700, Petri Gynther wrote:
> skb_len needs to be skb_headlen(skb) in bcmgenet_xmit_single().
>
> Fragmented skbs can have only Ethernet + IP + TCP headers (14+20+20=54 bytes)
> in the linear buffer, followed by the rest in fragments. Bumping skb_len to
> ETH_ZLEN would be incorrect for this case, as it would introduce garbage
> between TCP header and the fragment data.
>
> This also works with regular/non-fragmented small packets < ETH_ZLEN bytes.
> Successfully tested this on GENETv3 with 42-byte ARP frames.
>
> For testing, I used:
> ethtool -K eth0 tx-checksum-ipv4 off
> ethtool -K eth0 tx-checksum-ipv6 off
> echo 0 > /proc/sys/net/ipv4/tcp_timestamps
>
> Signed-off-by: Petri Gynther <pgynther@google.com>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 9990582..00da239 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1298,7 +1298,7 @@ static int bcmgenet_xmit_single(struct net_device *dev,
>
> tx_cb_ptr->skb = skb;
>
> - skb_len = skb_headlen(skb) < ETH_ZLEN ? ETH_ZLEN : skb_headlen(skb);
> + skb_len = skb_headlen(skb);
>
> mapping = dma_map_single(kdev, skb->data, skb_len, DMA_TO_DEVICE);
> ret = dma_mapping_error(kdev, mapping);
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* CISCO IP PHONES AND CPU's
From: Laison, Computech, Inc <support @ 2016-03-23 19:12 UTC (permalink / raw)
To: Recipients
Hi,
Our Stock list.
Brand NEW
96 x Cisco 7900 IP Phone
87 x Unified IP Phone 6900
12 x Unified IP Phone 8900
76 x Unified IP Phone 9900
55 x Unified IP Phone 8800
67 x Cisco 1921
67 x Cisco 1941
56 x Cisco CP-7961G 7961G
34 x Cisco CP-7971G-GE 7971G
19 x Cisco Unified IP Conference Station 7937G Model: 7937 CP-7937G
20 x Cisco CP-7975G 7975G
Brand New Sealed :
23 x CISCO1921-SEC/K9
Conditions: Brand New Sealed
Description: CISCO 1921 Security Bundle w/SEC license PAK
45 x CISCO1921/K9
Conditions: Brand New Sealed
Description: CISCO 1921 Modular Router, 2 GE, 2 EHWIC slots, 512DRAM, IP Base
(1) WS-C4500X-16SFP+
Serial number: JAE183501L3
US$2600
(1) WS-C3850-48PW-S
Serial number: FCW1823C0EW
US$2650
(1) WS-X6908-10G-2T
Serial number: SAL1620CKUB
US$3650
(1) ASR1000-ESP10
Serial number: JAE181306C3
US$3800
(1) AIR-CT5508-250-K9 (this is new but box is open!)
Serial number: FCW1521L038
US$4000
CPUs part number below
89 x X5650
975 x X5660
150 x X5680
265 x X5690
Kindly make your price offers.
Sincerely
Barbara Johnson
Laison Computech
210 N Scoring Ave,
Rialto California, 92376
Tel: +1-657-232-7047
Fax: +1-347-214-0478
Email: sales@laisoncomputertech.us
^ permalink raw reply
* Re: [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem
From: Vishal Thanki @ 2016-03-23 21:24 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, Matus Ujhelyi, netdev
In-Reply-To: <20160323211010.GF19953@lunn.ch>
On Wed, Mar 23, 2016 at 10:10 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Mar 23, 2016 at 09:24:00PM +0100, Vishal Thanki wrote:
>> Hi,
>>
>> > My suggestion was that the hardware needs to control the LEDs. You
>> > have software doing it. You might be able to do this with the PHY
>> > state machine for link. But activity is never going to be accepted if
>> > software control.
>> >
>> > The LED trigger attached to an LED should be used to configure the
>> > hardware to drive the LED as wanted.
>> >
>>
>> The eth-phy-activity trigger uses the blink_set which I think uses the
>> hardware acceleration if available. I am not sure how to handles LEDs
>> which does not have hardware acceleration for this (eth-phy-activity)
>> trigger.
>
> We want the LED to blink on activity, real packets coming in and
> out. The PHY can do this, so let the PHY control the LED. In this
> case, the trigger is just mechanism for the user to say what the LED
> should be used for. The trigger is not itself controlling the LED, it
> has no idea about packets coming and going.
>
Yes, I understand that. But PHY can only control the LEDs attached to
it directly. The at803x led driver configures the PHY to blink the
activity LED based on traffic but I think it is not possible for PHY
to control other LEDs in system, for example some other LEDs in system
controlled only via GPIO. In such cases, putting PHY activity trigger
on the GPIO LEDs would not make sense. Correct me if I am wrong.
Vishal
^ permalink raw reply
* Re: [PATCH net 1/2] net: bcmgenet: fix dev->stats.tx_bytes accounting
From: Florian Fainelli @ 2016-03-23 21:47 UTC (permalink / raw)
To: Eric Dumazet, Petri Gynther; +Cc: netdev, davem, jaedon.shin, edumazet
In-Reply-To: <1458768237.10868.46.camel@edumazet-glaptop3.roam.corp.google.com>
On March 23, 2016 2:23:57 PM PDT, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>On Wed, 2016-03-23 at 13:01 -0700, Petri Gynther wrote:
>> 1. Add bytes_compl local variable to collect transmitted bytes
>> - dev->stats updates can then be moved outside the while-loop
>> - bytes_compl is also needed for future BQL support
>> 2. When bcmgenet device uses Tx checksum offload, each transmitted
>> skb has an extra 64-byte header prepended to it. This needs to
>> be deducted when counting transmitted bytes.
>> 3. skb->len covers the entire length of skb, whether it is linear or
>> fragmented. Thus, when we clean the fragments, do not increase
>> transmitted bytes.
>>
>> Signed-off-by: Petri Gynther <pgynther@google.com>
>> ---
>> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> index 6746fd0..9990582 100644
>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> @@ -1171,6 +1171,7 @@ static unsigned int
>__bcmgenet_tx_reclaim(struct net_device *dev,
>> struct enet_cb *tx_cb_ptr;
>> struct netdev_queue *txq;
>> unsigned int pkts_compl = 0;
>> + unsigned int bytes_compl = 0;
>> unsigned int c_index;
>> unsigned int txbds_ready;
>> unsigned int txbds_processed = 0;
>> @@ -1193,16 +1194,14 @@ static unsigned int
>__bcmgenet_tx_reclaim(struct net_device *dev,
>> tx_cb_ptr = &priv->tx_cbs[ring->clean_ptr];
>> if (tx_cb_ptr->skb) {
>> pkts_compl++;
>> - dev->stats.tx_packets++;
>> - dev->stats.tx_bytes += tx_cb_ptr->skb->len;
>> + bytes_compl += tx_cb_ptr->skb->len -
>> + (priv->desc_64b_en ? 64 : 0);
>
>Please do not use this 64 scalar, prefer sizeof(struct status_64)
>
>I know this driver uses skb_pull(skb, 64); len -= 64; in
>bcmgenet_desc_rx(), but I would prefer not adding magic values.
Agreed, and just so we are more robust and ready to add BQL in the features please store this information in skb->cb, in the same spirit as what this does:
https://github.com/ffainelli/linux/commits/genet-bql
Thanks.
--
Florian
^ permalink raw reply
* [GIT PULL] target for-next-merge updates for v4.6-rc1
From: Nicholas A. Bellinger @ 2016-03-23 21:53 UTC (permalink / raw)
To: Linus Torvalds
Cc: target-devel, Linux-netdev, David Miller, Hariprasad Shenai,
Varun Prakash, swise, indranil, kxie
Hello Linus,
Here are the target-pending for-next-merge updates for v4.6-rc1.
Please go ahead and pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git for-next-merge
You'll hit one minor merge conflict in t4fw_api.h with net-next as
reported by SFR:
https://lkml.org/lkml/2016/2/29/34
This series contains cxgb4 driver prerequisites for supporting iscsi
segmentation offload (ISO), that will be utilized for a number of future
v4.7 developments in iscsi-target for supporting generic hw offloads.
Thank you,
--nab
Varun Prakash (12):
cxgb4: add new ULD type CXGB4_ULD_ISCSIT
cxgb4: allocate resources for CXGB4_ULD_ISCSIT
cxgb4: large receive offload support
cxgb4, iw_cxgb4: move definitions to common header file
cxgb4, iw_cxgb4, cxgb4i: remove duplicate definitions
cxgb4, cxgb4i: move struct cpl_rx_data_ddp definition
cxgb4: add definitions for iSCSI target ULD
cxgb4: update struct cxgb4_lld_info definition
cxgb4: move VLAN_NONE macro definition
cxgb4, iw_cxgb4: move delayed ack macro definitions
cxgb4: add iSCSI DDP page pod manager
cxgb4: update Kconfig and Makefile
drivers/infiniband/hw/cxgb4/t4fw_ri_api.h | 99 -----
drivers/net/ethernet/chelsio/Kconfig | 11 +
drivers/net/ethernet/chelsio/cxgb4/Makefile | 1 +
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 27 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 34 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 97 ++++-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_ppm.c | 464 +++++++++++++++++++++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_ppm.h | 310 ++++++++++++++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.h | 11 +
drivers/net/ethernet/chelsio/cxgb4/l2t.c | 2 -
drivers/net/ethernet/chelsio/cxgb4/l2t.h | 2 +
drivers/net/ethernet/chelsio/cxgb4/sge.c | 13 +-
drivers/net/ethernet/chelsio/cxgb4/t4_msg.h | 217 ++++++++++
drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h | 7 +
drivers/scsi/cxgbi/cxgb4i/cxgb4i.h | 17 -
15 files changed, 1174 insertions(+), 138 deletions(-)
create mode 100644 drivers/net/ethernet/chelsio/cxgb4/cxgb4_ppm.c
create mode 100644 drivers/net/ethernet/chelsio/cxgb4/cxgb4_ppm.h
^ permalink raw reply
* [PATCH net-next] hv_netvsc: Fix the order of num_sc_offered decrement
From: Haiyang Zhang @ 2016-03-23 21:54 UTC (permalink / raw)
To: davem, netdev; +Cc: olaf, haiyangz, driverdev-devel, linux-kernel
Reorder the code in netvsc_sc_open(), so num_sc_offered is only decremented
after vmbus_open() is called. This avoid pontential race of removing device
before all channels are setup.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/net/hyperv/rndis_filter.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index d5a54da..c4e1e04 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -986,12 +986,6 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
nvscdev = hv_get_drvdata(new_sc->primary_channel->device_obj);
- spin_lock_irqsave(&nvscdev->sc_lock, flags);
- nvscdev->num_sc_offered--;
- spin_unlock_irqrestore(&nvscdev->sc_lock, flags);
- if (nvscdev->num_sc_offered == 0)
- complete(&nvscdev->channel_init_wait);
-
if (chn_index >= nvscdev->num_chn)
return;
@@ -1004,6 +998,12 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
if (ret == 0)
nvscdev->chn_table[chn_index] = new_sc;
+
+ spin_lock_irqsave(&nvscdev->sc_lock, flags);
+ nvscdev->num_sc_offered--;
+ spin_unlock_irqrestore(&nvscdev->sc_lock, flags);
+ if (nvscdev->num_sc_offered == 0)
+ complete(&nvscdev->channel_init_wait);
}
int rndis_filter_device_add(struct hv_device *dev,
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH net 1/2] net: bcmgenet: fix dev->stats.tx_bytes accounting
From: Florian Fainelli @ 2016-03-23 22:02 UTC (permalink / raw)
To: Petri Gynther, netdev; +Cc: davem, jaedon.shin, edumazet
In-Reply-To: <1458763289-95167-1-git-send-email-pgynther@google.com>
On March 23, 2016 1:01:28 PM PDT, Petri Gynther <pgynther@google.com> wrote:
>1. Add bytes_compl local variable to collect transmitted bytes
> - dev->stats updates can then be moved outside the while-loop
> - bytes_compl is also needed for future BQL support
>2. When bcmgenet device uses Tx checksum offload, each transmitted
> skb has an extra 64-byte header prepended to it. This needs to
> be deducted when counting transmitted bytes.
>3. skb->len covers the entire length of skb, whether it is linear or
> fragmented. Thus, when we clean the fragments, do not increase
> transmitted bytes.
>
>Signed-off-by: Petri Gynther <pgynther@google.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
--
Florian
^ permalink raw reply
* Re: [PATCH net 1/2] net: bcmgenet: fix dev->stats.tx_bytes accounting
From: Florian Fainelli @ 2016-03-23 22:09 UTC (permalink / raw)
To: Petri Gynther, netdev; +Cc: davem, jaedon.shin, edumazet
In-Reply-To: <1458763289-95167-1-git-send-email-pgynther@google.com>
On March 23, 2016 1:01:28 PM PDT, Petri Gynther <pgynther@google.com> wrote:
>1. Add bytes_compl local variable to collect transmitted bytes
> - dev->stats updates can then be moved outside the while-loop
> - bytes_compl is also needed for future BQL support
>2. When bcmgenet device uses Tx checksum offload, each transmitted
> skb has an extra 64-byte header prepended to it. This needs to
> be deducted when counting transmitted bytes.
>3. skb->len covers the entire length of skb, whether it is linear or
> fragmented. Thus, when we clean the fragments, do not increase
> transmitted bytes.
>
>Signed-off-by: Petri Gynther <pgynther@google.com>
Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>index 6746fd0..9990582 100644
>--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>@@ -1171,6 +1171,7 @@ static unsigned int __bcmgenet_tx_reclaim(struct
>net_device *dev,
> struct enet_cb *tx_cb_ptr;
> struct netdev_queue *txq;
> unsigned int pkts_compl = 0;
>+ unsigned int bytes_compl = 0;
> unsigned int c_index;
> unsigned int txbds_ready;
> unsigned int txbds_processed = 0;
>@@ -1193,16 +1194,14 @@ static unsigned int
>__bcmgenet_tx_reclaim(struct net_device *dev,
> tx_cb_ptr = &priv->tx_cbs[ring->clean_ptr];
> if (tx_cb_ptr->skb) {
> pkts_compl++;
>- dev->stats.tx_packets++;
>- dev->stats.tx_bytes += tx_cb_ptr->skb->len;
>+ bytes_compl += tx_cb_ptr->skb->len -
>+ (priv->desc_64b_en ? 64 : 0);
> dma_unmap_single(&dev->dev,
> dma_unmap_addr(tx_cb_ptr, dma_addr),
> dma_unmap_len(tx_cb_ptr, dma_len),
> DMA_TO_DEVICE);
> bcmgenet_free_cb(tx_cb_ptr);
> } else if (dma_unmap_addr(tx_cb_ptr, dma_addr)) {
>- dev->stats.tx_bytes +=
>- dma_unmap_len(tx_cb_ptr, dma_len);
> dma_unmap_page(&dev->dev,
> dma_unmap_addr(tx_cb_ptr, dma_addr),
> dma_unmap_len(tx_cb_ptr, dma_len),
>@@ -1220,6 +1219,9 @@ static unsigned int __bcmgenet_tx_reclaim(struct
>net_device *dev,
> ring->free_bds += txbds_processed;
> ring->c_index = (ring->c_index + txbds_processed) & DMA_C_INDEX_MASK;
>
>+ dev->stats.tx_packets += pkts_compl;
>+ dev->stats.tx_bytes += bytes_compl;
>+
> if (ring->free_bds > (MAX_SKB_FRAGS + 1)) {
> txq = netdev_get_tx_queue(dev, ring->queue);
> if (netif_tx_queue_stopped(txq))
--
Florian
^ permalink raw reply
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Alexander Duyck @ 2016-03-23 22:36 UTC (permalink / raw)
To: Edward Cree
Cc: Or Gerlitz, Alexander Duyck, Netdev, David Miller, Tom Herbert
In-Reply-To: <56F30523.8080108@solarflare.com>
On Wed, Mar 23, 2016 at 2:05 PM, Edward Cree <ecree@solarflare.com> wrote:
> On 23/03/16 18:06, Alexander Duyck wrote:
>> On Wed, Mar 23, 2016 at 9:27 AM, Edward Cree <ecree@solarflare.com> wrote:
>>> My belief is that my way is (in the long run) simpler: ultimately it gets
>>> rid of per-protocol GSO callbacks entirely. Every header gets written
>>> correctly* when the packet initially traverses the stack, and then at
>>> transmit time you either hand that off to TSO, or do the equivalent thing
>>> in software: segment at the TCP layer while treating everything above it
>>> as an opaque pseudo-L2 header.
>> I'm pretty sure that isn't a safe approach to take. With GSO we are
>> holding off until we are about to hand the packets off to the device
>> before we segment it. If we do it earlier it will lead to more issues
>> as you could have the packet route off to somewhere you were not
>> expecting and having it already "soft segmented" could lead to issues
>> where the packet would no longer be coherent.
> Ah, yes, that is a potential problem. And now that I think about it, we
> might not know what the MSS is until segmentation time, either, even if
> we did know for sure we would want to segment.
> So my approach doesn't work after all, the superframe has to be coherent
> when it traverses the stack.
Right.
>>> You mean not supported by offloads, right? We can still _create_ a nested
>>> tunnel device, it just won't use hardware offloads. And in the long run,
>>> if we can make tunnel-agnostic TSO as I'm proposing, we should be able to
>>> support it for nested tunnels too (the giant L2 header just gets more giant).
>> Right. Basically what will currently happen is that if you were to
>> encapsulate an ipip tunnel inside of a VXLAN for instance the GSO
>> would kick in before the VXLAN tunnel has even added the outer headers
>> because the VXLAN netdev does not advertise NETIF_F_GSO_IPIP. That is
>> the kind of thing we want to have happen though anyway since a
>> physical device wouldn't know how to deal with such a scenario anyway.
> I disagree. Surely we should be able to "soft segment" the packet just
> before we give it to the physical device, and then tell it to do dumb copying
> of both the VXLAN and IPIP headers? At this point, we don't have the problem
> you identified above, because we've arrived at the device now.
One issue here is that all levels of IP headers would have to have the
DF bit set. I don't think that happens right now.
> So we can chase through some per-protocol callbacks to shorten all the outer
> lengths and adjust all the outer checksums, then hand it to the device for
> TSO. The device is treating the extra headers as an opaque blob, so it
> doesn't know or care whether it's one layer of encapsulation or forty-two.
So if we do pure software offloads this is doable. However the GSO
flags are meant to have hardware feature equivalents. The problem is
if you combine an IPIP and VXLAN header how do you know what header is
what and which order things are in, and what is the likelihood of
having a device that would get things right when dealing with 3 levels
of IP headers. This is one of the reasons why we don't support
multiple levels of tunnels in the GSO code. GSO is just meant to be a
fall-back for hardware offloads.
>>> I think an L2 header extension is a better semantic match for what's
>>> happening (the tunnel is an L2 device and we're sending L3 packets over
>>> it). But why does it matter? Are you giving the hardware the L2 and
>>> L3 headers in separate DMA descriptors or something? The way I see it,
>>> all hardware needs to be told is "where to start TSO from", and how it
>>> thinks of the stuff before that doesn't matter, because it's not
>>> supposed to touch it anyway.
>> The problem is setups like VFs where they want to know where the
>> network header starts so they know where to insert a VLAN tag. Most
>> hardware supports IP options or IPv6 extension headers. What I am
>> doing is exploiting that in the case of the Intel hardware by telling
>> it the IP header is the outer IP header and the transport header is
>> the inner transport header. Then all I have to deal with is the fact
>> that hardware will try to compute the entire IPv4 header checksum over
>> the range so I cancel that out by seeding the IP checksum with the
>> lco_csum added to the inner pseudo-header checksum.
> Ok, it sounds like the interface to Intel hardware is just Very Different
> to Solarflare hardware on this point: we don't tell our hardware anything
> about where the various headers start, it just parses them to figure it
> out. (And for new-style TSO we'd tell it where the TCP header starts, as
> I described before.)
That is kind of what I figured. So does that mean for IPv6 you guys
are parsing through extension headers? I believe that is one of the
reasons why Intel did things the way they did is to avoid having to
parse through any IPv4 options or IPv6 extension headers.
> But this sounds like a driver-level thing: you have to undo some of what
> your hardware will do because you're having to lie to it about what you're
> giving it. So that all happens in the driver, the stack's GSO code isn't
> affected. In which case I'm much less bothered by this; I thought it was
> an assumption you were baking into the stack. As far as I'm concerned you
> can do whatever ugly hacks you like in Intel drivers, as long as I don't
> have to do them in sfc ;)
The thing is I have to have the hardware recompute the outer IPv4
checksum because it is updating the ID field. If I were able to leave
the IP ID static then I wouldn't need to update the checksum. The
only bit that makes it ugly is the fact that the hardware is being
misled so it thinks the IPv4 header has 50 bytes of options when those
actually represent the outer L4 through to the inner L3 headers.
> Although, why is your device computing the IPv4 header checksum? Those
> aren't supposed to be offloaded, the stack always already filled them in
> in software. (I think sfc makes the same mistake actually.) Is there not
> a way for you to tell your device to skip IP header checksum offload?
> Then you wouldn't have this problem in the first place, and you could tell
> it the IP header was as big as you like without having to seed it with
> this correction value.
It is because we are rewriting the IP ID. We cannot have a static
checksum if the IP ID is getting updated.
>>> Like I say, I'm assuming we'll start setting DF on outer frames.
>>> Besides, it doesn't matter what happens "in transit" - as long as the
>>> outer headers aren't touched by the transmitting NIC, the network can
>>> do what it likes to them afterwards, without it breaking our TSO code.
>> The IP ID bit is going to be something where I don't want to break
>> things. One of the things I have seen is there ends up being a number
>> of occasions where VXLAN gets fragmented due to incorrectly configured
>> MTU. I would rather not have it get completely screwed up when this
>> occurs. A performance hit for fragmenting is one thing. Having
>> people start complaining because previously working tunnels suddenly
>> stop functioning is another. The fact is the whole point of VXLAN is
>> to support sending the tunneled frames between data centers. With the
>> DF bit set on a tunnel header we end up making it so that frames get
>> dropped instead of fragmented which would be a change of behavior.
> I agree this isn't something we can do silently. But we _can_ make it a
> condition for enabling gso-partial. And I think it's a necessary
> condition for truly generic TSO. Sure, your 'L3 extension header' works
> fine for a single tunnel. But if you nest tunnels, you now need to
> update the outer _and_ middle IP IDs, and you can't do that because you
> only have one L3 header pointer.
This is getting away from the 'less is more' concept. If we are doing
multiple levels of tunnels we have already made things far too
complicated and it is unlikely hardware will ever support anything
like that.
> OTOH, for a single tunnel I think we could implement your 'L3 extension
> header' trick in firmware, by making it always parse the outer packet up
> to the outer L3 header and increment the IP ID in that. So I could live
> with that approach if necessary.
Good to hear.
>> Yes, I am very confident of that. For Intel hardware the outer VLAN
>> tag would be the one inserted via software, the inner VLAN tag is the
>> one inserted via hardware.
> That's really weird; why does it do that?
> For sfc, the only reason we do hardware VLANs at all is to transparently
> tag a VF (or non-primary PF) that's being passed through into an
> (untrusted) VM. For that use case you'd always want to insert the outer
> VLAN tag, because otherwise the VM can escape the isolation by inserting
> a different VLAN tag in software.
The problem is I think the feature was implemented long before any of
the SR-IOV stuff was added and it has been carried that way through
the igb and ixgbe drivers. I haven't looked at i40e but it doesn't
report doing STAG so I don't know how it handles QinQ.
>>> _However_, if we don't need to update the IP IDs, then we can just take
>>> the offset of the inner L4 header, and it doesn't make any difference
>>> whether you choose to think of the stuff before that as L2 + giant L3
>>> or as giant L2 + normal L3, because it's not part of the OS->NIC
>>> interface (which is just "L4 starts here"). If your NIC needs to be
>>> told where the outer L3 starts as well, then, I guess that's just a
>>> wart you need in your drivers. You have skb->network_header, so that
>>> shouldn't be difficult - that will always point to the outer one.
>> Right. That is kind of what I was going for with this setup. The
>> only issue is that the VXLAN tunnels not setting the DF bit kind of
>> get in the way of the giant L3 approach.
Sorry meant giant L2 approach here, not L3.
> On the contrary; your giant L3 approach is exactly what solves this case
> (for non-nested tunnels) - the hardware has the outer IP and inner TCP
> header offsets, which are exactly the two headers it needs to alter.
> And if the hardware is sensible, it won't try to re-checksum the whole
> giant L3 header, it'll just decrement the (outer) IP checksum to account
> for incrementing the (outer) IP ID. If the hardware isn't sensible,
> then you have to play games like you are doing in the Intel drivers ;)
Agreed.
>> Dealing with the outer header needing the DF bit is something I have
>> left unaddressed at this point. The question would be what is the
>> correct approach to take for all this. I know RFC2003 for IPv4 in
>> IPv4 says you must set the DF bit if the inner header has the DF bit
>> set. I'm just wondering if we can apply the same type of logic to GRE
>> and UDP tunnels.
> I wonder if _not_ setting the DF bit in the outer header can harm TCP
> congestion control at all? If so, then we'd pretty much have to set it
> in that case.
Really the only concern with DF based on RFC 2003 is for path MTU
discovery. What happens right now if you set the VXLAN MTU
incorrectly is you end up taking a performance hit as the tunneled
frames are fragmented and have to be reassembled on the other end. It
ends up being a performance hit due to the fragmentation. I might
need to look into this. It is possible that we are already doing the
wrong stuff anyway with this and might need to look at setting the DF
bit anyway.
>> My concern is that I am not sure how much value there is to add with
>> this type of code if the hardware is parsing headers. In the case of
>> most of the Intel parts you specify offsets for the network and
>> transport headers so it gives you some degree of flexibility. If
>> however the hardware parses headers it becomes problematic as we can
>> only support protocols that can be parsed by the hardware.
> Solarflare parts will _normally_ parse headers to get that information.
> But, when doing TSO, we do get a chance to specify some extra
> information, in the TSO option descriptor. Enough of the datapath is
> under firmware control that that should be enough; as long as the outer
> frame is IP over Ethernet, the hardware will parse that fine, and we
> *should* be able to make it just accept that it doesn't know what's
> going on between that and the start of the TCP header. And, it
> shouldn't matter that the hardware can parse some types of tunnel
> headers, because we'll just tell it to ignore that.
Right. Just skipping the tunnel headers makes it quite a bit easier.
> Of course, that means changing the firmware; luckily we haven't got any
> parts in the wild doing tunnel offloads yet, so we still have a chance
> to do that without needing driver code to work around our past
> mistakes...
>
> But this stuff does definitely add value for us, it means we could TSO
> any tunnel type whatsoever; even nested tunnels as long as only the
> outermost IP ID needs to change.
Right. In your case it sounds like you would have the advantage of
just having to run essentially two counters, one increments the IPv4
ID and the other decrements the IPv4 checksum. Beyond that the outer
headers wouldn't need to change at all.
The only other issue would be determining how the inner pseudo-header
checksum is updated. If you were parsing out header fields from the
IP header previously to generate it you would instead need to update
things so that you could use the partial checksum that is already
stored in the TCP header checksum field.
- Alex
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox