Re: [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: convert batman iv algorithm to use dynamic infrastructure

[Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>]

[-- Attachment #1: Type: text/plain, Size: 1764 bytes --]

Hi Marek,

interesting approach, thanks for the patches! I don't really think that
we need modules at this point, but we can easily extend it later - for
example, net/mac80211/rate.c has a very similar API and works with modules.

A few comments to the API:

Generally, we should have a few lines per function in types.h to
describe what each function is doing - we don't need that for a RFC patch,
but that would be useful for the final version.

On Tue, Nov 29, 2011 at 12:28:36AM +0800, Marek Lindner wrote:
>  struct bat_algo {
>  	struct hlist_node list;
>  	char name[20];

you can also use a simple pointer here: 
  char *name;

(the name will most probably static in the module anyways)

> +	void (*bat_ogm_init)(struct hard_iface *hard_iface);
> +	void (*bat_ogm_init_primary)(struct hard_iface *hard_iface);
> +	void (*bat_ogm_update_mac)(struct hard_iface *hard_iface);
> +	void (*bat_ogm_schedule)(struct hard_iface *hard_iface,
> +				 int tt_num_changes);

can't we put tt_num_changes somewhere bat_priv?

> +	void (*bat_ogm_emit)(struct forw_packet *forw_packet);
> +	void (*bat_ogm_receive)(const struct ethhdr *ethhdr,
> +				unsigned char *packet_buff, int packet_len,
> +				struct hard_iface *if_incoming);

you can just pass the SKB instead of ethhdr, packet_buff, and packet_len
-> they are derived from the skb anyway.

Then, you have can do the API with always a hard_iface as the first
parameter. ;)

I also agree with Andrew on defining (a subset of) mandatory functions
which every algo has to provide. The register function could then check
and decline algos which don't define mandatory functions, and we don't have
to check availability in the real code later ...

best regards,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]