| Message ID | 20081121190110.32313.43591.stgit@gitlost.lost |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2008-11-21 at 11:01 -0800, Jeff Kirsher wrote: > From: Bruce Allan <bruce.w.allan@intel.com> > > The system log messages created on a link status change need to follow a > specific format to work with tools some customers use. Um, shouldn't those tools be listening to netlink for carrier events, or are these tools run on a separate machine using on some later date using the logs from the machine with the e1000e? Dan > Signed-off-by: Bruce Allan <bruce.w.allan@intel.com> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > --- > > drivers/net/e1000e/netdev.c | 9 +++++++-- > 1 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c > index 7eb1a36..cd6d132 100644 > --- a/drivers/net/e1000e/netdev.c > +++ b/drivers/net/e1000e/netdev.c > @@ -3408,7 +3408,10 @@ static void e1000_print_link_info(struct e1000_adapter *adapter) > struct e1000_hw *hw = &adapter->hw; > u32 ctrl = er32(CTRL); > > - e_info("Link is Up %d Mbps %s, Flow Control: %s\n", > + /* Link status message must follow this format for user tools */ > + printk(KERN_INFO "e1000e: %s NIC Link is Up %d Mbps %s, " > + "Flow Control: %s\n", > + adapter->netdev->name, > adapter->link_speed, > (adapter->link_duplex == FULL_DUPLEX) ? > "Full Duplex" : "Half Duplex", > @@ -3618,7 +3621,9 @@ static void e1000_watchdog_task(struct work_struct *work) > if (netif_carrier_ok(netdev)) { > adapter->link_speed = 0; > adapter->link_duplex = 0; > - e_info("Link is Down\n"); > + /* Link status message must follow this format */ > + printk(KERN_INFO "e1000e: %s NIC Link is Down\n", > + adapter->netdev->name); > netif_carrier_off(netdev); > netif_tx_stop_all_queues(netdev); > if (!test_bit(__E1000_DOWN, &adapter->state)) > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 21, 2008 at 11:04 AM, Dan Williams <dcbw@redhat.com> wrote: > On Fri, 2008-11-21 at 11:01 -0800, Jeff Kirsher wrote: >> From: Bruce Allan <bruce.w.allan@intel.com> >> >> The system log messages created on a link status change need to follow a >> specific format to work with tools some customers use. > > Um, shouldn't those tools be listening to netlink for carrier events, or > are these tools run on a separate machine using on some later date using > the logs from the machine with the e1000e? > > Dan > From my understanding these tools are looking at the logs and that is why we need to have a consistent log message.
On Fri, 21 Nov 2008 11:23:42 -0800 "Jeff Kirsher" <jeffrey.t.kirsher@intel.com> wrote: > On Fri, Nov 21, 2008 at 11:04 AM, Dan Williams <dcbw@redhat.com> wrote: > > On Fri, 2008-11-21 at 11:01 -0800, Jeff Kirsher wrote: > >> From: Bruce Allan <bruce.w.allan@intel.com> > >> > >> The system log messages created on a link status change need to follow a > >> specific format to work with tools some customers use. > > > > Um, shouldn't those tools be listening to netlink for carrier events, or > > are these tools run on a separate machine using on some later date using > > the logs from the machine with the e1000e? > > > > Dan > > > > From my understanding these tools are looking at the logs and that is > why we need to have a consistent log message. > These tools are tied to a specific driver (yours), because not all drivers generate a message or the same format message. This may be okay for Intel but is really stupid design... It would be good if link state transitions generated uevents (online/offline). Then udev, hal and others could use that without netlink. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 21, 2008 at 12:16 PM, Stephen Hemminger <shemminger@vyatta.com> wrote: > On Fri, 21 Nov 2008 11:23:42 -0800 > "Jeff Kirsher" <jeffrey.t.kirsher@intel.com> wrote: > >> On Fri, Nov 21, 2008 at 11:04 AM, Dan Williams <dcbw@redhat.com> wrote: >> > On Fri, 2008-11-21 at 11:01 -0800, Jeff Kirsher wrote: >> >> From: Bruce Allan <bruce.w.allan@intel.com> >> >> >> >> The system log messages created on a link status change need to follow a >> >> specific format to work with tools some customers use. >> > >> > Um, shouldn't those tools be listening to netlink for carrier events, or >> > are these tools run on a separate machine using on some later date using >> > the logs from the machine with the e1000e? >> > >> > Dan >> > >> >> From my understanding these tools are looking at the logs and that is >> why we need to have a consistent log message. >> > > These tools are tied to a specific driver (yours), because not all drivers > generate a message or the same format message. This may be okay for Intel > but is really stupid design... I have not checked all of the drivers, but I do see that our driver does follow what others driver (e.g. tg3, starfire) are doing. I would be open for standardizing on what messages get generated and in what format. I am sure that would help third parties generate tools that would work with every driver. > It would be good if link state transitions generated uevents (online/offline). > Then udev, hal and others could use that without netlink. > -- Again, I would be open to any standardization so that all drivers would deal with link up/down messages in the same manner.
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com> Date: Fri, 21 Nov 2008 11:01:10 -0800 > From: Bruce Allan <bruce.w.allan@intel.com> > > The system log messages created on a link status change need to follow a > specific format to work with tools some customers use. > > Signed-off-by: Bruce Allan <bruce.w.allan@intel.com> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> As stated by others, if anything we should have a global tree wide format for this. Even via a helper function or similar. But for now, how you print this message is your business, so patch applied :) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c index 7eb1a36..cd6d132 100644 --- a/drivers/net/e1000e/netdev.c +++ b/drivers/net/e1000e/netdev.c @@ -3408,7 +3408,10 @@ static void e1000_print_link_info(struct e1000_adapter *adapter) struct e1000_hw *hw = &adapter->hw; u32 ctrl = er32(CTRL); - e_info("Link is Up %d Mbps %s, Flow Control: %s\n", + /* Link status message must follow this format for user tools */ + printk(KERN_INFO "e1000e: %s NIC Link is Up %d Mbps %s, " + "Flow Control: %s\n", + adapter->netdev->name, adapter->link_speed, (adapter->link_duplex == FULL_DUPLEX) ? "Full Duplex" : "Half Duplex", @@ -3618,7 +3621,9 @@ static void e1000_watchdog_task(struct work_struct *work) if (netif_carrier_ok(netdev)) { adapter->link_speed = 0; adapter->link_duplex = 0; - e_info("Link is Down\n"); + /* Link status message must follow this format */ + printk(KERN_INFO "e1000e: %s NIC Link is Down\n", + adapter->netdev->name); netif_carrier_off(netdev); netif_tx_stop_all_queues(netdev); if (!test_bit(__E1000_DOWN, &adapter->state))