trunk shouldnt care if it's stacked

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

trunk shouldnt care if it's stacked

David Gwynne-5
nothing else does, and i want to clean up the code.

ok?

Index: if_trunk.c
===================================================================
RCS file: /cvs/src/sys/net/if_trunk.c,v
retrieving revision 1.137
diff -u -p -r1.137 if_trunk.c
--- if_trunk.c 12 Aug 2018 23:50:31 -0000 1.137
+++ if_trunk.c 9 Jan 2019 02:55:32 -0000
@@ -282,7 +282,6 @@ trunk_port_lladdr(struct trunk_port *tp,
 int
 trunk_port_create(struct trunk_softc *tr, struct ifnet *ifp)
 {
- struct trunk_softc *tr_ptr;
  struct trunk_port *tp;
  int error = 0;
 
@@ -317,18 +316,6 @@ trunk_port_create(struct trunk_softc *tr
  if ((tp = malloc(sizeof *tp, M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
  return (ENOMEM);
 
- /* Check if port is a stacked trunk */
- SLIST_FOREACH(tr_ptr, &trunk_list, tr_entries) {
- if (ifp == &tr_ptr->tr_ac.ac_if) {
- tp->tp_flags |= TRUNK_PORT_STACK;
- if (trunk_port_checkstacking(tr_ptr) >=
-    TRUNK_MAX_STACKING) {
- free(tp, M_DEVBUF, sizeof *tp);
- return (E2BIG);
- }
- }
- }
-
  /* Change the interface type */
  tp->tp_iftype = ifp->if_type;
  ifp->if_type = IFT_IEEE8023ADLAG;
@@ -380,23 +367,6 @@ trunk_port_create(struct trunk_softc *tr
  if_ih_insert(ifp, trunk_input, tp);
 
  return (error);
-}
-
-int
-trunk_port_checkstacking(struct trunk_softc *tr)
-{
- struct trunk_softc *tr_ptr;
- struct trunk_port *tp;
- int m = 0;
-
- SLIST_FOREACH(tp, &tr->tr_ports, tp_entries) {
- if (tp->tp_flags & TRUNK_PORT_STACK) {
- tr_ptr = (struct trunk_softc *)tp->tp_if->if_softc;
- m = MAX(m, trunk_port_checkstacking(tr_ptr));
- }
- }
-
- return (m + 1);
 }
 
 int
Index: if_trunk.h
===================================================================
RCS file: /cvs/src/sys/net/if_trunk.h,v
retrieving revision 1.26
diff -u -p -r1.26 if_trunk.h
--- if_trunk.h 12 Aug 2018 23:50:31 -0000 1.26
+++ if_trunk.h 9 Jan 2019 02:55:32 -0000
@@ -25,7 +25,6 @@
 
 #define TRUNK_MAX_PORTS 32 /* logically */
 #define TRUNK_MAX_NAMESIZE 32 /* name of a protocol */
-#define TRUNK_MAX_STACKING 4 /* maximum number of stacked trunks */
 
 /* Port flags */
 #define TRUNK_PORT_SLAVE 0x00000000 /* normal enslaved port */

Reply | Threaded
Open this post in threaded view
|

Re: trunk shouldnt care if it's stacked

Klemens Nanni-2
On Wed, Jan 09, 2019 at 01:12:31PM +1000, David Gwynne wrote:
> -#define TRUNK_MAX_STACKING 4 /* maximum number of stacked trunks */
Is this an arbitrary limit or does it conceal other limitiations?

The commit that added it lacks this information:

        revision 1.2
        date: 2005/05/24 07:51:53;  author: reyk;  state: Exp;  lines: +44 -10;
        support trunk stacking (trunks as trunk ports) and some fixes

        ok brad@

Since the flag TRUNK_PORT_STACK in if_trunk.h:33 is now unused, maybe
add a comment to it for clarity?

Reply | Threaded
Open this post in threaded view
|

Re: trunk shouldnt care if it's stacked

Philip Guenther-2
On Wed, Jan 9, 2019 at 1:11 AM Klemens Nanni <[hidden email]> wrote:

> On Wed, Jan 09, 2019 at 01:12:31PM +1000, David Gwynne wrote:
> > -#define TRUNK_MAX_STACKING   4       /* maximum number of stacked
> trunks */
> Is this an arbitrary limit or does it conceal other limitiations?
>

If stacking is handled via possibly recursive calls then there's an
effective limit imposed by size of the kernel stack.

Imposing a depth limit also prevents looping a stack by adding an upper
trunk to a bottom one.


Philip Guenther
Reply | Threaded
Open this post in threaded view
|

Re: trunk shouldnt care if it's stacked

Theo de Raadt-2
Philip Guenther <[hidden email]> wrote:

> On Wed, Jan 9, 2019 at 1:11 AM Klemens Nanni <[hidden email]> wrote:
>
> > On Wed, Jan 09, 2019 at 01:12:31PM +1000, David Gwynne wrote:
> > > -#define TRUNK_MAX_STACKING   4       /* maximum number of stacked
> > trunks */
> > Is this an arbitrary limit or does it conceal other limitiations?
> >
>
> If stacking is handled via possibly recursive calls then there's an
> effective limit imposed by size of the kernel stack.
>
> Imposing a depth limit also prevents looping a stack by adding an upper
> trunk to a bottom one.

David please stack 100 times.

Does it work?

Reply | Threaded
Open this post in threaded view
|

Re: trunk shouldnt care if it's stacked

Martin Pieuchot
On 09/01/19(Wed) 19:12, Theo de Raadt wrote:

> Philip Guenther <[hidden email]> wrote:
>
> > On Wed, Jan 9, 2019 at 1:11 AM Klemens Nanni <[hidden email]> wrote:
> >
> > > On Wed, Jan 09, 2019 at 01:12:31PM +1000, David Gwynne wrote:
> > > > -#define TRUNK_MAX_STACKING   4       /* maximum number of stacked
> > > trunks */
> > > Is this an arbitrary limit or does it conceal other limitiations?
> > >
> >
> > If stacking is handled via possibly recursive calls then there's an
> > effective limit imposed by size of the kernel stack.
> >
> > Imposing a depth limit also prevents looping a stack by adding an upper
> > trunk to a bottom one.
>
> David please stack 100 times.
>
> Does it work?

I understand David.  This logic is not coherent with the other
pseudo-drivers and not limited to trunk(4).

Could it be possible to move it out of trunk and turn it generic
now that we have a common SIOCSIFPARENT ioctl(2)?