* 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: [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 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 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
* [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] 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
* Re: [PATCH net] ppp: take reference on channels netns
From: David Miller @ 2016-03-23 18:37 UTC (permalink / raw)
To: g.nault; +Cc: netdev, gorcunov, sploving1, paulus
In-Reply-To: <4dbdfa8e264d5a635865c6e843c122daf9be4f8d.1458747209.git.g.nault@alphalink.fr>
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Wed, 23 Mar 2016 16:38:55 +0100
> Let channels hold a reference on their network namespace.
> Some channel types, like ppp_async and ppp_synctty, can have their
> userspace controller running in a different namespace. Therefore they
> can't rely on them to preclude their netns from being removed from
> under them.
...
> Fixes: 273ec51dd7ce ("net: ppp_generic - introduce net-namespace functionality v2")
> Reported-by: Baozeng Ding <sploving1@gmail.com>
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
Applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: [PATCH] vlan: propagate gso_min_segs
From: Eric Dumazet @ 2016-03-23 18:34 UTC (permalink / raw)
To: David Miller; +Cc: yanhaishuang, kaber, nicolas.dichtel, netdev, linux-kernel
In-Reply-To: <20160323.142556.2023977823696366302.davem@davemloft.net>
On Wed, 2016-03-23 at 14:25 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 22 Mar 2016 19:33:52 -0700
>
> > On Wed, 2016-03-23 at 09:35 +0800, Haishuang Yan wrote:
> >> vlan drivers lack proper propagation of gso_min_segs from lower device.
> >>
> >> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
> >> ---
> >
> > The plan was to get rid of gso_min_segs, as nothing uses it.
> >
> > Otherwise I would have included this in my recent patches...
> >
> > For such a rare 'issue' , we believe ndo_features_check() is better,
> > because it does not slow down the fast path.
>
> I'm not aplpying this.
I am adding to my TODO list a revert, when net-next opens again.
Thanks.
^ permalink raw reply
* Re: [PATCH] net: Fix typos and whitespace.
From: David Miller @ 2016-03-23 18:27 UTC (permalink / raw)
To: bhelgaas; +Cc: netdev, linux-kernel
In-Reply-To: <20160323134530.18742.35082.stgit@bhelgaas-glaptop2.roam.corp.google.com>
From: Bjorn Helgaas <bhelgaas@google.com>
Date: Wed, 23 Mar 2016 08:45:30 -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>
Does not apply to the current 'net' tree, please respin.
^ permalink raw reply
* Re: [PATCH] vlan: propagate gso_min_segs
From: David Miller @ 2016-03-23 18:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: yanhaishuang, kaber, nicolas.dichtel, netdev, linux-kernel
In-Reply-To: <1458700432.10868.32.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 22 Mar 2016 19:33:52 -0700
> On Wed, 2016-03-23 at 09:35 +0800, Haishuang Yan wrote:
>> vlan drivers lack proper propagation of gso_min_segs from lower device.
>>
>> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
>> ---
>
> The plan was to get rid of gso_min_segs, as nothing uses it.
>
> Otherwise I would have included this in my recent patches...
>
> For such a rare 'issue' , we believe ndo_features_check() is better,
> because it does not slow down the fast path.
I'm not aplpying this.
^ permalink raw reply
* Re: [PATCH] gre: fix return value of gre_rcv
From: David Miller @ 2016-03-23 18:25 UTC (permalink / raw)
To: yanhaishuang; +Cc: kuznet, jmorris, kaber, netdev, linux-kernel
In-Reply-To: <1458696883-15796-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
From: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
Date: Wed, 23 Mar 2016 09:34:43 +0800
> Dropped skb's should be documented by an appropriate return value.
> Use the correct NET_RX_DROP and NET_RX_SUCCESS values for that reason.
>
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
IP protocol handlers do not use NET_RX_* values like this.
They either return 0, to finish processing, or a negative value which
indicates the negated protocol number to use to demux in the protocol
switch again.
Many of your patches seem very straightforward, but usually upon
further inspection severe fundamental issues with your changes are
found because you don't know what the proper semantics are for the
pieces of infrastructure you are touching.
Therefore, reviewing your seemingly trivial changes is a large
inventment of time and energy.
Please put more care into your changes, or else people will simply
stop reviewing your work and instead put their effort into submissions
which are more properly researched and easier to verify.
Thanks.
^ permalink raw reply
* Re: [net PATCH] net: Reset encap_level to avoid resetting features on inner IP headers
From: David Miller @ 2016-03-23 18:19 UTC (permalink / raw)
To: aduyck; +Cc: tom, kernel-team, alexander.duyck, netdev
In-Reply-To: <20160322231732.13054.71337.stgit@localhost.localdomain>
From: Alexander Duyck <aduyck@mirantis.com>
Date: Tue, 22 Mar 2016 16:18:07 -0700
> This patch corrects an oversight in which we were allowing the encap_level
> value to pass from the outer headers to the inner headers. As a result we
> were incorrectly identifying UDP or GRE tunnels as also making use of ipip
> or sit when the second header actually represented a tunnel encapsulated in
> either a UDP or GRE tunnel which already had the features masked.
>
> Fixes: 76443456227097179c1482 ("net: Move GSO csum into SKB_GSO_CB")
> Reported-by: Tom Herbert <tom@herbertland.com>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
Applied, thanks.
^ permalink raw reply
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Alexander Duyck @ 2016-03-23 18:19 UTC (permalink / raw)
To: Tom Herbert
Cc: Alexander Duyck, Edward Cree, Linux Kernel Network Developers,
David S. Miller
In-Reply-To: <CALx6S35wK2_eR1e5OTHnQTkQRg763unS0sCMQU+Jk6e27F26Vw@mail.gmail.com>
On Wed, Mar 23, 2016 at 10:09 AM, Tom Herbert <tom@herbertland.com> wrote:
> On Fri, Mar 18, 2016 at 4:25 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> This patch adds support for something I am referring to as GSO partial.
>> The basic idea is that we can support a broader range of devices for
>> segmentation if we use fixed outer headers and have the hardware only
>> really deal with segmenting the inner header. The idea behind the naming
>> is due to the fact that everything before csum_start will be fixed headers,
>> and everything after will be the region that is handled by hardware.
>>
> Personally, I don't like the name ;-) This technique should be
> "generic" to handle almost all GSO except those case where headers are
> not fixed which we should be able to avoid as a design point in any
> new encapsulations. Besides, what if someday we perform GSO on
> something where csum_start is not set?
Well the "partial" component of it refers to the fact that we have to
do a certain amount of this in software before we offload the work to
hardware. So it isn't a full segmentation offload, it is a partial
one.
The csum_start is a bit of a red herring in terms of the naming. I
was using csum_start as that works for TCP offloads since it will
always point to the inner-most transport header. It was basically
just a convenient optimization for the drivers and makes it so that we
can essentially use just one code path in the case of ixgbe since
network header and checksum_start will always represent the two
headers we need to update for TSO regardless of it being partial or
complete offload.
> Can you add some description about strategy for dealing with ip_id?
Yeah. I still need to add more documentation. I just didn't want to
get into details on it until we have finalized things a bit more. I'm
still wondering if we should follow the pattern of ipip tunnels in
terms of setting the DF bit if the inner header has the DF bit set.
If we end up needing to add code to do that then it makes it so that
the ip_id value can be fixed for both inner and outer which makes the
segmentation much simpler since the only header that would ever really
need to be updated would be the transport header in order to get the
checksum correct.
- Alex
^ permalink raw reply
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Alexander Duyck @ 2016-03-23 18:06 UTC (permalink / raw)
To: Edward Cree, Or Gerlitz
Cc: Alexander Duyck, Netdev, David Miller, Tom Herbert
In-Reply-To: <56F2C3F8.8070308@solarflare.com>
On Wed, Mar 23, 2016 at 9:27 AM, Edward Cree <ecree@solarflare.com> wrote:
> On 22/03/16 21:38, Alexander Duyck wrote:
>> On Tue, Mar 22, 2016 at 12:40 PM, Edward Cree <ecree@solarflare.com> wrote:
>>> But won't the tunnel dev have the feature flag for GSO_PARTIAL depending
>>> on what the underlying dev advertises? (Or, at least, could we make it
>>> bethatway?)
>> This stuff doesn't work. That is why tunnels now advertise all
>> available features that can be offloaded via software. Basically if
>> we can advertise a feature we do, and then we sort things out in
>> software if we cannot actually do it in hardware.
> Fair enough; then go withthe other approach:
>>> Alternatively, have per-protocol GSO callbacks to do the fixup in the
>>> opposite direction to what you have now - in the long term we hope that
>>> hardware supporting GSO partial will become the common case, so that
>>> should be the fast path without bouncing backwards through GSO callbacks.
>>> Then, if you find out at GSO time that the hardware wants to do old-style
>>> TSO, you call those callbacks so as to give it a superframe with the long
>>> lengths filled in everywhere.
>> I thought about doing that but decided it was much simpler to simply
>> update all headers. For now I want to keep this as simple as possible
>> while we get the first few drivers on board. If we later want to
>> optimize and add complexity we can go that route, but for now this
>> change is more than fast enough as it allows me to push an i40e at
>> 20Gb/s while sending frames with outer checksums enabled.
> 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.
> *That is, correctly for a single segment, rather than correctly for the
> superframe.
Yes, but what about the cases where a packets gets switched from Tx
back to Rx? How would you expect the stack to handle that then?
>>> Yes; it's a clever idea. Only trouble is that we really want theinner IP
>>> header rather than the inner TCP header, so that we can (if we want to)
>>> increment the inner IP IDs. Of course, if we Officially Don't Care about
>>> inner IP IDs that's not a problem.
>> The inner IP IDs are the ones that are guaranteed to be the ones we
>> can leave fixed since TCP will require that the DF bit be set.
> I was worrying about the SLHC stuff, I thought the inner ones were precisely
> the ones where that was a problem. If we really don't care about it, then
> we do just need the inner TCP header, and we can stick with using your
> csum_start == gso_start trick :)
>> The
>> current VXLAN implementation does not set DF for the outer headers.
>> So really we don't have too many options right now if we are wanting
>> to support tunnels.
> I was predicating all this on the assumption that tunnels would be changed
> to set DF in their outer frames; I thought I saw a patch recently to do
> that, but maybe I was mistaken. In any case I think that's the right thing
> to do, and it's a necessary prerequisite for truly tunnel-agnostic TSO.
>>> Iwonder if we could just always fill in inner_network_headereven if we're
>>> not doing encapsulation. Or does it end up pointing to a 'middle' header
>>> in the case of nested encap?
>> Right now neseted encap is not supported because tunnels don't
>> advertise hw_encap_features.
> 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.
>>> Why not use the single-segment length in the pseudo-header, then the
>>> outer L4 checksum is already the right thing? (And if yourhardware
>>> can't be told to leave the outer L4 checksum alone, then it's not worth
>>> the trouble of trying to support GSO partial for it, since it clearly
>>> wants to do old-style "NIC knows best" TSO.)
>> The problem then becomes that I needs special case code. One for
>> standard TSO and one for this special case. If I leave it as is I can
>> use the same code to cancel out the length in the TCP pseudo-header
>> checksum for either case.
> I don't think that's true. Look at it this way: there are two choices.
> 1) The normal path builds things for standard TSO, and we have code to
> fix it up for this new-style TSO.
> 2) The normal path builds things for new-style TSO, and we have code to
> fix it up for standard TSO.
> Now the fix-up code should be of equal complexity in either case, because
> the two cases are inverses of each other. So we should choose whichever
> approach makes the normal path simpler. I think having TCP 'lie' to all
> the outer layers about the length of the packet is simpler, because then
> they don't even have to know that GSO is happening. They just append a
> header appropriate for an MSS-sized packet, blissfully unaware that the
> payload data carries on for longer than that.
> Moreover, the fixup in (2) can sometimes be avoided...
> I don't know how your hardware does encapsulated TSO, but what ours does
> (in the old "I'm-a-smart-NIC-I-know-best" world) is that it computes both
> checksums itself from scratch. So it doesn't care what it was given as
> the pre-seeded value, because it's reconstructing the pseudo-header and
> zeroing the checksum field to begin with.
> I would have expected this to be the common behaviour for "smart" NICs
> doing encap TSO, in which case (2) is a clear winner. And, of course,
> once "less is more" hardware becomes common, we will want (2) anyway.
The argument for here is a matter of complexity. I really don't think
we gain much by recomputing the pseudo-header with the segmented
length versus the entire packet length. In the case of gso it is more
complicated to figure out the length if we are having to take gso_size
and add the size for a TCP header rather than just subtracting the
inner transport offset from skb->len.
>> The general idea to my approach is to treat the the UDP or GRE header
>> to the inner IP header as an IP header extension type value. That
>> will allow us to trick most hardware into being able to accept it.
> 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.
>>> Again, the idea is that we optimise for GSO partial by making it a plain
>>> header copy everywhere, and put all the 'fix things up' on the _other_
>>> path.
>> That is what I went for. The only part that isn't a plain header copy
>> is the TCP pseudo-header checksum.
> Right, but that means someone, somewhere, has to fix up the outer UDP
> checksum to account for that (because the TCP phdr sum leaks out of LCO).
> I realise that's a per-superframe job, rather than a per-segment job, but
> it still means that code at GSO time (when we decide whether to do GSO or
> GSO-partial) has to know enough about the tunnel headers to find the outer
> checksum and fix it up.
The fix-up already happens in the GSO code path I have coded up. The
outer checksum already takes it into account.
If you look at my patches you should see a block in both the UDP
tunnel and GRE tunnel code that handles the "skb_is_gso()" case. That
is where we deal with the checksum adjustment. In the case of UDP
tunnels it actually requires very little work as the only field that
actually has to be updated is uh->len since the outer pseduo-header
checksum effectively cancels out the length for the inner anyway. In
the case of GRE it took a bit more as I had to seed the checksum with
the delta of the length in order to correctly account for the change.
> And it's a question of who pays that cost, the old TSO or the new one; see
> above for why I think the old TSO should pay it.
Generally I don't agree with slowing down existing paths in order to
improve the performance for new ones. One of my goals with this was
to keep the code minimally intrusive. I would rather not have to
rewrite all driver TSO paths in order to support a new segmentation
approach.
>> The only issue I see is the expectation that the outer headers go
>> untouched is problematic. The outer headers are where things like
>> fragmentation will occur in transit.
> 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.
>> In addition I suspect a number
>> of devices other than the Intel NICs probably use the network header
>> to determine where to insert L2 tags such as VLAN.
> Ah, VLAN is interesting, because there's two things you might want:
> VLAN inside the tunnel, or outside of it. Presumably if you're having
> the NIC insert the VLAN, you want it outside (e.g. you're doing SR-IOV
> and you're putting the VF on a VLAN).
> But then it doesn't make sense to work backwards from the network
> header, because that'll get confused by traffic that's already VLAN
> tagged - because again, you want to insert the new VLAN as the outer
> VLAN. You need to find the Ethertype, as well; it just seems like
> working backwards from L3 is crazy. Particularly when working forwards
> from L2 is so easy - you know your L2 header begins at the start of the
> packet, you (hopefully) know it's Ethernet, so you know where the VLAN
> tags and Ethertype go. (Are you /sure/ Intel works backwards from the
> L3 header? I'm pretty confident we don't.)
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.
> Then of course this works fine with the 'giant L2 header', because the
> NIC just inserts the VLAN as normal in the outer Ethernet header and
> shunts the remaining headers along to make room for it. In fact, in
> our NICs I think that's done by an entirely separate bit of hardware
> that doesn't even have to know that TSO was considering much more of
> the packet to be "L2 header".
Well the Intel hardware doesn't work that way. It would be nice if it
did because then you could actually support Q-in-Q on the VFs without
much issue, but due to the limiting nature of a 64b transmit
descriptor they opted to insert the VLAN tags at the start of the
network header.
> _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.
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.
>> Like I have said with the current solution I could probably update
>> igb, igbvf, fm10k, ixgbe, ixgbevf, i40e, and i40evf to support this.
>> That covers pretty much the entire Intel series of drivers in terms of
>> enterprise. The question is what do I need to enable to support other
>> drivers. It doesn't seem like there would be too much but the bit I
>> would need to know is what the expectations are when computing outer
>> IPv4 checksums.
> Like I say, the plan we had for Solarflare NICs before "less is more"
> happened was that the device would reconstruct the outer pseudo-header
> (same as it does with the inner) and ignore any pre-seeded value in the
> checksum field. I'd expected other vendors to have gone down the same
> route, but if Intel didn't then maybe others didn't either. It'd be
> nice if some of them would chime in and let us know what they want...
Right. I added Or to the Cc. Maybe we can get some input from
Mellanox on how they do TSO and what changes they would need in order
to support TSO for GRE or UDP tunnels with checksum.
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. So for
example on the Intel parts using the drivers igb and ixgbe I will be
able to support VXLAN-GPE using a partial GSO approach. However for
fm10k which does some parsing for the tunnel headers I probably won't
as it will not know what to do if there are any extra headers included
and won't be able to determine the offset of the inner transport
header.
- Alex
^ permalink raw reply
* [PATCH 3/3] led: at8030: Add LED driver for AT8030 ethernet PHY
From: Vishal Thanki @ 2016-03-23 17:51 UTC (permalink / raw)
To: andrew, f.fainelli, ujhelyi.m, netdev; +Cc: Vishal Thanki
In-Reply-To: <1458755500-15571-1-git-send-email-vishalthanki@gmail.com>
This patch adds support for controlling the Link and
Activity LED of AT8030 ethernet PHY through LED subsystem.
Signed-off-by: Vishal Thanki <vishalthanki@gmail.com>
---
drivers/leds/Kconfig | 7 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-at803x.c | 158 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 166 insertions(+)
create mode 100644 drivers/leds/leds-at803x.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1f64151..07781ac 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -615,6 +615,13 @@ config LEDS_VERSATILE
This option enabled support for the LEDs on the ARM Versatile
and RealView boards. Say Y to enabled these.
+config LEDS_AT803X
+ tristate "LED support for the AT803X Ethernet PHY (Experimental)"
+ depends on LEDS_CLASS
+ help
+ This option enabled support for the LEDs on AT803X ethernet PHY.
+
+
comment "LED Triggers"
source "drivers/leds/trigger/Kconfig"
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index cb2013d..2d1ae65 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_KTD2692) += leds-ktd2692.o
obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o
obj-$(CONFIG_LEDS_SEAD3) += leds-sead3.o
obj-$(CONFIG_LEDS_IS31FL32XX) += leds-is31fl32xx.o
+obj-$(CONFIG_LEDS_AT803X) += leds-at803x.o
# LED SPI Drivers
obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
diff --git a/drivers/leds/leds-at803x.c b/drivers/leds/leds-at803x.c
new file mode 100644
index 0000000..58eb0c4
--- /dev/null
+++ b/drivers/leds/leds-at803x.c
@@ -0,0 +1,158 @@
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/leds.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/phy/at803x.h>
+
+#define LED_TX_NORMAL 0x0
+#define LED_TX_BLINK 0x1
+#define LED_TX_OFF 0x2
+#define LED_TX_ON 0x3
+
+#define LED_RX_NORMAL 0x0
+#define LED_RX_BLINK 0x1
+#define LED_RX_OFF 0x2
+#define LED_RX_ON 0x3
+
+#define LED_ACT_BLINK 0x1
+
+#define LED_LINK_NORMAL 0x0
+#define LED_LINK_BLINK LED_ACT_BLINK
+#define LED_LINK_OFF 0x2
+#define LED_LINK_ON 0x3
+
+#define LED_ACT_ACTIVE 0x0
+#define LED_ACT_LINK 0x1
+
+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);
+}
+
+static int at803x_blink_set(struct led_classdev *cdev,
+ unsigned long *delay_on,
+ unsigned long *delay_off)
+{
+ struct at803x_phy_led *led =
+ container_of(cdev, struct at803x_phy_led, cdev);
+ union at803x_led_manual_ctrl regval = {.value = 0};
+ int id = led->id;
+ unsigned long flags;
+
+ spin_lock_irqsave(&led->lock, flags);
+ switch (id) {
+ case AT803X_LINK:
+ /* Can't configure LED_LINK to blink */
+ break;
+ case AT803X_ACT:
+ /*
+ * LED will be ON when link is established
+ * and it will blink when link is active.
+ * delay_on/delay_off values are ignored.
+ */
+ regval.field.led_act_ctrl = LED_ACT_LINK;
+ break;
+ }
+ led->regval = regval;
+
+ schedule_work(&led->work);
+ spin_unlock_irqrestore(&led->lock, flags);
+ return 0;
+}
+
+
+static void at803x_phy_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ struct at803x_phy_led *led =
+ container_of(led_cdev, struct at803x_phy_led, cdev);
+ union at803x_led_manual_ctrl regval = led->regval;
+ int id = led->id;
+ unsigned long flags;
+
+ spin_lock_irqsave(&led->lock, flags);
+ switch (id) {
+ case AT803X_LINK:
+ regval.field.led_lnk_10_100_ctrl = value == LED_FULL ?
+ LED_LINK_ON : LED_LINK_OFF;
+ break;
+ case AT803X_ACT:
+ /* LED_ACT status is controlled via LED_TX and LED_RX */
+ regval.field.led_tx = value == LED_FULL ?
+ LED_TX_ON : LED_TX_OFF;
+ regval.field.led_rx = value == LED_FULL ?
+ LED_RX_ON : LED_RX_OFF;
+ break;
+ }
+
+ led->value = value;
+ led->regval = regval;
+ schedule_work(&led->work);
+ spin_unlock_irqrestore(&led->lock, flags);
+}
+
+static int at803x_phy_led_probe(struct platform_device *pdev)
+{
+ int i;
+ int ret;
+ struct at803x_phy_leds *led_grp = platform_get_drvdata(pdev);
+ struct at803x_phy_led *leds = led_grp->leds;
+
+ for (i = 0; i < led_grp->nr_leds; i++) {
+
+ leds[i].cdev.brightness_set = at803x_phy_led_brightness_set;
+ leds[i].cdev.blink_set = at803x_blink_set;
+
+ ret = led_classdev_register(&pdev->dev, &leds[i].cdev);
+ if (ret < 0)
+ goto err;
+
+ leds[i].led_grp = led_grp;
+ leds[i].regval.value = phy_read(led_grp->phydev, leds[i].reg);
+ INIT_WORK(&leds[i].work, at803x_led_work);
+ spin_lock_init(&leds[i].lock);
+
+ }
+
+ return 0;
+
+err:
+ for (i = i - 1; i >= 0; i--)
+ led_classdev_unregister(&leds[i].cdev);
+
+ return ret;
+}
+
+static int at803x_phy_led_remove(struct platform_device *pdev)
+{
+ int i;
+ struct at803x_phy_leds *led_grp = platform_get_drvdata(pdev);
+ struct at803x_phy_led *leds = led_grp->leds;
+
+
+ for (i = 0; i < led_grp->nr_leds; i++)
+ led_classdev_unregister(&leds[i].cdev);
+
+ return 0;
+}
+
+static struct platform_driver at803x_phy_led_driver = {
+ .probe = at803x_phy_led_probe,
+ .remove = at803x_phy_led_remove,
+ .driver = {
+ .name = "at803x-led",
+ },
+};
+
+module_platform_driver(at803x_phy_led_driver);
+
+MODULE_DESCRIPTION("at803x PHY LED driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:leds-at803x");
--
2.4.3
^ permalink raw reply related
* [PATCH 2/3] net: phy: at8030: Expose the Link and Activity LEDs
From: Vishal Thanki @ 2016-03-23 17:51 UTC (permalink / raw)
To: andrew, f.fainelli, ujhelyi.m, netdev; +Cc: Vishal Thanki
In-Reply-To: <1458755500-15571-1-git-send-email-vishalthanki@gmail.com>
Expose these LEDs by means of a platform device which
can be controlled by a suitable LED driver.
Signed-off-by: Vishal Thanki <vishalthanki@gmail.com>
---
drivers/net/phy/at803x.c | 55 +++++++++++++++++++++++++++++++++++++++++++++-
include/linux/phy/at803x.h | 45 +++++++++++++++++++++++++++++++++++++
2 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644 include/linux/phy/at803x.h
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 1e901c7..8fdb5ff 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -18,6 +18,8 @@
#include <linux/etherdevice.h>
#include <linux/of_gpio.h>
#include <linux/gpio/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/phy/at803x.h>
#define AT803X_INTR_ENABLE 0x12
#define AT803X_INTR_ENABLE_AUTONEG_ERR BIT(15)
@@ -34,6 +36,7 @@
#define AT803X_SMART_SPEED 0x14
#define AT803X_LED_CONTROL 0x18
+#define AT803X_LED_MANUAL_CTRL 0x19
#define AT803X_DEVICE_ADDR 0x03
#define AT803X_LOC_MAC_ADDR_0_15_OFFSET 0x804C
@@ -117,6 +120,46 @@ static inline int at803x_enable_tx_delay(struct phy_device *phydev)
AT803X_DEBUG_TX_CLK_DLY_EN);
}
+static struct at803x_phy_led leds[] = {
+ {
+ .cdev.name = "at803x-link",
+ .id = AT803X_LINK,
+ .reg = AT803X_LED_MANUAL_CTRL,
+ },
+ {
+ .cdev.name = "at803x-act",
+ .id = AT803X_ACT,
+ .reg = AT803X_LED_MANUAL_CTRL,
+ },
+};
+
+static struct at803x_phy_leds led_grp = {
+ .leds = leds,
+ .nr_leds = ARRAY_SIZE(leds),
+};
+
+static struct platform_device *pdev;
+
+static int at803x_led_init(struct phy_device *phydev)
+{
+ led_grp.phydev = phydev;
+
+ pdev = platform_device_alloc("at803x-led", 0);
+ if (!pdev) {
+ pr_info("Failed to allocate LED\n");
+ return -ENOMEM;
+ }
+
+ platform_set_drvdata(pdev, &led_grp);
+
+ return platform_device_add(pdev);
+}
+
+static void at803x_led_cleanup(struct phy_device *phydev)
+{
+ platform_device_del(pdev);
+}
+
/* save relevant PHY registers to private copy */
static void at803x_context_save(struct phy_device *phydev,
struct at803x_context *context)
@@ -285,9 +328,18 @@ static int at803x_probe(struct phy_device *phydev)
phydev->priv = priv;
+ if (phydev->drv->flags & PHY_HAS_LED_CTRL)
+ return at803x_led_init(phydev);
+
return 0;
}
+static void at803x_remove(struct phy_device *phydev)
+{
+ if (phydev->drv->flags & PHY_HAS_LED_CTRL)
+ at803x_led_cleanup(phydev);
+}
+
static int at803x_config_init(struct phy_device *phydev)
{
int ret;
@@ -411,11 +463,12 @@ static struct phy_driver at803x_driver[] = {
.suspend = at803x_suspend,
.resume = at803x_resume,
.features = PHY_BASIC_FEATURES,
- .flags = PHY_HAS_INTERRUPT,
+ .flags = PHY_HAS_INTERRUPT | PHY_HAS_LED_CTRL,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = at803x_ack_interrupt,
.config_intr = at803x_config_intr,
+ .remove = at803x_remove,
}, {
/* ATHEROS 8031 */
.phy_id = ATH8031_PHY_ID,
diff --git a/include/linux/phy/at803x.h b/include/linux/phy/at803x.h
new file mode 100644
index 0000000..f33bfcf
--- /dev/null
+++ b/include/linux/phy/at803x.h
@@ -0,0 +1,45 @@
+#ifndef _AT803X_H
+#define _AT803X_H
+
+#include <linux/leds.h>
+#include <linux/phy/phy.h>
+#include <linux/workqueue.h>
+#include <linux/spinlock.h>
+
+enum at803x_led_type {
+ AT803X_LINK,
+ AT803X_ACT,
+};
+
+union at803x_led_manual_ctrl {
+ struct {
+ u16 led_tx:2;
+ u16 led_rx:2;
+ u16 res3:2;
+ u16 led_lnk_10_100_ctrl:2;
+ u16 res2:4;
+ u16 led_act_ctrl:1;
+ u16 res1:3;
+
+ } field;
+ u16 value;
+};
+
+struct at803x_phy_led {
+ struct work_struct work;
+ struct led_classdev cdev;
+ enum led_brightness value;
+ union at803x_led_manual_ctrl regval;
+ enum at803x_led_type id;
+ struct at803x_phy_leds *led_grp;
+ spinlock_t lock;
+ u16 reg;
+};
+
+struct at803x_phy_leds {
+ struct phy_device *phydev;
+ struct at803x_phy_led *leds;
+ int nr_leds;
+};
+
+#endif
--
2.4.3
^ permalink raw reply related
* [PATCH 1/3] net: phy: Add ethernet PHY LED triggers
From: Vishal Thanki @ 2016-03-23 17:51 UTC (permalink / raw)
To: andrew, f.fainelli, ujhelyi.m, netdev; +Cc: Vishal Thanki
In-Reply-To: <1458755500-15571-1-git-send-email-vishalthanki@gmail.com>
Add the LED triggers for ethernet PHY link and activity
status. The triggers are set mainly based on the
PHY state machine.
Signed-off-by: Vishal Thanki <vishalthanki@gmail.com>
---
drivers/net/phy/Kconfig | 7 +++++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/phy.c | 20 +++++++++++--
drivers/net/phy/phy_device.c | 4 +++
drivers/net/phy/phy_led.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
drivers/net/phy/phy_led.h | 37 +++++++++++++++++++++++
include/linux/leds.h | 1 +
include/linux/phy.h | 6 ++++
8 files changed, 144 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/phy/phy_led.c
create mode 100644 drivers/net/phy/phy_led.h
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 6dad9a9..15712da 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -271,6 +271,13 @@ config MDIO_BCM_IPROC
This module provides a driver for the MDIO busses found in the
Broadcom iProc SoC's.
+config ETH_PHY_LED
+ bool "Ethernet PHY LED trigger support (Experimental)"
+ depends on LEDS_TRIGGERS
+ help
+ This feature enables the Ethernet PHY LED triggers for PHY link
+ and data transfer activities.
+
endif # PHYLIB
config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index fcdbb92..76ebd83 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -44,3 +44,4 @@ obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o
obj-$(CONFIG_MDIO_BCM_UNIMAC) += mdio-bcm-unimac.o
obj-$(CONFIG_MICROCHIP_PHY) += microchip.o
obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
+obj-$(CONFIG_ETH_PHY_LED) += phy_led.o
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 5590b9c..e422ff6 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -35,6 +35,7 @@
#include <linux/io.h>
#include <linux/uaccess.h>
#include <linux/atomic.h>
+#include "phy_led.h"
#include <asm/irq.h>
@@ -831,15 +832,15 @@ void phy_state_machine(struct work_struct *work)
switch (phydev->state) {
case PHY_DOWN:
+ phy_trig_led_link(phydev, 0);
case PHY_STARTING:
case PHY_READY:
case PHY_PENDING:
break;
case PHY_UP:
needs_aneg = true;
-
phydev->link_timeout = PHY_AN_TIMEOUT;
-
+ phy_trig_led_link(phydev, 1);
break;
case PHY_AN:
err = phy_read_status(phydev);
@@ -849,6 +850,7 @@ void phy_state_machine(struct work_struct *work)
/* If the link is down, give up on negotiation for now */
if (!phydev->link) {
phydev->state = PHY_NOLINK;
+ phy_trig_led_link(phydev, 0);
netif_carrier_off(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
break;
@@ -862,6 +864,7 @@ void phy_state_machine(struct work_struct *work)
/* If AN is done, we're running */
if (err > 0) {
phydev->state = PHY_RUNNING;
+ phy_trig_led_act(phydev);
netif_carrier_on(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
@@ -889,6 +892,7 @@ void phy_state_machine(struct work_struct *work)
}
}
phydev->state = PHY_RUNNING;
+ phy_trig_led_act(phydev);
netif_carrier_on(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
}
@@ -900,6 +904,8 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->state = PHY_RUNNING;
+ phy_trig_led_act(phydev);
+ phy_trig_led_link(phydev, 1);
netif_carrier_on(phydev->attached_dev);
} else {
if (0 == phydev->link_timeout--)
@@ -929,9 +935,12 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->state = PHY_RUNNING;
+ phy_trig_led_act(phydev);
+ phy_trig_led_link(phydev, 1);
netif_carrier_on(phydev->attached_dev);
} else {
phydev->state = PHY_NOLINK;
+ phy_trig_led_link(phydev, 0);
netif_carrier_off(phydev->attached_dev);
}
@@ -948,6 +957,7 @@ void phy_state_machine(struct work_struct *work)
phydev->adjust_link(phydev->attached_dev);
do_suspend = true;
}
+ led_trigger_event(phydev->phy_act, LED_OFF);
break;
case PHY_RESUMING:
if (AUTONEG_ENABLE == phydev->autoneg) {
@@ -965,9 +975,12 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->state = PHY_RUNNING;
+ phy_trig_led_link(phydev, 1);
+ phy_trig_led_act(phydev);
netif_carrier_on(phydev->attached_dev);
} else {
phydev->state = PHY_NOLINK;
+ phy_trig_led_link(phydev, 0);
}
phydev->adjust_link(phydev->attached_dev);
} else {
@@ -981,9 +994,12 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->state = PHY_RUNNING;
+ phy_trig_led_link(phydev, 1);
+ phy_trig_led_act(phydev);
netif_carrier_on(phydev->attached_dev);
} else {
phydev->state = PHY_NOLINK;
+ phy_trig_led_link(phydev, 0);
}
phydev->adjust_link(phydev->attached_dev);
}
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e551f3a..fafcc3c 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -34,6 +34,7 @@
#include <linux/io.h>
#include <linux/uaccess.h>
#include <linux/of.h>
+#include "phy_led.h"
#include <asm/irq.h>
@@ -1592,6 +1593,8 @@ static int phy_probe(struct device *dev)
of_set_phy_supported(phydev);
phydev->advertising = phydev->supported;
+ phy_led_init(phydev);
+
/* Set the state to READY by default */
phydev->state = PHY_READY;
@@ -1608,6 +1611,7 @@ static int phy_remove(struct device *dev)
struct phy_device *phydev = to_phy_device(dev);
mutex_lock(&phydev->lock);
+ phy_led_cleanup(phydev);
phydev->state = PHY_DOWN;
mutex_unlock(&phydev->lock);
diff --git a/drivers/net/phy/phy_led.c b/drivers/net/phy/phy_led.c
new file mode 100644
index 0000000..240355a
--- /dev/null
+++ b/drivers/net/phy/phy_led.c
@@ -0,0 +1,70 @@
+#include <linux/phy.h>
+#include <linux/leds.h>
+
+static void phy_link_trig_activate(struct led_classdev *led)
+{
+ struct phy_device *phydev = led->trigger->data;
+ enum led_brightness value = LED_OFF;
+
+ if (phydev && phydev->state != PHY_DOWN)
+ value = LED_FULL;
+ led_set_brightness(led, value);
+}
+
+static void phy_act_trig_activate(struct led_classdev *led)
+{
+ struct phy_device *phydev = led->trigger->data;
+ unsigned long on = 0, off = 0;
+
+ if (phydev && (phydev->state == PHY_RUNNING ||
+ phydev->state == PHY_CHANGELINK))
+ led_blink_set(led, &on, &off);
+}
+
+void phy_led_init(struct phy_device *phydev)
+{
+ phydev->phy_link = kzalloc(sizeof(*phydev->phy_link), GFP_KERNEL);
+ if (!phydev->phy_link)
+ return;
+
+ snprintf(phydev->phy_link_name, sizeof(phydev->phy_link_name),
+ "%x-eth-phy-link", phydev->drv->phy_id);
+ phydev->phy_link->name = phydev->phy_link_name;
+ if (led_trigger_register(phydev->phy_link)) {
+ kfree(phydev->phy_link);
+ phydev->phy_link = NULL;
+ }
+ phydev->phy_link->data = phydev;
+ phydev->phy_link->activate = phy_link_trig_activate;
+
+ phydev->phy_act = kzalloc(sizeof(*phydev->phy_act), GFP_KERNEL);
+ if (!phydev->phy_act) {
+ led_trigger_unregister(phydev->phy_link);
+ kfree(phydev->phy_link);
+ return;
+ }
+ phydev->phy_act->data = phydev;
+ phydev->phy_act->activate = phy_act_trig_activate;
+
+ snprintf(phydev->phy_act_name, sizeof(phydev->phy_act_name),
+ "%x-eth-phy-activity", phydev->drv->phy_id);
+ phydev->phy_act->name = phydev->phy_act_name;
+ if (led_trigger_register(phydev->phy_act)) {
+ kfree(phydev->phy_act);
+ phydev->phy_act = NULL;
+ return;
+ }
+}
+
+void phy_led_cleanup(struct phy_device *phydev)
+{
+ if (phydev->phy_link) {
+ led_trigger_unregister(phydev->phy_link);
+ kfree(phydev->phy_link);
+ }
+ if (phydev->phy_act) {
+ led_trigger_unregister(phydev->phy_act);
+ kfree(phydev->phy_act);
+ }
+}
+
diff --git a/drivers/net/phy/phy_led.h b/drivers/net/phy/phy_led.h
new file mode 100644
index 0000000..ff5bd98
--- /dev/null
+++ b/drivers/net/phy/phy_led.h
@@ -0,0 +1,37 @@
+#include <linux/leds.h>
+
+#ifdef CONFIG_ETH_PHY_LED
+static inline void phy_trig_led_act(struct phy_device *phydev)
+{
+ unsigned long on = 0, off = 0;
+
+ led_trigger_blink(phydev->phy_act, &on, &off);
+}
+
+static inline void phy_trig_led_link(struct phy_device *phydev, int on)
+{
+ enum led_brightness value = (on) ? LED_FULL : LED_OFF;
+
+ led_trigger_event(phydev->phy_link, value);
+}
+
+void phy_led_init(struct phy_device *phydev);
+void phy_led_cleanup(struct phy_device *phydev);
+#else
+static inline void phy_trig_led_act(struct phy_device *phydev)
+{
+}
+
+static inline void phy_trig_led_link(struct phy_device *phydev, int on)
+{
+}
+
+static inline void phy_led_init(struct phy_device *phydev)
+{
+}
+
+static inline void phy_led_cleanup(struct phy_device *phydev)
+{
+}
+#endif
+
diff --git a/include/linux/leds.h b/include/linux/leds.h
index f203a8f..11ec574 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -233,6 +233,7 @@ struct led_trigger {
const char *name;
void (*activate)(struct led_classdev *led_cdev);
void (*deactivate)(struct led_classdev *led_cdev);
+ void *data;
/* LEDs under control by this trigger (for simple triggers) */
rwlock_t leddev_list_lock;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 2abd791..94fee77 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -25,6 +25,7 @@
#include <linux/timer.h>
#include <linux/workqueue.h>
#include <linux/mod_devicetable.h>
+#include <linux/leds.h>
#include <linux/atomic.h>
@@ -60,6 +61,7 @@
#define PHY_HAS_INTERRUPT 0x00000001
#define PHY_HAS_MAGICANEG 0x00000002
#define PHY_IS_INTERNAL 0x00000004
+#define PHY_HAS_LED_CTRL 0x00000008
#define MDIO_DEVICE_IS_PHY 0x80000000
/* Interface Mode definitions */
@@ -424,6 +426,10 @@ struct phy_device {
u8 mdix;
void (*adjust_link)(struct net_device *dev);
+
+ struct led_trigger *phy_link, *phy_act;
+ char phy_link_name[32], phy_act_name[32];
+
};
#define to_phy_device(d) container_of(to_mdio_device(d), \
struct phy_device, mdio)
--
2.4.3
^ permalink raw reply related
* [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem
From: Vishal Thanki @ 2016-03-23 17:51 UTC (permalink / raw)
To: andrew, f.fainelli, ujhelyi.m, netdev; +Cc: Vishal Thanki
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.
Please provide the review comments so that I can work on this patchset to
make it complete.
Thanks,
Vishal
Vishal Thanki (3):
net: phy: Add ethernet PHY LED triggers
net: phy: at8030: Expose the Link and Activity LEDs
led: at8030: Add LED driver for AT8030 ethernet PHY
drivers/leds/Kconfig | 7 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-at803x.c | 158 +++++++++++++++++++++++++++++++++++++++++++
drivers/net/phy/Kconfig | 7 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/at803x.c | 55 ++++++++++++++-
drivers/net/phy/phy.c | 20 +++++-
drivers/net/phy/phy_device.c | 4 ++
drivers/net/phy/phy_led.c | 70 +++++++++++++++++++
drivers/net/phy/phy_led.h | 37 ++++++++++
include/linux/leds.h | 1 +
include/linux/phy.h | 6 ++
include/linux/phy/at803x.h | 45 ++++++++++++
13 files changed, 409 insertions(+), 3 deletions(-)
create mode 100644 drivers/leds/leds-at803x.c
create mode 100644 drivers/net/phy/phy_led.c
create mode 100644 drivers/net/phy/phy_led.h
create mode 100644 include/linux/phy/at803x.h
--
2.4.3
^ permalink raw reply
* Re: [PATCH] net: mediatek: fix checking for NULL instead of IS_ERR() in .probe
From: David Miller @ 2016-03-23 17:50 UTC (permalink / raw)
To: vz; +Cc: nbd, blogic, netdev, linux-arm-kernel, linux-mediatek
In-Reply-To: <1458687964-9690-1-git-send-email-vz@mleia.com>
From: Vladimir Zapolskiy <vz@mleia.com>
Date: Wed, 23 Mar 2016 01:06:04 +0200
> devm_ioremap_resource() returns ERR_PTR() value on error, it never
> returns NULL, fix it and propagate the returned error upwards.
>
> Fixes: 656e705243fd ("net-next: mediatek: add support for MT7623 ethernet")
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 1/2] net: ethernet: dlink: dl2k: fix code style
From: David Miller @ 2016-03-23 17:41 UTC (permalink / raw)
To: mussitantesmortem; +Cc: linux, netdev, linux-kernel
In-Reply-To: <1458743800-30606-1-git-send-email-mussitantesmortem@gmail.com>
Such cleanups and tidies are not appropriate to be submitted at this time.
Only bug fixes should be going into the tree right now.
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: Request 'reset' GPIO only for AT8030 PHY
From: David Miller @ 2016-03-23 17:40 UTC (permalink / raw)
To: sf84
Cc: u.kleine-koenig, daniel, netdev, linux-kernel, slash.tmp,
f.fainelli, mans, festevam, martin.blumenstingl, linus.walleij
In-Reply-To: <56F274A5.6030502@laposte.net>
From: Sebastian Frias <sf84@laposte.net>
Date: Wed, 23 Mar 2016 11:49:09 +0100
> 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>
Applied.
^ permalink raw reply
* Re: [PATCH] at803x: fix reset handling
From: David Miller @ 2016-03-23 17:40 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: netdev, f.fainelli, u.kleine-koenig
In-Reply-To: <1525241.UQIRf9ZOB3@wasted.cogentembedded.com>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 23 Mar 2016 00:44:40 +0300
> The driver of course "knows" that the chip's reset signal is active low,
> so it drives the GPIO to 0 to reset the PHY and to 1 otherwise; however
> all this will only work iff the GPIO is specified as active-high in the
> device tree! I think both the driver and the device trees (if there are
> any -- I was unable to find them) need to be fixed in this case...
>
> Fixes: 13a56b449325 ("net: phy: at803x: Add support for hardware reset")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Applied.
^ permalink raw reply
* Re: net/sctp: stack-out-of-bounds in sctp_getsockopt
From: Pablo Neira Ayuso @ 2016-03-23 17:38 UTC (permalink / raw)
To: Baozeng
Cc: Eric Dumazet, kaber, kadlec, coreteam, netfilter-devel,
Vladislav Yasevich, nhorman, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <CAP=GMUFA-qAOxF1W=aeWDYBiR-GSohJYGHiMDsrqwsD0xdg3_Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 998 bytes --]
On Thu, Mar 24, 2016 at 12:42:43AM +0800, Baozeng wrote:
> 2016-03-22 23:27 GMT+08:00 Eric Dumazet <eric.dumazet@gmail.com>:
> > Untested patch would be :
> >
> > diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
> > index 67b2e27999aa..fceb7354d169 100644
> > --- a/net/bridge/netfilter/ebtables.c
> > +++ b/net/bridge/netfilter/ebtables.c
> > @@ -346,7 +346,7 @@ find_inlist_lock(struct list_head *head, const char *name, const char *prefix,
> > {
> > return try_then_request_module(
> > find_inlist_lock_noload(head, name, error, mutex),
> > - "%s%s", prefix, name);
> > + "%.*s%s", EBT_TABLE_MAXNAMELEN, prefix, name);
> > }
> >
> > static inline struct ebt_table *
> >
> >
>
> Thanks for your quick patch. I tested it but it still reproduce the
> bug. We should limit the length of the name,
> not the prefix. The following patch fixs it.
Could you give a try to this patch? Thanks.
[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 2686 bytes --]
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 67b2e27..1e3a707 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1521,6 +1521,8 @@ static int do_ebt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
if (copy_from_user(&tmp, user, sizeof(tmp)))
return -EFAULT;
+ tmp.name[sizeof(tmp.name) - 1] = '\0';
+
t = find_table_lock(net, tmp.name, &ret, &ebt_mutex);
if (!t)
return ret;
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index a2002ff..06e9fb7 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -960,6 +960,7 @@ static int get_entries(struct net *net, struct arpt_get_entries __user *uptr,
sizeof(struct arpt_get_entries) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
t = xt_find_table_lock(net, NFPROTO_ARP, get.name);
if (!IS_ERR_OR_NULL(t)) {
@@ -1654,6 +1655,7 @@ static int compat_get_entries(struct net *net,
*len, sizeof(get) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
xt_compat_lock(NFPROTO_ARP);
t = xt_find_table_lock(net, NFPROTO_ARP, get.name);
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 45b1d97..4642dc2 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1146,6 +1146,7 @@ get_entries(struct net *net, struct ipt_get_entries __user *uptr,
*len, sizeof(get) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
t = xt_find_table_lock(net, AF_INET, get.name);
if (!IS_ERR_OR_NULL(t)) {
@@ -1925,6 +1926,7 @@ compat_get_entries(struct net *net, struct compat_ipt_get_entries __user *uptr,
*len, sizeof(get) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
xt_compat_lock(AF_INET);
t = xt_find_table_lock(net, AF_INET, get.name);
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 85c0942..db47815 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1158,6 +1158,7 @@ get_entries(struct net *net, struct ip6t_get_entries __user *uptr,
*len, sizeof(get) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
t = xt_find_table_lock(net, AF_INET6, get.name);
if (!IS_ERR_OR_NULL(t)) {
@@ -1934,6 +1935,7 @@ compat_get_entries(struct net *net, struct compat_ip6t_get_entries __user *uptr,
*len, sizeof(get) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
xt_compat_lock(AF_INET6);
t = xt_find_table_lock(net, AF_INET6, get.name);
^ permalink raw reply related
* Re: [PATCH 2/2] drivers: net: ethernet: dlink: dl2k: fix indent
From: Joe Perches @ 2016-03-23 17:34 UTC (permalink / raw)
To: Maxim Zhukov, davem; +Cc: linux, netdev, linux-kernel
In-Reply-To: <1458748660-10571-2-git-send-email-mussitantesmortem@gmail.com>
If dl2k is modified at all, maybe convert the
printks to netdev_<level> too so that the logging
output is more like other networking drivers.
Something like:
---
drivers/net/ethernet/dlink/dl2k.c | 181 +++++++++++++++++++++-----------------
1 file changed, 100 insertions(+), 81 deletions(-)
diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
index f92b6d9..0f0326b 100644
--- a/drivers/net/ethernet/dlink/dl2k.c
+++ b/drivers/net/ethernet/dlink/dl2k.c
@@ -10,9 +10,9 @@
(at your option) any later version.
*/
-#define DRV_NAME "DL2000/TC902x-based linux driver"
#define DRV_VERSION "v1.19"
#define DRV_RELDATE "2007/08/12"
+
#include "dl2k.h"
#include <linux/dma-mapping.h>
@@ -23,8 +23,6 @@
#define dr16(reg) ioread16(ioaddr + (reg))
#define dr8(reg) ioread8(ioaddr + (reg))
-static char version[] =
- KERN_INFO DRV_NAME " " DRV_VERSION " " DRV_RELDATE "\n";
#define MAX_UNITS 8
static int mtu[MAX_UNITS];
static int vlan[MAX_UNITS];
@@ -118,12 +116,11 @@ rio_probe1 (struct pci_dev *pdev, const struct pci_device_id *ent)
int chip_idx = ent->driver_data;
int err, irq;
void __iomem *ioaddr;
- static int version_printed;
void *ring_space;
dma_addr_t ring_dma;
- if (!version_printed++)
- printk ("%s", version);
+ pr_info_once("DL2000/TC902x-based linux driver %s %s\n",
+ DRV_VERSION, DRV_RELDATE);
err = pci_enable_device (pdev);
if (err)
@@ -274,18 +271,20 @@ rio_probe1 (struct pci_dev *pdev, const struct pci_device_id *ent)
card_idx++;
- printk (KERN_INFO "%s: %s, %pM, IRQ %d\n",
- dev->name, np->name, dev->dev_addr, irq);
+ netdev_info(dev, "%s, %pM, IRQ %d\n",
+ np->name, dev->dev_addr, irq);
if (tx_coalesce > 1)
- printk(KERN_INFO "tx_coalesce:\t%d packets\n",
- tx_coalesce);
- if (np->coalesce)
- printk(KERN_INFO
- "rx_coalesce:\t%d packets\n"
- "rx_timeout: \t%d ns\n",
- np->rx_coalesce, np->rx_timeout*640);
+ netdev_info(dev, "tx_coalesce:\t%d packets\n",
+ tx_coalesce);
+ if (np->coalesce) {
+ netdev_info(dev, "rx_coalesce:\t%d packets\n",
+ np->rx_coalesce);
+ netdev_info(dev, "rx_timeout: \t%d ns\n",
+ np->rx_timeout * 640);
+ }
if (np->vlan)
- printk(KERN_INFO "vlan(id):\t%d\n", np->vlan);
+ netdev_info(dev, "vlan(id):\t%d\n", np->vlan);
+
return 0;
err_out_unmap_rx:
@@ -322,7 +321,7 @@ find_miiphy (struct net_device *dev)
}
}
if (!phy_found) {
- printk (KERN_ERR "%s: No MII PHY found!\n", dev->name);
+ netdev_err(dev, "No MII PHY found!\n");
return -ENODEV;
}
return 0;
@@ -348,8 +347,7 @@ parse_eeprom (struct net_device *dev)
/* Check CRC */
crc = ~ether_crc_le (256 - 4, sromdata);
if (psrom->crc != cpu_to_le32(crc)) {
- printk (KERN_ERR "%s: EEPROM data CRC error.\n",
- dev->name);
+ netdev_err(dev, "EEPROM data CRC error\n");
return -1;
}
}
@@ -374,7 +372,7 @@ parse_eeprom (struct net_device *dev)
cid = psib[i++];
next = psib[i++];
if ((cid == 0 && next == 0) || (cid == 0xff && next == 0xff)) {
- printk (KERN_ERR "Cell data error\n");
+ netdev_err(dev, "Cell data error\n");
return -1;
}
switch (cid) {
@@ -664,7 +662,7 @@ rio_timer (unsigned long data)
spin_lock_irqsave(&np->rx_lock, flags);
/* Recover rx ring exhausted error */
if (np->cur_rx - np->old_rx >= RX_RING_SIZE) {
- printk(KERN_INFO "Try to recover rx ring exhausted...\n");
+ netdev_info(dev, "Trying to recover, rx ring exhausted...\n");
/* Re-allocate skbuffs to fill the descriptor ring */
for (; np->cur_rx - np->old_rx > 0; np->old_rx++) {
struct sk_buff *skb;
@@ -675,9 +673,8 @@ rio_timer (unsigned long data)
np->rx_buf_sz);
if (skb == NULL) {
np->rx_ring[entry].fraginfo = 0;
- printk (KERN_INFO
- "%s: Still unable to re-allocate Rx skbuff.#%d\n",
- dev->name, entry);
+ netdev_info(dev, "Still unable to re-allocate Rx skbuff.#%d\n",
+ entry);
break;
}
np->rx_skbuff[entry] = skb;
@@ -702,8 +699,8 @@ rio_tx_timeout (struct net_device *dev)
struct netdev_private *np = netdev_priv(dev);
void __iomem *ioaddr = np->ioaddr;
- printk (KERN_INFO "%s: Tx timed out (%4.4x), is buffer full?\n",
- dev->name, dr32(TxStatus));
+ netdev_info(dev, "Tx timed out (%04x), is buffer full?\n",
+ dr32(TxStatus));
rio_free_tx(dev, 0);
dev->if_port = 0;
dev->trans_start = jiffies; /* prevent tx timeout */
@@ -874,8 +871,8 @@ tx_error (struct net_device *dev, int tx_status)
int i;
frame_id = (tx_status & 0xffff0000);
- printk (KERN_ERR "%s: Transmit error, TxStatus %4.4x, FrameId %d.\n",
- dev->name, tx_status, frame_id);
+ netdev_err(dev, "Transmit error, TxStatus %04x, FrameId %d\n",
+ tx_status, frame_id);
np->stats.tx_errors++;
/* Ttransmit Underrun */
if (tx_status & 0x10) {
@@ -1007,10 +1004,8 @@ receive_packet (struct net_device *dev)
skb = netdev_alloc_skb_ip_align(dev, np->rx_buf_sz);
if (skb == NULL) {
np->rx_ring[entry].fraginfo = 0;
- printk (KERN_INFO
- "%s: receive_packet: "
- "Unable to re-allocate Rx skbuff.#%d\n",
- dev->name, entry);
+ netdev_info(dev, "receive_packet: Unable to re-allocate Rx skbuff.#%d\n",
+ entry);
break;
}
np->rx_skbuff[entry] = skb;
@@ -1039,7 +1034,7 @@ rio_error (struct net_device *dev, int int_status)
/* Link change event */
if (int_status & LinkEvent) {
if (mii_wait_link (dev, 10) == 0) {
- printk (KERN_INFO "%s: Link up\n", dev->name);
+ netdev_info(dev, "Link up\n");
if (np->phy_media)
mii_get_media_pcs (dev);
else
@@ -1059,7 +1054,7 @@ rio_error (struct net_device *dev, int int_status)
np->link_status = 1;
netif_carrier_on(dev);
} else {
- printk (KERN_INFO "%s: Link off\n", dev->name);
+ netdev_info(dev, "Link off\n");
np->link_status = 0;
netif_carrier_off(dev);
}
@@ -1073,8 +1068,7 @@ rio_error (struct net_device *dev, int int_status)
/* PCI Error, a catastronphic error related to the bus interface
occurs, set GlobalReset and HostReset to reset. */
if (int_status & HostError) {
- printk (KERN_ERR "%s: HostError! IntStatus %4.4x.\n",
- dev->name, int_status);
+ netdev_err(dev, "HostError! IntStatus %04x\n", int_status);
dw16(ASICCtrl + 2, GlobalReset | HostReset);
mdelay (500);
rio_set_led_mode(dev);
@@ -1325,7 +1319,7 @@ static int rio_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
if (np->speed == 1000) {
ethtool_cmd_speed_set(cmd, SPEED_100);
cmd->duplex = DUPLEX_FULL;
- printk("Warning!! Can't disable Auto negotiation in 1000Mbps, change to Manual 100Mbps, Full duplex.\n");
+ netdev_warn(dev, "Can't disable Auto negotiation in 1000Mbps, change to Manual 100Mbps, Full duplex\n");
}
switch (ethtool_cmd_speed(cmd)) {
case SPEED_10:
@@ -1515,6 +1509,9 @@ mii_get_media (struct net_device *dev)
__u16 mssr;
int phy_addr;
struct netdev_private *np;
+ const char *how;
+ const char *speed;
+ const char *duplex;
np = netdev_priv(dev);
phy_addr = np->phy_addr;
@@ -1525,6 +1522,7 @@ mii_get_media (struct net_device *dev)
/* Auto-Negotiation not completed */
return -1;
}
+ how = "Auto";
negotiate = mii_read (dev, phy_addr, MII_ADVERTISE) &
mii_read (dev, phy_addr, MII_LPA);
mscr = mii_read (dev, phy_addr, MII_CTRL1000);
@@ -1532,27 +1530,36 @@ mii_get_media (struct net_device *dev)
if (mscr & ADVERTISE_1000FULL && mssr & LPA_1000FULL) {
np->speed = 1000;
np->full_duplex = 1;
- printk (KERN_INFO "Auto 1000 Mbps, Full duplex\n");
+ speed = "1000";
+ duplex = "Full";
} else if (mscr & ADVERTISE_1000HALF && mssr & LPA_1000HALF) {
np->speed = 1000;
np->full_duplex = 0;
- printk (KERN_INFO "Auto 1000 Mbps, Half duplex\n");
+ speed = "1000";
+ duplex = "Half";
} else if (negotiate & ADVERTISE_100FULL) {
np->speed = 100;
np->full_duplex = 1;
- printk (KERN_INFO "Auto 100 Mbps, Full duplex\n");
+ speed = "100";
+ duplex = "Full";
} else if (negotiate & ADVERTISE_100HALF) {
np->speed = 100;
np->full_duplex = 0;
- printk (KERN_INFO "Auto 100 Mbps, Half duplex\n");
+ speed = "100";
+ duplex = "Half";
} else if (negotiate & ADVERTISE_10FULL) {
np->speed = 10;
np->full_duplex = 1;
- printk (KERN_INFO "Auto 10 Mbps, Full duplex\n");
+ speed = "10";
+ duplex = "Full";
} else if (negotiate & ADVERTISE_10HALF) {
np->speed = 10;
np->full_duplex = 0;
- printk (KERN_INFO "Auto 10 Mbps, Half duplex\n");
+ speed = "10";
+ duplex = "Half";
+ } else {
+ speed = "unknown";
+ duplex = "unknown";
}
if (negotiate & ADVERTISE_PAUSE_CAP) {
np->tx_flow &= 1;
@@ -1564,30 +1571,33 @@ mii_get_media (struct net_device *dev)
/* else tx_flow, rx_flow = user select */
} else {
__u16 bmcr = mii_read (dev, phy_addr, MII_BMCR);
+
+ how = "Operating at";
switch (bmcr & (BMCR_SPEED100 | BMCR_SPEED1000)) {
case BMCR_SPEED1000:
- printk (KERN_INFO "Operating at 1000 Mbps, ");
+ speed = "1000";
break;
case BMCR_SPEED100:
- printk (KERN_INFO "Operating at 100 Mbps, ");
+ speed = "100";
break;
case 0:
- printk (KERN_INFO "Operating at 10 Mbps, ");
- }
- if (bmcr & BMCR_FULLDPLX) {
- printk (KERN_CONT "Full duplex\n");
- } else {
- printk (KERN_CONT "Half duplex\n");
+ speed = "10";
+ break;
+ default:
+ speed = "unknown";
+ break;
}
+ if (bmcr & BMCR_FULLDPLX)
+ duplex = "Full";
+ else
+ duplex = "Half";
+
}
- if (np->tx_flow)
- printk(KERN_INFO "Enable Tx Flow Control\n");
- else
- printk(KERN_INFO "Disable Tx Flow Control\n");
- if (np->rx_flow)
- printk(KERN_INFO "Enable Rx Flow Control\n");
- else
- printk(KERN_INFO "Disable Rx Flow Control\n");
+
+ netdev_info(dev, "%s %s Mbps, %s duplex, Tx Flow Control: %s Rx Flow Control: %s\n",
+ how, speed, duplex,
+ np->tx_flow ? "Enabled" : "Disabled",
+ np->rx_flow ? "Enabled" : "Disabled");
return 0;
}
@@ -1636,6 +1646,10 @@ mii_set_media (struct net_device *dev)
mii_write (dev, phy_addr, MII_BMCR, bmcr);
mdelay(1);
} else {
+ const char *how = "Manual";
+ const char *speed = "unknown";
+ const char *duplex = "unknown";
+
/* Force speed setting */
/* 1) Disable Auto crossover */
pscr = mii_read (dev, phy_addr, MII_PHY_SCR);
@@ -1659,16 +1673,18 @@ mii_set_media (struct net_device *dev)
bmcr = BMCR_PDOWN;
if (np->speed == 100) {
bmcr |= BMCR_SPEED100;
- printk (KERN_INFO "Manual 100 Mbps, ");
+ speed = "100";
} else if (np->speed == 10) {
- printk (KERN_INFO "Manual 10 Mbps, ");
+ speed = "10";
}
if (np->full_duplex) {
bmcr |= BMCR_FULLDPLX;
- printk (KERN_CONT "Full duplex\n");
+ duplex = "Full";
} else {
- printk (KERN_CONT "Half duplex\n");
+ duplex = "Half";
}
+ netdev_info(dev, "%s %s Mbps, %s duplex\n",
+ how, speed, duplex);
#if 0
/* Set 1000BaseT Master/Slave setting */
mscr = mii_read (dev, phy_addr, MII_CTRL1000);
@@ -1688,6 +1704,9 @@ mii_get_media_pcs (struct net_device *dev)
__u16 bmsr;
int phy_addr;
struct netdev_private *np;
+ const char *how;
+ const char *speed;
+ const char *duplex;
np = netdev_priv(dev);
phy_addr = np->phy_addr;
@@ -1698,15 +1717,17 @@ mii_get_media_pcs (struct net_device *dev)
/* Auto-Negotiation not completed */
return -1;
}
+ how = "Auto";
negotiate = mii_read (dev, phy_addr, PCS_ANAR) &
mii_read (dev, phy_addr, PCS_ANLPAR);
np->speed = 1000;
+ speed = "1000";
if (negotiate & PCS_ANAR_FULL_DUPLEX) {
- printk (KERN_INFO "Auto 1000 Mbps, Full duplex\n");
np->full_duplex = 1;
+ duplex = "Full";
} else {
- printk (KERN_INFO "Auto 1000 Mbps, half duplex\n");
np->full_duplex = 0;
+ duplex = "Half";
}
if (negotiate & PCS_ANAR_PAUSE) {
np->tx_flow &= 1;
@@ -1718,21 +1739,19 @@ mii_get_media_pcs (struct net_device *dev)
/* else tx_flow, rx_flow = user select */
} else {
__u16 bmcr = mii_read (dev, phy_addr, PCS_BMCR);
- printk (KERN_INFO "Operating at 1000 Mbps, ");
- if (bmcr & BMCR_FULLDPLX) {
- printk (KERN_CONT "Full duplex\n");
- } else {
- printk (KERN_CONT "Half duplex\n");
- }
+
+ how = "Operating at";
+ speed = "1000";
+ if (bmcr & BMCR_FULLDPLX)
+ duplex = "Full";
+ else
+ duplex = "Half";
}
- if (np->tx_flow)
- printk(KERN_INFO "Enable Tx Flow Control\n");
- else
- printk(KERN_INFO "Disable Tx Flow Control\n");
- if (np->rx_flow)
- printk(KERN_INFO "Enable Rx Flow Control\n");
- else
- printk(KERN_INFO "Disable Rx Flow Control\n");
+
+ netdev_info(dev, "%s %s Mbps, %s duplex, Tx Flow Control: %s Rx Flow Control: %s\n",
+ how, speed, duplex,
+ np->tx_flow ? "Enabled" : "Disabled",
+ np->rx_flow ? "Enabled" : "Disabled");
return 0;
}
@@ -1775,10 +1794,10 @@ mii_set_media_pcs (struct net_device *dev)
mdelay(10);
if (np->full_duplex) {
bmcr = BMCR_FULLDPLX;
- printk (KERN_INFO "Manual full duplex\n");
+ netdev_info(dev, "Manual full duplex\n");
} else {
bmcr = 0;
- printk (KERN_INFO "Manual half duplex\n");
+ netdev_info(dev, "Manual half duplex\n");
}
mii_write (dev, phy_addr, MII_BMCR, bmcr);
mdelay(10);
^ permalink raw reply related
* Re: [PATCH net v2] xfrm: Fix crash observed during device unregistration and decryption
From: Eric Dumazet @ 2016-03-23 17:29 UTC (permalink / raw)
To: Herbert Xu
Cc: Subash Abhinov Kasiviswanathan, 'Steffen Klassert',
netdev, jeromes
In-Reply-To: <20160323132947.GA29938@gondor.apana.org.au>
On Wed, 2016-03-23 at 21:29 +0800, Herbert Xu wrote:
> On Wed, Mar 23, 2016 at 06:25:47AM -0700, Eric Dumazet wrote:
> >
> > Wont this prevent device from being dismantled ?
>
> This is only held while the crypto processing is ongoing.
>
> > Where is this xfrm queue purged at device dismantle ?
>
> There is no way to cancel an ongoing crypto processing so you'll
> just have to wait it out.
OK, but before calling netif_rx() are we properly testing dev->flags
IFF_UP status ?
Otherwise, we still allow packets being queued after flush_backlog() had
been called.
Thanks.
^ 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