On Thu, Jan 22, 2015 at 08:26:57AM +0100, Hannes Reinecke wrote:
> On 01/22/2015 07:14 AM, Chris Leech wrote:
> > The and_start part would always just segfault* after vlan creation, and
> > it's duplicated in the newlink handling now anyway.
> >
> > * except for maybe a reported VID of 0 to run on the physical interface,
> > I'm not surethis change won't break that ...
> > ---
> > fipvlan.c | 20 +++-----------------
> > 1 file changed, 3 insertions(+), 17 deletions(-)
> >
> > diff --git a/fipvlan.c b/fipvlan.c
> > index 211da53..ccba914 100644
> > --- a/fipvlan.c
> > +++ b/fipvlan.c
> > @@ -160,7 +160,7 @@ struct fcf {
> > struct fcf_list_head fcfs = TAILQ_HEAD_INITIALIZER(fcfs);
> > static struct fcf_list_head vn2vns = TAILQ_HEAD_INITIALIZER(vn2vns);
> >
> > -static int create_and_start_vlan(struct fcf *fcf, bool vn2vn);
> > +static int create_vlan(struct fcf *fcf, bool vn2vn);
> >
> > static struct fcf *lookup_fcf(struct fcf_list_head *head, int ifindex,
> > uint16_t vlan, unsigned char *mac)
> > @@ -324,7 +324,7 @@ static int fip_recv_vlan_note(struct fiphdr *fh, int
> > ifindex, bool vn2vn)
> > TAILQ_INSERT_TAIL(head, fcf, list_node);
> > if (!config.create)
> > continue;
> > - create_and_start_vlan(fcf, vn2vn);
> > + create_vlan(fcf, vn2vn);
> > }
> >
> > return 0;
> > @@ -604,7 +604,7 @@ static int rtnl_listener_handler(struct nlmsghdr *nh,
> > UNUSED void *arg)
> > }
> >
> > static int
> > -create_and_start_vlan(struct fcf *fcf, bool vn2vn)
> > +create_vlan(struct fcf *fcf, bool vn2vn)
> > {
> > struct iff *real_dev, *vlan;
> > char vlan_name[IFNAMSIZ];
> > @@ -642,20 +642,6 @@ create_and_start_vlan(struct fcf *fcf, bool vn2vn)
> > }
> > printf("Created VLAN device %s\n", vlan_name);
> > }
> > - if (!config.start)
> > - return rc;
> > -
> > - if (!vlan->running) {
> > - FIP_LOG_DBG("%s if %d not running, starting",
> > - vlan == real_dev ? "real" : "vlan",
> > - vlan->ifindex);
> > - rtnl_set_iff_up(vlan->ifindex, NULL);
> > - } else if (!vlan->fcoe_started) {
> > - printf("Starting FCoE on interface %s\n",
> > - vlan->ifname);
> > - fcoe_instance_start(vlan->ifname);
> > - vlan->fcoe_started = true;
> > - }
> > return rc;
> > }
> >
> >
> Hmm. Not sure this will work.
> Note that this is running in the netlink receive loop, so we only
> would get one event here and have to figure out the next step.
> Omitting these lines will probably break this.
>
> Better to just insert a 'return 0' statement after
> printf("Created VLAN device %s\n", vlan_name);
>
> That's what the original code did.
Yeah, that would not break the vid == 0 case. Other than that, with a
return added after a successful VLAN create, it can never continue past
the else block of (!fcf->vlan).
Actual VLAN use always gets started from the rtnl handler, not the FIP
handler, so this is kind of misleading and only dealing with the vid ==
0 case. But it fixes the segfault, and works.
- Chris
_______________________________________________
fcoe-devel mailing list
[email protected]
http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel
