Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v2 1/1] ARM: dts: imx6sx: correct i.MX6sx sdb board enet phy address
From: David Miller @ 2015-01-25  5:55 UTC (permalink / raw)
  To: b38611; +Cc: shawn.guo, netdev, s.hauer, stefan, linux-arm-kernel
In-Reply-To: <1421743697-26918-1-git-send-email-b38611@freescale.com>

From: Fugang Duan <b38611@freescale.com>
Date: Tue, 20 Jan 2015 16:48:17 +0800

> The commit (3d125f9c91c5) cause i.MX6SX sdb enet cannot work. The cause is
> the commit add mdio node with un-correct phy address.
> 
> The patch just correct i.MX6sx sdb board enet phy address.
> 
> V2:
> * As Shawn's suggestion that unit-address should match 'reg' property, so
>   update ethernet-phy unit-address.
> 
> Acked-by: Stefan Agner <stefan@agner.ch>
> Signed-off-by: Fugang Duan <B38611@freescale.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 1/1] net: fec: fix the warning found by dma debug
From: David Miller @ 2015-01-25  5:54 UTC (permalink / raw)
  To: b38611; +Cc: netdev, festevam, christian.gmeiner, moon.linux, bhutchings,
	stephen
In-Reply-To: <1421734235-6563-1-git-send-email-b38611@freescale.com>

From: Fugang Duan <b38611@freescale.com>
Date: Tue, 20 Jan 2015 14:10:35 +0800

> Enable kernel config "CONFIG_HAVE_DMA_API_DEBUG", FEC have kernel warning:
 ...
> There have one bug in .fec_enet_tx_queue() function to unmap the DMA memory:
> For SG or TSO, get one buffer descriptor and then unmap the related DMA memory, and then
> get the next buffer descriptor, loop to while() to check "TX_READY". If "TX_READY" bit
> still __IS__ existed in the BD (The next fraglist or next TSO packet is not transmited
> complitely), exit the current clean work. When the next work is triggered, it still repeat
> above step with the same BD. The potential issue is that unmap the same DMA memory for
> multiple times.
> 
> The patch fix the clean work for SG and TSO packet.
> 
> Reported-by: Anand Moon <moon.linux@yahoo.com>
> Reported-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> Signed-off-by: Fugang Duan <B38611@freescale.com>

Applied.

^ permalink raw reply

* [PATCH net] ipvlan: fix incorrect usage of IS_ERR() macro in IPv6 code path.
From: Mahesh Bandewar @ 2015-01-25  5:53 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Eric Dumazet, Dan Carpenter, Mahesh Bandewar

The ip6_route_output() always returns a valid dst pointer unlike in IPv4
case. So the validation has to be different from the IPv4 path. Correcting
that error in this patch.

This was picked up by a static checker with a following warning -

   drivers/net/ipvlan/ipvlan_core.c:380 ipvlan_process_v6_outbound()
        warn: 'dst' isn't an ERR_PTR

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/net/ipvlan/ipvlan_core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index a14d87783245..2e195289ddf4 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -377,9 +377,11 @@ static int ipvlan_process_v6_outbound(struct sk_buff *skb)
 	};
 
 	dst = ip6_route_output(dev_net(dev), NULL, &fl6);
-	if (IS_ERR(dst))
+	if (dst->error) {
+		ret = dst->error;
+		dst_release(dst);
 		goto err;
-
+	}
 	skb_dst_drop(skb);
 	skb_dst_set(skb, dst);
 	err = ip6_local_out(skb);
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Ahmed S. Darwish @ 2015-01-25  3:21 UTC (permalink / raw)
  To: Andri Yngvason
  Cc: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
	Marc Kleine-Budde, Linux-CAN, netdev, LKML
In-Reply-To: <20150121162025.2933.13836@shannon>

Hi!

On Wed, Jan 21, 2015 at 04:20:25PM +0000, Andri Yngvason wrote:
> Quoting Ahmed S. Darwish (2015-01-20 21:45:37)
> > From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> > 
> > Replace most of the can interface's state and error counters
> > handling with the new can-dev can_change_state() mechanism.
> > 
> > Suggested-by: Andri Yngvason <andri.yngvason@marel.com>
> > Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> > ---
> >  drivers/net/can/usb/kvaser_usb.c | 114 +++++++++++++++++++--------------------
> >  1 file changed, 55 insertions(+), 59 deletions(-)
> > 
> > diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> > index 971c5f9..0386d3f 100644
> > --- a/drivers/net/can/usb/kvaser_usb.c
> > +++ b/drivers/net/can/usb/kvaser_usb.c
> > @@ -620,40 +620,43 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> >  }
> >  
> >  static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
> > -                                                const struct kvaser_usb_error_summary *es)
> > +                                                const struct kvaser_usb_error_summary *es,
> > +                                                struct can_frame *cf)
> >  {
> >         struct net_device_stats *stats;
> > -       enum can_state new_state;
> > -
> > -       stats = &priv->netdev->stats;
> > -       new_state = priv->can.state;
> > +       enum can_state cur_state, new_state, tx_state, rx_state;
> >  
> >         netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
> >  
> > -       if (es->status & M16C_STATE_BUS_OFF) {
> > -               priv->can.can_stats.bus_off++;
> > +       stats = &priv->netdev->stats;
> > +       new_state = cur_state = priv->can.state;
> > +
> > +       if (es->status & M16C_STATE_BUS_OFF)
> >                 new_state = CAN_STATE_BUS_OFF;
> > -       } else if (es->status & M16C_STATE_BUS_PASSIVE) {
> > -               if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
> > -                       priv->can.can_stats.error_passive++;
> > +       else if (es->status & M16C_STATE_BUS_PASSIVE)
> >                 new_state = CAN_STATE_ERROR_PASSIVE;
> > -       }
> >  
> >         if (es->status == M16C_STATE_BUS_ERROR) {
> > -               if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> > -                   ((es->txerr >= 96) || (es->rxerr >= 96))) {
> > -                       priv->can.can_stats.error_warning++;
> > +               if ((cur_state < CAN_STATE_ERROR_WARNING) &&
> > +                   ((es->txerr >= 96) || (es->rxerr >= 96)))
> >                         new_state = CAN_STATE_ERROR_WARNING;
> > -               } else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
> > +               else if (cur_state > CAN_STATE_ERROR_ACTIVE)
> >                         new_state = CAN_STATE_ERROR_ACTIVE;
> > -               }
> >         }
> >  
> >         if (!es->status)
> >                 new_state = CAN_STATE_ERROR_ACTIVE;
> >  
> > +       if (new_state != cur_state) {
> > +               tx_state = (es->txerr >= es->rxerr) ? new_state : 0;
> > +               rx_state = (es->txerr <= es->rxerr) ? new_state : 0;
> > +
> > +               can_change_state(priv->netdev, cf, tx_state, rx_state);
>
> This (below) is redundant. It doesn't harm but at this point can_change_state
> has set priv->can.state to new_state.
>
> > +               new_state = priv->can.state;
> > +       }
> > +

Correct; I will remove it.

> >         if (priv->can.restart_ms &&
> > -           (priv->can.state >= CAN_STATE_BUS_OFF) &&
> > +           (cur_state >= CAN_STATE_BUS_OFF) &&
> >             (new_state < CAN_STATE_BUS_OFF)) {
> >                 priv->can.can_stats.restarts++;
> >         }
> > @@ -665,18 +668,17 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
> >  
> >         priv->bec.txerr = es->txerr;
> >         priv->bec.rxerr = es->rxerr;
> > -       priv->can.state = new_state;
> >  }
> >  
> >  static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> >                                 const struct kvaser_msg *msg)
> >  {
> > -       struct can_frame *cf;
> > +       struct can_frame *cf, tmp_cf = { .can_id = CAN_ERR_FLAG, .can_dlc = CAN_ERR_DLC };
> >         struct sk_buff *skb;
> >         struct net_device_stats *stats;
> >         struct kvaser_usb_net_priv *priv;
> >         struct kvaser_usb_error_summary es = { };
> > -       enum can_state old_state;
> > +       enum can_state old_state, new_state;
> >  
> >         switch (msg->id) {
> >         case CMD_CAN_ERROR_EVENT:
> > @@ -721,60 +723,54 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> >         }
> >  
> >         /* Update all of the can interface's state and error counters before
> > -        * trying any skb allocation that can actually fail with -ENOMEM.
> > +        * trying any memory allocation that can actually fail with -ENOMEM.
> > +        *
> > +        * We send a temporary stack-allocated error can frame to
> > +        * can_change_state() for the very same reason.
> > +        *
> > +        * TODO: Split can_change_state() responsibility between updating the
> > +        * can interface's state and counters, and the setting up of can error
> > +        * frame ID and data to userspace. Remove stack allocation afterwards.
> >          */
> >         old_state = priv->can.state;
> > -       kvaser_usb_rx_error_update_can_state(priv, &es);
> > +       kvaser_usb_rx_error_update_can_state(priv, &es, &tmp_cf);
> > +       new_state = priv->can.state;
> >  
> >         skb = alloc_can_err_skb(priv->netdev, &cf);
> >         if (!skb) {
> >                 stats->rx_dropped++;
> >                 return;
> >         }
> > +       memcpy(cf, &tmp_cf, sizeof(*cf));
> >  
> > -       if (es.status & M16C_STATE_BUS_OFF) {
> > -               cf->can_id |= CAN_ERR_BUSOFF;
> > -
> > -               if (!priv->can.restart_ms)
> > -                       kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> > -               netif_carrier_off(priv->netdev);
> > -       } else if (es.status & M16C_STATE_BUS_PASSIVE) {
> > -               if (old_state != CAN_STATE_ERROR_PASSIVE) {
> > -                       cf->can_id |= CAN_ERR_CRTL;
> > -
> > -                       if (es.txerr || es.rxerr)
> > -                               cf->data[1] = (es.txerr > es.rxerr)
> > -                                               ? CAN_ERR_CRTL_TX_PASSIVE
> > -                                               : CAN_ERR_CRTL_RX_PASSIVE;
> > -                       else
> > -                               cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
> > -                                             CAN_ERR_CRTL_RX_PASSIVE;
> > +       if (new_state != old_state) {
> > +               if (es.status & M16C_STATE_BUS_OFF) {
> > +                       if (!priv->can.restart_ms)
> > +                               kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> > +                       netif_carrier_off(priv->netdev);
> > +               }
> > +
>
> This block [below] is wrong. The usage of PROT_ACTIVE is based on a misunderstanding.
> It's used in some drivers to signify back-to-error-active but its original
> meaning is something completely different, AFAIK.
> This is handled in can_change_state() using a new CTRL message; namely:
> CAN_ERR_CTRL_ACTIVE. The newest version of can-utils is up to date with this.
>
> > +               if (es.status == M16C_STATE_BUS_ERROR) {
> > +                       if ((old_state >= CAN_STATE_ERROR_WARNING) ||
> > +                           (es.txerr < 96 && es.rxerr < 96)) {
> > +                               if (old_state > CAN_STATE_ERROR_ACTIVE) {
> > +                                       cf->can_id |= CAN_ERR_PROT;
> > +                                       cf->data[2] = CAN_ERR_PROT_ACTIVE;
> > +                               }
> > +                       }
> >                 }

So I would just make the new_state equals CAN_STATE_ERROR_ACTIVE,
and then can_change_state() will do the right thing? Excellent!!
This means the entire block above can be removed ;-)

[ On another note, the block's if conditions above are faulty and
  fixed in patch #3. This patch (#2) used can_change_state()
  without changing any of that faulty logic to ease any future
  bisection. ]

> > -       }
> >  
> > -       if (es.status == M16C_STATE_BUS_ERROR) {
> > -               if ((old_state < CAN_STATE_ERROR_WARNING) &&
> > -                   ((es.txerr >= 96) || (es.rxerr >= 96))) {
> > -                       cf->can_id |= CAN_ERR_CRTL;
> > -                       cf->data[1] = (es.txerr > es.rxerr)
> > -                                       ? CAN_ERR_CRTL_TX_WARNING
> > -                                       : CAN_ERR_CRTL_RX_WARNING;
> > -               } else if (old_state > CAN_STATE_ERROR_ACTIVE) {
>
> This is also redundant, and wrong:
>
> > +               if (!es.status) {
> >                         cf->can_id |= CAN_ERR_PROT;
> >                         cf->data[2] = CAN_ERR_PROT_ACTIVE;
> >                 }
> > -       }

As in the above; extra code to be removed, yay! ;-)

> >  
> > -       if (!es.status) {
> > -               cf->can_id |= CAN_ERR_PROT;
> > -               cf->data[2] = CAN_ERR_PROT_ACTIVE;
> > -       }
> > -
> > -       if (priv->can.restart_ms &&
> > -           (old_state >= CAN_STATE_BUS_OFF) &&
> > -           (priv->can.state < CAN_STATE_BUS_OFF)) {
> > -               cf->can_id |= CAN_ERR_RESTARTED;
> > -               netif_carrier_on(priv->netdev);
> > +               if (priv->can.restart_ms &&
> > +                   (old_state >= CAN_STATE_BUS_OFF) &&
> > +                   (new_state < CAN_STATE_BUS_OFF)) {
> > +                       cf->can_id |= CAN_ERR_RESTARTED;
> > +                       netif_carrier_on(priv->netdev);
> > +               }
> >         }
> >  
> >         if (es.error_factor) {
> > -- 
> > 1.9.1
> 
> Looking over the patch again, I've noticed that there
> are a few things that are not quite right.
> 

The state-handlig code has become much simpler since your
reveiw and Kleine-Budde's one. Thanks a lot for all the effort.

Regards,
Darwish

^ permalink raw reply

* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Ahmed S. Darwish @ 2015-01-25  2:49 UTC (permalink / raw)
  To: Andri Yngvason
  Cc: Marc Kleine-Budde, Olivier Sobrie, Oliver Hartkopp,
	Wolfgang Grandegger, Linux-CAN, netdev, LKML
In-Reply-To: <20150122101447.25198.24222@shannon>

On Thu, Jan 22, 2015 at 10:14:47AM +0000, Andri Yngvason wrote:
> Quoting Marc Kleine-Budde (2015-01-21 22:59:23)
> > On 01/21/2015 05:20 PM, Andri Yngvason wrote:
> > > Marc, could you merge the "move bus_off++" patch before you merge this so that I
> > > won't have to incorporate this patch-set into it?
> > 
> > ...included in the lastest pull-request to David. Use
> > tags/linux-can-next-for-3.20-20150121 of the can-next repo as you new base.
> > 
> 
> Thanks!
>

I guess I'll re-base my next submission over this tag too.
Nothing in the new 5 patches is substantial enough to be
included in the current kernel release.

Thanks!
Darwish

^ permalink raw reply

* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Ahmed S. Darwish @ 2015-01-25  2:43 UTC (permalink / raw)
  To: Andri Yngvason
  Cc: Wolfgang Grandegger, Olivier Sobrie, Oliver Hartkopp,
	Marc Kleine-Budde, Linux-CAN, netdev, LKML
In-Reply-To: <20150123103213.6205.54618@shannon>

On Fri, Jan 23, 2015 at 10:32:13AM +0000, Andri Yngvason wrote:
> Quoting Ahmed S. Darwish (2015-01-23 06:07:34)
> > On Wed, Jan 21, 2015 at 05:13:45PM +0100, Wolfgang Grandegger wrote:
> > > On Wed, 21 Jan 2015 10:36:47 -0500, "Ahmed S. Darwish"
> > > <darwish.07@gmail.com> wrote:
> > > > On Wed, Jan 21, 2015 at 03:00:15PM +0000, Andri Yngvason wrote:
> > > >> Quoting Ahmed S. Darwish (2015-01-21 14:43:23)
> > > >> > Hi!
> > > > 
> > > > ...
> > > > 
> > > >> > <-- Unplug the cable -->
> > > >> > 
> > > >> >  (000.009106)  can0  20000080   [8]  00 00 00 00 00 00 08 00  
> > > >> >  ERRORFRAME
> > > >> >         bus-error
> > > >> >         error-counter-tx-rx{{8}{0}}
> > > >> >  (000.001872)  can0  20000080   [8]  00 00 00 00 00 00 10 00  
> > > 
> > > For a bus-errors I would also expcect some more information in the
> > > data[2..3] fields. But these are always zero.
> > > 
> > 
> > M16C error factors made it possible to report things like
> > CAN_ERR_PROT_FORM/STUFF/BIT0/BIT1/TX in data[2], and
> > CAN_ERR_PROT_LOC_ACK/CRC_DEL in data[3].
> > 
> > Unfortunately such error factors are only reported in Leaf, but
> > not in USBCan-II due to the wire format change in the error event:
> > 
> >         struct leaf_msg_error_event {
> >                 u8 tid;
> >                 u8 flags;
> >                 __le16 time[3];
> >                 u8 channel;
> >                 u8 padding;
> >                 u8 tx_errors_count;
> >                 u8 rx_errors_count;
> >                 u8 status;
> >                 u8 error_factor;
> >         } __packed;
> > 
> >         struct usbcan_msg_error_event {
> >                 u8 tid;
> >                 u8 padding;
> >                 u8 tx_errors_count_ch0;
> >                 u8 rx_errors_count_ch0;
> >                 u8 tx_errors_count_ch1;
> >                 u8 rx_errors_count_ch1;
> >                 u8 status_ch0;
> >                 u8 status_ch1;
> >                 __le16 time;
> >         } __packed;
> > 
> > I speculate that the wire format was changed due to controller
> > bugs in the USBCan-II, which was slightly mentioned in their
> > data sheets here:
> > 
> >         http://www.kvaser.com/canlib-webhelp/page_hardware_specific_can_controllers.html
> > 
> > So it seems there's really no way for filling such bus error
> > info given the very limited amount of data exported :-(
> >
> We experienced similar problems with FlexCAN.

Hmm, I'll have a look there then...

Although my initial instincts imply that the FlexCAN driver has
access to the raw CAN registers, something I'm unable to do here.
But maybe there's some black magic I'm missing :-)

[...]

> > 
> > I've dumped _every_ message I receive from the firmware while
> > disconnecting the CAN bus, waiting a while, and connecting it again.
> > I really received _nothing_ from the firmware when the CAN bus was
> > reconnected and the data packets were flowing again. Not even a
> > single CHIP_STATE_EVENT, even after waiting for a long time.
> > 
> > So it's basically:
> > ...
> > ERR EVENT, txerr=128, rxerr=0
> > ERR EVENT, txerr=128, rxerr=0
> > ERR EVENT, txerr=128, rxerr=0
> > ...
> > 
> > then complete silence, except the data frames. I've even tried with
> > different versions of the firmware, but the same behaviour persisted.
> > 
> > > > So, What can the driver do given the above?
> > > 
> > > Little if the notification does not come.
> > > 
> > 
> > We can poll the state by sending CMD_GET_CHIP_STATE to the firmware,
> > and it will hopefully reply with a CHIP_STATE_EVENT response
> > containing the new txerr and rxerr values that we can use for
> > reverse state transitions.
> >
> > But do we _really_ want to go through the path? I feel that it will
> > open some cans of worms w.r.t. concurrent access to both the netdev
> > and USB stacks from a single driver.
> >
> Honestly, I don't know.
> >
> > A possible solution can be setting up a kernel thread that queries
> > for a CHIP_STATE_EVENT every second?
> > 
> Have you considered polling in kvaser_usb_tx_acknowledge? You could do something
> like:
> if(unlikely(dev->can.state != CAN_STATE_ERROR_ACTIVE))
> {
>     request_state();
> }
> 

OK, I have four important updates on this issue:

a) My initial testing was done on high-speed channel, at a bitrate
   of 50K. After setting the bus to a more reasonable bitrate 500K
   or 1M, I was _consistently_ able to receive CHIP_STATE_EVENTs
   when plugging the CAN connector again after an unplug.

b) The error counters on this device do not get reset on plugging
   after an unplug. I've setup a kernel thread [2] that queries
   the chip state event every second, and the error counters stays
   the same all the time. [1]

c) There's a single case when the erro counters do indeed get
   reversed, and it happens only when introducing some noise in
   the bus after the re-plug. In that case, the new error events
   get raised with new error counters starting from 0/1 again.

d) I've discovered a bug that forbids the CAN state from
   returning to ERROR_ACTIVE in case of the error counters
   numbers getting decreased. But independent from that bug, the
   verbose debugging messages clearly imply that we only get the
   error counters decreased in the case mentioned at `c)' above.

So from [1] and [2], it's now clear that the device do not reset
these counters back in the re-plug case. I'll give a check to
flexcan as advised, but unfortunately I don't really think there's
much I can do about this.

[1]

[  877.207082] CAN_ERROR_: channel=0, txerr=88, rxerr=0
[  877.207090] CAN_ERROR_: channel=0, txerr=136, rxerr=0
[  877.207094] CAN_ERROR_: channel=0, txerr=144, rxerr=0
[  877.207098] CAN_ERROR_: channel=0, txerr=152, rxerr=0
[  877.207100] CAN_ERROR_: channel=0, txerr=160, rxerr=0
[  877.207102] CAN_ERROR_: channel=0, txerr=168, rxerr=0
[  877.208075] CAN_ERROR_: channel=0, txerr=200, rxerr=0 

(( The above error event, staying the same at txerr=200 keeps
   flooding the bus until the CAN cable is re-plugged ))

[  878.225116] CHIP_STATE: channel=0, txerr=200, rxerr=0
[  878.225143] CHIP_STATE: channel=1, txerr=0, rxerr=0
[  879.265167] CHIP_STATE: channel=0, txerr=200, rxerr=0
[  879.267152] CHIP_STATE: channel=1, txerr=0, rxerr=0
[  879.265167] CHIP_STATE: channel=0, txerr=200, rxerr=0
[  879.267152] CHIP_STATE: channel=1, txerr=0, rxerr=0

(( The same counters get repeated every second ))

[2] State was polled using:

static int kvaser_usb_poll_chip_state(void *vpriv) {
      struct kvaser_usb_net_priv *priv = vpriv;

      while (!kthread_should_stop()) {
              kvaser_usb_simple_msg_async(priv, CMD_GET_CHIP_STATE);
              ssleep(1);
      }

      return 0;
}

> I don't think that anything beyond that would be worth pursuing.
> 

I agree, but given the new input, it seems that our problem
extends to the error counters themselves not getting decreased
on re-plug. So, even polling will not solve the issue: we'll
get the same txerr/rxerr values again and again :-(

> Best regards,
> Andri

Regards,
Darwish


^ permalink raw reply

* Re: [PATCH net-next 0/7] be2net: patch set
From: David Miller @ 2015-01-25  1:23 UTC (permalink / raw)
  To: sathya.perla; +Cc: netdev
In-Reply-To: <1421743909-14716-1-git-send-email-sathya.perla@emulex.com>

From: Sathya Perla <sathya.perla@emulex.com>
Date: Tue, 20 Jan 2015 03:51:42 -0500

> Hi David, as the below patch-set includes minor bug fixes and some
> code re-org, pls consider applying it to the "net-next" tree. Thanks!

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH v2 0/4] net: stmmac: dwmac-rk: Fix phy regulator issues
From: David Miller @ 2015-01-25  1:17 UTC (permalink / raw)
  To: romain.perier
  Cc: peppe.cavallaro, netdev, linux-kernel, heiko, linux-rockchip,
	linux-arm-kernel, devicetree, roger.chen
In-Reply-To: <1421737780-1533-1-git-send-email-romain.perier@gmail.com>

From: Romain Perier <romain.perier@gmail.com>
Date: Tue, 20 Jan 2015 07:09:36 +0000

> This series fixes few issues in dwmac-rk:
> 
> 1. Voltage settings was hardcoded into the driver for the phy regulator.
>    The driver now uses the default voltage settings found in the devicetree,
>    which are applied throught the regulator framework.
> 2. The regulator name used to power on or power off the phy was put in the devicetree
>    variable "phy_regulator", which is not standard and added a lot of code for nothing.
>    The driver now uses the devicetree property "phy-supply" and the corresponding functions
>    to manipulate this regulator.
> 
> The corresponding devicetree files are also updated. As this new binding for
> rk3288 has not been released with any official kernel yet (not until 3.20),
> I don't need to care about keeping compatibility with the old non standard property.

Series applied to net-next, thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/2] Add support to dump flash and rss config
From: David Miller @ 2015-01-25  1:13 UTC (permalink / raw)
  To: hariprasad; +Cc: netdev, leedom, anish, nirranjan, praveenm
In-Reply-To: <1421735541-32400-1-git-send-email-hariprasad@chelsio.com>

From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Tue, 20 Jan 2015 12:02:19 +0530

> This patch series adds support to dump flash, rss, rss_key, rss_config,
> rss_pf_config and rss_vf_config debugfs entries.
> 
> The patches series is created against 'net-next' tree.
> And includes patches on cxgb4 driver.
> 
> We have included all the maintainers of respective drivers. Kindly review the
> change and let us know in case of any review comments.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next v3 2/5] swdevice: add new api to set and del bridge port attributes
From: roopa @ 2015-01-25  1:03 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, vyasevic,
	ronen.arad, netdev, davem, shm, gospo
In-Reply-To: <20150124112657.GA1879@nanopsycho.orion>

On 1/24/15, 3:26 AM, Jiri Pirko wrote:
> Sat, Jan 24, 2015 at 12:10:01AM CET, roopa@cumulusnetworks.com wrote:
>> On 1/23/15, 2:45 PM, roopa wrote:
>>> On 1/23/15, 8:06 AM, Jiri Pirko wrote:
>>>> Fri, Jan 23, 2015 at 04:58:57PM CET, roopa@cumulusnetworks.com wrote:
>>>>> On 1/23/15, 2:41 AM, Jiri Pirko wrote:
>>>>>
>>>>> <snip..>
>>>>>> +
>>>>>> +/**
>>>>>> + *    netdev_switch_port_bridge_dellink - Notify switch device port
>>>>>> of bridge
>>>>>> + *    attribute delete
>>>>>> + *
>>>>>> + *    @dev: port device
>>>>>> + *    @nlh: netlink msg with bridge port attributes
>>>>>> + *
>>>>>> + *    Notify switch device port of bridge port attribute delete
>>>>>> + */
>>>>>> +int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>>>> +                      struct nlmsghdr *nlh, u16 flags)
>>>>>> +{
>>>>>> +    const struct net_device_ops *ops = dev->netdev_ops;
>>>>>> +    struct net_device *lower_dev;
>>>>>> +    struct list_head *iter;
>>>>>> +    int ret = 0, err = 0;
>>>>>> +
>>>>>> +    if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>>>> +        return err;
>>>>>> +
>>>>>> +    if (ops->ndo_bridge_dellink) {
>>>>>> +        WARN_ON(!ops->ndo_switch_parent_id_get);
>>>>>> +        return ops->ndo_bridge_dellink(dev, nlh, flags);
>>>>>> +    }
>>>>>> +
>>>>>> +    netdev_for_each_lower_dev(dev, lower_dev, iter) {
>>>>>> +        err = netdev_switch_port_bridge_dellink(lower_dev, nlh,
>>>>>> flags);
>>>>>> +        if (err)
>>>>>> +            ret = err;
>>>>>> +    }
>>>>>> +
>>>>>> +    return ret;
>>>>>> +}
>>>>>> +EXPORT_SYMBOL(netdev_switch_port_bridge_dellink);
>>>>>> -- 
>>>>>> 1.7.10.4
>>>>>>
>>>>>> Is there any other place, other than bridge code, this functions are
>>>>>> suppored to be called from?
>>>>> No other place today. Its usually the master that implements
>>>>> ndo_bridge_setlink/dellink.
>>>>>
>>>>>> If not, which I consider likely, it would
>>>>>> make more sense to me to:
>>>>>>
>>>>>> - move netdev_for_each_lower_dev iterations directly to bridge code
>>>>>> - let the masters (bond, team, ..) implement ndo_bridge_*link and do
>>>>>>    the traversing there (can be in a form of pre-prepared default
>>>>>>    ndo callback (ndo_dflt_netdev_switch_port_bridge_*link)
>>>>> But, i am still not understanding why i would modify bond, team and
>>>>> other
>>>>> slaves
>>>> Well, that is the usual way to propagate ndo calls. People are used to
>>>> this. It is visible right away in bonding/other code that is propagated
>>>> some ndo call to slaves. With your code, that is somehow hidden and only
>>>> dependent on NETIF_F_HW_NETFUNC_OFFLOAD flag.
>>>>
>>>> Note that there are only couple of "master drivers" (for this, most
>>>> likely
>>>> only bond and team modifications are needed).
>>> ndo_bridge_setlink today is only implemented by drivers that implement
>>> bridging function.
>>> So, having the bond and team driver implement it...seems odd.
> Well, it is not odd. It is ok I believe. Same as for example:
> .ndo_set_mac_address
> .ndo_change_mtu
> .ndo_vlan_rx_add_vid
> .ndo_vlan_rx_kill_vid
> .ndo_poll_controller
> .ndo_netpoll_setup
> .ndo_netpoll_cleanup
>
> All take care of propagating the ndo to slaves.
These mostly operate on netdevs directly.
I do have some hesitation, But, I will take your suggestion and add it 
to the bond and team drivers,
>
>>> But if you insist, i am going to do just that.
>> A side note, I dont see any reason for ndo_bridge_setlink to be renamed to
>> ndo_setlink. Because it seems to take the whole netlink msg anyways. It can
>> be used to offload other link attributes besides bridging (vxlan and so on).
>> Any thoughts on that ?.
> Well, it is PF_BRIDGE so I think that the name is accurate.
The ndo op does not necessarily have to be tied to the netlink msg family.
I don't plan to rename it now. But, in future if i see more value in 
reusing it for other offloads,
i will come back to this.

Thanks for the review.

^ permalink raw reply

* Business Overture
From: Ong Eng @ 2015-01-24  2:44 UTC (permalink / raw)




Help me buy properties/equipments in your country:ongenghuat005@gmail.com

^ permalink raw reply

* Re: [net-next PATCH v3 00/12] Flow API
From: Thomas Graf @ 2015-01-24 13:48 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: Pablo Neira Ayuso, John Fastabend, simon.horman, sfeldma, netdev,
	davem, gerlitz.or, andy, ast, Jiri Pirko
In-Reply-To: <54C39161.9030300@mojatatu.com>

On 01/24/15 at 07:34am, Jamal Hadi Salim wrote:
> They are not the same. The API lowers the barrier immensely; refer to
> my response to John. And we are actually handing it to them.
> 
> From an equivalance perspective:
> Youd have to convince Dave to allow all those TOE vendors to get in
> with their direct hardware APIs (if those guys are still around).

I'm not advocating TOE, not sure why that pops up. I made the point
that various ways already exist for an out of tree driver to expose
a Netlink or other interface which provides direct access to the
hardware and that we can't prevent that.

Anyways, looks like you agree to the general direction the thread has
taken with Jiri's input.

^ permalink raw reply

* Re: [PATCH v3] net: mv643xx_eth: Fix highmem support in non-TSO egress path
From: Ezequiel Garcia @ 2015-01-24 13:45 UTC (permalink / raw)
  To: David Miller, Russell King; +Cc: netdev, B38611, fabio.estevam, David.Laight
In-Reply-To: <1421937182-1708-1-git-send-email-ezequiel.garcia@free-electrons.com>

Hi all,

On 01/22/2015 11:33 AM, Ezequiel Garcia wrote:
> Commit 69ad0dd7af22b61d9e0e68e56b6290121618b0fb
> Author: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> Date:   Mon May 19 13:59:59 2014 -0300
> 
>     net: mv643xx_eth: Use dma_map_single() to map the skb fragments
> 
> caused a nasty regression by removing the support for highmem skb
> fragments. By using page_address() to get the address of a fragment's
> page, we are assuming a lowmem page. However, such assumption is incorrect,
> as fragments can be in highmem pages, resulting in very nasty issues.
> 
> This commit fixes this by using the skb_frag_dma_map() helper,
> which takes care of mapping the skb fragment properly. Additionally,
> the type of mapping is now tracked, so it can be unmapped using
> dma_unmap_page or dma_unmap_single when appropriate.
> 
> This commit also fixes the error path in txq_init() to release the
> resources properly.
> 
> Fixes: 69ad0dd7af22 ("net: mv643xx_eth: Use dma_map_single() to map the skb fragments")
> Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>

Russell, when you have some time, please test this patch. I think it
solves the regression and it manages to unmap the descriptors properly.

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply

* Re: [net-next PATCH v3 00/12] Flow API
From: Thomas Graf @ 2015-01-24 13:34 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: John Fastabend, Jiri Pirko, Pablo Neira Ayuso, simon.horman,
	sfeldma, netdev, davem, gerlitz.or, andy, ast
In-Reply-To: <54C39C8E.4@mojatatu.com>

On 01/24/15 at 08:22am, Jamal Hadi Salim wrote:
> It is up to user space to decide on what the policy should do.
> The kernel is not paid to think. You tell it what to do and it does it
> efficiently. So if you are going to tell it to have a mix and match
> of some things to execute in hardware and some in software then
> it may shoot someone's big toe.

OK. We seem agree on this part. In order to do so, user space needs
to know about hardware capabilities. If that should happen through
tc, so be it. John raised some open question around this and the
rtnl lock is currently a blocker on this architecture as well.

> IOW, user space should decide how a packet is going to flow.
> Agreed that we would need a good way to provide this knowledge
> to user space.
> BTW: Thomas, reading your other email quickly:
> the idea that metadata would be carried around on OF pipeline and
> some script at the end executes the actions is imo  a hardware
> pipeline hack limitation. Why do i want to defer dropping a packet
> when some action is telling me to drop it? ;->

There is obviously no reason to defer a drop.

An example of deferred actions would be if only certain tables allow
certain actions but the matching to chose the action is done in a 
previous tables. Or if you have multiple tables matching on the
original packet header and you need to defer the L2/L3 rewrite until
all matching and action construction is done.

> For some reason, brcm hardware in particulat requires that i
> complete the pipeline first.
> I dont know why we need such a limitation in s/ware (and tc will kill
> the pipeline when needed).

Not sure what "killing the pipeline" means ;-)

^ permalink raw reply

* Re: [net-next PATCH v3 00/12] Flow API
From: Jamal Hadi Salim @ 2015-01-24 13:22 UTC (permalink / raw)
  To: Thomas Graf, John Fastabend, Jiri Pirko
  Cc: Pablo Neira Ayuso, simon.horman, sfeldma, netdev, davem,
	gerlitz.or, andy, ast
In-Reply-To: <20150123174609.GA23242@casper.infradead.org>

On 01/23/15 12:46, Thomas Graf wrote:

> I'm not sure I understand the offload qdisc yet. My interpretation
> so far is that it would contain childs which *must* be offloaded.
>
> How would one transparently offload tc in this model? e.g. let's
> assume we have a simple prio qdisc with u32 cls:
>
> eth0
>    prio
>        class
>        class
>        ...
>      u32 ...
>      u32 ...
>

My view is:
It is up to user space to decide on what the policy should do.
The kernel is not paid to think. You tell it what to do and it does it
efficiently. So if you are going to tell it to have a mix and match
of some things to execute in hardware and some in software then
it may shoot someone's big toe.
IOW, user space should decide how a packet is going to flow.
Agreed that we would need a good way to provide this knowledge
to user space.
BTW: Thomas, reading your other email quickly:
the idea that metadata would be carried around on OF pipeline and
some script at the end executes the actions is imo  a hardware
pipeline hack limitation. Why do i want to defer dropping a packet
when some action is telling me to drop it? ;->
For some reason, brcm hardware in particulat requires that i
complete the pipeline first.
I dont know why we need such a limitation in s/ware (and tc will kill
the pipeline when needed).

Sorry, trying to post while doing other things so not paying close
attention to possibly other important details.

cheers,
jamal

^ permalink raw reply

* Re: [net-next PATCH v3 00/12] Flow API
From: Jamal Hadi Salim @ 2015-01-24 13:04 UTC (permalink / raw)
  To: Jiri Pirko, Thomas Graf
  Cc: John Fastabend, Pablo Neira Ayuso, simon.horman, sfeldma, netdev,
	davem, gerlitz.or, andy, ast
In-Reply-To: <20150123161605.GN2065@nanopsycho.orion>

I am not big on high fives but here's my +1
Excellent diagram below.

cheers,
jamal
On 01/23/15 11:16, Jiri Pirko wrote:
>
> As I wrote earlier, the value is that userspace can easily use single
> xflows api to take care of all ways to handle flows (ovs kernel dp,
> rocker, other device, u32 tc filter + actions, you name it)
>
>
>      my flow managing app
>            |
> uspc      |
>    --------|----------------------------------------------------
> krnl      |
>         tc xflows api
>            |  |  |
>            |  |  ---------------------------------------------------
>            |  |                                                    |
>            |  ------------------                                other xflows backend
>            |                   |
>       ovs xflows backend     rocker driver xflows backend
>            |                   |
>           ovs dp               |
> krnl	                      |
>    ----------------------------|--------------------------------
> hw                            |
>                             rocker switch
>

^ permalink raw reply

* Re: [net-next PATCH v3 00/12] Flow API
From: Jamal Hadi Salim @ 2015-01-24 13:01 UTC (permalink / raw)
  To: John Fastabend, Thomas Graf
  Cc: Jiri Pirko, Pablo Neira Ayuso, simon.horman, sfeldma, netdev,
	davem, gerlitz.or, andy, ast
In-Reply-To: <54C26A1F.6060603@gmail.com>

On 01/23/15 10:34, John Fastabend wrote:

> First 'tc' infrastructure doesn't have any classifier that would map
> well to this today so you are talking about a new classifier looks like
> Jiri is calling it xflows. This is fine.

I know you know this (and apologies for the little Australian Bike
Shed tangent):
You can do _any_ classifier you want. xflows just happens to make the
OF people happy. Someone else who wants to classify on pcre like
strings can go ahead and write another one.
i.e there is no monopoly on what a classifier should be.

> Now 'xflows' needs to implement the same get operations that exist in
> this flow API otherwise writing meaningful policies as Thomas points out
> is crude at best.

It is crude only if you assume the kernel is doing your policies
and fixing any conflicts. Let the kernel do mechanisms and have user
space do the brainy part. No need to give total autonomy to the kernel.


> So this tc classifier supports 'get headers',
> 'get actions', and 'get tables' and then there associated graphs. All
> good so far. This is just an embedding of the existing API in the 'tc'
> netlink family. I've never had any issues with this. Finally you build
> up the 'get_flow' and 'set_flow' operations I still so no issue with
> this and its just an embedding of the existing API into a 'tc
> classifier'. My flow tool becomes one of the classifier tools.
>

You have very few generic verbs really within tc and i dont see
much more needed.
GET/SET(mods for create/append/replace)/DEL with the object
being a noun. Add a handful for capabilities exercising verbs and
you should be on your way.
BTW: I did have capabilities in actions for years but Cong sent a
patch about a year or so ago to kill them because they were not being
exercised from user space tc - I protested but Dave overruled me.
There are still remnants - look at struct tcf_common field
tcfc_capab - the original intent was to have that look like netdev
features bitmask. In any case i never got to proper implementation
and have gained a lot of experience since those early days
and my thinking has changed.


> Now what should I attach my filter to? Typically we attach it to qdiscs
> today. But what does that mean for a switch device? I guess I need an
> _offloaded qdisc_? I don't want to run the same qdisc in my dataplane
> of the switch as I run on the ports going into/out of the sw dataplane.

I dont know if you need a qdisc necessarily that sits in hardware.
But you need to anchor your policy somewhere. The ingress qdisc is
really a dummy for this purpose. It is the beggining of the pipeline.
Most of the hardware i have looked at has some anchor point for the
hardware ACLs. Typically around a queue or a port. Sometime i find it
hard to use this model because of vendor SDKs and APIs they offer.

> Similarly I don't want to run the same set of filters. So at this point
> I have a set of qdiscs per port to represent the switch dataplane and
> a set of qdiscs attached to the software dataplane. If people think this
> is worth doing lets do it. It may get you a nice way to manage QOS while
> your @ it.
>

Lets discuss at the meeting. I am just skimming these emails (the
conference is chewing a lot of my time so i will mostly be absent).
Sorry if i am not responding to some things.

cheers,
jamal


>
>

^ permalink raw reply

* Re: [net-next PATCH v3 00/12] Flow API
From: Jamal Hadi Salim @ 2015-01-24 12:36 UTC (permalink / raw)
  To: Jiri Pirko, Thomas Graf
  Cc: Pablo Neira Ayuso, John Fastabend, simon.horman, sfeldma, netdev,
	davem, gerlitz.or, andy, ast
In-Reply-To: <20150123113934.GD2065@nanopsycho.orion>

On 01/23/15 06:39, Jiri Pirko wrote:

> Maybe I did not express myself correctly. I do not care if this is
> exposed by rtnl or a separate genetlink. The issue still stands. And the
> issue is that the user have to use "the way A" to setup sw datapath and
> "the way B" to setup hw datapath. The preferable would be to have
> "the way X" which can be used to setup both sw and hw.
>
> And I believe that could be achieved. Consider something like this:
>
> - have cls_xflows tc classifier and act_xflows tc action as a wrapper
>    (or api) for John's work. With possibility for multiple backends. The
>    backend iface would looke very similar to what John has now.
> - other tc clses and acts will implement xflows backend
> - openvswitch datapath will implement xflows backend
> - rocker switch will implement xflows backend
> - other drivers will implement xflows backend
>
> Now if user wants to manipulate with any flow setting, he can just use
> cls_xflows and act_xflows to to that.
>
> This is very rough, but I just wanted to draw the picture. This would
> provide single entry to flow world manipulation in kernel, no matter if
> sw or hw.
>
> Thoughts?
>


Exactly my thinking as well.
I guess skipping a few emails helps.

cheers,
jamal

^ permalink raw reply

* Re: [net-next PATCH v3 00/12] Flow API
From: Jamal Hadi Salim @ 2015-01-24 12:34 UTC (permalink / raw)
  To: Thomas Graf, Pablo Neira Ayuso
  Cc: John Fastabend, simon.horman, sfeldma, netdev, davem, gerlitz.or,
	andy, ast, Jiri Pirko
In-Reply-To: <20150122174445.GD25797@casper.infradead.org>

On 01/22/15 12:44, Thomas Graf wrote:
> On 01/22/15 at 05:49pm, Pablo Neira Ayuso wrote:

>
> You can achieve the exact same thing with an out of tree tc action,
> classifier or even a new link type. Nothing prevents an out of tree
> driver to register a new rtnetlink link type and do vendor specific
> crap.
>

They are not the same. The API lowers the barrier immensely; refer to
my response to John. And we are actually handing it to them.

 From an equivalance perspective:
Youd have to convince Dave to allow all those TOE vendors to get in
with their direct hardware APIs (if those guys are still around).

cheers,
jamal

^ permalink raw reply

* Re: [net-next PATCH v3 00/12] Flow API
From: Jamal Hadi Salim @ 2015-01-24 12:29 UTC (permalink / raw)
  To: John Fastabend, Thomas Graf
  Cc: Pablo Neira Ayuso, simon.horman, sfeldma, netdev, davem,
	gerlitz.or, andy, ast, Jiri Pirko
In-Reply-To: <54C12C1F.706@gmail.com>

Sorry I have been running around like a lunatic chicken so havent
had time to join the fun discussion. I hope we can make progress at
the meeting.
I am going to skim and jump through the emails and comment.

On 01/22/15 11:58, John Fastabend wrote:
> On 01/22/2015 07:13 AM, Thomas Graf wrote:
>> On 01/22/15 at 10:00am, Jamal Hadi Salim wrote:

>
> Correct this is fully exposed to user space, but it is also self
> contained inside the API meaning I can learn when to use it and what it
> does by looking at the other operations tables the table graph and
> supported headers. The assumption I am making that is not in the API
> explicitly yet. Is that actions named "set_field_name" perform the
> set operation on that field. We can and plan to extend the API to make
> this assumption explicit in the API.
>

 From what you describe, you are running into a danger of going too low
level such that the interface will end up weighing too much into
flexibility/perfomance and less into usability. If there is one lesson
i learnt from netfilter is usability counts for something. You dont
want another u32 api (otherwise Jiri wouldnt have to write that new
classifier - there is nothing he is doing that cant be done with
u32).

> In this case I can "learn" that I can match on group_id in some tables
> and then use the above action to set the group_id in others.
>

And this discoverability was part of my concern especially when there is
no "stickiness" to the kernel or Linux tooling for that matter by
going direct to hardware. It is a tactical issue more than anything
else. With the approach, you need a little bit of clue of course, you 
really dont even care about compiling the kernel. Essentially the 
barrier to entry for SDKs is immensely lowered.
SDK joy. I know you were intending to replace ethtool - but you are
replacing it with a turbo engine and we need to look at a much bigger
scope.

cheers,
jamal

^ permalink raw reply

* Re: [PATCH net-next v3 2/5] swdevice: add new api to set and del bridge port attributes
From: Jiri Pirko @ 2015-01-24 11:26 UTC (permalink / raw)
  To: roopa
  Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, vyasevic,
	ronen.arad, netdev, davem, shm, gospo
In-Reply-To: <54C2D4C9.5000400@cumulusnetworks.com>

Sat, Jan 24, 2015 at 12:10:01AM CET, roopa@cumulusnetworks.com wrote:
>On 1/23/15, 2:45 PM, roopa wrote:
>>On 1/23/15, 8:06 AM, Jiri Pirko wrote:
>>>Fri, Jan 23, 2015 at 04:58:57PM CET, roopa@cumulusnetworks.com wrote:
>>>>On 1/23/15, 2:41 AM, Jiri Pirko wrote:
>>>>
>>>><snip..>
>>>>>+
>>>>>+/**
>>>>>+ *    netdev_switch_port_bridge_dellink - Notify switch device port
>>>>>of bridge
>>>>>+ *    attribute delete
>>>>>+ *
>>>>>+ *    @dev: port device
>>>>>+ *    @nlh: netlink msg with bridge port attributes
>>>>>+ *
>>>>>+ *    Notify switch device port of bridge port attribute delete
>>>>>+ */
>>>>>+int netdev_switch_port_bridge_dellink(struct net_device *dev,
>>>>>+                      struct nlmsghdr *nlh, u16 flags)
>>>>>+{
>>>>>+    const struct net_device_ops *ops = dev->netdev_ops;
>>>>>+    struct net_device *lower_dev;
>>>>>+    struct list_head *iter;
>>>>>+    int ret = 0, err = 0;
>>>>>+
>>>>>+    if (!(dev->features & NETIF_F_HW_NETFUNC_OFFLOAD))
>>>>>+        return err;
>>>>>+
>>>>>+    if (ops->ndo_bridge_dellink) {
>>>>>+        WARN_ON(!ops->ndo_switch_parent_id_get);
>>>>>+        return ops->ndo_bridge_dellink(dev, nlh, flags);
>>>>>+    }
>>>>>+
>>>>>+    netdev_for_each_lower_dev(dev, lower_dev, iter) {
>>>>>+        err = netdev_switch_port_bridge_dellink(lower_dev, nlh,
>>>>>flags);
>>>>>+        if (err)
>>>>>+            ret = err;
>>>>>+    }
>>>>>+
>>>>>+    return ret;
>>>>>+}
>>>>>+EXPORT_SYMBOL(netdev_switch_port_bridge_dellink);
>>>>>-- 
>>>>>1.7.10.4
>>>>>
>>>>>Is there any other place, other than bridge code, this functions are
>>>>>suppored to be called from?
>>>>No other place today. Its usually the master that implements
>>>>ndo_bridge_setlink/dellink.
>>>>
>>>>>If not, which I consider likely, it would
>>>>>make more sense to me to:
>>>>>
>>>>>- move netdev_for_each_lower_dev iterations directly to bridge code
>>>>>- let the masters (bond, team, ..) implement ndo_bridge_*link and do
>>>>>   the traversing there (can be in a form of pre-prepared default
>>>>>   ndo callback (ndo_dflt_netdev_switch_port_bridge_*link)
>>>>But, i am still not understanding why i would modify bond, team and
>>>>other
>>>>slaves
>>>Well, that is the usual way to propagate ndo calls. People are used to
>>>this. It is visible right away in bonding/other code that is propagated
>>>some ndo call to slaves. With your code, that is somehow hidden and only
>>>dependent on NETIF_F_HW_NETFUNC_OFFLOAD flag.
>>>
>>>Note that there are only couple of "master drivers" (for this, most
>>>likely
>>>only bond and team modifications are needed).
>>ndo_bridge_setlink today is only implemented by drivers that implement
>>bridging function.
>>So, having the bond and team driver implement it...seems odd.

Well, it is not odd. It is ok I believe. Same as for example:
.ndo_set_mac_address
.ndo_change_mtu
.ndo_vlan_rx_add_vid
.ndo_vlan_rx_kill_vid
.ndo_poll_controller
.ndo_netpoll_setup
.ndo_netpoll_cleanup

All take care of propagating the ndo to slaves.

>>
>>But if you insist, i am going to do just that.
>A side note, I dont see any reason for ndo_bridge_setlink to be renamed to
>ndo_setlink. Because it seems to take the whole netlink msg anyways. It can
>be used to offload other link attributes besides bridging (vxlan and so on).
>Any thoughts on that ?.

Well, it is PF_BRIDGE so I think that the name is accurate.

>
>
>
>

^ permalink raw reply

* Re: [PATCH v2 1/4] net: stmmac: dwmac-rk: Don't set the regulator voltage for phy from the driver
From: Heiko Stübner @ 2015-01-24 10:17 UTC (permalink / raw)
  To: Romain Perier
  Cc: peppe.cavallaro-qxv4g6HH51o, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	roger.chen-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <1421737780-1533-2-git-send-email-romain.perier-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Am Dienstag, 20. Januar 2015, 07:09:37 schrieb Romain Perier:
> As these settings can be directly expressed from devicetree for both fixed
> regulators and pmic-integrated regulators, it is more standard to set them
> from dts and let the regulator framework use the right voltage informations
> when it is used in the driver.
> 
> Signed-off-by: Romain Perier <romain.perier-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Romain suggested to resend my tags against a real patch of the series, so it 
turns up in patchwork, so

This series
Tested-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Reviewed-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>

@Dave: as said in the v1 mail, this would be nice to have for 3.20


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* NEED AN URGENT NEW YEAR LOAN FOR ANY PURPOSE ? REPLY US FOR MORE INFO?
From: Macdonald Erick @ 2015-01-24  1:18 UTC (permalink / raw)
  To: Recipients



---
This email has been checked for viruses by Avast antivirus software.
http://www.avast.com

^ permalink raw reply

* Re: [net-next] net: netcp: remove unused kconfig option and code
From: David Miller @ 2015-01-24  6:21 UTC (permalink / raw)
  To: m-karicheri2; +Cc: netdev, linux-kernel
In-Reply-To: <1421781756-11151-1-git-send-email-m-karicheri2@ti.com>

From: Murali Karicheri <m-karicheri2@ti.com>
Date: Tue, 20 Jan 2015 14:22:36 -0500

> Currently CPTS is built into the netcp driver even though there is no
> call out to the CPTS driver. This patch removes the dependency in Kconfig
> and remove cpts.o from the Makefile for NetCP.
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
>  drivers/net/ethernet/ti/Kconfig  |    2 +-
>  drivers/net/ethernet/ti/Makefile |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
> index e11bcfa..824e376 100644
> --- a/drivers/net/ethernet/ti/Kconfig
> +++ b/drivers/net/ethernet/ti/Kconfig
> @@ -73,7 +73,7 @@ config TI_CPSW
>  config TI_CPTS
>  	boolean "TI Common Platform Time Sync (CPTS) Support"
>  	depends on TI_CPSW
> -	depends on TI_CPSW || TI_KEYSTONE_NET
> +	depends on TI_CPSW

Just remove the second dependency line, it's redundant because you've
made it identical to the line before it.

^ permalink raw reply

* Re: [PATCH net] amd-xgbe: Use proper Rx flow control register
From: David Miller @ 2015-01-24  6:18 UTC (permalink / raw)
  To: thomas.lendacky; +Cc: netdev
In-Reply-To: <20150120182031.20236.62843.stgit@tlendack-t1.amdoffice.net>

From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Tue, 20 Jan 2015 12:20:31 -0600

> Updated hardware documention shows the Rx flow control settings were
> moved from the Rx queue operation mode register to a new Rx queue flow
> control register.  The old flow control settings are now reserved areas
> of the Rx queue operation mode register. Update the code to use the new
> register.
> 
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>

Applied, thanks TOm.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox