From: Stephens, A. <all...@wi...> - 2007-12-20 19:37:42
|
Hi Randy: I took a quick look at the patch. Here are my comments. 0) It's great that you've been able to do this without having to change any of the TIPC common files! This means that we don't have to make any changes to other versions of TIPC (like the VxWorks one) which may not want to support this capability. 1) Validation of tipc_stack_id in tipc_init() doesn't appear to catch case where a negative stack number is supplied. 2) Why pass role & stack arguments to routines like start_core_stack() and many others? You should be able to use the global variables tipc_role and tipc_stack_id. (Standard TIPC is already full of similar references to globals like tipc_own_addr and other globals.) 3) Do start_core_stack() and start_core_base() really need to call stop_core() upon failure? It looks like start_core() already does that. 4) I'm not sure why start_core_base() initializes the socket code. This is deliberately done as the last thing in standard TIPC to ensure there is no risk of an application invoking a TIPC socket before the rest of TIPC has been initialized to support it. Wouldn't it make more sense to leave it until the end of start_core_stack()? ... I think you may not be initializing things in quite the right way, perhaps due to some confusion about the relationship between a TIPC stack and the socket API for that stack. It is entirely possible for someone to have a node with two or more TIPC stacks in which some (or all!) of the stacks do not support the socket API (which is theoretically optional, due to the existence of the native API). I would suggest that you alter things to initialize the appropriate element of tipc_families[] in start_core(), then allow the netlink and socket initialization code bind themselves into the appropriate entry for their stack as initialization proceeds. 5) I'm confused by the stack number range checking done by handler_start(). Why is it necessary, and why would it have a different range than the stack number stuff in core.c? 6) You can eliminate the unused arguments to handler_stop() -- see comment 2). 7) I think the range checking for the stack number in tipc_create() should be done in handle_protocol(), since it would be nice to do the checking in one place. Or perhaps there needs to be a get_stack() routine returns a pointer to the appropriate stack element if the stack exists or NULL if it isn't valid? 8) You shouldn't be passing the stack id using the flags field of the netlink message header, since there seem to be some standard meanings for these bits. You may need to create a new field for the stack number, in between the netlink header and the TIPC TLV's. 9) The "-s" option has already be used in TIPC 1.7 to mean "status", so we need to come up with another mnemonic for this. One approach might be to use "-N" (where N is numeric) to indicate the stack number -- i.e. "tipc-config -3" would indicate stack 3; this seems intuitive, but there might be some issues if we ever want to support more than 9 stacks since I'm not sure how things like "tipc-config -31" would get parsed. I'm open to other ideas as well ... Regards, Al -----Original Message----- From: tip...@li... [mailto:tip...@li...] On Behalf Of Randy Macleod Sent: Tuesday, December 18, 2007 4:22 PM To: tipc Cc: Ravi Rao Subject: [tipc-discussion] vTIPC-1.5.12 patch. Hi, Attached is a patch to tipc-1.5.12 that enables multiple independent TIPC stacks to run on a single Linux OS. This patch is referred to as virtualized TIPC or vTIPC. The typical usage is an ATCA chassis that has separate control and data networks (aka base and data in ATCA lingo). Please take a look at the patch and the notes below and let us know what you think of the design and implementation. We've tested on: ppc64, linux-2.6.10 ppc32, linux-2.6.14 x86, linux-2.6.20-16-generic (Ubuntu) There are some issues with 2.6.22 (Ubuntu 7.10) so unfortunately testing on that system will require some work. Signed-off-by: Randy MacLeod <mac...@no...> Thanks, Ravi Rao - TCS Randy MacLeod - Nortel How this works =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D We've logically separated the TIPC module into a CORE and a STACK. The CORE is responsible for interfacing with the kernel - registering AF_TIPC, netlink and handling the creation of new sockets. The STACK is responsbile for the TIPC specific behaviour: name_table, bearers, links, etc. When you load TIPC you specify a role for the module. Valid roles are: - TIPC_FULL =3D 0 - default, usual tipc behaviour. - TIPC_CORE =3D 1 - just register AF_TIPC and dispatch socket = creation. - TIPC_STACK =3D 2 - register with CORE and handle all internals of this particular stack_id. When a STACK is loaded, it is also assigned a stack_id. The STACK binds with the core to register callback functions for - socket creation: tipc_create and - netlink commands: cfg_do_cmd The patch supports up to 8 STACKs but that can be easily changed. To interact with each of these stacks, tipc-config has been modified to accept an addtional parameter: -s=3D<stack_id> The default stack id is zero. From a user process, you indicate which TIPC stack a socket is associated with by specifying the "protocol" parameter in: int socket(AF_TIPC, int type, int protocol); I suggest that: 0 =3D control network, 1 =3D data network. A single process can bind sockets to any TIPC stack that is loaded. The stacks are completely isolated so if you want to transfer a message from the control network to the data network, you must do so via userspace (or using the tipc kernel API). Compiling =3D=3D=3D=3D=3D=3D=3D=3D=3D 1. unpack a fresh tipc-1.5.12.tar.gz 2. cd tipc-1.5.12 3. patch -p1 < /tmp/tipc-1.5.12-vtipc-01.patch 4. make - as you would normally. 5. cp net/tipc/tipc.ko .. or cp tipc.ko .. depending on how you build. 6. cp tipcstack.ko .. 7. cd .. Basics of loading =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D This is all backwards compatible so: # insmod tipc.ko and # tipc-config <any option> # rmmod tipc work as you would expect. New behaviour requires one or two module parameters to be specified at module load time: # modinfo tipc.ko ... parm: tipc_role:Role of tipc: 0 =3D full stack, 1 =3D core only, 2 =3D stack only (int) parm: tipc_stack_id:TIPC stack id: 0-7 permitted. (int) # insmod tipc.ko tipc_role=3D1 would load tipc as TIPC_CORE. It will not initialize a TIPC stack at all. You might want to do this to keep the core klm as simple as possible. # insmod tipcstack.ko tipc_role=3D2 tipc_stack_id=3D1 would load tipc (stack variant), initialize stack_id to 1, and register it with the previously loaded tipc core module. You can add up to 8 (0-7) separate stacks: # insmod tipcstack.ko tipc_role=3D2 tipc_stack_id=3D0 # insmod = tipcstack.ko tipc_role=3D2 tipc_stack_id=3D2 # insmod tipcstack.ko tipc_role=3D2 tipc_stack_id=3D3 # insmod tipcstack.ko tipc_role=3D2 tipc_stack_id=3D4 = # insmod tipcstack.ko tipc_role=3D2 tipc_stack_id=3D5 # insmod = tipcstack.ko tipc_role=3D2 tipc_stack_id=3D6 # insmod tipcstack.ko tipc_role=3D2 tipc_stack_id=3D7 # insmod tipcstack.ko tipc_role=3D2 tipc_stack_id=3D8 = <-- error! Testing =3D=3D=3D=3D=3D=3D=3D 1. sudo insmod /tipc.ko 2. sudo insmod tipcstack.ko tipc_role=3D2 tipc_stack_id=3D1 3. configure network: nodeA: sudo ./tipc-config -s=3D0 -addr=3D1.1.1 -be=3Deth:eth0 nodeA: sudo ./tipc-config -s=3D1 -addr=3D1.1.1 -be=3Deth:eth1 nodeB: sudo ./tipc-config -s=3D0 -addr=3D1.1.2 -be=3Deth:eth0 nodeB: sudo ./tipc-config -s=3D1 -addr=3D1.1.2 -be=3Deth:eth1 Since the tipc stacks are completely independent, the Z.C.N addresses don't have to be the same - you could have: nodeA: sudo ./tipc-config -s=3D0 -addr=3D1.1.1 -be=3Deth:eth0 nodeA: sudo ./tipc-config -s=3D1 -addr=3D1.1.21 -be=3Deth:eth1 nodeB: sudo ./tipc-config -s=3D0 -addr=3D1.1.2 -be=3Deth:eth0 nodeB: sudo ./tipc-config -s=3D1 -addr=3D1.1.22 -be=3Deth:eth1 4. take a look at the links: sudo ./tools/tipc-config -l sudo ./tools/tipc-config -s=3D1 -l ------------------- Limitations =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D When you bind each TIPC stack to a bearer you should ensure that separate LANs are used. Should we enforce this? There is one issue marked with RKR (Ravi's initials) that should be fixed - please read the comment and provide feedback. We should figure out how to make this work with Kconfig. Questions =3D=3D=3D=3D=3D=3D=3D=3D=3D 1. We'd like to rebase this onto a newer version of TIPC. What version would you suggest Allan? 2. Are the copyright notices okay? Shouldn't we add a GPL or dual notice like this: ------------------------------------- * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. --------------------------------------- be included? That's all. // Randy and Ravi. |