| Message ID | 50F17178.3090705@linux-ipv6.org |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
That looks good to me and is more or less exactly I wanted to do. > diff --git a/include/net/ndisc.h b/include/net/ndisc.h > index 3c53257..1de4e0c 100644 > --- a/include/net/ndisc.h > +++ b/include/net/ndisc.h > @@ -52,6 +52,7 @@ enum { > #include <linux/hash.h> > > #include <net/neighbour.h> > +#include <net/firewire.h> -- snip -- > diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c > index 99cd286..9a0ba9c 100644 > --- a/net/ipv6/ndisc.c > +++ b/net/ipv6/ndisc.c > @@ -72,6 +72,8 @@ > #include <linux/netfilter.h> > #include <linux/netfilter_ipv6.h> > > +#include <net/firewire.h> -- snip -- But now we have to include firewire.h in the ndisc code. The advantage is that we the firewire_net structure is now private. Regards, Stephan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jan 12 YOSHIFUJI Hideaki wrote: > CC: Stephan Gatzka <stephan.gatzka@gmail.com> > CC: Stefan Richter <stefanr@s5r6.in-berlin.de> > Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> > --- > include/net/ndisc.h | 14 +++++++++++- > net/ipv6/addrconf.c | 4 +++- > net/ipv6/ndisc.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++---- > net/ipv6/route.c | 2 ++ > 4 files changed, 74 insertions(+), 6 deletions(-) > > diff --git a/include/net/ndisc.h b/include/net/ndisc.h > index 3c53257..1de4e0c 100644 > --- a/include/net/ndisc.h > +++ b/include/net/ndisc.h > @@ -52,6 +52,7 @@ enum { > #include <linux/hash.h> > > #include <net/neighbour.h> > +#include <net/firewire.h> > > struct ctl_table; > struct inet6_dev; > @@ -127,10 +128,19 @@ static int ndisc_addr_option_pad(unsigned short type) > } > } > > +static int ndisc_addr_option_postpad(unsigned short type) > +{ > + switch (type) { > + case ARPHRD_IEEE1394: return sizeof(struct fwnet_peerinfo); > + default: return 0; > + } > +} > + > static inline int ndisc_opt_addr_space(struct net_device *dev) > { > return NDISC_OPT_SPACE(dev->addr_len + > - ndisc_addr_option_pad(dev->type)); > + ndisc_addr_option_pad(dev->type) + > + ndisc_addr_option_postpad(dev->type)); > } > > static inline u8 *ndisc_opt_addr_data(struct nd_opt_hdr *p, > @@ -203,6 +213,8 @@ extern void ndisc_send_redirect(struct sk_buff *skb, > extern int ndisc_mc_map(const struct in6_addr *addr, char *buf, > struct net_device *dev, int dir); > > +extern void ndisc_update_peerinfo(struct net_device *dev, > + void *lladdr); > > /* > * IGMP > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c > index 408cac4a..9a0728a 100644 > --- a/net/ipv6/addrconf.c > +++ b/net/ipv6/addrconf.c > @@ -1729,6 +1729,7 @@ static int ipv6_generate_eui64(u8 *eui, struct net_device *dev) > case ARPHRD_IPGRE: > return addrconf_ifid_gre(eui, dev); > case ARPHRD_IEEE802154: > + case ARPHRD_IEEE1394: > return addrconf_ifid_eui64(eui, dev); > } > return -1; > @@ -2571,7 +2572,8 @@ static void addrconf_dev_config(struct net_device *dev) > (dev->type != ARPHRD_FDDI) && > (dev->type != ARPHRD_ARCNET) && > (dev->type != ARPHRD_INFINIBAND) && > - (dev->type != ARPHRD_IEEE802154)) { > + (dev->type != ARPHRD_IEEE802154) && > + (dev->type != ARPHRD_IEEE1394)) { > /* Alas, we support only Ethernet autoconfiguration. */ > return; > } > diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c > index 99cd286..9a0ba9c 100644 > --- a/net/ipv6/ndisc.c > +++ b/net/ipv6/ndisc.c > @@ -72,6 +72,8 @@ > #include <linux/netfilter.h> > #include <linux/netfilter_ipv6.h> > > +#include <net/firewire.h> > + > /* Set to 3 to get tracing... */ > #define ND_DEBUG 1 > > @@ -143,6 +145,22 @@ struct neigh_table nd_tbl = { > .gc_thresh3 = 1024, > }; > > +#if defined(CONFIG_FIREWIRE_NET) > +static u8 *__ndisc_fill_addr_option_firewire_postpad(u8 *opt, int space, void *data, > + struct net_device *dev) > +{ > + if (likely(space >= sizeof(struct fwnet_peerinfo))) { > + fwnet_fill_peerinfo(dev, (__be64 *)data, (struct fwnet_peerinfo *)opt); > + opt += sizeof(struct fwnet_peerinfo); > + space -= sizeof(struct fwnet_peerinfo); > + } As noted, please do not add EXPORTs to drivers/firewire/net.c and call those from net/ipv6/*.c or anywhere else. Instead, let drivers/firewire/net.c provide a function pointer to ndisc_build_skb() or, if possible, just let it provide a pointer to struct fwnet_peerinfo source data to be copied here. Whether the function pointer or the source data go via struct net_device or some other struct or via an extended version of ndisc_build_skb() with added arguments is something I can't decide, not having worked with the networking code myself yet. > + if (space > 0) > + memset(opt, 0, space); > + > + return opt + space; > +} > +#endif > + > static u8 *ndisc_fill_addr_option(u8 *opt, int type, void *data, > struct net_device *dev) > { > @@ -160,9 +178,20 @@ static u8 *ndisc_fill_addr_option(u8 *opt, int type, void *data, > memcpy(opt+2, data, data_len); > data_len += 2; > opt += data_len; > - if ((space -= data_len) > 0) > - memset(opt, 0, space); > - return opt + space; > + > + if ((space -= data_len) > 0) { > + switch (dev->type) { > +#if defined(CONFIG_FIREWIRE_NET) > + case ARPHRD_IEEE1394: > + opt = __ndisc_fill_addr_option_firewire_postpad(opt, space, > + data, dev); > +#endif > + default: > + memset(opt, 0, space); > + opt += space; > + } > + } > + return opt; > } > > static struct nd_opt_hdr *ndisc_next_option(struct nd_opt_hdr *cur, > @@ -366,6 +395,19 @@ static void pndisc_destructor(struct pneigh_entry *n) > ipv6_dev_mc_dec(dev, &maddr); > } > > +void ndisc_update_peerinfo(struct net_device *dev, void *lladdr) > +{ > +#if defined(CONFIG_FIREWIRE_NET) > + switch (dev->type) { > + case ARPHRD_IEEE1394: > + fwnet_update_peerinfo(dev, lladdr, (struct fwnet_peerinfo *)((__u64 *)lladdr + 1)); > + break; > + default: > + break; > + } > +#endif [...] Dito, please do not solve this with an EXPORT from drivers/firewire/net.c. Either a new driver callback is required, and the function pointer to this callback needs to be reachable indirectly via the skb pointer. Or drivers/firewire/net.c needs to be aware that it just received an NDP packet, and then call a parser function in net/ipv6/ndisc.c which returns the data that drivers/firewire/net.c wants to know.
> Dito, please do not solve this with an EXPORT from drivers/firewire/net.c. Again, I totally agree from the architectural point of view. > > Either a new driver callback is required, and the function pointer to this > callback needs to be reachable indirectly via the skb pointer. How I understand the whole ndisc code, ndisc just copes with struct net_devices. So to break the dependency to the firewire net driver an optional callback routine in struct net_device might be the easiest way. > Or drivers/firewire/net.c needs to be aware that it just received an NDP > packet, and then call a parser function in net/ipv6/ndisc.c which returns > the data that drivers/firewire/net.c wants to know. I think that is definitely to late. All cryptographic stuff (IPSEC) will be done and I don't think we want to recalculate that afterwards. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Stefan Richter wrote: > On Jan 12 YOSHIFUJI Hideaki wrote: >> static struct nd_opt_hdr *ndisc_next_option(struct nd_opt_hdr *cur, >> @@ -366,6 +395,19 @@ static void pndisc_destructor(struct pneigh_entry *n) >> ipv6_dev_mc_dec(dev, &maddr); >> } >> >> +void ndisc_update_peerinfo(struct net_device *dev, void *lladdr) >> +{ >> +#if defined(CONFIG_FIREWIRE_NET) >> + switch (dev->type) { >> + case ARPHRD_IEEE1394: >> + fwnet_update_peerinfo(dev, lladdr, (struct fwnet_peerinfo *)((__u64 *)lladdr + 1)); >> + break; >> + default: >> + break; >> + } >> +#endif > [...] > > Dito, please do not solve this with an EXPORT from drivers/firewire/net.c. > > Either a new driver callback is required, and the function pointer to this > callback needs to be reachable indirectly via the skb pointer. > > Or drivers/firewire/net.c needs to be aware that it just received an NDP > packet, and then call a parser function in net/ipv6/ndisc.c which returns > the data that drivers/firewire/net.c wants to know. How about putting EUI64, maxrec, sspd and fifo in dev->dev_addr? This enable us to send NDISC/ARP packet easily (based on neighbor cache entry), and driver can be notified for new neighbors (thus new peers). Driver can manage peer information based on this notification. Note that notification mechanism is used in Chelsio cxgbe4 driver, for example. It requires some further modification to driver itself, but I guess we do not need any exports with this, and I guess most of most of arp related code can be removed from the driver. --yoshfuji -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> How about putting EUI64, maxrec, sspd and fifo in dev->dev_addr? > This enable us to send NDISC/ARP packet easily (based on neighbor > cache entry), and driver can be notified for new neighbors (thus > new peers). Hm, that looks a bit strange to me, because we need that only for link layer option packets and not for every IPv6 packet transmitted via firewire. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Stephan Gatzka wrote: >> How about putting EUI64, maxrec, sspd and fifo in dev->dev_addr? >> This enable us to send NDISC/ARP packet easily (based on neighbor >> cache entry), and driver can be notified for new neighbors (thus >> new peers). > Hm, that looks a bit strange to me, because we need that only for link layer option packets and not for every IPv6 packet transmitted via firewire. It seems the stack will be clearner, overall. In addition to ARP/NDP things, for example, it will become easier to implement multicast as well; IP layer can tell net_device layer multicast listener information via special mapping. Given multicast bit is disabled in the Unique ID (well, it should be disabled; otherwise, disable multicast bit and trun on locally administrated bit), IP layer can tell full IPv6 address via ip6_mc_map() by copying full IPv6 address to EUI-64 + additional space, e.g: IPv6: FF xx xx xx xx xx xx xx xx xx xx xx xx xx xx xx (IPv6 mutlicast address (FF00::/8)) Similar in IPv4: IPv4: 01 00 00 00 00 00 00 00 00 00 00 00 xx xx xx xx (IPv4 multicast address) Device will be notified via dev_mc_add() / dev_mc_del(), then net_device_opts->ndo_set_rx_mode(), and you can manage multicast lists based on that. When sending the driver can determine if the packet is multicast or not by seeing 0x01 bit in the first octet in the destination virtual "MAC" address, and then MCAP manager can determine IP version by seeing the first octet of our virtual "MAC" address. --yoshfuji -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/ndisc.h b/include/net/ndisc.h index 3c53257..1de4e0c 100644 --- a/include/net/ndisc.h +++ b/include/net/ndisc.h @@ -52,6 +52,7 @@ enum { #include <linux/hash.h> #include <net/neighbour.h> +#include <net/firewire.h> struct ctl_table; struct inet6_dev; @@ -127,10 +128,19 @@ static int ndisc_addr_option_pad(unsigned short type) } } +static int ndisc_addr_option_postpad(unsigned short type) +{ + switch (type) { + case ARPHRD_IEEE1394: return sizeof(struct fwnet_peerinfo); + default: return 0; + } +} + static inline int ndisc_opt_addr_space(struct net_device *dev) { return NDISC_OPT_SPACE(dev->addr_len + - ndisc_addr_option_pad(dev->type)); + ndisc_addr_option_pad(dev->type) + + ndisc_addr_option_postpad(dev->type)); } static inline u8 *ndisc_opt_addr_data(struct nd_opt_hdr *p, @@ -203,6 +213,8 @@ extern void ndisc_send_redirect(struct sk_buff *skb, extern int ndisc_mc_map(const struct in6_addr *addr, char *buf, struct net_device *dev, int dir); +extern void ndisc_update_peerinfo(struct net_device *dev, + void *lladdr); /* * IGMP diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 408cac4a..9a0728a 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -1729,6 +1729,7 @@ static int ipv6_generate_eui64(u8 *eui, struct net_device *dev) case ARPHRD_IPGRE: return addrconf_ifid_gre(eui, dev); case ARPHRD_IEEE802154: + case ARPHRD_IEEE1394: return addrconf_ifid_eui64(eui, dev); } return -1; @@ -2571,7 +2572,8 @@ static void addrconf_dev_config(struct net_device *dev) (dev->type != ARPHRD_FDDI) && (dev->type != ARPHRD_ARCNET) && (dev->type != ARPHRD_INFINIBAND) && - (dev->type != ARPHRD_IEEE802154)) { + (dev->type != ARPHRD_IEEE802154) && + (dev->type != ARPHRD_IEEE1394)) { /* Alas, we support only Ethernet autoconfiguration. */ return; } diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index 99cd286..9a0ba9c 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -72,6 +72,8 @@ #include <linux/netfilter.h> #include <linux/netfilter_ipv6.h> +#include <net/firewire.h> + /* Set to 3 to get tracing... */ #define ND_DEBUG 1 @@ -143,6 +145,22 @@ struct neigh_table nd_tbl = { .gc_thresh3 = 1024, }; +#if defined(CONFIG_FIREWIRE_NET) +static u8 *__ndisc_fill_addr_option_firewire_postpad(u8 *opt, int space, void *data, + struct net_device *dev) +{ + if (likely(space >= sizeof(struct fwnet_peerinfo))) { + fwnet_fill_peerinfo(dev, (__be64 *)data, (struct fwnet_peerinfo *)opt); + opt += sizeof(struct fwnet_peerinfo); + space -= sizeof(struct fwnet_peerinfo); + } + if (space > 0) + memset(opt, 0, space); + + return opt + space; +} +#endif + static u8 *ndisc_fill_addr_option(u8 *opt, int type, void *data, struct net_device *dev) { @@ -160,9 +178,20 @@ static u8 *ndisc_fill_addr_option(u8 *opt, int type, void *data, memcpy(opt+2, data, data_len); data_len += 2; opt += data_len; - if ((space -= data_len) > 0) - memset(opt, 0, space); - return opt + space; + + if ((space -= data_len) > 0) { + switch (dev->type) { +#if defined(CONFIG_FIREWIRE_NET) + case ARPHRD_IEEE1394: + opt = __ndisc_fill_addr_option_firewire_postpad(opt, space, + data, dev); +#endif + default: + memset(opt, 0, space); + opt += space; + } + } + return opt; } static struct nd_opt_hdr *ndisc_next_option(struct nd_opt_hdr *cur, @@ -366,6 +395,19 @@ static void pndisc_destructor(struct pneigh_entry *n) ipv6_dev_mc_dec(dev, &maddr); } +void ndisc_update_peerinfo(struct net_device *dev, void *lladdr) +{ +#if defined(CONFIG_FIREWIRE_NET) + switch (dev->type) { + case ARPHRD_IEEE1394: + fwnet_update_peerinfo(dev, lladdr, (struct fwnet_peerinfo *)((__u64 *)lladdr + 1)); + break; + default: + break; + } +#endif +} + static struct sk_buff *ndisc_build_skb(struct net_device *dev, const struct in6_addr *daddr, const struct in6_addr *saddr, @@ -798,6 +840,9 @@ static void ndisc_recv_ns(struct sk_buff *skb) neigh_update(neigh, lladdr, NUD_STALE, NEIGH_UPDATE_F_WEAK_OVERRIDE| NEIGH_UPDATE_F_OVERRIDE); + + ndisc_update_peerinfo(dev, lladdr); + if (neigh || !dev->header_ops) { ndisc_send_na(dev, neigh, saddr, &msg->target, is_router, @@ -905,6 +950,8 @@ static void ndisc_recv_na(struct sk_buff *skb) NEIGH_UPDATE_F_OVERRIDE_ISROUTER| (msg->icmph.icmp6_router ? NEIGH_UPDATE_F_ISROUTER : 0)); + ndisc_update_peerinfo(dev, lladdr); + if ((old_flags & ~neigh->flags) & NTF_ROUTER) { /* * Change: router to host @@ -970,6 +1017,8 @@ static void ndisc_recv_rs(struct sk_buff *skb) NEIGH_UPDATE_F_OVERRIDE| NEIGH_UPDATE_F_OVERRIDE_ISROUTER); neigh_release(neigh); + + ndisc_update_peerinfo(skb->dev, lladdr); } out: return; @@ -1223,6 +1272,8 @@ skip_linkparms: NEIGH_UPDATE_F_OVERRIDE| NEIGH_UPDATE_F_OVERRIDE_ISROUTER| NEIGH_UPDATE_F_ISROUTER); + + ndisc_update_peerinfo(skb->dev, lladdr); } if (!ipv6_accept_ra(in6_dev)) @@ -1405,7 +1456,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target) if (!ret) goto release; - if (dev->addr_len) { + /* XXX* We have not idea about IEEE1394 peer infomation */ + if (dev->addr_len && dev->type != ARPHRD_IEEE1394) { struct neighbour *neigh = dst_neigh_lookup(skb_dst(skb), target); if (!neigh) { ND_PRINTK(2, warn, diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 621b68e..27c5127 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -1795,6 +1795,8 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu NEIGH_UPDATE_F_ISROUTER)) ); + ndisc_update_peerinfo(skb->dev, lladdr); + nrt = ip6_rt_copy(rt, &msg->dest); if (!nrt) goto out;
CC: Stephan Gatzka <stephan.gatzka@gmail.com> CC: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> --- include/net/ndisc.h | 14 +++++++++++- net/ipv6/addrconf.c | 4 +++- net/ipv6/ndisc.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++---- net/ipv6/route.c | 2 ++ 4 files changed, 74 insertions(+), 6 deletions(-)