[Linuxptp-devel] [PATCH 2/5] raw: separate src and dst addresses

[Jiri Benc]

In order to be able to convert to a generic address struct, separate source
and destination address into separate fields.

Signed-off-by: Jiri Benc <***@redhat.com>
---
ether.h | 13 ++++++-------
raw.c | 22 +++++++++++-----------
2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/ether.h b/ether.h
index 07d0f31dae26..89e22c879058 100644
--- a/ether.h
+++ b/ether.h
@@ -26,25 +26,24 @@
#define PTP_DST_MAC 0x01, 0x1B, 0x19, 0x00, 0x00, 0x00
#define P2P_DST_MAC 0x01, 0x80, 0xC2, 0x00, 0x00, 0x0E

-struct eth_addr {
- uint8_t dst[MAC_LEN];
- uint8_t src[MAC_LEN];
-} __attribute__((packed));
+typedef uint8_t eth_addr[MAC_LEN];

struct eth_hdr {
- struct eth_addr mac;
+ eth_addr dst;
+ eth_addr src;
uint16_t type;
} __attribute__((packed));

#define VLAN_HLEN 4

struct vlan_hdr {
- struct eth_addr mac;
+ eth_addr dst;
+ eth_addr src;
uint16_t tpid;
uint16_t tci;
uint16_t type;
} __attribute__((packed));

-#define OFF_ETYPE sizeof(struct eth_addr)
+#define OFF_ETYPE (2 * sizeof(eth_addr))

#endif
diff --git a/raw.c b/raw.c
index 2afe8a25f85a..2de0793745ee 100644
--- a/raw.c
+++ b/raw.c
@@ -45,8 +45,9 @@

struct raw {
struct transport t;
- struct eth_addr ptp_addr;
- struct eth_addr p2p_addr;
+ eth_addr src_addr;
+ eth_addr ptp_addr;
+ eth_addr p2p_addr;
int vlan;
};

@@ -190,14 +191,12 @@ static int raw_open(struct transport *t, const char *name,
struct raw *raw = container_of(t, struct raw, t);
int efd, gfd;

- memcpy(raw->ptp_addr.dst, ptp_dst_mac, MAC_LEN);
- memcpy(raw->p2p_addr.dst, p2p_dst_mac, MAC_LEN);
+ memcpy(raw->ptp_addr, ptp_dst_mac, MAC_LEN);
+ memcpy(raw->p2p_addr, p2p_dst_mac, MAC_LEN);

- if (sk_interface_macaddr(name, raw->ptp_addr.src, MAC_LEN))
+ if (sk_interface_macaddr(name, raw->src_addr, MAC_LEN))
goto no_mac;

- memcpy(raw->p2p_addr.src, raw->ptp_addr.src, MAC_LEN);
-
efd = open_socket(name, 1);
if (efd < 0)
goto no_event;
@@ -277,9 +276,10 @@ static int raw_send(struct transport *t, struct fdarray *fda, int event, int pee

hdr = (struct eth_hdr *) ptr;
if (peer)
- memcpy(&hdr->mac, &raw->p2p_addr, sizeof(hdr->mac));
+ memcpy(&hdr->dst, &raw->p2p_addr, MAC_LEN);
else
- memcpy(&hdr->mac, &raw->ptp_addr, sizeof(hdr->mac));
+ memcpy(&hdr->dst, &raw->ptp_addr, MAC_LEN);
+ memcpy(&hdr->src, &raw->src_addr, MAC_LEN);

hdr->type = htons(ETH_P_1588);

@@ -303,14 +303,14 @@ static void raw_release(struct transport *t)
static int raw_physical_addr(struct transport *t, uint8_t *addr)
{
struct raw *raw = container_of(t, struct raw, t);
- memcpy(addr, raw->ptp_addr.src, MAC_LEN);
+ memcpy(addr, raw->src_addr, MAC_LEN);
return MAC_LEN;
}

static int raw_protocol_addr(struct transport *t, uint8_t *addr)
{
struct raw *raw = container_of(t, struct raw, t);
- memcpy(addr, raw->ptp_addr.src, MAC_LEN);
+ memcpy(addr, raw->src_addr, MAC_LEN);
return MAC_LEN;
}

[Jiri Benc]

Signed-off-by: Jiri Benc <***@redhat.com>
---
raw.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/raw.c b/raw.c
index 795292fab358..2afe8a25f85a 100644
--- a/raw.c
+++ b/raw.c
@@ -102,14 +102,14 @@ static int raw_configure(int fd, int event, int index,

mreq.mr_ifindex = index;
mreq.mr_type = PACKET_MR_MULTICAST;
- mreq.mr_alen = 6;
- memcpy(mreq.mr_address, addr1, 6);
+ mreq.mr_alen = MAC_LEN;
+ memcpy(mreq.mr_address, addr1, MAC_LEN);

err1 = setsockopt(fd, SOL_PACKET, option, &mreq, sizeof(mreq));
if (err1)
pr_warning("setsockopt PACKET_MR_MULTICAST failed: %m");

- memcpy(mreq.mr_address, addr2, 6);
+ memcpy(mreq.mr_address, addr2, MAC_LEN);

err2 = setsockopt(fd, SOL_PACKET, option, &mreq, sizeof(mreq));
if (err2)

[Richard Cochran]

---

Applied.

Thanks,
Richard

[Jiri Benc]

The callers of those functions are all using ptp_message. As we're going to
return more information (the address), let those functions just fill in the
ptp_message fields directly.

Some minor reshuffling needed to prevent circular header dependencies.

Signed-off-by: Jiri Benc <***@redhat.com>
---
msg.h | 14 +++++++++++++-
pmc_common.c | 6 ++----
pmc_common.h | 1 +
port.c | 8 +++-----
transport.c | 19 ++++++++++++-------
transport.h | 21 ++++-----------------
6 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/msg.h b/msg.h
index 7f471ca555aa..564cc9bfa0df 100644
--- a/msg.h
+++ b/msg.h
@@ -25,7 +25,6 @@
#include <time.h>

#include "ddt.h"
-#include "transport.h"
#include "tlv.h"

#define PTP_VERSION 2
@@ -55,6 +54,19 @@
#define TIME_TRACEABLE (1<<4)
#define FREQ_TRACEABLE (1<<5)

+enum timestamp_type {
+ TS_SOFTWARE,
+ TS_HARDWARE,
+ TS_LEGACY_HW,
+ TS_ONESTEP,
+};
+
+struct hw_timestamp {
+ enum timestamp_type type;
+ struct timespec ts;
+ struct timespec sw;
+};
+
enum controlField {
CTL_SYNC,
CTL_DELAY_REQ,
diff --git a/pmc_common.c b/pmc_common.c
index e52d68f5b5f7..a7201f92454d 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -149,8 +149,7 @@ static int pmc_send(struct pmc *pmc, struct ptp_message *msg, int pdulen)
pr_err("msg_pre_send failed");
return -1;
}
- cnt = transport_send(pmc->transport, &pmc->fdarray, 0,
- msg, pdulen, &msg->hwts);
+ cnt = transport_send(pmc->transport, &pmc->fdarray, 0, msg);
if (cnt < 0) {
pr_err("failed to send message");
return -1;
@@ -298,8 +297,7 @@ struct ptp_message *pmc_recv(struct pmc *pmc)
return NULL;
}
msg->hwts.type = TS_SOFTWARE;
- cnt = transport_recv(pmc->transport, pmc_get_transport_fd(pmc),
- msg, sizeof(msg->data), &msg->hwts);
+ cnt = transport_recv(pmc->transport, pmc_get_transport_fd(pmc), msg);
if (cnt <= 0) {
pr_err("recv message failed");
goto failed;
diff --git a/pmc_common.h b/pmc_common.h
index 850d85fe13e8..9fcb51da3fd4 100644
--- a/pmc_common.h
+++ b/pmc_common.h
@@ -22,6 +22,7 @@
#define HAVE_PMC_COMMON_H

#include "msg.h"
+#include "transport.h"

struct pmc;

diff --git a/port.c b/port.c
index 568bd30c8815..390aa389adc4 100644
--- a/port.c
+++ b/port.c
@@ -2092,7 +2092,7 @@ enum fsm_event port_event(struct port *p, int fd_index)

msg->hwts.type = p->timestamping;

- cnt = transport_recv(p->trp, fd, msg, sizeof(msg->data), &msg->hwts);
+ cnt = transport_recv(p->trp, fd, msg);
if (cnt <= 0) {
pr_err("port %hu: recv message failed", portnum(p));
msg_put(msg);
@@ -2166,19 +2166,17 @@ enum fsm_event port_event(struct port *p, int fd_index)
int port_forward(struct port *p, struct ptp_message *msg, int msglen)
{
int cnt;
- cnt = transport_send(p->trp, &p->fda, 0, msg, msglen, &msg->hwts);
+ cnt = transport_send(p->trp, &p->fda, 0, msg);
return cnt <= 0 ? -1 : 0;
}

int port_prepare_and_send(struct port *p, struct ptp_message *msg, int event)
{
- UInteger16 msg_len;
int cnt;

- msg_len = msg->header.messageLength;
if (msg_pre_send(msg))
return -1;
- cnt = transport_send(p->trp, &p->fda, event, msg, msg_len, &msg->hwts);
+ cnt = transport_send(p->trp, &p->fda, event, msg);
return cnt <= 0 ? -1 : 0;
}

diff --git a/transport.c b/transport.c
index b5346e5e51f4..cb799a68aa9f 100644
--- a/transport.c
+++ b/transport.c
@@ -17,6 +17,8 @@
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/

+#include <arpa/inet.h>
+
#include "transport.h"
#include "transport_private.h"
#include "raw.h"
@@ -35,22 +37,25 @@ int transport_open(struct transport *t, const char *name,
return t->open(t, name, fda, tt);
}

-int transport_recv(struct transport *t, int fd,
- void *buf, int buflen, struct hw_timestamp *hwts)
+int transport_recv(struct transport *t, int fd, struct ptp_message *msg)
{
- return t->recv(t, fd, buf, buflen, hwts);
+ return t->recv(t, fd, msg, sizeof(msg->data), &msg->hwts);
}

int transport_send(struct transport *t, struct fdarray *fda, int event,
- void *buf, int buflen, struct hw_timestamp *hwts)
+ struct ptp_message *msg)
{
- return t->send(t, fda, event, 0, buf, buflen, hwts);
+ int len = ntohs(msg->header.messageLength);
+
+ return t->send(t, fda, event, 0, msg, len, &msg->hwts);
}

int transport_peer(struct transport *t, struct fdarray *fda, int event,
- void *buf, int buflen, struct hw_timestamp *hwts)
+ struct ptp_message *msg)
{
- return t->send(t, fda, event, 1, buf, buflen, hwts);
+ int len = ntohs(msg->header.messageLength);
+
+ return t->send(t, fda, event, 1, msg, len, &msg->hwts);
}

int transport_physical_addr(struct transport *t, uint8_t *addr)
diff --git a/transport.h b/transport.h
index aa2018b4ecdf..5153c46d7887 100644
--- a/transport.h
+++ b/transport.h
@@ -24,6 +24,7 @@
#include <inttypes.h>

#include "fd.h"
+#include "msg.h"

/* Values from networkProtocol enumeration 7.4.1 Table 3 */
enum transport_type {
@@ -47,19 +48,6 @@ enum transport_event {
TRANS_ONESTEP,
};

-enum timestamp_type {
- TS_SOFTWARE,
- TS_HARDWARE,
- TS_LEGACY_HW,
- TS_ONESTEP,
-};
-
-struct hw_timestamp {
- enum timestamp_type type;
- struct timespec ts;
- struct timespec sw;
-};
-
struct transport;

int transport_close(struct transport *t, struct fdarray *fda);
@@ -67,14 +55,13 @@ int transport_close(struct transport *t, struct fdarray *fda);
int transport_open(struct transport *t, const char *name,
struct fdarray *fda, enum timestamp_type tt);

-int transport_recv(struct transport *t, int fd,
- void *buf, int buflen, struct hw_timestamp *hwts);
+int transport_recv(struct transport *t, int fd, struct ptp_message *msg);

int transport_send(struct transport *t, struct fdarray *fda, int event,
- void *buf, int buflen, struct hw_timestamp *hwts);
+ struct ptp_message *msg);

int transport_peer(struct transport *t, struct fdarray *fda, int event,
- void *buf, int buflen, struct hw_timestamp *hwts);
+ struct ptp_message *msg);

/**
* Returns the transport's type.

[Richard Cochran]

Post by Jiri Benc
The callers of those functions are all using ptp_message. As we're going to
return more information (the address), let those functions just fill in the
ptp_message fields directly.

No, no, no, this violates the transport abstraction. The transport
code knows nothing about the PTP payload, on purpose.

Thanks,
Richard

[Jiri Benc]

Post by Richard Cochran
No, no, no, this violates the transport abstraction. The transport
code knows nothing about the PTP payload, on purpose.

And this is not changed by this patch.

The actual transport code (udp.c, etc.) knows nothing about
ptp_message. The thin wrappers in transport.c take care of converting
ptp_message to lower level representation, which I think should be the
role of transport.c: to serve as a translator between the low-level
transport and high-level PTP code.

We could surely make yet another wrapper around transport_send etc. (as
you also suggest in your reply to patch 5) but I don't really see a
point of a wrapper of wrapper, especially when the inner wrapper won't
be used by anything.

Jiri

[Richard Cochran]

Post by Jiri Benc
We could surely make yet another wrapper around transport_send etc. (as
you also suggest in your reply to patch 5) but I don't really see a
point of a wrapper of wrapper, especially when the inner wrapper won't
be used by anything.

Okay, I see. Sorry for over-reacting.

Thanks,
Richard

[Richard Cochran]

Post by Jiri Benc
The callers of those functions are all using ptp_message. As we're going to
return more information (the address), let those functions just fill in the
ptp_message fields directly.
Some minor reshuffling needed to prevent circular header dependencies.
---

Applied.

Thanks,
Richard

[Jiri Benc]

Also, document transport_send, transport_peer and transport_sendto usage.

Signed-off-by: Jiri Benc <***@redhat.com>
---
transport.c | 8 ++++++++
transport.h | 33 +++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/transport.c b/transport.c
index 25d569607f71..ad41f678bb19 100644
--- a/transport.c
+++ b/transport.c
@@ -60,6 +60,14 @@ int transport_peer(struct transport *t, struct fdarray *fda, int event,
return t->send(t, fda, event, msg, len, addr, &msg->hwts);
}

+int transport_sendto(struct transport *t, struct fdarray *fda, int event,
+ struct ptp_message *msg)
+{
+ int len = ntohs(msg->header.messageLength);
+
+ return t->send(t, fda, event, msg, len, &msg->address, &msg->hwts);
+}
+
int transport_physical_addr(struct transport *t, uint8_t *addr)
{
if (t->physical_addr) {
diff --git a/transport.h b/transport.h
index 5153c46d7887..8e0d4214af5a 100644
--- a/transport.h
+++ b/transport.h
@@ -57,13 +57,46 @@ int transport_open(struct transport *t, const char *name,

int transport_recv(struct transport *t, int fd, struct ptp_message *msg);

+/**
+ * Sends the PTP message using the given transport. The message is sent to
+ * the default (usually multicast) address, any address field in the
+ * ptp_message itself is ignored.
+ * @param t The transport.
+ * @param fda The array of descriptors filled in by transport_open.
+ * @param event 1 for event message, 0 for general message.
+ * @param msg The message to send.
+ * @return Number of bytes send, or negative value in case of an error.
+ */
int transport_send(struct transport *t, struct fdarray *fda, int event,
struct ptp_message *msg);

+/**
+ * Sends the PTP message using the given transport. The message is sent to
+ * the address used for p2p delay measurements (usually a multicast
+ * address), any address field in the ptp_message itself is ignored.
+ * @param t The transport.
+ * @param fda The array of descriptors filled in by transport_open.
+ * @param event 1 for event message, 0 for general message.
+ * @param msg The message to send.
+ * @return Number of bytes send, or negative value in case of an error.
+ */
int transport_peer(struct transport *t, struct fdarray *fda, int event,
struct ptp_message *msg);

/**
+ * Sends the PTP message using the given transport. The address has to be
+ * provided in the address field of the message.
+ * @param t The transport.
+ * @param fda The array of descriptors filled in by transport_open.
+ * @param event 1 for event message, 0 for general message.
+ * @param msg The message to send. The address of the destination has to
+ * be set in the address field.
+ * @return Number of bytes send, or negative value in case of an error.
+ */
+int transport_sendto(struct transport *t, struct fdarray *fda, int event,
+ struct ptp_message *msg);
+
+/**
* Returns the transport's type.
*/
enum transport_type transport_type(struct transport *t);

[Richard Cochran]

Post by Jiri Benc
Also, document transport_send, transport_peer and transport_sendto usage.
---
transport.c | 8 ++++++++
transport.h | 33 +++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/transport.c b/transport.c
index 25d569607f71..ad41f678bb19 100644
--- a/transport.c
+++ b/transport.c
@@ -60,6 +60,14 @@ int transport_peer(struct transport *t, struct fdarray *fda, int event,
return t->send(t, fda, event, msg, len, addr, &msg->hwts);
}
+int transport_sendto(struct transport *t, struct fdarray *fda, int event,
+ struct ptp_message *msg)
+{
+ int len = ntohs(msg->header.messageLength);
+
+ return t->send(t, fda, event, msg, len, &msg->address, &msg->hwts);
+}

So this kind of wrapper function that marries the transport which the
ptp_message is fine, but please put it outside of transport.h.

Maybe call these msg_sendto() or something similar?

(msg.h already includes transport.h)

Thanks,
Richard

[Jiri Benc]

This modifies all transports to use a new common address type, struct
address. This address is stored in a ptp_message for all received messages.

For sending, the "default" address is used with the default sending
functions, transport_send and transport_peer. The default address depends on
the transport; it's supposed to be the multicast address assigned by the
transport specification.

Later, a new transport_sendto function will be implemented that sends to the
address contained in the passed ptp_message.

Signed-off-by: Jiri Benc <***@redhat.com>
---
address.h | 38 ++++++++++++++++++++++++
msg.h | 6 ++++
raw.c | 56 ++++++++++++++++++++++++-----------
sk.c | 34 ++++++++++++---------
sk.h | 11 ++++---
transport.c | 8 +++--
transport_private.h | 11 +++++--
udp.c | 70 ++++++++++++++++++++++++++-----------------
udp6.c | 81 ++++++++++++++++++++++++++++++---------------------
uds.c | 34 ++++++++++++++-------
util.c | 18 ++++++-----
11 files changed, 244 insertions(+), 123 deletions(-)
create mode 100644 address.h

diff --git a/address.h b/address.h
new file mode 100644
index 000000000000..b7803879f8b2
--- /dev/null
+++ b/address.h
@@ -0,0 +1,38 @@
+/**
+ * @file address.h
+ * @brief Definition of a structure to hold an address.
+ * @note Copyright (C) 2014 Red Hat, Inc., Jiri Benc <***@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#ifndef HAVE_ADDRESS_H
+#define HAVE_ADDRESS_H
+
+#include <netinet/in.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+
+struct address {
+ socklen_t len;
+ union {
+ struct sockaddr_storage ss;
+ struct sockaddr_in sin;
+ struct sockaddr_in6 sin6;
+ struct sockaddr_un sun;
+ struct sockaddr sa;
+ };
+};
+
+#endif
diff --git a/msg.h b/msg.h
index 564cc9bfa0df..3fa58338f07a 100644
--- a/msg.h
+++ b/msg.h
@@ -24,6 +24,7 @@
#include <sys/queue.h>
#include <time.h>

+#include "address.h"
#include "ddt.h"
#include "tlv.h"

@@ -214,6 +215,11 @@ struct ptp_message {
*/
struct hw_timestamp hwts;
/**
+ * Contains the address this message was received from or should be
+ * sent to.
+ */
+ struct address address;
+ /**
* Contains the number of TLVs in the suffix.
*/
int tlv_count;
diff --git a/raw.c b/raw.c
index 2de0793745ee..b71dbc7f5f88 100644
--- a/raw.c
+++ b/raw.c
@@ -36,6 +36,7 @@
#include <linux/net_tstamp.h>
#include <linux/sockios.h>

+#include "address.h"
#include "contain.h"
#include "ether.h"
#include "print.h"
@@ -45,9 +46,9 @@

struct raw {
struct transport t;
- eth_addr src_addr;
- eth_addr ptp_addr;
- eth_addr p2p_addr;
+ struct address src_addr;
+ struct address ptp_addr;
+ struct address p2p_addr;
int vlan;
};

@@ -185,16 +186,28 @@ no_socket:
return -1;
}

+static void mac_to_addr(struct address *addr, void *mac)
+{
+ addr->sa.sa_family = AF_UNSPEC;
+ memcpy(&addr->sa.sa_data, mac, MAC_LEN);
+ addr->len = sizeof(addr->sa.sa_family) + MAC_LEN;
+}
+
+static void addr_to_mac(void *mac, struct address *addr)
+{
+ memcpy(mac, &addr->sa.sa_data, MAC_LEN);
+}
+
static int raw_open(struct transport *t, const char *name,
struct fdarray *fda, enum timestamp_type ts_type)
{
struct raw *raw = container_of(t, struct raw, t);
int efd, gfd;

- memcpy(raw->ptp_addr, ptp_dst_mac, MAC_LEN);
- memcpy(raw->p2p_addr, p2p_dst_mac, MAC_LEN);
+ mac_to_addr(&raw->ptp_addr, ptp_dst_mac);
+ mac_to_addr(&raw->p2p_addr, p2p_dst_mac);

- if (sk_interface_macaddr(name, raw->src_addr, MAC_LEN))
+ if (sk_interface_macaddr(name, &raw->src_addr))
goto no_mac;

efd = open_socket(name, 1);
@@ -225,7 +238,7 @@ no_mac:
}

static int raw_recv(struct transport *t, int fd, void *buf, int buflen,
- struct hw_timestamp *hwts)
+ struct address *addr, struct hw_timestamp *hwts)
{
int cnt, hlen;
unsigned char *ptr = buf;
@@ -241,7 +254,7 @@ static int raw_recv(struct transport *t, int fd, void *buf, int buflen,
buflen += hlen;
hdr = (struct eth_hdr *) ptr;

- cnt = sk_receive(fd, ptr, buflen, hwts, 0);
+ cnt = sk_receive(fd, ptr, buflen, addr, hwts, 0);

if (cnt >= 0)
cnt -= hlen;
@@ -262,8 +275,9 @@ static int raw_recv(struct transport *t, int fd, void *buf, int buflen,
return cnt;
}

-static int raw_send(struct transport *t, struct fdarray *fda, int event, int peer,
- void *buf, int len, struct hw_timestamp *hwts)
+static int raw_send(struct transport *t, struct fdarray *fda, int event,
+ void *buf, int len, struct address *addr,
+ struct hw_timestamp *hwts)
{
struct raw *raw = container_of(t, struct raw, t);
ssize_t cnt;
@@ -275,11 +289,8 @@ static int raw_send(struct transport *t, struct fdarray *fda, int event, int pee
len += sizeof(*hdr);

hdr = (struct eth_hdr *) ptr;
- if (peer)
- memcpy(&hdr->dst, &raw->p2p_addr, MAC_LEN);
- else
- memcpy(&hdr->dst, &raw->ptp_addr, MAC_LEN);
- memcpy(&hdr->src, &raw->src_addr, MAC_LEN);
+ addr_to_mac(&hdr->dst, addr);
+ addr_to_mac(&hdr->src, &raw->src_addr);

hdr->type = htons(ETH_P_1588);

@@ -291,7 +302,7 @@ static int raw_send(struct transport *t, struct fdarray *fda, int event, int pee
/*
* Get the time stamp right away.
*/
- return event == TRANS_EVENT ? sk_receive(fd, pkt, len, hwts, MSG_ERRQUEUE) : cnt;
+ return event == TRANS_EVENT ? sk_receive(fd, pkt, len, NULL, hwts, MSG_ERRQUEUE) : cnt;
}

static void raw_release(struct transport *t)
@@ -300,17 +311,25 @@ static void raw_release(struct transport *t)
free(raw);
}

+static struct address *raw_default_addr(struct transport *t,
+ int event, int peer)
+{
+ struct raw *raw = container_of(t, struct raw, t);
+
+ return peer ? &raw->p2p_addr : &raw->ptp_addr;
+}
+
static int raw_physical_addr(struct transport *t, uint8_t *addr)
{
struct raw *raw = container_of(t, struct raw, t);
- memcpy(addr, raw->src_addr, MAC_LEN);
+ addr_to_mac(addr, &raw->src_addr);
return MAC_LEN;
}

static int raw_protocol_addr(struct transport *t, uint8_t *addr)
{
struct raw *raw = container_of(t, struct raw, t);
- memcpy(addr, raw->src_addr, MAC_LEN);
+ addr_to_mac(addr, &raw->src_addr);
return MAC_LEN;
}

@@ -325,6 +344,7 @@ struct transport *raw_transport_create(void)
raw->t.recv = raw_recv;
raw->t.send = raw_send;
raw->t.release = raw_release;
+ raw->t.default_addr = raw_default_addr;
raw->t.physical_addr = raw_physical_addr;
raw->t.protocol_addr = raw_protocol_addr;
return &raw->t;
diff --git a/sk.c b/sk.c
index 8d70d1acf5dd..838004eab14e 100644
--- a/sk.c
+++ b/sk.c
@@ -30,6 +30,8 @@
#include <stdlib.h>
#include <poll.h>

+#include "address.h"
+#include "ether.h"
#include "print.h"
#include "sk.h"

@@ -146,7 +148,7 @@ failed:
return -1;
}

-int sk_interface_macaddr(const char *name, unsigned char *mac, int len)
+int sk_interface_macaddr(const char *name, struct address *mac)
{
struct ifreq ifreq;
int err, fd;
@@ -167,16 +169,17 @@ int sk_interface_macaddr(const char *name, unsigned char *mac, int len)
return -1;
}

- memcpy(mac, ifreq.ifr_hwaddr.sa_data, len);
+ memcpy(&mac->sa, &ifreq.ifr_hwaddr, sizeof(ifreq.ifr_hwaddr));
+ mac->len = sizeof(ifreq.ifr_hwaddr.sa_family) + MAC_LEN;
close(fd);
return 0;
}

-int sk_interface_addr(const char *name, int family, uint8_t *addr, int len)
+int sk_interface_addr(const char *name, int family, struct address *addr)
{
struct ifaddrs *ifaddr, *i;
- int copy_len, result = -1;
- void *copy_from;
+ int result = -1;
+
if (getifaddrs(&ifaddr) == -1) {
pr_err("getifaddrs failed: %m");
return -1;
@@ -187,20 +190,16 @@ int sk_interface_addr(const char *name, int family, uint8_t *addr, int len)
{
switch (family) {
case AF_INET:
- copy_len = 4;
- copy_from = &((struct sockaddr_in *)i->ifa_addr)->sin_addr.s_addr;
+ addr->len = sizeof(addr->sin);
break;
case AF_INET6:
- copy_len = 16;
- copy_from = &((struct sockaddr_in6 *)i->ifa_addr)->sin6_addr.s6_addr;
+ addr->len = sizeof(addr->sin6);
break;
default:
continue;
}
- if (copy_len > len)
- copy_len = len;
- memcpy(addr, copy_from, copy_len);
- result = copy_len;
+ memcpy(&addr->sa, &i->ifa_addr, addr->len);
+ result = 0;
break;
}
}
@@ -209,7 +208,7 @@ int sk_interface_addr(const char *name, int family, uint8_t *addr, int len)
}

int sk_receive(int fd, void *buf, int buflen,
- struct hw_timestamp *hwts, int flags)
+ struct address *addr, struct hw_timestamp *hwts, int flags)
{
char control[256];
int cnt = 0, res = 0, level, type;
@@ -220,6 +219,10 @@ int sk_receive(int fd, void *buf, int buflen,

memset(control, 0, sizeof(control));
memset(&msg, 0, sizeof(msg));
+ if (addr) {
+ msg.msg_name = &addr->ss;
+ msg.msg_namelen = sizeof(addr->ss);
+ }
msg.msg_iov = &iov;
msg.msg_iovlen = 1;
msg.msg_control = control;
@@ -265,6 +268,9 @@ int sk_receive(int fd, void *buf, int buflen,
}
}

+ if (addr)
+ addr->len = msg.msg_namelen;
+
if (!ts) {
memset(&hwts->ts, 0, sizeof(hwts->ts));
return cnt;
diff --git a/sk.h b/sk.h
index ea338c13f021..f05d1df5ff86 100644
--- a/sk.h
+++ b/sk.h
@@ -20,6 +20,7 @@
#ifndef HAVE_SK_H
#define HAVE_SK_H

+#include "address.h"
#include "transport.h"

/**
@@ -65,32 +66,32 @@ int sk_get_ts_info(const char *name, struct sk_ts_info *sk_info);
* Obtain the MAC address of a network interface.
* @param name The name of the interface
* @param mac Buffer to hold the result
- * @param len Length of 'mac'
* @return Zero on success, non-zero otherwise.
*/
-int sk_interface_macaddr(const char *name, unsigned char *mac, int len);
+int sk_interface_macaddr(const char *name, struct address *mac);

/**
* Obtains the first IP address assigned to a network interface.
* @param name The name of the interface
* @param family The family of the address to get: AF_INET or AF_INET6
* @param addr Buffer to hold the result
- * @param len Length of 'addr'
* @return The number of bytes written to addr on success, -1 otherwise.
*/
-int sk_interface_addr(const char *name, int family, uint8_t *addr, int len);
+int sk_interface_addr(const char *name, int family, struct address *addr);

/**
* Read a message from a socket.
* @param fd An open socket.
* @param buf Buffer to receive the message.
* @param buflen Size of 'buf' in bytes.
+ * @param addr Pointer to a buffer to receive the message's source
+ * address. May be NULL.
* @param hwts Pointer to a buffer to receive the message's time stamp.
* @param flags Flags to pass to RECV(2).
* @return
*/
int sk_receive(int fd, void *buf, int buflen,
- struct hw_timestamp *hwts, int flags);
+ struct address *addr, struct hw_timestamp *hwts, int flags);

/**
* Enable time stamping on a given network interface.
diff --git a/transport.c b/transport.c
index cb799a68aa9f..25d569607f71 100644
--- a/transport.c
+++ b/transport.c
@@ -39,23 +39,25 @@ int transport_open(struct transport *t, const char *name,

int transport_recv(struct transport *t, int fd, struct ptp_message *msg)
{
- return t->recv(t, fd, msg, sizeof(msg->data), &msg->hwts);
+ return t->recv(t, fd, msg, sizeof(msg->data), &msg->address, &msg->hwts);
}

int transport_send(struct transport *t, struct fdarray *fda, int event,
struct ptp_message *msg)
{
int len = ntohs(msg->header.messageLength);
+ struct address *addr = t->default_addr(t, event, 0);

- return t->send(t, fda, event, 0, msg, len, &msg->hwts);
+ return t->send(t, fda, event, msg, len, addr, &msg->hwts);
}

int transport_peer(struct transport *t, struct fdarray *fda, int event,
struct ptp_message *msg)
{
int len = ntohs(msg->header.messageLength);
+ struct address *addr = t->default_addr(t, event, 1);

- return t->send(t, fda, event, 1, msg, len, &msg->hwts);
+ return t->send(t, fda, event, msg, len, addr, &msg->hwts);
}

int transport_physical_addr(struct transport *t, uint8_t *addr)
diff --git a/transport_private.h b/transport_private.h
index 7124f94af424..d9302e218f81 100644
--- a/transport_private.h
+++ b/transport_private.h
@@ -22,6 +22,7 @@

#include <time.h>

+#include "address.h"
#include "fd.h"
#include "transport.h"

@@ -34,13 +35,17 @@ struct transport {
enum timestamp_type tt);

int (*recv)(struct transport *t, int fd, void *buf, int buflen,
- struct hw_timestamp *hwts);
+ struct address *addr, struct hw_timestamp *hwts);

- int (*send)(struct transport *t, struct fdarray *fda, int event, int peer,
- void *buf, int buflen, struct hw_timestamp *hwts);
+ int (*send)(struct transport *t, struct fdarray *fda, int event,
+ void *buf, int buflen, struct address *addr,
+ struct hw_timestamp *hwts);

void (*release)(struct transport *t);

+ struct address *(*default_addr)(struct transport *t,
+ int event, int peer);
+
int (*physical_addr)(struct transport *t, uint8_t *addr);

int (*protocol_addr)(struct transport *t, uint8_t *addr);
diff --git a/udp.c b/udp.c
index 818cbfd53b6a..9f8ac2825ef2 100644
--- a/udp.c
+++ b/udp.c
@@ -29,6 +29,7 @@
#include <sys/socket.h>
#include <unistd.h>

+#include "address.h"
#include "contain.h"
#include "print.h"
#include "sk.h"
@@ -43,10 +44,8 @@

struct udp {
struct transport t;
- uint8_t ip[4];
- int ip_len;
- uint8_t mac[MAC_LEN];
- int mac_len;
+ struct address ip;
+ struct address mac;
};

static int mcast_bind(int fd, int index)
@@ -154,13 +153,11 @@ static int udp_open(struct transport *t, const char *name, struct fdarray *fda,
struct udp *udp = container_of(t, struct udp, t);
int efd, gfd;

- udp->mac_len = 0;
- if (sk_interface_macaddr(name, udp->mac, MAC_LEN) == 0)
- udp->mac_len = MAC_LEN;
+ udp->mac.len = 0;
+ sk_interface_macaddr(name, &udp->mac);

- udp->ip_len = sk_interface_addr(name, AF_INET, udp->ip, sizeof(udp->ip));
- if (udp->ip_len == -1)
- udp->ip_len = 0;
+ udp->ip.len = 0;
+ sk_interface_addr(name, AF_INET, &udp->ip);

if (!inet_aton(PTP_PRIMARY_MCAST_IPADDR, &mcast_addr[MC_PRIMARY]))
return -1;
@@ -195,24 +192,19 @@ no_event:
}

static int udp_recv(struct transport *t, int fd, void *buf, int buflen,
- struct hw_timestamp *hwts)
+ struct address *addr, struct hw_timestamp *hwts)
{
- return sk_receive(fd, buf, buflen, hwts, 0);
+ return sk_receive(fd, buf, buflen, addr, hwts, 0);
}

-static int udp_send(struct transport *t, struct fdarray *fda, int event, int peer,
- void *buf, int len, struct hw_timestamp *hwts)
+static int udp_send(struct transport *t, struct fdarray *fda, int event,
+ void *buf, int len, struct address *addr,
+ struct hw_timestamp *hwts)
{
ssize_t cnt;
int fd = event ? fda->fd[FD_EVENT] : fda->fd[FD_GENERAL];
- struct sockaddr_in addr;
unsigned char junk[1600];

- memset(&addr, 0, sizeof(addr));
- addr.sin_family = AF_INET;
- addr.sin_addr = peer ? mcast_addr[MC_PDELAY] : mcast_addr[MC_PRIMARY];
- addr.sin_port = htons(event ? EVENT_PORT : GENERAL_PORT);
-
/*
* Extend the payload by two, for UDP checksum correction.
* This is not really part of the standard, but it is the way
@@ -221,7 +213,7 @@ static int udp_send(struct transport *t, struct fdarray *fda, int event, int pee
if (event == TRANS_ONESTEP)
len += 2;

- cnt = sendto(fd, buf, len, 0, (struct sockaddr *)&addr, sizeof(addr));
+ cnt = sendto(fd, buf, len, 0, &addr->sa, sizeof(addr->sin));
if (cnt < 1) {
pr_err("sendto failed: %m");
return cnt;
@@ -229,7 +221,7 @@ static int udp_send(struct transport *t, struct fdarray *fda, int event, int pee
/*
* Get the time stamp right away.
*/
- return event == TRANS_EVENT ? sk_receive(fd, junk, len, hwts, MSG_ERRQUEUE) : cnt;
+ return event == TRANS_EVENT ? sk_receive(fd, junk, len, NULL, hwts, MSG_ERRQUEUE) : cnt;
}

static void udp_release(struct transport *t)
@@ -238,20 +230,41 @@ static void udp_release(struct transport *t)
free(udp);
}

+static struct address *udp_default_addr(struct transport *t,
+ int event, int peer)
+{
+ static struct address addr;
+
+ memset(&addr, 0, sizeof(addr));
+ addr.sin.sin_family = AF_INET;
+ addr.sin.sin_addr = peer ? mcast_addr[MC_PDELAY] : mcast_addr[MC_PRIMARY];
+ addr.sin.sin_port = htons(event ? EVENT_PORT : GENERAL_PORT);
+ addr.len = sizeof(addr.sin);
+ return &addr;
+}
+
static int udp_physical_addr(struct transport *t, uint8_t *addr)
{
struct udp *udp = container_of(t, struct udp, t);
- if (udp->mac_len)
- memcpy(addr, udp->mac, udp->mac_len);
- return udp->mac_len;
+ int len = 0;
+
+ if (udp->mac.len) {
+ len = MAC_LEN;
+ memcpy(addr, udp->mac.sa.sa_data, len);
+ }
+ return len;
}

static int udp_protocol_addr(struct transport *t, uint8_t *addr)
{
struct udp *udp = container_of(t, struct udp, t);
- if (udp->ip_len)
- memcpy(addr, &udp->ip, udp->ip_len);
- return udp->ip_len;
+ int len = 0;
+
+ if (udp->ip.len) {
+ len = sizeof(udp->ip.sin.sin_addr.s_addr);
+ memcpy(addr, &udp->ip.sin.sin_addr.s_addr, len);
+ }
+ return len;
}

struct transport *udp_transport_create(void)
@@ -264,6 +277,7 @@ struct transport *udp_transport_create(void)
udp->t.recv = udp_recv;
udp->t.send = udp_send;
udp->t.release = udp_release;
+ udp->t.default_addr = udp_default_addr;
udp->t.physical_addr = udp_physical_addr;
udp->t.protocol_addr = udp_protocol_addr;
return &udp->t;
diff --git a/udp6.c b/udp6.c
index e26195b4fffd..aaad02804c95 100644
--- a/udp6.c
+++ b/udp6.c
@@ -29,6 +29,7 @@
#include <sys/socket.h>
#include <unistd.h>

+#include "address.h"
#include "contain.h"
#include "print.h"
#include "sk.h"
@@ -46,10 +47,8 @@ unsigned char udp6_scope = 0x0E;
struct udp6 {
struct transport t;
int index;
- uint8_t ip[16];
- int ip_len;
- uint8_t mac[MAC_LEN];
- int mac_len;
+ struct address ip;
+ struct address mac;
};

static int is_link_local(struct in6_addr *addr)
@@ -164,13 +163,11 @@ static int udp6_open(struct transport *t, const char *name, struct fdarray *fda,
struct udp6 *udp6 = container_of(t, struct udp6, t);
int efd, gfd;

- udp6->mac_len = 0;
- if (sk_interface_macaddr(name, udp6->mac, MAC_LEN) == 0)
- udp6->mac_len = MAC_LEN;
+ udp6->mac.len = 0;
+ sk_interface_macaddr(name, &udp6->mac);

- udp6->ip_len = sk_interface_addr(name, AF_INET6, udp6->ip, sizeof(udp6->ip));
- if (udp6->ip_len == -1)
- udp6->ip_len = 0;
+ udp6->ip.len = 0;
+ sk_interface_addr(name, AF_INET6, &udp6->ip);

if (1 != inet_pton(AF_INET6, PTP_PRIMARY_MCAST_IP6ADDR, &mc6_addr[MC_PRIMARY]))
return -1;
@@ -207,32 +204,22 @@ no_event:
}

static int udp6_recv(struct transport *t, int fd, void *buf, int buflen,
- struct hw_timestamp *hwts)
+ struct address *addr, struct hw_timestamp *hwts)
{
- return sk_receive(fd, buf, buflen, hwts, 0);
+ return sk_receive(fd, buf, buflen, addr, hwts, 0);
}

-static int udp6_send(struct transport *t, struct fdarray *fda, int event, int peer,
- void *buf, int len, struct hw_timestamp *hwts)
+static int udp6_send(struct transport *t, struct fdarray *fda, int event,
+ void *buf, int len, struct address *addr,
+ struct hw_timestamp *hwts)
{
- struct udp6 *udp6 = container_of(t, struct udp6, t);
ssize_t cnt;
int fd = event ? fda->fd[FD_EVENT] : fda->fd[FD_GENERAL];
- struct sockaddr_in6 addr;
unsigned char junk[1600];

- memset(&addr, 0, sizeof(addr));
- addr.sin6_family = AF_INET6;
- addr.sin6_addr = peer ? mc6_addr[MC_PDELAY] : mc6_addr[MC_PRIMARY];
- addr.sin6_port = htons(event ? EVENT_PORT : GENERAL_PORT);
-
- if (is_link_local(&addr.sin6_addr)) {
- addr.sin6_scope_id = udp6->index;
- }
-
len += 2; /* Extend the payload by two, for UDP checksum corrections. */

- cnt = sendto(fd, buf, len, 0, (struct sockaddr *)&addr, sizeof(addr));
+ cnt = sendto(fd, buf, len, 0, &addr->sa, sizeof(addr->sin6));
if (cnt < 1) {
pr_err("sendto failed: %m");
return cnt;
@@ -240,7 +227,7 @@ static int udp6_send(struct transport *t, struct fdarray *fda, int event, int pe
/*
* Get the time stamp right away.
*/
- return event == TRANS_EVENT ? sk_receive(fd, junk, len, hwts, MSG_ERRQUEUE) : cnt;
+ return event == TRANS_EVENT ? sk_receive(fd, junk, len, NULL, hwts, MSG_ERRQUEUE) : cnt;
}

static void udp6_release(struct transport *t)
@@ -249,20 +236,47 @@ static void udp6_release(struct transport *t)
free(udp6);
}

+static struct address *udp6_default_addr(struct transport *t,
+ int event, int peer)
+{
+ struct udp6 *udp6 = container_of(t, struct udp6, t);
+ static struct address addr;
+
+ memset(&addr, 0, sizeof(addr));
+ addr.sin6.sin6_family = AF_INET6;
+ addr.sin6.sin6_addr = peer ? mc6_addr[MC_PDELAY] : mc6_addr[MC_PRIMARY];
+ addr.sin6.sin6_port = htons(event ? EVENT_PORT : GENERAL_PORT);
+
+ if (is_link_local(&addr.sin6.sin6_addr)) {
+ addr.sin6.sin6_scope_id = udp6->index;
+ }
+
+ addr.len = sizeof(addr.sin6);
+ return &addr;
+}
+
static int udp6_physical_addr(struct transport *t, uint8_t *addr)
{
struct udp6 *udp6 = container_of(t, struct udp6, t);
- if (udp6->mac_len)
- memcpy(addr, udp6->mac, udp6->mac_len);
- return udp6->mac_len;
+ int len = 0;
+
+ if (udp6->mac.len) {
+ len = MAC_LEN;
+ memcpy(addr, udp6->mac.sa.sa_data, len);
+ }
+ return len;
}

static int udp6_protocol_addr(struct transport *t, uint8_t *addr)
{
struct udp6 *udp6 = container_of(t, struct udp6, t);
- if (udp6->ip_len)
- memcpy(addr, &udp6->ip, udp6->ip_len);
- return udp6->ip_len;
+ int len = 0;
+
+ if (udp6->ip.len) {
+ len = sizeof(udp6->ip.sin6.sin6_addr.s6_addr);
+ memcpy(addr, &udp6->ip.sin6.sin6_addr.s6_addr, len);
+ }
+ return len;
}

struct transport *udp6_transport_create(void)
@@ -276,6 +290,7 @@ struct transport *udp6_transport_create(void)
udp6->t.recv = udp6_recv;
udp6->t.send = udp6_send;
udp6->t.release = udp6_release;
+ udp6->t.default_addr = udp6_default_addr;
udp6->t.physical_addr = udp6_physical_addr;
udp6->t.protocol_addr = udp6_protocol_addr;
return &udp6->t;
diff --git a/uds.c b/uds.c
index 35f5eccf0545..0ed28658437b 100644
--- a/uds.c
+++ b/uds.c
@@ -25,6 +25,7 @@
#include <sys/un.h>
#include <unistd.h>

+#include "address.h"
#include "contain.h"
#include "print.h"
#include "transport_private.h"
@@ -36,8 +37,7 @@ char uds_path[MAX_IFNAME_SIZE + 1] = "/var/run/ptp4l";

struct uds {
struct transport t;
- struct sockaddr_un sa;
- socklen_t len;
+ struct address address;
};

static int uds_close(struct transport *t, struct fdarray *fda)
@@ -75,8 +75,8 @@ static int uds_open(struct transport *t, const char *name, struct fdarray *fda,
memset(&sa, 0, sizeof(sa));
sa.sun_family = AF_LOCAL;
strncpy(sa.sun_path, uds_path, sizeof(sa.sun_path) - 1);
- uds->sa = sa;
- uds->len = sizeof(sa);
+ uds->address.sun = sa;
+ uds->address.len = sizeof(sa);

chmod(name, UDS_FILEMODE);
fda->fd[FD_EVENT] = -1;
@@ -85,25 +85,28 @@ static int uds_open(struct transport *t, const char *name, struct fdarray *fda,
}

static int uds_recv(struct transport *t, int fd, void *buf, int buflen,
- struct hw_timestamp *hwts)
+ struct address *addr, struct hw_timestamp *hwts)
{
int cnt;
struct uds *uds = container_of(t, struct uds, t);
- socklen_t *len = &uds->len;
- uds->len = sizeof(uds->sa);
- cnt = recvfrom(fd, buf, buflen, 0, (struct sockaddr *) &uds->sa, len);
+
+ addr->len = sizeof(addr->sun);
+ cnt = recvfrom(fd, buf, buflen, 0, &addr->sa, &addr->len);
if (cnt <= 0) {
pr_err("uds: recvfrom failed: %m");
+ return cnt;
}
+ uds->address = *addr;
return cnt;
}

static int uds_send(struct transport *t, struct fdarray *fda, int event,
- int peer, void *buf, int buflen, struct hw_timestamp *hwts)
+ void *buf, int buflen, struct address *addr,
+ struct hw_timestamp *hwts)
{
int cnt, fd = fda->fd[FD_GENERAL];
- struct uds *uds = container_of(t, struct uds, t);
- cnt = sendto(fd, buf, buflen, 0, (struct sockaddr *) &uds->sa, uds->len);
+
+ cnt = sendto(fd, buf, buflen, 0, &addr->sa, addr->len);
if (cnt <= 0) {
pr_err("uds: sendto failed: %m");
}
@@ -116,6 +119,14 @@ static void uds_release(struct transport *t)
free(uds);
}

+struct address *uds_default_addr(struct transport *t,
+ int event, int peer)
+{
+ struct uds *uds = container_of(t, struct uds, t);
+
+ return &uds->address;
+}
+
struct transport *uds_transport_create(void)
{
struct uds *uds;
@@ -127,6 +138,7 @@ struct transport *uds_transport_create(void)
uds->t.recv = uds_recv;
uds->t.send = uds_send;
uds->t.release = uds_release;
+ uds->t.default_addr = uds_default_addr;
return &uds->t;
}

diff --git a/util.c b/util.c
index 56af8efcb0fa..1e86040c61f7 100644
--- a/util.c
+++ b/util.c
@@ -21,6 +21,7 @@
#include <stdlib.h>
#include <string.h>

+#include "address.h"
#include "sk.h"
#include "util.h"

@@ -100,17 +101,18 @@ int str2pid(const char *s, struct PortIdentity *result)

int generate_clock_identity(struct ClockIdentity *ci, const char *name)
{
- unsigned char mac[6];
- if (sk_interface_macaddr(name, mac, sizeof(mac)))
+ struct address addr;
+
+ if (sk_interface_macaddr(name, &addr))
return -1;
- ci->id[0] = mac[0];
- ci->id[1] = mac[1];
- ci->id[2] = mac[2];
+ ci->id[0] = addr.sa.sa_data[0];
+ ci->id[1] = addr.sa.sa_data[1];
+ ci->id[2] = addr.sa.sa_data[2];
ci->id[3] = 0xFF;
ci->id[4] = 0xFE;
- ci->id[5] = mac[3];
- ci->id[6] = mac[4];
- ci->id[7] = mac[5];
+ ci->id[5] = addr.sa.sa_data[3];
+ ci->id[6] = addr.sa.sa_data[4];
+ ci->id[7] = addr.sa.sa_data[5];
return 0;
}

[Richard Cochran]

Post by Jiri Benc
This modifies all transports to use a new common address type, struct
address. This address is stored in a ptp_message for all received messages.
For sending, the "default" address is used with the default sending
functions, transport_send and transport_peer. The default address depends on
the transport; it's supposed to be the multicast address assigned by the
transport specification.
Later, a new transport_sendto function will be implemented that sends to the
address contained in the passed ptp_message.
---
address.h | 38 ++++++++++++++++++++++++
msg.h | 6 ++++
raw.c | 56 ++++++++++++++++++++++++-----------
sk.c | 34 ++++++++++++---------
sk.h | 11 ++++---
transport.c | 8 +++--
transport_private.h | 11 +++++--
udp.c | 70 ++++++++++++++++++++++++++-----------------
udp6.c | 81 ++++++++++++++++++++++++++++++---------------------
uds.c | 34 ++++++++++++++-------
util.c | 18 ++++++-----
11 files changed, 244 insertions(+), 123 deletions(-)
create mode 100644 address.h
diff --git a/address.h b/address.h
new file mode 100644
index 000000000000..b7803879f8b2
--- /dev/null
+++ b/address.h
@@ -0,0 +1,38 @@
+/**
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#ifndef HAVE_ADDRESS_H
+#define HAVE_ADDRESS_H
+
+#include <netinet/in.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+
+struct address {
+ socklen_t len;
+ union {
+ struct sockaddr_storage ss;
+ struct sockaddr_in sin;
+ struct sockaddr_in6 sin6;
+ struct sockaddr_un sun;
+ struct sockaddr sa;

Can't we use struct sockaddr_storage here?

Thanks,
Richard

[Jiri Benc]

Post by Richard Cochran

Post by Jiri Benc
+struct address {
+ socklen_t len;
+ union {
+ struct sockaddr_storage ss;
+ struct sockaddr_in sin;
+ struct sockaddr_in6 sin6;
+ struct sockaddr_un sun;
+ struct sockaddr sa;

Can't we use struct sockaddr_storage here?

It's the first member of the union. I originally had only struct
sockaddr_storage here but it led to horrible typecasting in the code
(which is quite error prone when you accidentally put an extra & before
the variable). In the end, I added the union here, which led to a
shorter and cleaner code, and the compiler being able to do type
checking as a bonus.

Jiri

[Richard Cochran]

Post by Jiri Benc
It's the first member of the union. I originally had only struct
sockaddr_storage here but it led to horrible typecasting in the code
(which is quite error prone when you accidentally put an extra & before
the variable). In the end, I added the union here, which led to a
shorter and cleaner code, and the compiler being able to do type
checking as a bonus.

Fair enough.

Thanks,
Richard

[Richard Cochran]

Jiri,

I think this series brings a nice improvement, on second look. But the
change in this patch could be better yet?

See below...

Post by Jiri Benc
diff --git a/transport.c b/transport.c
index cb799a68aa9f..25d569607f71 100644
--- a/transport.c
+++ b/transport.c
@@ -39,23 +39,25 @@ int transport_open(struct transport *t, const char *name,
int transport_recv(struct transport *t, int fd, struct ptp_message *msg)
{
- return t->recv(t, fd, msg, sizeof(msg->data), &msg->hwts);
+ return t->recv(t, fd, msg, sizeof(msg->data), &msg->address, &msg->hwts);
}
int transport_send(struct transport *t, struct fdarray *fda, int event,
struct ptp_message *msg)
{
int len = ntohs(msg->header.messageLength);
+ struct address *addr = t->default_addr(t, event, 0);

I don't see why you need to call into the transport twice.

Can't you pass addr=NULL in order to get the default address?

Post by Jiri Benc
- return t->send(t, fda, event, 0, msg, len, &msg->hwts);
+ return t->send(t, fda, event, msg, len, addr, &msg->hwts);
}

Thanks,
Richard

[Jiri Benc]

Post by Richard Cochran
I don't see why you need to call into the transport twice.
Can't you pass addr=NULL in order to get the default address?

Makes sense.

Jiri

[Richard Cochran]

Post by Jiri Benc
In order to be able to convert to a generic address struct, separate source
and destination address into separate fields.
---
ether.h | 13 ++++++-------
raw.c | 22 +++++++++++-----------
2 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/ether.h b/ether.h
index 07d0f31dae26..89e22c879058 100644
--- a/ether.h
+++ b/ether.h
@@ -26,25 +26,24 @@
#define PTP_DST_MAC 0x01, 0x1B, 0x19, 0x00, 0x00, 0x00
#define P2P_DST_MAC 0x01, 0x80, 0xC2, 0x00, 0x00, 0x0E
-struct eth_addr {
- uint8_t dst[MAC_LEN];
- uint8_t src[MAC_LEN];
-} __attribute__((packed));
+typedef uint8_t eth_addr[MAC_LEN];

Nit: I don't like this typedef for the reasons explained in the kernel
CodingStyle.

Also, I did not really understand why this change is needed.

Thanks,
Richard

[Jiri Benc]

Post by Richard Cochran

Post by Jiri Benc
+typedef uint8_t eth_addr[MAC_LEN];

Nit: I don't like this typedef for the reasons explained in the kernel
CodingStyle.

I don't like it either but the other option was

struct eth_addr {
uint8_t addr[MAC_LEN];
};

which I liked even less. I actually had it this way in an earlier
version of the patchset.

The third option would be just open coding uint8_t foo[MAC_LEN]. Works
for me but we'll probably lose some type checking.

Post by Richard Cochran
Also, I did not really understand why this change is needed.

I need to have a separate source address in raw_send for patch 4, as
the destination address can come from outside of the raw transport
code. And I need to return the destination address(es) by
raw_default_addr, thus converting all of them to struct address seems
to be the cleanest way. But even without the conversion, the separation
of the addresses is needed.

Jiri

[Richard Cochran]

FYI, I am inclined to merge this, but I am going to review it once
more, in the next couple of days.

Thanks,
Richard

[Richard Cochran]

Post by Jiri Benc
In order to be able to convert to a generic address struct, separate source
and destination address into separate fields.
---

Applied.

Thanks,
Richard